diff --git a/packages/flutter/lib/src/rendering/object.dart b/packages/flutter/lib/src/rendering/object.dart index e1cd6b1de0..868b05100a 100644 --- a/packages/flutter/lib/src/rendering/object.dart +++ b/packages/flutter/lib/src/rendering/object.dart @@ -2608,7 +2608,7 @@ abstract class RenderObject with DiagnosticableTreeMixin implements HitTestTarge @pragma('vm:notify-debugger-on-exception') void _layoutWithoutResize() { assert(_needsLayout); - assert(_relayoutBoundary == this); + assert(_relayoutBoundary == this || this is RenderObjectWithLayoutCallbackMixin); RenderObject? debugPreviousActiveLayout; assert(!_debugMutationsLocked); assert(!_doingThisLayoutWithCallback); @@ -4132,6 +4132,77 @@ mixin RenderObjectWithChildMixin on RenderObject } } +/// A mixin for managing [RenderObject] with a [layoutCallback], which will be +/// invoked during this [RenderObject]'s layout process if scheduled using +/// [scheduleLayoutCallback]. +/// +/// A layout callback is typically a callback that mutates the [RenderObject]'s +/// render subtree during the [RenderObject]'s layout process. When an ancestor +/// [RenderObject] chooses to skip laying out this [RenderObject] in its +/// [performLayout] implementation (for example, for performance reasons, an +/// [Overlay] may skip laying out an offstage [OverlayEntry] while keeping it in +/// the tree), normally the [layoutCallback] will not be invoked because the +/// [layout] method will not be called. This can be undesirable when the +/// [layoutCallback] involves rebuilding dirty widgets (most notably, the +/// [LayoutBuilder] widget). Unlike render subtrees, typically all dirty widgets +/// (even off-screen ones) in a widget tree must be rebuilt. This mixin makes +/// sure once scheduled, the [layoutCallback] method will be invoked even if it's +/// skipped by an ancestor [RenderObject], unless this [RenderObject] has never +/// been laid out. +/// +/// Subclasses must not invoke the layout callback directly. Instead, call +/// [runLayoutCallback] in the [performLayout] implementation. +/// +/// See also: +/// +/// * [LayoutBuilder] and [SliverLayoutBuilder], which use the mixin. +mixin RenderObjectWithLayoutCallbackMixin on RenderObject { + // The initial value of this flag must be set to true to prevent the layout + // callback from being scheduled when the subtree has never been laid out (in + // which case the `constraints` or any other layout information is unknown). + bool _needsRebuild = true; + + /// The layout callback to be invoked during [performLayout]. + /// + /// This method should not be invoked directly. Instead, call + /// [runLayoutCallback] in the [performLayout] implementation. This callback + /// will be invoked using [invokeLayoutCallback]. + @visibleForOverriding + void layoutCallback(); + + /// Invokes [layoutCallback] with [invokeLayoutCallback]. + /// + /// This method must be called in [performLayout], typically as early as + /// possible before any layout work is done, to avoid re-dirtying any child + /// [RenderObject]s. + @mustCallSuper + void runLayoutCallback() { + assert(debugDoingThisLayout); + invokeLayoutCallback((_) => layoutCallback()); + _needsRebuild = false; + } + + /// Informs the framework that the layout callback has been updated and must be + /// invoked again when this [RenderObject] is ready for layout, even when an + /// ancestor [RenderObject] chooses to skip laying out this render subtree. + @mustCallSuper + void scheduleLayoutCallback() { + if (_needsRebuild) { + assert(debugNeedsLayout); + return; + } + _needsRebuild = true; + // This ensures that the layout callback will be run even if an ancestor + // chooses to not lay out this subtree (for example, obstructed OverlayEntries + // with `maintainState` set to true), to maintain the widget tree integrity + // (making sure global keys are unique, for example). + owner?._nodesNeedingLayout.add(this); + // In an active tree, markNeedsLayout is needed to inform the layout boundary + // that its child size may change. + super.markNeedsLayout(); + } +} + /// Parent data to support a doubly-linked list of children. /// /// The children can be traversed using [nextSibling] or [previousSibling], diff --git a/packages/flutter/lib/src/widgets/layout_builder.dart b/packages/flutter/lib/src/widgets/layout_builder.dart index 7d40c10187..79980bb458 100644 --- a/packages/flutter/lib/src/widgets/layout_builder.dart +++ b/packages/flutter/lib/src/widgets/layout_builder.dart @@ -92,6 +92,10 @@ abstract class AbstractLayoutBuilder extends RenderObjectWidget /// /// The [builder] function is _not_ called during layout if the parent passes /// the same constraints repeatedly. +/// +/// In the event that an ancestor skips the layout of this subtree so the +/// constraints become outdated, the `builder` rebuilds with the last known +/// constraints. /// {@endtemplate} abstract class ConstrainedLayoutBuilder extends AbstractLayoutBuilder { @@ -133,7 +137,7 @@ class _LayoutBuilderElement extends RenderObjectElement { SchedulerPhase.persistentCallbacks => false, }; if (!deferMarkNeedsLayout) { - renderObject.markNeedsLayout(); + renderObject.scheduleLayoutCallback(); return; } _deferredCallbackScheduled = true; @@ -145,7 +149,7 @@ class _LayoutBuilderElement extends RenderObjectElement { // This method is only called when the render tree is stable, if the Element // is deactivated it will never be reincorporated back to the tree. if (mounted) { - renderObject.markNeedsLayout(); + renderObject.scheduleLayoutCallback(); } } @@ -180,7 +184,7 @@ class _LayoutBuilderElement extends RenderObjectElement { renderObject._updateCallback(_rebuildWithConstraints); if (newWidget.updateShouldRebuild(oldWidget)) { _needsBuild = true; - renderObject.markNeedsLayout(); + renderObject.scheduleLayoutCallback(); } } @@ -190,7 +194,7 @@ class _LayoutBuilderElement extends RenderObjectElement { // to performRebuild since this call already does what performRebuild does, // So the element is clean as soon as this method returns and does not have // to be added to the dirty list or marked as dirty. - renderObject.markNeedsLayout(); + renderObject.scheduleLayoutCallback(); _needsBuild = true; } @@ -202,14 +206,14 @@ class _LayoutBuilderElement extends RenderObjectElement { // Force the callback to be called, even if the layout constraints are the // same. This is because that callback may depend on the updated widget // configuration, or an inherited widget. - renderObject.markNeedsLayout(); + renderObject.scheduleLayoutCallback(); _needsBuild = true; super.performRebuild(); // Calls widget.updateRenderObject (a no-op in this case). } @override void unmount() { - renderObject._updateCallback(null); + renderObject._callback = null; super.unmount(); } @@ -295,39 +299,36 @@ class _LayoutBuilderElement extends RenderObjectElement { /// Generic mixin for [RenderObject]s created by an [AbstractLayoutBuilder] with /// the the same `LayoutInfoType`. /// -/// Provides a [rebuildIfNecessary] method that should be called at layout time, -/// typically in [RenderObject.performLayout]. The method invokes -/// [AbstractLayoutBuilder]'s builder callback if needed. +/// Provides a [layoutCallback] implementation which, if needed, invokes +/// [AbstractLayoutBuilder]'s builder callback. /// /// Implementers must provide a [layoutInfo] implementation that is safe to -/// access in [rebuildIfNecessary], which is typically called in [performLayout]. +/// access in [layoutCallback], which is called in [performLayout]. mixin RenderAbstractLayoutBuilderMixin - on RenderObjectWithChildMixin { + on RenderObjectWithChildMixin, RenderObjectWithLayoutCallbackMixin { LayoutCallback? _callback; /// Change the layout callback. - void _updateCallback(LayoutCallback? value) { + void _updateCallback(LayoutCallback value) { if (value == _callback) { return; } _callback = value; - markNeedsLayout(); + scheduleLayoutCallback(); } - /// Invoke the builder callback supplied via [AbstractLayoutBuilder] and + /// Invokes the builder callback supplied via [AbstractLayoutBuilder] and /// rebuilds the [AbstractLayoutBuilder]'s widget tree, if needed. /// - /// No work will be done if [layoutInfo] has not changed since the last time - /// this method was called, and [AbstractLayoutBuilder.updateShouldRebuild] + /// No further work will be done if [layoutInfo] has not changed since the last + /// time this method was called, and [AbstractLayoutBuilder.updateShouldRebuild] /// returned `false` when the widget was rebuilt. /// /// This method should typically be called as soon as possible in the class's /// [performLayout] implementation, before any layout work is done. - @protected - void rebuildIfNecessary() { - assert(_callback != null); - invokeLayoutCallback(_callback!); - } + @visibleForOverriding + @override + void layoutCallback() => _callback!(constraints); /// The information to invoke the [AbstractLayoutBuilder.builder] callback with. /// @@ -381,6 +382,7 @@ class LayoutBuilder extends ConstrainedLayoutBuilder { class _RenderLayoutBuilder extends RenderBox with RenderObjectWithChildMixin, + RenderObjectWithLayoutCallbackMixin, RenderAbstractLayoutBuilderMixin { @override double computeMinIntrinsicWidth(double height) { @@ -433,7 +435,7 @@ class _RenderLayoutBuilder extends RenderBox @override void performLayout() { final BoxConstraints constraints = this.constraints; - rebuildIfNecessary(); + runLayoutCallback(); if (child != null) { child!.layout(constraints, parentUsesSize: true); size = constraints.constrain(child!.size); diff --git a/packages/flutter/lib/src/widgets/overlay.dart b/packages/flutter/lib/src/widgets/overlay.dart index a7733d4983..bd01fd9814 100644 --- a/packages/flutter/lib/src/widgets/overlay.dart +++ b/packages/flutter/lib/src/widgets/overlay.dart @@ -2632,7 +2632,10 @@ class _OverlayChildLayoutBuilder extends AbstractLayoutBuilder { + with + _RenderTheaterMixin, + RenderObjectWithLayoutCallbackMixin, + RenderAbstractLayoutBuilderMixin { @override Iterable _childrenInPaintOrder() { final RenderBox? child = this.child; @@ -2709,19 +2712,25 @@ class _RenderLayoutBuilder extends RenderProxyBox return OverlayChildLayoutInfo._((overlayPortalSize, paintTransform, size)); } + @override + @visibleForOverriding + void layoutCallback() { + _layoutInfo = _computeNewLayoutInfo(); + super.layoutCallback(); + } + int? _callbackId; @override void performLayout() { - // The invokeLayoutCallback allows arbitrary access to the sizes of - // RenderBoxes the we know that have finished doing layout. - invokeLayoutCallback((_) => _layoutInfo = _computeNewLayoutInfo()); - rebuildIfNecessary(); + runLayoutCallback(); + if (child case final RenderBox child?) { + layoutChild(child, constraints); + } assert(_callbackId == null); _callbackId ??= SchedulerBinding.instance.scheduleFrameCallback( _frameCallback, scheduleNewFrame: false, ); - layoutChild(child!, constraints); } // This RenderObject is a child of _RenderDeferredLayouts which in turn is a diff --git a/packages/flutter/lib/src/widgets/sliver_layout_builder.dart b/packages/flutter/lib/src/widgets/sliver_layout_builder.dart index dea8f283c6..a7daaae35d 100644 --- a/packages/flutter/lib/src/widgets/sliver_layout_builder.dart +++ b/packages/flutter/lib/src/widgets/sliver_layout_builder.dart @@ -37,6 +37,7 @@ class SliverLayoutBuilder extends ConstrainedLayoutBuilder { class _RenderSliverLayoutBuilder extends RenderSliver with RenderObjectWithChildMixin, + RenderObjectWithLayoutCallbackMixin, RenderAbstractLayoutBuilderMixin { @override double childMainAxisPosition(RenderObject child) { @@ -50,7 +51,7 @@ class _RenderSliverLayoutBuilder extends RenderSliver @override void performLayout() { - rebuildIfNecessary(); + runLayoutCallback(); child?.layout(constraints, parentUsesSize: true); geometry = child?.geometry ?? SliverGeometry.zero; } diff --git a/packages/flutter/test/widgets/layout_builder_test.dart b/packages/flutter/test/widgets/layout_builder_test.dart index bdba550b44..e11dfd5202 100644 --- a/packages/flutter/test/widgets/layout_builder_test.dart +++ b/packages/flutter/test/widgets/layout_builder_test.dart @@ -872,7 +872,6 @@ void main() { ), ), ); - WidgetsBinding.instance.buildOwner!.reassemble(WidgetsBinding.instance.rootElement!); await tester.pump(); WidgetsBinding.instance.buildOwner!.reassemble(WidgetsBinding.instance.rootElement!); @@ -880,6 +879,111 @@ void main() { expect(tester.takeException(), isNull); }, ); + + testWidgets( + 'LayoutBuilder in a subtree that skips layout does not rebuild during the initial treewalk', + (WidgetTester tester) async { + bool rebuilt = false; + final LayoutBuilder layoutBuilder = LayoutBuilder( + builder: (BuildContext context, BoxConstraints constraints) { + rebuilt = true; + return const Placeholder(); + }, + ); + final OverlayEntry overlayEntry1 = OverlayEntry( + maintainState: true, + builder: (BuildContext context) => layoutBuilder, + ); + // OverlayEntry2 obstructs OverlayEntry1 and forces it to skip layout. + final OverlayEntry overlayEntry2 = OverlayEntry( + opaque: true, + canSizeOverlay: true, + builder: (BuildContext context) => Container(), + ); + addTearDown( + () => + overlayEntry1 + ..remove() + ..dispose(), + ); + addTearDown( + () => + overlayEntry2 + ..remove() + ..dispose(), + ); + await tester.pumpWidget( + Directionality( + textDirection: TextDirection.ltr, + // The UnconstrainedBox makes sure the OverlayEntries are not relayout boundaries. + child: UnconstrainedBox( + child: Overlay(initialEntries: [overlayEntry1, overlayEntry2]), + ), + ), + ); + + final Element layoutBuilderElement = tester.element( + find.byWidget(layoutBuilder, skipOffstage: false), + ); + layoutBuilderElement.markNeedsBuild(); + await tester.pump(); + expect(rebuilt, isFalse); + expect(tester.takeException(), isNull); + }, + ); + + testWidgets('LayoutBuilder in a subtree that skips layout still rebuilds', ( + WidgetTester tester, + ) async { + bool rebuilt = false; + final LayoutBuilder layoutBuilder = LayoutBuilder( + builder: (BuildContext context, BoxConstraints constraints) { + rebuilt = true; + return const Placeholder(); + }, + ); + final OverlayEntry overlayEntry1 = OverlayEntry( + maintainState: true, + canSizeOverlay: true, + builder: (BuildContext context) => layoutBuilder, + ); + // OverlayEntry2 obstructs OverlayEntry1 and forces it to skip layout. + final OverlayEntry overlayEntry2 = OverlayEntry( + opaque: true, + canSizeOverlay: true, + builder: (BuildContext context) => const Placeholder(), + ); + addTearDown( + () => + overlayEntry1 + ..remove() + ..dispose(), + ); + addTearDown( + () => + overlayEntry2 + ..remove() + ..dispose(), + ); + await tester.pumpWidget( + Directionality( + textDirection: TextDirection.ltr, + // The UnconstrainedBox makes sure the OverlayEntries are not relayout boundaries. + child: UnconstrainedBox(child: Overlay(initialEntries: [overlayEntry1])), + ), + ); + tester.state(find.byType(Overlay)).insert(overlayEntry2); + await tester.pump(); + + rebuilt = false; + final Element layoutBuilderElement = tester.element( + find.byWidget(layoutBuilder, skipOffstage: false), + ); + layoutBuilderElement.markNeedsBuild(); + expect(rebuilt, isFalse); + await tester.pump(); + expect(rebuilt, isTrue); + }); } class _SmartLayoutBuilder extends ConstrainedLayoutBuilder { @@ -921,7 +1025,9 @@ class _SmartLayoutBuilder extends ConstrainedLayoutBuilder { typedef _OnChildWasPaintedCallback = void Function(Offset extraOffset); class _RenderSmartLayoutBuilder extends RenderProxyBox - with RenderAbstractLayoutBuilderMixin { + with + RenderObjectWithLayoutCallbackMixin, + RenderAbstractLayoutBuilderMixin { _RenderSmartLayoutBuilder({required double offsetPercentage, required this.onChildWasPainted}) : _offsetPercentage = offsetPercentage; @@ -946,7 +1052,7 @@ class _RenderSmartLayoutBuilder extends RenderProxyBox @override void performLayout() { - rebuildIfNecessary(); + runLayoutCallback(); child?.layout(constraints); } diff --git a/packages/flutter/test/widgets/overlay_layout_builder_test.dart b/packages/flutter/test/widgets/overlay_layout_builder_test.dart index 892b8d1645..ab7841a638 100644 --- a/packages/flutter/test/widgets/overlay_layout_builder_test.dart +++ b/packages/flutter/test/widgets/overlay_layout_builder_test.dart @@ -282,7 +282,7 @@ void main() { expect(paintTransform, Matrix4.translationValues(10.0, 20.0, 0.0) * transform); }); - testWidgets('Still works if child is null', (WidgetTester tester) async { + testWidgets('Still works if child and overlay child are null', (WidgetTester tester) async { late final OverlayEntry overlayEntry; addTearDown( () => @@ -307,7 +307,7 @@ void main() { controller: controller1, overlayChildBuilder: (BuildContext context, OverlayChildLayoutInfo layoutInfo) { regularChildSize = layoutInfo.childSize; - return const SizedBox(); + return const _NullLeaf(); }, child: null, ), @@ -363,3 +363,22 @@ void main() { ); }); } + +class _NullLeaf extends Widget { + const _NullLeaf(); + @override + Element createElement() => _NullElement(this); +} + +class _NullElement extends Element { + _NullElement(super.widget); + + @override + void mount(Element? parent, Object? newSlot) { + super.mount(parent, newSlot); + rebuild(force: true); + } + + @override + bool get debugDoingBuild => throw UnimplementedError(); +}