Leader not always dirty (flutter#92598)

Author: goderbauer

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

@@ -2354,7 +2354,9 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin,
       _clipRectLayer.layer = null;
       _paintContents(context, offset);
     }
-    _paintHandleLayers(context, getEndpointsForSelection(selection!));
+    if (selection!.isValid) {
+      _paintHandleLayers(context, getEndpointsForSelection(selection!));
+    }
   }
 
   final LayerHandle<ClipRectLayer> _clipRectLayer = LayerHandle<ClipRectLayer>();

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

@@ -2104,14 +2104,41 @@ class PhysicalModelLayer extends ContainerLayer {
 ///  * [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 @@ class LayerLink {
   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 @@ class LeaderLayer extends ContainerLayer {
   ///
   /// 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 @@ class LeaderLayer extends ContainerLayer {
   ///
   /// 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 @@ class FollowerLayer extends ContainerLayer {
   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 @@ class FollowerLayer extends ContainerLayer {
   ///  * [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 @@ class FollowerLayer extends ContainerLayer {
 
   @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 @@ class FollowerLayer extends ContainerLayer {
   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 @@ class FollowerLayer extends ContainerLayer {
   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;

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

@@ -5287,7 +5287,7 @@ class RenderFollowerLayer extends RenderProxyBox {
   @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 @@ class RenderFollowerLayer extends RenderProxyBox {
   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).',
     );

packages/flutter/test/material/text_field_test.dart

@@ -9967,4 +9967,73 @@ void main() {
       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.
 }

packages/flutter/test/rendering/layers_test.dart

@@ -153,18 +153,71 @@ void main() {
     }
   });
 
-  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);
     leaderLayer.debugMarkClean();
     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();