From ebe72d3f32a8995b8ebfee5a88f84b365f70de7e Mon Sep 17 00:00:00 2001 From: LongCatIsLooong <31859944+LongCatIsLooong@users.noreply.github.com> Date: Fri, 6 Oct 2023 15:12:20 -0700 Subject: [PATCH] Call `markNeedsPaint` when adding overlayChild to `Overlay` (#135941) Fixes https://github.com/flutter/flutter/issues/134656 `_skipMarkNeesLayout` was meant to only skip `markNeedsLayout` calls. Re-painting is still needed when a child gets added/removed from the `Overlay`. --- .../lib/src/rendering/mouse_tracker.dart | 10 +-- packages/flutter/lib/src/widgets/overlay.dart | 7 ++ .../test/material/tooltip_theme_test.dart | 73 +++++++------------ .../test/widgets/overlay_portal_test.dart | 59 +++++++++++++++ 4 files changed, 98 insertions(+), 51 deletions(-) diff --git a/packages/flutter/lib/src/rendering/mouse_tracker.dart b/packages/flutter/lib/src/rendering/mouse_tracker.dart index aa2efc7c6c..a503d686b4 100644 --- a/packages/flutter/lib/src/rendering/mouse_tracker.dart +++ b/packages/flutter/lib/src/rendering/mouse_tracker.dart @@ -412,10 +412,8 @@ class MouseTracker extends ChangeNotifier { // hit-test order. final PointerExitEvent baseExitEvent = PointerExitEvent.fromMouseEvent(latestEvent); lastAnnotations.forEach((MouseTrackerAnnotation annotation, Matrix4 transform) { - if (!nextAnnotations.containsKey(annotation)) { - if (annotation.validForMouseTracker && annotation.onExit != null) { - annotation.onExit!(baseExitEvent.transformed(lastAnnotations[annotation])); - } + if (annotation.validForMouseTracker && !nextAnnotations.containsKey(annotation)) { + annotation.onExit?.call(baseExitEvent.transformed(lastAnnotations[annotation])); } }); @@ -426,8 +424,8 @@ class MouseTracker extends ChangeNotifier { ).toList(); final PointerEnterEvent baseEnterEvent = PointerEnterEvent.fromMouseEvent(latestEvent); for (final MouseTrackerAnnotation annotation in enteringAnnotations.reversed) { - if (annotation.validForMouseTracker && annotation.onEnter != null) { - annotation.onEnter!(baseEnterEvent.transformed(nextAnnotations[annotation])); + if (annotation.validForMouseTracker) { + annotation.onEnter?.call(baseEnterEvent.transformed(nextAnnotations[annotation])); } } } diff --git a/packages/flutter/lib/src/widgets/overlay.dart b/packages/flutter/lib/src/widgets/overlay.dart index d689e10769..0ed11fe8e7 100644 --- a/packages/flutter/lib/src/widgets/overlay.dart +++ b/packages/flutter/lib/src/widgets/overlay.dart @@ -1032,6 +1032,10 @@ class _RenderTheater extends RenderBox with ContainerRenderObjectMixin key = GlobalKey(); late final OverlayEntry entry; addTearDown(() => entry..remove()..dispose()); await tester.pumpWidget( @@ -152,7 +135,7 @@ void main() { ), ), ); - _ensureTooltipVisible(key); + key.currentState!.ensureTooltipVisible(); await tester.pump(const Duration(seconds: 2)); // faded in, show timer started (and at 0.0) /********************* 800x600 screen @@ -172,7 +155,7 @@ void main() { }); testWidgetsWithLeakTracking('Tooltip verticalOffset, preferBelow; center prefer above fits - TooltipTheme', (WidgetTester tester) async { - final GlobalKey key = GlobalKey(); + final GlobalKey key = GlobalKey(); late final OverlayEntry entry; addTearDown(() => entry..remove()..dispose()); @@ -211,7 +194,7 @@ void main() { ), ), ); - _ensureTooltipVisible(key); + key.currentState!.ensureTooltipVisible(); await tester.pump(const Duration(seconds: 2)); // faded in, show timer started (and at 0.0) /********************* 800x600 screen @@ -231,7 +214,7 @@ void main() { }); testWidgetsWithLeakTracking('Tooltip verticalOffset, preferBelow; center prefer above does not fit - ThemeData.tooltipTheme', (WidgetTester tester) async { - final GlobalKey key = GlobalKey(); + final GlobalKey key = GlobalKey(); late final OverlayEntry entry; addTearDown(() => entry..remove()..dispose()); @@ -271,7 +254,7 @@ void main() { ), ), ); - _ensureTooltipVisible(key); + key.currentState!.ensureTooltipVisible(); await tester.pump(const Duration(seconds: 2)); // faded in, show timer started (and at 0.0) // we try to put it here but it doesn't fit: @@ -302,7 +285,7 @@ void main() { }); testWidgetsWithLeakTracking('Tooltip verticalOffset, preferBelow; center prefer above does not fit - TooltipTheme', (WidgetTester tester) async { - final GlobalKey key = GlobalKey(); + final GlobalKey key = GlobalKey(); late final OverlayEntry entry; addTearDown(() => entry..remove()..dispose()); @@ -341,7 +324,7 @@ void main() { ), ), ); - _ensureTooltipVisible(key); + key.currentState!.ensureTooltipVisible(); await tester.pump(const Duration(seconds: 2)); // faded in, show timer started (and at 0.0) // we try to put it here but it doesn't fit: @@ -372,7 +355,7 @@ void main() { }); testWidgetsWithLeakTracking('Tooltip verticalOffset, preferBelow; center preferBelow fits - ThemeData.tooltipTheme', (WidgetTester tester) async { - final GlobalKey key = GlobalKey(); + final GlobalKey key = GlobalKey(); late final OverlayEntry entry; addTearDown(() => entry..remove()..dispose()); await tester.pumpWidget( @@ -411,7 +394,7 @@ void main() { ), ), ); - _ensureTooltipVisible(key); + key.currentState!.ensureTooltipVisible(); await tester.pumpAndSettle(); // faded in, show timer started (and at 0.0) /********************* 800x600 screen @@ -430,7 +413,7 @@ void main() { }); testWidgetsWithLeakTracking('Tooltip verticalOffset, preferBelow; center prefer below fits - TooltipTheme', (WidgetTester tester) async { - final GlobalKey key = GlobalKey(); + final GlobalKey key = GlobalKey(); late final OverlayEntry entry; addTearDown(() => entry..remove()..dispose()); @@ -469,7 +452,7 @@ void main() { ), ), ); - _ensureTooltipVisible(key); + key.currentState!.ensureTooltipVisible(); await tester.pumpAndSettle(); // faded in, show timer started (and at 0.0) /********************* 800x600 screen @@ -488,7 +471,7 @@ void main() { }); testWidgetsWithLeakTracking('Tooltip margin - ThemeData', (WidgetTester tester) async { - final GlobalKey key = GlobalKey(); + final GlobalKey key = GlobalKey(); late final OverlayEntry entry; addTearDown(() => entry..remove()..dispose()); @@ -519,7 +502,7 @@ void main() { ), ), ); - _ensureTooltipVisible(key); + key.currentState!.ensureTooltipVisible(); await tester.pump(const Duration(seconds: 2)); // faded in, show timer started (and at 0.0) final RenderBox tip = tester.renderObject(find.text(tooltipText)).parent!.parent!.parent!.parent!.parent! as RenderBox; @@ -547,7 +530,7 @@ void main() { }); testWidgetsWithLeakTracking('Tooltip margin - TooltipTheme', (WidgetTester tester) async { - final GlobalKey key = GlobalKey(); + final GlobalKey key = GlobalKey(); late final OverlayEntry entry; addTearDown(() => entry..remove()..dispose()); @@ -575,7 +558,7 @@ void main() { ), ), ); - _ensureTooltipVisible(key); + key.currentState!.ensureTooltipVisible(); await tester.pump(const Duration(seconds: 2)); // faded in, show timer started (and at 0.0) final RenderBox tip = tester.renderObject(find.text(tooltipText)).parent!.parent!.parent!.parent!.parent! as RenderBox; @@ -603,7 +586,7 @@ void main() { }); testWidgetsWithLeakTracking('Tooltip message textStyle - ThemeData.tooltipTheme', (WidgetTester tester) async { - final GlobalKey key = GlobalKey(); + final GlobalKey key = GlobalKey(); await tester.pumpWidget(MaterialApp( theme: ThemeData( tooltipTheme: const TooltipThemeData( @@ -623,7 +606,7 @@ void main() { ), ), )); - _ensureTooltipVisible(key); + key.currentState!.ensureTooltipVisible(); await tester.pump(const Duration(seconds: 2)); // faded in, show timer started (and at 0.0) final TextStyle textStyle = tester.widget(find.text(tooltipText)).style!; @@ -633,7 +616,7 @@ void main() { }); testWidgetsWithLeakTracking('Tooltip message textStyle - TooltipTheme', (WidgetTester tester) async { - final GlobalKey key = GlobalKey(); + final GlobalKey key = GlobalKey(); await tester.pumpWidget(MaterialApp( home: TooltipTheme( data: const TooltipThemeData(), @@ -652,7 +635,7 @@ void main() { ), ), )); - _ensureTooltipVisible(key); + key.currentState!.ensureTooltipVisible(); await tester.pump(const Duration(seconds: 2)); // faded in, show timer started (and at 0.0) final TextStyle textStyle = tester.widget(find.text(tooltipText)).style!; @@ -701,7 +684,7 @@ void main() { }); testWidgetsWithLeakTracking('Tooltip decoration - ThemeData.tooltipTheme', (WidgetTester tester) async { - final GlobalKey key = GlobalKey(); + final GlobalKey key = GlobalKey(); const Decoration customDecoration = ShapeDecoration( shape: StadiumBorder(), color: Color(0x80800000), @@ -734,7 +717,7 @@ void main() { ), ), ); - _ensureTooltipVisible(key); + key.currentState!.ensureTooltipVisible(); await tester.pump(const Duration(seconds: 2)); // faded in, show timer started (and at 0.0) final RenderBox tip = tester.renderObject(find.text(tooltipText)).parent!.parent!.parent!.parent! as RenderBox; @@ -745,7 +728,7 @@ void main() { }); testWidgetsWithLeakTracking('Tooltip decoration - TooltipTheme', (WidgetTester tester) async { - final GlobalKey key = GlobalKey(); + final GlobalKey key = GlobalKey(); const Decoration customDecoration = ShapeDecoration( shape: StadiumBorder(), color: Color(0x80800000), @@ -778,7 +761,7 @@ void main() { ), ), ); - _ensureTooltipVisible(key); + key.currentState!.ensureTooltipVisible(); await tester.pump(const Duration(seconds: 2)); // faded in, show timer started (and at 0.0) final RenderBox tip = tester.renderObject(find.text(tooltipText)).parent!.parent!.parent!.parent! as RenderBox; @@ -789,7 +772,7 @@ void main() { }); testWidgetsWithLeakTracking('Tooltip height and padding - ThemeData.tooltipTheme', (WidgetTester tester) async { - final GlobalKey key = GlobalKey(); + final GlobalKey key = GlobalKey(); const double customTooltipHeight = 100.0; const double customPaddingVal = 20.0; @@ -821,7 +804,7 @@ void main() { ), ), ); - _ensureTooltipVisible(key); + key.currentState!.ensureTooltipVisible(); await tester.pumpAndSettle(); final RenderBox tip = tester.renderObject(find.ancestor( @@ -839,7 +822,7 @@ void main() { }); testWidgetsWithLeakTracking('Tooltip height and padding - TooltipTheme', (WidgetTester tester) async { - final GlobalKey key = GlobalKey(); + final GlobalKey key = GlobalKey(); const double customTooltipHeight = 100.0; const double customPaddingValue = 20.0; late final OverlayEntry entry; @@ -868,7 +851,7 @@ void main() { ), ), ); - _ensureTooltipVisible(key); + key.currentState!.ensureTooltipVisible(); await tester.pumpAndSettle(); final RenderBox tip = tester.renderObject(find.ancestor( diff --git a/packages/flutter/test/widgets/overlay_portal_test.dart b/packages/flutter/test/widgets/overlay_portal_test.dart index 22d624b9c0..2d9d7d476a 100644 --- a/packages/flutter/test/widgets/overlay_portal_test.dart +++ b/packages/flutter/test/widgets/overlay_portal_test.dart @@ -777,6 +777,65 @@ void main() { verifyTreeIsClean(); }); + group('Adding/removing overlay child causes repaint', () { + // Regression test for https://github.com/flutter/flutter/issues/134656. + const Key childKey = Key('child'); + final OverlayEntry overlayEntry = OverlayEntry( + builder: (BuildContext context) { + return RepaintBoundary( + child: OverlayPortal( + controller: controller1, + overlayChildBuilder: (BuildContext context) => const SizedBox(), + child: const SizedBox(key: childKey), + ), + ); + }, + ); + final Widget widget = Directionality( + key: GlobalKey(debugLabel: 'key'), + textDirection: TextDirection.ltr, + child: Overlay(initialEntries: [overlayEntry]), + ); + tearDown(overlayEntry.remove); + tearDownAll(overlayEntry.dispose); + + testWidgetsWithLeakTracking('Adding child', (WidgetTester tester) async { + controller1.hide(); + await tester.pumpWidget(widget); + + final RenderBox renderTheater = tester.renderObject(find.byType(Overlay)); + final RenderBox renderChild = tester.renderObject(find.byKey(childKey)); + assert(!renderTheater.debugNeedsPaint); + assert(!renderChild.debugNeedsPaint); + + controller1.show(); + await tester.pump(null, EnginePhase.layout); + expect(renderTheater.debugNeedsPaint, isTrue); + expect(renderChild.debugNeedsPaint, isFalse); + + // Discard the dirty render tree. + await tester.pumpWidget(const SizedBox()); + }); + + testWidgetsWithLeakTracking('Removing child', (WidgetTester tester) async { + controller1.show(); + await tester.pumpWidget(widget); + + final RenderBox renderTheater = tester.renderObject(find.byType(Overlay)); + final RenderBox renderChild = tester.renderObject(find.byKey(childKey)); + assert(!renderTheater.debugNeedsPaint); + assert(!renderChild.debugNeedsPaint); + + controller1.hide(); + await tester.pump(null, EnginePhase.layout); + expect(renderTheater.debugNeedsPaint, isTrue); + expect(renderChild.debugNeedsPaint, isFalse); + + // Discard the dirty render tree. + await tester.pumpWidget(const SizedBox()); + }); + }); + testWidgetsWithLeakTracking('Adding/Removing OverlayPortal in LayoutBuilder during layout', (WidgetTester tester) async { final GlobalKey widgetKey = GlobalKey(debugLabel: 'widget'); final GlobalKey overlayKey = GlobalKey(debugLabel: 'overlay');