Skip to content

Commit

Permalink
Call markNeedsPaint when adding overlayChild to Overlay (#135941)
Browse files Browse the repository at this point in the history 
Fixes flutter/flutter#134656

`_skipMarkNeesLayout` was meant to only skip `markNeedsLayout` calls. Re-painting is still needed when a child gets added/removed from the `Overlay`.
  • Loading branch information
@LongCatIsLooong
LongCatIsLooong authored Oct 6, 2023
1 parent 68d47e5 commit ebe72d3
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 51 deletions.
10 changes: 4 additions & 6 deletions packages/flutter/lib/src/rendering/mouse_tracker.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]));
}
});

Expand All @@ -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]));
}
}
}
Expand Down
7 changes: 7 additions & 0 deletions packages/flutter/lib/src/widgets/overlay.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1032,6 +1032,10 @@ class _RenderTheater extends RenderBox with ContainerRenderObjectMixin<RenderBox
assert(!_skipMarkNeedsLayout);
_skipMarkNeedsLayout = true;
adoptChild(child);
// The Overlay still needs repainting when a deferred child is added. Usually
// `markNeedsLayout` implies `markNeedsPaint`, but here `markNeedsLayout` is
// skipped when the `_skipMarkNeedsLayout` flag is set.
markNeedsPaint();
_skipMarkNeedsLayout = false;

// After adding `child` to the render tree, we want to make sure it will be
Expand All @@ -1045,6 +1049,9 @@ class _RenderTheater extends RenderBox with ContainerRenderObjectMixin<RenderBox
assert(!_skipMarkNeedsLayout);
_skipMarkNeedsLayout = true;
dropChild(child);
// The Overlay still needs repainting when a deferred child is dropped. See
// the comment in `_addDeferredChild`.
markNeedsPaint();
_skipMarkNeedsLayout = false;
}

Expand Down
73 changes: 28 additions & 45 deletions packages/flutter/test/material/tooltip_theme_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,23 +13,6 @@ import '../widgets/semantics_tester.dart';
const String tooltipText = 'TIP';
const double _customPaddingValue = 10.0;

void _ensureTooltipVisible(GlobalKey key) {
// This function uses "as dynamic"to defeat the static analysis. In general
// you want to avoid using this style in your code, as it will cause the
// analyzer to be unable to help you catch errors.
//
// In this case, we do it because we are trying to call internal methods of
// the tooltip code in order to test it. Normally, the state of a tooltip is a
// private class, but by using a GlobalKey we can get a handle to that object
// and by using "as dynamic" we can bypass the analyzer's type checks and call
// methods that we aren't supposed to be able to know about.
//
// It's ok to do this in tests, but you really don't want to do it in
// production code.
// ignore: avoid_dynamic_calls
(key.currentState as dynamic).ensureTooltipVisible();
}

