Unverified Commit 9881aa69 authored by Jonah Williams's avatar Jonah Williams Committed by GitHub

[framework] attempt non-key solution (#128273)

Adds a special opacity widget that does not act like a repaint boundary.

Better solution for https://github.com/flutter/flutter/pull/128138
parent 2be476d4
...@@ -6,6 +6,7 @@ import 'dart:math' as math; ...@@ -6,6 +6,7 @@ import 'dart:math' as math;
import 'dart:ui' as ui; import 'dart:ui' as ui;
import 'package:flutter/foundation.dart' show clampDouble; import 'package:flutter/foundation.dart' show clampDouble;
import 'package:flutter/rendering.dart';
import 'package:flutter/widgets.dart'; import 'package:flutter/widgets.dart';
import 'colors.dart'; import 'colors.dart';
...@@ -251,17 +252,12 @@ class _FlexibleSpaceBarState extends State<FlexibleSpaceBar> { ...@@ -251,17 +252,12 @@ class _FlexibleSpaceBarState extends State<FlexibleSpaceBar> {
left: 0.0, left: 0.0,
right: 0.0, right: 0.0,
height: height, height: height,
child: Opacity( child: _FlexibleSpaceHeaderOpacity(
// We need the child widget to repaint, however both the opacity
// and potentially `widget.background` can be constant which won't
// lead to repainting.
// see: https://github.com/flutter/flutter/issues/127836
key: ValueKey<double>(topPadding),
// IOS is relying on this semantics node to correctly traverse // IOS is relying on this semantics node to correctly traverse
// through the app bar when it is collapsed. // through the app bar when it is collapsed.
alwaysIncludeSemantics: true, alwaysIncludeSemantics: true,
opacity: opacity, opacity: opacity,
child: widget.background, child: widget.background
), ),
)); ));
...@@ -426,3 +422,33 @@ class FlexibleSpaceBarSettings extends InheritedWidget { ...@@ -426,3 +422,33 @@ class FlexibleSpaceBarSettings extends InheritedWidget {
|| isScrolledUnder != oldWidget.isScrolledUnder; || isScrolledUnder != oldWidget.isScrolledUnder;
} }
} }
// We need the child widget to repaint, however both the opacity
// and potentially `widget.background` can be constant which won't
// lead to repainting.
// see: https://github.com/flutter/flutter/issues/127836
class _FlexibleSpaceHeaderOpacity extends SingleChildRenderObjectWidget {
const _FlexibleSpaceHeaderOpacity({required this.opacity, required super.child, required this.alwaysIncludeSemantics});
final double opacity;
final bool alwaysIncludeSemantics;
@override
RenderObject createRenderObject(BuildContext context) {
return _RenderFlexibleSpaceHeaderOpacity(opacity: opacity, alwaysIncludeSemantics: alwaysIncludeSemantics);
}
@override
void updateRenderObject(BuildContext context, covariant _RenderFlexibleSpaceHeaderOpacity renderObject) {
renderObject
..alwaysIncludeSemantics = alwaysIncludeSemantics
..opacity = opacity;
}
}
class _RenderFlexibleSpaceHeaderOpacity extends RenderOpacity {
_RenderFlexibleSpaceHeaderOpacity({super.opacity, super.alwaysIncludeSemantics});
@override
bool get isRepaintBoundary => false;
}
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
library; library;
import 'package:flutter/material.dart'; import 'package:flutter/material.dart';
import 'package:flutter/rendering.dart';
import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test/flutter_test.dart';
import '../widgets/semantics_tester.dart'; import '../widgets/semantics_tester.dart';
...@@ -159,7 +160,10 @@ void main() { ...@@ -159,7 +160,10 @@ void main() {
), ),
); );
final Opacity backgroundOpacity = tester.firstWidget(find.byType(Opacity)); final dynamic backgroundOpacity = tester.firstWidget(
find.byWidgetPredicate((Widget widget) => widget.runtimeType.toString() == '_FlexibleSpaceHeaderOpacity'));
// accessing private type member.
// ignore: avoid_dynamic_calls
expect(backgroundOpacity.opacity, 1.0); expect(backgroundOpacity.opacity, 1.0);
}); });
...@@ -348,7 +352,7 @@ void main() { ...@@ -348,7 +352,7 @@ void main() {
rect: const Rect.fromLTRB(0.0, 0.0, 800.0, 56.0), rect: const Rect.fromLTRB(0.0, 0.0, 800.0, 56.0),
children: <TestSemantics>[ children: <TestSemantics>[
TestSemantics( TestSemantics(
id: 18, id: 11,
rect: const Rect.fromLTRB(0.0, 36.0, 800.0, 92.0), rect: const Rect.fromLTRB(0.0, 36.0, 800.0, 92.0),
label: 'Expanded title', label: 'Expanded title',
textDirection: TextDirection.ltr, textDirection: TextDirection.ltr,
...@@ -790,7 +794,7 @@ void main() { ...@@ -790,7 +794,7 @@ void main() {
home: SubCategoryScreenView(), home: SubCategoryScreenView(),
)); ));
expect(RebuildTracker.count, 1); expect(RenderRebuildTracker.count, 1);
// We drag up to fully collapse the space bar. // We drag up to fully collapse the space bar.
for (int i = 0; i < 20; i++) { for (int i = 0; i < 20; i++) {
...@@ -798,7 +802,7 @@ void main() { ...@@ -798,7 +802,7 @@ void main() {
await tester.pumpAndSettle(); await tester.pumpAndSettle();
} }
expect(RebuildTracker.count, greaterThan(1)); expect(RenderRebuildTracker.count, greaterThan(1));
}); });
} }
...@@ -825,15 +829,22 @@ class TestDelegate extends SliverPersistentHeaderDelegate { ...@@ -825,15 +829,22 @@ class TestDelegate extends SliverPersistentHeaderDelegate {
bool shouldRebuild(TestDelegate oldDelegate) => false; bool shouldRebuild(TestDelegate oldDelegate) => false;
} }
class RebuildTracker extends StatelessWidget { class RebuildTracker extends SingleChildRenderObjectWidget {
const RebuildTracker({super.key}); const RebuildTracker({super.key});
@override
RenderObject createRenderObject(BuildContext context) {
return RenderRebuildTracker();
}
}
class RenderRebuildTracker extends RenderProxyBox {
static int count = 0; static int count = 0;
@override @override
Widget build(BuildContext context) { void paint(PaintingContext context, Offset offset) {
count++; count++;
return const SizedBox(width: 100, height: 100); super.paint(context, offset);
} }
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment