enable more retained rendering opportunities for LeaderLayer (#96486)
diff --git a/packages/flutter/lib/src/rendering/layer.dart b/packages/flutter/lib/src/rendering/layer.dart
index eee4ee8..73310e7 100644
--- a/packages/flutter/lib/src/rendering/layer.dart
+++ b/packages/flutter/lib/src/rendering/layer.dart
@@ -2109,6 +2109,8 @@
/// * [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 @@
}());
}
- 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 @@
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 @@
}
}
- /// {@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 @@
@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 @@
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 @@
/// * [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 @@
@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 @@
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 @@
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 @@
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;
diff --git a/packages/flutter/lib/src/rendering/proxy_box.dart b/packages/flutter/lib/src/rendering/proxy_box.dart
index b3aee53..9e1caa3 100644
--- a/packages/flutter/lib/src/rendering/proxy_box.dart
+++ b/packages/flutter/lib/src/rendering/proxy_box.dart
@@ -5289,7 +5289,7 @@
@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 @@
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).',
);
diff --git a/packages/flutter/test/rendering/layers_test.dart b/packages/flutter/test/rendering/layers_test.dart
index cc63c23..361fb6a 100644
--- a/packages/flutter/test/rendering/layers_test.dart
+++ b/packages/flutter/test/rendering/layers_test.dart
@@ -176,7 +176,7 @@
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 @@
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 @@
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();
diff --git a/packages/flutter/test/widgets/composited_transform_test.dart b/packages/flutter/test/widgets/composited_transform_test.dart
index 4e18b41..daaff69 100644
--- a/packages/flutter/test/widgets/composited_transform_test.dart
+++ b/packages/flutter/test/widgets/composited_transform_test.dart
@@ -300,4 +300,22 @@
}
}
});
+
+ 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'),
+ );
+ });
}
