Skip to content

Commit d378515

Browse files
Merge dirty relayout boundaries after RenderObject.invokeLayoutCallback (#105175)
1 parent dc55ca6 commit d378515

File tree

2 files changed

+114
-2
lines changed

2 files changed

+114
-2
lines changed

packages/flutter/lib/src/rendering/object.dart

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -927,6 +927,21 @@ class PipelineOwner {
927927
_rootNode?.attach(this);
928928
}
929929

930+
// Whether the current [flushLayout] call should pause to incorporate the
931+
// [RenderObject]s in `_nodesNeedingLayout` into the current dirty list,
932+
// before continuing to process dirty relayout boundaries.
933+
//
934+
// This flag is set to true when a [RenderObject.invokeLayoutCallback]
935+
// returns, to avoid laying out dirty relayout boundaries in an incorrect
936+
// order and causing them to be laid out more than once per frame. See
937+
// layout_builder_mutations_test.dart for an example.
938+
//
939+
// The new dirty nodes are not immediately merged after a
940+
// [RenderObject.invokeLayoutCallback] call because we may encounter multiple
941+
// such calls while processing a single relayout boundary in [flushLayout].
942+
// Batching new dirty nodes can reduce the number of merges [flushLayout]
943+
// has to perform.
944+
bool _shouldMergeDirtyNodes = false;
930945
List<RenderObject> _nodesNeedingLayout = <RenderObject>[];
931946

932947
/// Whether this pipeline is currently in the layout phase.
@@ -968,15 +983,29 @@ class PipelineOwner {
968983
}());
969984
try {
970985
while (_nodesNeedingLayout.isNotEmpty) {
986+
assert(!_shouldMergeDirtyNodes);
971987
final List<RenderObject> dirtyNodes = _nodesNeedingLayout;
972988
_nodesNeedingLayout = <RenderObject>[];
973-
for (final RenderObject node in dirtyNodes..sort((RenderObject a, RenderObject b) => a.depth - b.depth)) {
989+
dirtyNodes.sort((RenderObject a, RenderObject b) => a.depth - b.depth);
990+
for (int i = 0; i < dirtyNodes.length; i++) {
991+
if (_shouldMergeDirtyNodes) {
992+
_shouldMergeDirtyNodes = false;
993+
if (_nodesNeedingLayout.isNotEmpty) {
994+
_nodesNeedingLayout.addAll(dirtyNodes.getRange(i, dirtyNodes.length));
995+
break;
996+
}
997+
}
998+
final RenderObject node = dirtyNodes[i];
974999
if (node._needsLayout && node.owner == this) {
9751000
node._layoutWithoutResize();
9761001
}
9771002
}
1003+
// No need to merge dirty nodes generated from processing the last
1004+
// relayout boundary back.
1005+
_shouldMergeDirtyNodes = false;
9781006
}
9791007
} finally {
1008+
_shouldMergeDirtyNodes = false;
9801009
assert(() {
9811010
_debugDoingLayout = false;
9821011
return true;
@@ -1006,6 +1035,7 @@ class PipelineOwner {
10061035
try {
10071036
callback();
10081037
} finally {
1038+
_shouldMergeDirtyNodes = true;
10091039
assert(() {
10101040
_debugAllowMutationsToDirtySubtrees = oldState!;
10111041
return true;

packages/flutter/test/widgets/layout_builder_mutations_test.dart

Lines changed: 83 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,12 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5-
import 'package:flutter/src/rendering/sliver.dart';
5+
import 'package:flutter/rendering.dart';
6+
import 'package:flutter/services.dart';
67
import 'package:flutter/src/widgets/basic.dart';
78
import 'package:flutter/src/widgets/framework.dart';
89
import 'package:flutter/src/widgets/layout_builder.dart';
10+
import 'package:flutter/src/widgets/media_query.dart';
911
import 'package:flutter/src/widgets/scroll_view.dart';
1012
import 'package:flutter/src/widgets/sliver_layout_builder.dart';
1113
import 'package:flutter_test/flutter_test.dart';
@@ -125,4 +127,84 @@ void main() {
125127

126128
expect(tester.takeException(), null);
127129
});
130+
131+
testWidgets('LayoutBuilder does not layout twice', (WidgetTester tester) async {
132+
// This widget marks itself dirty when the closest MediaQuery changes.
133+
final _LayoutCount widget = _LayoutCount();
134+
late StateSetter setState;
135+
bool updated = false;
136+
137+
await tester.pumpWidget(
138+
Directionality(
139+
textDirection: TextDirection.ltr,
140+
child: StatefulBuilder(
141+
builder: (BuildContext context, StateSetter setter) {
142+
setState = setter;
143+
return MediaQuery(
144+
data: updated
145+
? const MediaQueryData(platformBrightness: Brightness.dark)
146+
: const MediaQueryData(),
147+
child: LayoutBuilder(
148+
builder: (BuildContext context, BoxConstraints constraints) {
149+
return Center(
150+
child: SizedBox.square(
151+
dimension: 20,
152+
child: Center(
153+
child: SizedBox.square(
154+
dimension: updated ? 10 : 20,
155+
child: widget,
156+
),
157+
),
158+
),
159+
);
160+
},
161+
),
162+
);
163+
}
164+
),
165+
),
166+
);
167+
168+
assert(widget._renderObject.layoutCount == 1);
169+
setState(() { updated = true; });
170+
171+
await tester.pump();
172+
expect(widget._renderObject.layoutCount, 2);
173+
});
174+
}
175+
176+
class _LayoutCount extends LeafRenderObjectWidget {
177+
late final _RenderLayoutCount _renderObject;
178+
179+
@override
180+
RenderObject createRenderObject(BuildContext context) {
181+
return _renderObject = _RenderLayoutCount(MediaQuery.of(context));
182+
}
183+
184+
@override
185+
void updateRenderObject(BuildContext context, _RenderLayoutCount renderObject) {
186+
renderObject.mediaQuery = MediaQuery.of(context);
187+
}
188+
}
189+
190+
class _RenderLayoutCount extends RenderProxyBox {
191+
_RenderLayoutCount(this._mediaQuery);
192+
int layoutCount = 0;
193+
194+
MediaQueryData get mediaQuery => _mediaQuery;
195+
MediaQueryData _mediaQuery;
196+
set mediaQuery(MediaQueryData newValue) {
197+
if (newValue != _mediaQuery) {
198+
_mediaQuery = newValue;
199+
markNeedsLayout();
200+
}
201+
}
202+
203+
@override
204+
bool get sizedByParent => true;
205+
206+
@override
207+
void performLayout() {
208+
layoutCount += 1;
209+
}
128210
}

0 commit comments

Comments
 (0)