enable more retained rendering opportunities for LeaderLayer (#96486)

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

@@ -2109,6 +2109,8 @@ class PhysicalModelLayer extends ContainerLayer {
 ///  * [RenderLeaderLayer] and [RenderFollowerLayer], the corresponding
 ///    render objects.
 class LayerLink {
+  /// The [LeaderLayer] connected to this link.
+  LeaderLayer? get leader => _leader;
   LeaderLayer? _leader;
 
   void _registerLeader(LeaderLayer leader) {
@@ -2160,35 +2162,6 @@ class LayerLink {
     }());
   }
 
-  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
@@ -2201,30 +2174,6 @@ 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
@@ -2279,41 +2228,18 @@ class LeaderLayer extends ContainerLayer {
     }
   }
 
-  /// {@macro flutter.rendering.FollowerLayer.alwaysNeedsAddToScene}
-  @override
-  bool get alwaysNeedsAddToScene => _link._connectedFollowers > 0;
-
   @override
   void attach(Object owner) {
     super.attach(owner);
-    assert(_debugSetLastOffset(null));
     _link._registerLeader(this);
   }
 
   @override
   void detach() {
-    assert(_debugSetLastOffset(null));
     _link._unregisterLeader(this);
     super.detach();
   }
 
-  /// The offset the last time this layer was composited.
-  ///
-  /// This is reset to null when the layer is attached or detached, to help
-  /// catch cases where the follower layer ends up before the leader layer, but
-  /// not every case can be detected.
-  Offset? _debugLastOffset;
-
-  bool _debugSetLastOffset(Offset? offset) {
-    bool result = false;
-    assert(() {
-      _debugLastOffset = offset;
-      result = true;
-      return true;
-    }());
-    return result;
-  }
-
   @override
   bool findAnnotations<S extends Object>(AnnotationResult<S> result, Offset localPosition, { required bool onlyFirst }) {
     return super.findAnnotations<S>(result, localPosition - offset, onlyFirst: onlyFirst);
@@ -2322,7 +2248,6 @@ class LeaderLayer extends ContainerLayer {
   @override
   void addToScene(ui.SceneBuilder builder) {
     assert(offset != null);
-    assert(_debugSetLastOffset(offset));
     if (offset != Offset.zero)
       engineLayer = builder.pushTransform(
         Matrix4.translationValues(offset.dx, offset.dy, 0.0).storage,
@@ -2386,10 +2311,6 @@ 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;
@@ -2436,21 +2357,6 @@ 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;
@@ -2470,7 +2376,7 @@ class FollowerLayer extends ContainerLayer {
 
   @override
   bool findAnnotations<S extends Object>(AnnotationResult<S> result, Offset localPosition, { required bool onlyFirst }) {
-    if (_leaderHandle!.leader == null) {
+    if (_link.leader == null) {
       if (showWhenUnlinked!) {
         return super.findAnnotations(result, localPosition - unlinkedOffset!, onlyFirst: onlyFirst);
       }
@@ -2545,11 +2451,39 @@ class FollowerLayer extends ContainerLayer {
     return _pathsToCommonAncestor(a.parent, b.parent, ancestorsA, ancestorsB);
   }
 
+  bool _debugCheckLeaderBeforeFollower(
+    List<ContainerLayer> leaderToCommonAncestor,
+    List<ContainerLayer> followerToCommonAncestor,
+  ) {
+    if (followerToCommonAncestor.length <= 1) {
+      // Follower is the common ancestor, ergo the leader must come AFTER the follower.
+      return false;
+    }
+    if (leaderToCommonAncestor.length <= 1) {
+      // Leader is the common ancestor, ergo the leader must come BEFORE the follower.
+      return true;
+    }
+
+    // Common ancestor is neither the leader nor the follower.
+    final ContainerLayer leaderSubtreeBelowAncestor = leaderToCommonAncestor[leaderToCommonAncestor.length - 2];
+    final ContainerLayer followerSubtreeBelowAncestor = followerToCommonAncestor[followerToCommonAncestor.length - 2];
+
+    Layer? sibling = leaderSubtreeBelowAncestor;
+    while (sibling != null) {
+      if (sibling == followerSubtreeBelowAncestor) {
+        return true;
+      }
+      sibling = sibling.nextSibling;
+    }
+    // The follower subtree didn't come after the leader subtree.
+    return false;
+  }
+
   /// Populate [_lastTransform] given the current state of the tree.
   void _establishTransform() {
     assert(link != null);
     _lastTransform = null;
-    final LeaderLayer? leader = _leaderHandle!.leader;
+    final LeaderLayer? leader = _link.leader;
     // Check to see if we are linked.
     if (leader == null)
       return;
@@ -2558,22 +2492,25 @@ class FollowerLayer extends ContainerLayer {
       leader.owner == owner,
       'Linked LeaderLayer anchor is not in the same layer tree as the FollowerLayer.',
     );
-    assert(
-      leader._debugLastOffset != null,
-      'LeaderLayer anchor must come before FollowerLayer in paint order, but the reverse was true.',
-    );
 
     // Stores [leader, ..., commonAncestor] after calling _pathsToCommonAncestor.
-    final List<ContainerLayer?> forwardLayers = <ContainerLayer>[leader];
+    final List<ContainerLayer> forwardLayers = <ContainerLayer>[leader];
     // Stores [this (follower), ..., commonAncestor] after calling
     // _pathsToCommonAncestor.
-    final List<ContainerLayer?> inverseLayers = <ContainerLayer>[this];
+    final List<ContainerLayer> inverseLayers = <ContainerLayer>[this];
 
     final Layer? ancestor = _pathsToCommonAncestor(
       leader, this,
       forwardLayers, inverseLayers,
     );
-    assert(ancestor != null);
+    assert(
+      ancestor != null,
+      'LeaderLayer and FollowerLayer do not have a common ancestor.',
+    );
+    assert(
+      _debugCheckLeaderBeforeFollower(forwardLayers, inverseLayers),
+      'LeaderLayer anchor must come before FollowerLayer in paint order, but the reverse was true.',
+    );
 
     final Matrix4 forwardTransform = _collectTransformForLayerChain(forwardLayers);
     // Further transforms the coordinate system to a hypothetical child (null)
@@ -2611,7 +2548,7 @@ class FollowerLayer extends ContainerLayer {
   void addToScene(ui.SceneBuilder builder) {
     assert(link != null);
     assert(showWhenUnlinked != null);
-    if (_leaderHandle!.leader == null && !showWhenUnlinked!) {
+    if (_link.leader == null && !showWhenUnlinked!) {
       _lastTransform = null;
       _lastOffset = null;
       _inverseDirty = true;

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

@@ -5289,7 +5289,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.leaderConnected && !showWhenUnlinked)
+    if (link.leader == null && !showWhenUnlinked)
       return false;
     // RenderFollowerLayer objects don't check if they are
     // themselves hit, because it's confusing to think about
@@ -5313,8 +5313,8 @@ class RenderFollowerLayer extends RenderProxyBox {
   void paint(PaintingContext context, Offset offset) {
     final Size? leaderSize = link.leaderSize;
     assert(
-      link.leaderSize != null || (!link.leaderConnected || leaderAnchor == Alignment.topLeft),
-      '$link: layer is linked to ${link.debugLeader} but a valid leaderSize is not set. '
+      link.leaderSize != null || (link.leader == null || leaderAnchor == Alignment.topLeft),
+      '$link: layer is linked to ${link.leader} but a valid leaderSize is not set. '
       'leaderSize is required when leaderAnchor is not Alignment.topLeft '
       '(current value is $leaderAnchor).',
     );

packages/flutter/test/rendering/layers_test.dart

@@ -176,7 +176,7 @@ void main() {
     expect(leaderLayer.link, link2);
   });
 
-  test('leader layers are always dirty when connected to follower layer', () {
+  test('leader layers not dirty when connected to follower layer', () {
     final ContainerLayer root = ContainerLayer()..attach(Object());
 
     final LayerLink link = LayerLink();
@@ -190,7 +190,7 @@ void main() {
     followerLayer.debugMarkClean();
     leaderLayer.updateSubtreeNeedsAddToScene();
     followerLayer.updateSubtreeNeedsAddToScene();
-    expect(leaderLayer.debugSubtreeNeedsAddToScene, true);
+    expect(leaderLayer.debugSubtreeNeedsAddToScene, false);
   });
 
   test('leader layers are not dirty when all followers disconnects', () {
@@ -204,24 +204,24 @@ void main() {
     leaderLayer.updateSubtreeNeedsAddToScene();
     expect(leaderLayer.debugSubtreeNeedsAddToScene, false);
 
-    // Connecting a follower requires adding to scene.
+    // Connecting a follower does not require adding to scene
     final FollowerLayer follower1 = FollowerLayer(link: link);
     root.append(follower1);
     leaderLayer.debugMarkClean();
     leaderLayer.updateSubtreeNeedsAddToScene();
-    expect(leaderLayer.debugSubtreeNeedsAddToScene, true);
+    expect(leaderLayer.debugSubtreeNeedsAddToScene, false);
 
     final FollowerLayer follower2 = FollowerLayer(link: link);
     root.append(follower2);
     leaderLayer.debugMarkClean();
     leaderLayer.updateSubtreeNeedsAddToScene();
-    expect(leaderLayer.debugSubtreeNeedsAddToScene, true);
+    expect(leaderLayer.debugSubtreeNeedsAddToScene, false);
 
-    // Disconnecting one follower, still needs add to scene.
+    // Disconnecting one follower, still does not needs add to scene.
     follower2.remove();
     leaderLayer.debugMarkClean();
     leaderLayer.updateSubtreeNeedsAddToScene();
-    expect(leaderLayer.debugSubtreeNeedsAddToScene, true);
+    expect(leaderLayer.debugSubtreeNeedsAddToScene, false);
 
     // Disconnecting all followers goes back to not requiring add to scene.
     follower1.remove();

packages/flutter/test/widgets/composited_transform_test.dart

@@ -300,4 +300,22 @@ void main() {
       }
     }
   });
+
+  testWidgets('Leader after Follower asserts', (WidgetTester tester) async {
+    final LayerLink link = LayerLink();
+    await tester.pumpWidget(
+      CompositedTransformFollower(
+        link: link,
+        child: CompositedTransformTarget(
+          link: link,
+          child: const SizedBox(height: 20, width: 20),
+        ),
+      ),
+    );
+
+    expect(
+      (tester.takeException() as AssertionError).message,
+      contains('LeaderLayer anchor must come before FollowerLayer in paint order'),
+    );
+  });
 }