Leader not always dirty (#92598)
diff --git a/packages/flutter/lib/src/rendering/editable.dart b/packages/flutter/lib/src/rendering/editable.dart index 234c533..dd19cf4 100644 --- a/packages/flutter/lib/src/rendering/editable.dart +++ b/packages/flutter/lib/src/rendering/editable.dart
@@ -2354,7 +2354,9 @@ _clipRectLayer.layer = null; _paintContents(context, offset); } - _paintHandleLayers(context, getEndpointsForSelection(selection!)); + if (selection!.isValid) { + _paintHandleLayers(context, getEndpointsForSelection(selection!)); + } } final LayerHandle<ClipRectLayer> _clipRectLayer = LayerHandle<ClipRectLayer>();
diff --git a/packages/flutter/lib/src/rendering/layer.dart b/packages/flutter/lib/src/rendering/layer.dart index 3568ffb..711303d 100644 --- a/packages/flutter/lib/src/rendering/layer.dart +++ b/packages/flutter/lib/src/rendering/layer.dart
@@ -2104,14 +2104,41 @@ /// * [RenderLeaderLayer] and [RenderFollowerLayer], the corresponding /// render objects. class LayerLink { - /// The currently-registered [LeaderLayer], if any. - LeaderLayer? get leader => _leader; LeaderLayer? _leader; - /// The total size of [leader]'s contents. + int _connectedFollowers = 0; + + /// Whether a [LeaderLayer] is currently connected to this link. + bool get leaderConnected => _leader != null; + + /// Called by the [FollowerLayer] to establish a link to a [LeaderLayer]. + /// + /// The returned [LayerLinkHandle] provides access to the leader via + /// [LayerLinkHandle.leader]. + /// + /// When the [FollowerLayer] no longer wants to follow the [LeaderLayer], + /// [LayerLinkHandle.dispose] must be called to disconnect the link. + _LayerLinkHandle _registerFollower() { + assert(_connectedFollowers >= 0); + _connectedFollowers++; + return _LayerLinkHandle(this); + } + + /// Returns the [LeaderLayer] currently connected to this link. + /// + /// Valid in debug mode only. Returns null in all other modes. + LeaderLayer? get debugLeader { + LeaderLayer? result; + if (kDebugMode) { + result = _leader; + } + return result; + } + + /// The total size of the content of the connected [LeaderLayer]. /// /// Generally this should be set by the [RenderObject] that paints on the - /// registered [leader] layer (for instance a [RenderLeaderLayer] that shares + /// registered [LeaderLayer] (for instance a [RenderLeaderLayer] that shares /// this link with its followers). This size may be outdated before and during /// layout. Size? leaderSize; @@ -2120,6 +2147,30 @@ String toString() => '${describeIdentity(this)}(${ _leader != null ? "<linked>" : "<dangling>" })'; } +/// A handle provided by [LayerLink.registerFollower] to a calling +/// [FollowerLayer] to establish a link between that [FollowerLayer] and a +/// [LeaderLayer]. +/// +/// If the link is no longer needed, [dispose] must be called to disconnect it. +class _LayerLinkHandle { + _LayerLinkHandle(this._link); + + LayerLink? _link; + + /// The currently-registered [LeaderLayer], if any. + LeaderLayer? get leader => _link!._leader; + + /// Disconnects the link between the [FollowerLayer] owning this handle and + /// the [leader]. + /// + /// The [LayerLinkHandle] becomes unusable after calling this method. + void dispose() { + assert(_link!._connectedFollowers > 0); + _link!._connectedFollowers--; + _link = null; + } +} + /// A composited layer that can be followed by a [FollowerLayer]. /// /// This layer collapses the accumulated offset into a transform and passes @@ -2134,18 +2185,22 @@ /// /// The [offset] property must be non-null before the compositing phase of the /// pipeline. - LeaderLayer({ required LayerLink link, this.offset = Offset.zero }) : assert(link != null), _link = link; + LeaderLayer({ required LayerLink link, Offset offset = Offset.zero }) : assert(link != null), _link = link, _offset = offset; /// The object with which this layer should register. /// /// The link will be established when this layer is [attach]ed, and will be /// cleared when this layer is [detach]ed. LayerLink get link => _link; + LayerLink _link; set link(LayerLink value) { assert(value != null); + if (_link == value) { + return; + } + _link._leader = null; _link = value; } - LayerLink _link; /// Offset from parent in the parent's coordinate system. /// @@ -2154,23 +2209,34 @@ /// /// The [offset] property must be non-null before the compositing phase of the /// pipeline. - Offset offset; + Offset get offset => _offset; + Offset _offset; + set offset(Offset value) { + assert(value != null); + if (value == _offset) { + return; + } + _offset = value; + if (!alwaysNeedsAddToScene) { + markNeedsAddToScene(); + } + } /// {@macro flutter.rendering.FollowerLayer.alwaysNeedsAddToScene} @override - bool get alwaysNeedsAddToScene => true; + bool get alwaysNeedsAddToScene => _link._connectedFollowers > 0; @override void attach(Object owner) { super.attach(owner); - assert(link.leader == null); + assert(link._leader == null); _lastOffset = null; link._leader = this; } @override void detach() { - assert(link.leader == this); + assert(link._leader == this); link._leader = null; _lastOffset = null; super.detach(); @@ -2256,6 +2322,10 @@ LayerLink get link => _link; set link(LayerLink value) { assert(value != null); + if (value != _link && _leaderHandle != null) { + _leaderHandle!.dispose(); + _leaderHandle = value._registerFollower(); + } _link = value; } LayerLink _link; @@ -2302,6 +2372,21 @@ /// * [unlinkedOffset], for when the layer is not linked. Offset? linkedOffset; + _LayerLinkHandle? _leaderHandle; + + @override + void attach(Object owner) { + super.attach(owner); + _leaderHandle = _link._registerFollower(); + } + + @override + void detach() { + super.detach(); + _leaderHandle?.dispose(); + _leaderHandle = null; + } + Offset? _lastOffset; Matrix4? _lastTransform; Matrix4? _invertedTransform; @@ -2321,7 +2406,7 @@ @override bool findAnnotations<S extends Object>(AnnotationResult<S> result, Offset localPosition, { required bool onlyFirst }) { - if (link.leader == null) { + if (_leaderHandle!.leader == null) { if (showWhenUnlinked!) { return super.findAnnotations(result, localPosition - unlinkedOffset!, onlyFirst: onlyFirst); } @@ -2400,7 +2485,7 @@ void _establishTransform() { assert(link != null); _lastTransform = null; - final LeaderLayer? leader = link.leader; + final LeaderLayer? leader = _leaderHandle!.leader; // Check to see if we are linked. if (leader == null) return; @@ -2462,7 +2547,7 @@ void addToScene(ui.SceneBuilder builder) { assert(link != null); assert(showWhenUnlinked != null); - if (link.leader == null && !showWhenUnlinked!) { + if (_leaderHandle!.leader == null && !showWhenUnlinked!) { _lastTransform = null; _lastOffset = null; _inverseDirty = true;
diff --git a/packages/flutter/lib/src/rendering/proxy_box.dart b/packages/flutter/lib/src/rendering/proxy_box.dart index c215f65..39680da 100644 --- a/packages/flutter/lib/src/rendering/proxy_box.dart +++ b/packages/flutter/lib/src/rendering/proxy_box.dart
@@ -5287,7 +5287,7 @@ @override bool hitTest(BoxHitTestResult result, { required Offset position }) { // Disables the hit testing if this render object is hidden. - if (link.leader == null && !showWhenUnlinked) + if (!link.leaderConnected && !showWhenUnlinked) return false; // RenderFollowerLayer objects don't check if they are // themselves hit, because it's confusing to think about @@ -5311,8 +5311,8 @@ void paint(PaintingContext context, Offset offset) { final Size? leaderSize = link.leaderSize; assert( - link.leaderSize != null || (link.leader == null || leaderAnchor == Alignment.topLeft), - '$link: layer is linked to ${link.leader} but a valid leaderSize is not set. ' + link.leaderSize != null || (!link.leaderConnected || leaderAnchor == Alignment.topLeft), + '$link: layer is linked to ${link.debugLeader} but a valid leaderSize is not set. ' 'leaderSize is required when leaderAnchor is not Alignment.topLeft ' '(current value is $leaderAnchor).', );
diff --git a/packages/flutter/test/material/text_field_test.dart b/packages/flutter/test/material/text_field_test.dart index 98a815d..6f369d9 100644 --- a/packages/flutter/test/material/text_field_test.dart +++ b/packages/flutter/test/material/text_field_test.dart
@@ -9967,4 +9967,73 @@ containsPair('hintText', 'placeholder text'), ); }); + + testWidgets('TextField at rest does not push any layers with alwaysNeedsAddToScene', (WidgetTester tester) async { + await tester.pumpWidget( + const MaterialApp( + home: Material( + child: Center( + child: TextField(), + ), + ), + ), + ); + + expect(tester.layers.any((Layer layer) => layer.debugSubtreeNeedsAddToScene!), isFalse); + }); + + testWidgets('Focused TextField does not push any layers with alwaysNeedsAddToScene', (WidgetTester tester) async { + final FocusNode focusNode = FocusNode(); + await tester.pumpWidget( + MaterialApp( + home: Material( + child: Center( + child: TextField(focusNode: focusNode), + ), + ), + ), + ); + await tester.showKeyboard(find.byType(TextField)); + + expect(focusNode.hasFocus, isTrue); + expect(tester.layers.any((Layer layer) => layer.debugSubtreeNeedsAddToScene!), isFalse); + }); + + testWidgets('TextField does not push any layers with alwaysNeedsAddToScene after toolbar is dismissed', (WidgetTester tester) async { + final FocusNode focusNode = FocusNode(); + await tester.pumpWidget( + MaterialApp( + home: Material( + child: Center( + child: TextField(focusNode: focusNode), + ), + ), + ), + ); + + await tester.showKeyboard(find.byType(TextField)); + + // Bring up the toolbar. + const String testValue = 'A B C'; + tester.testTextInput.updateEditingValue( + const TextEditingValue( + text: testValue, + ), + ); + await tester.pump(); + final EditableTextState state = tester.state<EditableTextState>(find.byType(EditableText)); + state.renderEditable.selectWordsInRange(from: Offset.zero, cause: SelectionChangedCause.tap); + expect(state.showToolbar(), true); + await tester.pumpAndSettle(); + await tester.pump(const Duration(seconds: 1)); + expect(find.text('Copy'), findsOneWidget); // Toolbar is visible + + // Hide the toolbar + focusNode.unfocus(); + await tester.pumpAndSettle(); + await tester.pump(const Duration(seconds: 1)); + expect(find.text('Copy'), findsNothing); // Toolbar is not visible + + expect(tester.layers.any((Layer layer) => layer.debugSubtreeNeedsAddToScene!), isFalse); + }, skip: isContextMenuProvidedByPlatform); // [intended] only applies to platforms where we supply the context menu. }
diff --git a/packages/flutter/test/rendering/layers_test.dart b/packages/flutter/test/rendering/layers_test.dart index 11c03fb..8d4e2dc 100644 --- a/packages/flutter/test/rendering/layers_test.dart +++ b/packages/flutter/test/rendering/layers_test.dart
@@ -153,7 +153,7 @@ } }); - test('leader and follower layers are always dirty', () { + test('follower layers are always dirty', () { final LayerLink link = LayerLink(); final LeaderLayer leaderLayer = LeaderLayer(link: link); final FollowerLayer followerLayer = FollowerLayer(link: link); @@ -161,10 +161,63 @@ followerLayer.debugMarkClean(); leaderLayer.updateSubtreeNeedsAddToScene(); followerLayer.updateSubtreeNeedsAddToScene(); - expect(leaderLayer.debugSubtreeNeedsAddToScene, true); expect(followerLayer.debugSubtreeNeedsAddToScene, true); }); + test('leader layers are always dirty when connected to follower layer', () { + final ContainerLayer root = ContainerLayer()..attach(Object()); + + final LayerLink link = LayerLink(); + final LeaderLayer leaderLayer = LeaderLayer(link: link); + final FollowerLayer followerLayer = FollowerLayer(link: link); + + root.append(leaderLayer); + root.append(followerLayer); + + leaderLayer.debugMarkClean(); + followerLayer.debugMarkClean(); + leaderLayer.updateSubtreeNeedsAddToScene(); + followerLayer.updateSubtreeNeedsAddToScene(); + expect(leaderLayer.debugSubtreeNeedsAddToScene, true); + }); + + test('leader layers are not dirty when all followers disconnects', () { + final ContainerLayer root = ContainerLayer()..attach(Object()); + final LayerLink link = LayerLink(); + final LeaderLayer leaderLayer = LeaderLayer(link: link); + root.append(leaderLayer); + + // Does not need add to scene when nothing is connected to link. + leaderLayer.debugMarkClean(); + leaderLayer.updateSubtreeNeedsAddToScene(); + expect(leaderLayer.debugSubtreeNeedsAddToScene, false); + + // Connecting a follower requires adding to scene. + final FollowerLayer follower1 = FollowerLayer(link: link); + root.append(follower1); + leaderLayer.debugMarkClean(); + leaderLayer.updateSubtreeNeedsAddToScene(); + expect(leaderLayer.debugSubtreeNeedsAddToScene, true); + + final FollowerLayer follower2 = FollowerLayer(link: link); + root.append(follower2); + leaderLayer.debugMarkClean(); + leaderLayer.updateSubtreeNeedsAddToScene(); + expect(leaderLayer.debugSubtreeNeedsAddToScene, true); + + // Disconnecting one follower, still needs add to scene. + follower2.remove(); + leaderLayer.debugMarkClean(); + leaderLayer.updateSubtreeNeedsAddToScene(); + expect(leaderLayer.debugSubtreeNeedsAddToScene, true); + + // Disconnecting all followers goes back to not requiring add to scene. + follower1.remove(); + leaderLayer.debugMarkClean(); + leaderLayer.updateSubtreeNeedsAddToScene(); + expect(leaderLayer.debugSubtreeNeedsAddToScene, false); + }); + test('depthFirstIterateChildren', () { final ContainerLayer a = ContainerLayer(); final ContainerLayer b = ContainerLayer();