From d3785152c4e624227345d7766faa4f0e41d7edc0 Mon Sep 17 00:00:00 2001 From: LongCatIsLooong <31859944+LongCatIsLooong@users.noreply.github.com> Date: Thu, 2 Jun 2022 18:28:10 -0700 Subject: [PATCH] Merge dirty relayout boundaries after `RenderObject.invokeLayoutCallback` (#105175) --- .../flutter/lib/src/rendering/object.dart | 32 ++++++- .../layout_builder_mutations_test.dart | 84 ++++++++++++++++++- 2 files changed, 114 insertions(+), 2 deletions(-) diff --git a/packages/flutter/lib/src/rendering/object.dart b/packages/flutter/lib/src/rendering/object.dart index d59ef1e046e..1da0ad75f52 100644 --- a/packages/flutter/lib/src/rendering/object.dart +++ b/packages/flutter/lib/src/rendering/object.dart @@ -927,6 +927,21 @@ class PipelineOwner { _rootNode?.attach(this); } + // Whether the current [flushLayout] call should pause to incorporate the + // [RenderObject]s in `_nodesNeedingLayout` into the current dirty list, + // before continuing to process dirty relayout boundaries. + // + // This flag is set to true when a [RenderObject.invokeLayoutCallback] + // returns, to avoid laying out dirty relayout boundaries in an incorrect + // order and causing them to be laid out more than once per frame. See + // layout_builder_mutations_test.dart for an example. + // + // The new dirty nodes are not immediately merged after a + // [RenderObject.invokeLayoutCallback] call because we may encounter multiple + // such calls while processing a single relayout boundary in [flushLayout]. + // Batching new dirty nodes can reduce the number of merges [flushLayout] + // has to perform. + bool _shouldMergeDirtyNodes = false; List _nodesNeedingLayout = []; /// Whether this pipeline is currently in the layout phase. @@ -968,15 +983,29 @@ class PipelineOwner { }()); try { while (_nodesNeedingLayout.isNotEmpty) { + assert(!_shouldMergeDirtyNodes); final List dirtyNodes = _nodesNeedingLayout; _nodesNeedingLayout = []; - for (final RenderObject node in dirtyNodes..sort((RenderObject a, RenderObject b) => a.depth - b.depth)) { + dirtyNodes.sort((RenderObject a, RenderObject b) => a.depth - b.depth); + for (int i = 0; i < dirtyNodes.length; i++) { + if (_shouldMergeDirtyNodes) { + _shouldMergeDirtyNodes = false; + if (_nodesNeedingLayout.isNotEmpty) { + _nodesNeedingLayout.addAll(dirtyNodes.getRange(i, dirtyNodes.length)); + break; + } + } + final RenderObject node = dirtyNodes[i]; if (node._needsLayout && node.owner == this) { node._layoutWithoutResize(); } } + // No need to merge dirty nodes generated from processing the last + // relayout boundary back. + _shouldMergeDirtyNodes = false; } } finally { + _shouldMergeDirtyNodes = false; assert(() { _debugDoingLayout = false; return true; @@ -1006,6 +1035,7 @@ class PipelineOwner { try { callback(); } finally { + _shouldMergeDirtyNodes = true; assert(() { _debugAllowMutationsToDirtySubtrees = oldState!; return true; diff --git a/packages/flutter/test/widgets/layout_builder_mutations_test.dart b/packages/flutter/test/widgets/layout_builder_mutations_test.dart index 1c764a3cc12..f20ca55cc80 100644 --- a/packages/flutter/test/widgets/layout_builder_mutations_test.dart +++ b/packages/flutter/test/widgets/layout_builder_mutations_test.dart @@ -2,10 +2,12 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import 'package:flutter/src/rendering/sliver.dart'; +import 'package:flutter/rendering.dart'; +import 'package:flutter/services.dart'; import 'package:flutter/src/widgets/basic.dart'; import 'package:flutter/src/widgets/framework.dart'; import 'package:flutter/src/widgets/layout_builder.dart'; +import 'package:flutter/src/widgets/media_query.dart'; import 'package:flutter/src/widgets/scroll_view.dart'; import 'package:flutter/src/widgets/sliver_layout_builder.dart'; import 'package:flutter_test/flutter_test.dart'; @@ -125,4 +127,84 @@ void main() { expect(tester.takeException(), null); }); + + testWidgets('LayoutBuilder does not layout twice', (WidgetTester tester) async { + // This widget marks itself dirty when the closest MediaQuery changes. + final _LayoutCount widget = _LayoutCount(); + late StateSetter setState; + bool updated = false; + + await tester.pumpWidget( + Directionality( + textDirection: TextDirection.ltr, + child: StatefulBuilder( + builder: (BuildContext context, StateSetter setter) { + setState = setter; + return MediaQuery( + data: updated + ? const MediaQueryData(platformBrightness: Brightness.dark) + : const MediaQueryData(), + child: LayoutBuilder( + builder: (BuildContext context, BoxConstraints constraints) { + return Center( + child: SizedBox.square( + dimension: 20, + child: Center( + child: SizedBox.square( + dimension: updated ? 10 : 20, + child: widget, + ), + ), + ), + ); + }, + ), + ); + } + ), + ), + ); + + assert(widget._renderObject.layoutCount == 1); + setState(() { updated = true; }); + + await tester.pump(); + expect(widget._renderObject.layoutCount, 2); + }); +} + +class _LayoutCount extends LeafRenderObjectWidget { + late final _RenderLayoutCount _renderObject; + + @override + RenderObject createRenderObject(BuildContext context) { + return _renderObject = _RenderLayoutCount(MediaQuery.of(context)); + } + + @override + void updateRenderObject(BuildContext context, _RenderLayoutCount renderObject) { + renderObject.mediaQuery = MediaQuery.of(context); + } +} + +class _RenderLayoutCount extends RenderProxyBox { + _RenderLayoutCount(this._mediaQuery); + int layoutCount = 0; + + MediaQueryData get mediaQuery => _mediaQuery; + MediaQueryData _mediaQuery; + set mediaQuery(MediaQueryData newValue) { + if (newValue != _mediaQuery) { + _mediaQuery = newValue; + markNeedsLayout(); + } + } + + @override + bool get sizedByParent => true; + + @override + void performLayout() { + layoutCount += 1; + } }