void main() {
test('TooltipThemeData copyWith, ==, hashCode basics', () {
expect(const TooltipThemeData(), const TooltipThemeData().copyWith());
Expand Down Expand Up @@ -113,7 +96,7 @@ void main() {
});

testWidgetsWithLeakTracking('Tooltip verticalOffset, preferBelow; center prefer above fits - ThemeData.tooltipTheme', (WidgetTester tester) async {
final GlobalKey key = GlobalKey();
final GlobalKey<TooltipState> key = GlobalKey<TooltipState>();
late final OverlayEntry entry;
addTearDown(() => entry..remove()..dispose());
await tester.pumpWidget(
Expand Down Expand Up @@ -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
Expand All @@ -172,7 +155,7 @@ void main() {
});

testWidgetsWithLeakTracking('Tooltip verticalOffset, preferBelow; center prefer above fits - TooltipTheme', (WidgetTester tester) async {
final GlobalKey key = GlobalKey();
final GlobalKey<TooltipState> key = GlobalKey<TooltipState>();

late final OverlayEntry entry;
addTearDown(() => entry..remove()..dispose());
Expand Down Expand Up @@ -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
Expand All @@ -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<TooltipState> key = GlobalKey<TooltipState>();
late final OverlayEntry entry;
addTearDown(() => entry..remove()..dispose());

Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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<TooltipState> key = GlobalKey<TooltipState>();

late final OverlayEntry entry;
addTearDown(() => entry..remove()..dispose());
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -372,7 +355,7 @@ void main() {
});

testWidgetsWithLeakTracking('Tooltip verticalOffset, preferBelow; center preferBelow fits - ThemeData.tooltipTheme', (WidgetTester tester) async {
final GlobalKey key = GlobalKey();
final GlobalKey<TooltipState> key = GlobalKey<TooltipState>();
late final OverlayEntry entry;
addTearDown(() => entry..remove()..dispose());
await tester.pumpWidget(
Expand Down Expand Up @@ -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
Expand All @@ -430,7 +413,7 @@ void main() {
});

testWidgetsWithLeakTracking('Tooltip verticalOffset, preferBelow; center prefer below fits - TooltipTheme', (WidgetTester tester) async {
final GlobalKey key = GlobalKey();
final GlobalKey<TooltipState> key = GlobalKey<TooltipState>();

late final OverlayEntry entry;
addTearDown(() => entry..remove()..dispose());
Expand Down Expand Up @@ -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
Expand All @@ -488,7 +471,7 @@ void main() {
});

testWidgetsWithLeakTracking('Tooltip margin - ThemeData', (WidgetTester tester) async {
final GlobalKey key = GlobalKey();
final GlobalKey<TooltipState> key = GlobalKey<TooltipState>();

late final OverlayEntry entry;
addTearDown(() => entry..remove()..dispose());
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -547,7 +530,7 @@ void main() {
});

testWidgetsWithLeakTracking('Tooltip margin - TooltipTheme', (WidgetTester tester) async {
final GlobalKey key = GlobalKey();
final GlobalKey<TooltipState> key = GlobalKey<TooltipState>();
late final OverlayEntry entry;
addTearDown(() => entry..remove()..dispose());

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -603,7 +586,7 @@ void main() {
});

testWidgetsWithLeakTracking('Tooltip message textStyle - ThemeData.tooltipTheme', (WidgetTester tester) async {
final GlobalKey key = GlobalKey();
final GlobalKey<TooltipState> key = GlobalKey<TooltipState>();
await tester.pumpWidget(MaterialApp(
theme: ThemeData(
tooltipTheme: const TooltipThemeData(
Expand All @@ -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<Text>(find.text(tooltipText)).style!;
Expand All @@ -633,7 +616,7 @@ void main() {
});

testWidgetsWithLeakTracking('Tooltip message textStyle - TooltipTheme', (WidgetTester tester) async {
final GlobalKey key = GlobalKey();
final GlobalKey<TooltipState> key = GlobalKey<TooltipState>();
await tester.pumpWidget(MaterialApp(
home: TooltipTheme(
data: const TooltipThemeData(),
Expand All @@ -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<Text>(find.text(tooltipText)).style!;
Expand Down Expand Up @@ -701,7 +684,7 @@ void main() {
});

testWidgetsWithLeakTracking('Tooltip decoration - ThemeData.tooltipTheme', (WidgetTester tester) async {
final GlobalKey key = GlobalKey();
final GlobalKey<TooltipState> key = GlobalKey<TooltipState>();
const Decoration customDecoration = ShapeDecoration(
shape: StadiumBorder(),
color: Color(0x80800000),
Expand Down Expand Up @@ -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;
Expand All @@ -745,7 +728,7 @@ void main() {
});

testWidgetsWithLeakTracking('Tooltip decoration - TooltipTheme', (WidgetTester tester) async {
final GlobalKey key = GlobalKey();
final GlobalKey<TooltipState> key = GlobalKey<TooltipState>();
const Decoration customDecoration = ShapeDecoration(
shape: StadiumBorder(),
color: Color(0x80800000),
Expand Down Expand Up @@ -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;
Expand All @@ -789,7 +772,7 @@ void main() {
});

testWidgetsWithLeakTracking('Tooltip height and padding - ThemeData.tooltipTheme', (WidgetTester tester) async {
final GlobalKey key = GlobalKey();
final GlobalKey<TooltipState> key = GlobalKey<TooltipState>();
const double customTooltipHeight = 100.0;
const double customPaddingVal = 20.0;

Expand Down Expand Up @@ -821,7 +804,7 @@ void main() {
),
),
);
_ensureTooltipVisible(key);
key.currentState!.ensureTooltipVisible();
await tester.pumpAndSettle();

final RenderBox tip = tester.renderObject(find.ancestor(
Expand All @@ -839,7 +822,7 @@ void main() {
});

testWidgetsWithLeakTracking('Tooltip height and padding - TooltipTheme', (WidgetTester tester) async {
final GlobalKey key = GlobalKey();
final GlobalKey<TooltipState> key = GlobalKey<TooltipState>();
const double customTooltipHeight = 100.0;
const double customPaddingValue = 20.0;
late final OverlayEntry entry;
Expand Down Expand Up @@ -868,7 +851,7 @@ void main() {
),
),
);
_ensureTooltipVisible(key);
key.currentState!.ensureTooltipVisible();
await tester.pumpAndSettle();

final RenderBox tip = tester.renderObject(find.ancestor(
Expand Down
59 changes: 59 additions & 0 deletions packages/flutter/test/widgets/overlay_portal_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>[overlayEntry]),
);
tearDown(overlayEntry.remove);
tearDownAll(overlayEntry.dispose);

testWidgetsWithLeakTracking('Adding child', (WidgetTester tester) async {
controller1.hide();
await tester.pumpWidget(widget);

final RenderBox renderTheater = tester.renderObject<RenderBox>(find.byType(Overlay));
final RenderBox renderChild = tester.renderObject<RenderBox>(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<RenderBox>(find.byType(Overlay));
final RenderBox renderChild = tester.renderObject<RenderBox>(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');
Expand Down

0 comments on commit ebe72d3

Please sign in to comment.