Unverified Commit ca788d89 authored by Michael Goderbauer's avatar Michael Goderbauer Committed by GitHub

Leader not always dirty (#92598)

parent 7f177087
...@@ -2354,8 +2354,10 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin, ...@@ -2354,8 +2354,10 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin,
_clipRectLayer.layer = null; _clipRectLayer.layer = null;
_paintContents(context, offset); _paintContents(context, offset);
} }
if (selection!.isValid) {
_paintHandleLayers(context, getEndpointsForSelection(selection!)); _paintHandleLayers(context, getEndpointsForSelection(selection!));
} }
}
final LayerHandle<ClipRectLayer> _clipRectLayer = LayerHandle<ClipRectLayer>(); final LayerHandle<ClipRectLayer> _clipRectLayer = LayerHandle<ClipRectLayer>();
......
...@@ -2104,14 +2104,41 @@ class PhysicalModelLayer extends ContainerLayer { ...@@ -2104,14 +2104,41 @@ class PhysicalModelLayer extends ContainerLayer {
/// * [RenderLeaderLayer] and [RenderFollowerLayer], the corresponding /// * [RenderLeaderLayer] and [RenderFollowerLayer], the corresponding
/// render objects. /// render objects.
class LayerLink { class LayerLink {
/// The currently-registered [LeaderLayer], if any.
LeaderLayer? get leader => _leader;
LeaderLayer? _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 /// 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 /// this link with its followers). This size may be outdated before and during
/// layout. /// layout.
Size? leaderSize; Size? leaderSize;
...@@ -2120,6 +2147,30 @@ class LayerLink { ...@@ -2120,6 +2147,30 @@ class LayerLink {
String toString() => '${describeIdentity(this)}(${ _leader != null ? "<linked>" : "<dangling>" })'; 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]. /// A composited layer that can be followed by a [FollowerLayer].
/// ///
/// This layer collapses the accumulated offset into a transform and passes /// This layer collapses the accumulated offset into a transform and passes
...@@ -2134,18 +2185,22 @@ class LeaderLayer extends ContainerLayer { ...@@ -2134,18 +2185,22 @@ class LeaderLayer extends ContainerLayer {
/// ///
/// The [offset] property must be non-null before the compositing phase of the /// The [offset] property must be non-null before the compositing phase of the
/// pipeline. /// 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 object with which this layer should register.
/// ///
/// The link will be established when this layer is [attach]ed, and will be /// The link will be established when this layer is [attach]ed, and will be
/// cleared when this layer is [detach]ed. /// cleared when this layer is [detach]ed.
LayerLink get link => _link; LayerLink get link => _link;
LayerLink _link;
set link(LayerLink value) { set link(LayerLink value) {
assert(value != null); assert(value != null);
if (_link == value) {
return;
}
_link._leader = null;
_link = value; _link = value;
} }
LayerLink _link;
/// Offset from parent in the parent's coordinate system. /// Offset from parent in the parent's coordinate system.
/// ///
...@@ -2154,23 +2209,34 @@ class LeaderLayer extends ContainerLayer { ...@@ -2154,23 +2209,34 @@ class LeaderLayer extends ContainerLayer {
/// ///
/// The [offset] property must be non-null before the compositing phase of the /// The [offset] property must be non-null before the compositing phase of the
/// pipeline. /// 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} /// {@macro flutter.rendering.FollowerLayer.alwaysNeedsAddToScene}
@override @override
bool get alwaysNeedsAddToScene => true; bool get alwaysNeedsAddToScene => _link._connectedFollowers > 0;
@override @override
void attach(Object owner) { void attach(Object owner) {
super.attach(owner); super.attach(owner);
assert(link.leader == null); assert(link._leader == null);
_lastOffset = null; _lastOffset = null;
link._leader = this; link._leader = this;
} }
@override @override
void detach() { void detach() {
assert(link.leader == this); assert(link._leader == this);
link._leader = null; link._leader = null;
_lastOffset = null; _lastOffset = null;
super.detach(); super.detach();
...@@ -2256,6 +2322,10 @@ class FollowerLayer extends ContainerLayer { ...@@ -2256,6 +2322,10 @@ class FollowerLayer extends ContainerLayer {
LayerLink get link => _link; LayerLink get link => _link;
set link(LayerLink value) { set link(LayerLink value) {
assert(value != null); assert(value != null);
if (value != _link && _leaderHandle != null) {
_leaderHandle!.dispose();
_leaderHandle = value._registerFollower();
}
_link = value; _link = value;
} }
LayerLink _link; LayerLink _link;
...@@ -2302,6 +2372,21 @@ class FollowerLayer extends ContainerLayer { ...@@ -2302,6 +2372,21 @@ class FollowerLayer extends ContainerLayer {
/// * [unlinkedOffset], for when the layer is not linked. /// * [unlinkedOffset], for when the layer is not linked.
Offset? linkedOffset; 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; Offset? _lastOffset;
Matrix4? _lastTransform; Matrix4? _lastTransform;
Matrix4? _invertedTransform; Matrix4? _invertedTransform;
...@@ -2321,7 +2406,7 @@ class FollowerLayer extends ContainerLayer { ...@@ -2321,7 +2406,7 @@ class FollowerLayer extends ContainerLayer {
@override @override
bool findAnnotations<S extends Object>(AnnotationResult<S> result, Offset localPosition, { required bool onlyFirst }) { bool findAnnotations<S extends Object>(AnnotationResult<S> result, Offset localPosition, { required bool onlyFirst }) {
if (link.leader == null) { if (_leaderHandle!.leader == null) {
if (showWhenUnlinked!) { if (showWhenUnlinked!) {
return super.findAnnotations(result, localPosition - unlinkedOffset!, onlyFirst: onlyFirst); return super.findAnnotations(result, localPosition - unlinkedOffset!, onlyFirst: onlyFirst);
} }
...@@ -2400,7 +2485,7 @@ class FollowerLayer extends ContainerLayer { ...@@ -2400,7 +2485,7 @@ class FollowerLayer extends ContainerLayer {
void _establishTransform() { void _establishTransform() {
assert(link != null); assert(link != null);
_lastTransform = null; _lastTransform = null;
final LeaderLayer? leader = link.leader; final LeaderLayer? leader = _leaderHandle!.leader;
// Check to see if we are linked. // Check to see if we are linked.
if (leader == null) if (leader == null)
return; return;
...@@ -2462,7 +2547,7 @@ class FollowerLayer extends ContainerLayer { ...@@ -2462,7 +2547,7 @@ class FollowerLayer extends ContainerLayer {
void addToScene(ui.SceneBuilder builder) { void addToScene(ui.SceneBuilder builder) {
assert(link != null); assert(link != null);
assert(showWhenUnlinked != null); assert(showWhenUnlinked != null);
if (link.leader == null && !showWhenUnlinked!) { if (_leaderHandle!.leader == null && !showWhenUnlinked!) {
_lastTransform = null; _lastTransform = null;
_lastOffset = null; _lastOffset = null;
_inverseDirty = true; _inverseDirty = true;
......
...@@ -5287,7 +5287,7 @@ class RenderFollowerLayer extends RenderProxyBox { ...@@ -5287,7 +5287,7 @@ class RenderFollowerLayer extends RenderProxyBox {
@override @override
bool hitTest(BoxHitTestResult result, { required Offset position }) { bool hitTest(BoxHitTestResult result, { required Offset position }) {
// Disables the hit testing if this render object is hidden. // Disables the hit testing if this render object is hidden.
if (link.leader == null && !showWhenUnlinked) if (!link.leaderConnected && !showWhenUnlinked)
return false; return false;
// RenderFollowerLayer objects don't check if they are // RenderFollowerLayer objects don't check if they are
// themselves hit, because it's confusing to think about // themselves hit, because it's confusing to think about
...@@ -5311,8 +5311,8 @@ class RenderFollowerLayer extends RenderProxyBox { ...@@ -5311,8 +5311,8 @@ class RenderFollowerLayer extends RenderProxyBox {
void paint(PaintingContext context, Offset offset) { void paint(PaintingContext context, Offset offset) {
final Size? leaderSize = link.leaderSize; final Size? leaderSize = link.leaderSize;
assert( assert(
link.leaderSize != null || (link.leader == null || leaderAnchor == Alignment.topLeft), link.leaderSize != null || (!link.leaderConnected || leaderAnchor == Alignment.topLeft),
'$link: layer is linked to ${link.leader} but a valid leaderSize is not set. ' '$link: layer is linked to ${link.debugLeader} but a valid leaderSize is not set. '
'leaderSize is required when leaderAnchor is not Alignment.topLeft ' 'leaderSize is required when leaderAnchor is not Alignment.topLeft '
'(current value is $leaderAnchor).', '(current value is $leaderAnchor).',
); );
......
...@@ -9967,4 +9967,73 @@ void main() { ...@@ -9967,4 +9967,73 @@ void main() {
containsPair('hintText', 'placeholder text'), 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.
} }
...@@ -153,7 +153,7 @@ void main() { ...@@ -153,7 +153,7 @@ void main() {
} }
}); });
test('leader and follower layers are always dirty', () { test('follower layers are always dirty', () {
final LayerLink link = LayerLink(); final LayerLink link = LayerLink();
final LeaderLayer leaderLayer = LeaderLayer(link: link); final LeaderLayer leaderLayer = LeaderLayer(link: link);
final FollowerLayer followerLayer = FollowerLayer(link: link); final FollowerLayer followerLayer = FollowerLayer(link: link);
...@@ -161,10 +161,63 @@ void main() { ...@@ -161,10 +161,63 @@ void main() {
followerLayer.debugMarkClean(); followerLayer.debugMarkClean();
leaderLayer.updateSubtreeNeedsAddToScene(); leaderLayer.updateSubtreeNeedsAddToScene();
followerLayer.updateSubtreeNeedsAddToScene(); followerLayer.updateSubtreeNeedsAddToScene();
expect(leaderLayer.debugSubtreeNeedsAddToScene, true);
expect(followerLayer.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', () { test('depthFirstIterateChildren', () {
final ContainerLayer a = ContainerLayer(); final ContainerLayer a = ContainerLayer();
final ContainerLayer b = ContainerLayer(); final ContainerLayer b = ContainerLayer();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment