From 3a1a25339e4877f6f2fc837d228e769febe22c81 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Tue, 20 Sep 2022 10:28:57 -0700 Subject: [PATCH] [framework] avoid compositing with visibility (#111844) --- .../flutter/lib/src/widgets/visibility.dart | 148 ++++++++++++++++-- .../flutter/test/widgets/visibility_test.dart | 47 ++++++ 2 files changed, 185 insertions(+), 10 deletions(-) diff --git a/packages/flutter/lib/src/widgets/visibility.dart b/packages/flutter/lib/src/widgets/visibility.dart index d44af1eec6..5c8b91a5cb 100644 --- a/packages/flutter/lib/src/widgets/visibility.dart +++ b/packages/flutter/lib/src/widgets/visibility.dart @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import 'package:flutter/foundation.dart'; +import 'package:flutter/rendering.dart'; import 'basic.dart'; import 'framework.dart'; @@ -168,7 +168,7 @@ class Visibility extends StatelessWidget { /// [child] subtree is not trivial then it is significantly cheaper to not /// even keep the state (see [maintainState]). /// - /// If this property is true, [Opacity] is used instead of [Offstage]. + /// If this property is false, [Offstage] is used. /// /// If this property is false, then [maintainSemantics] and /// [maintainInteractivity] must also be false. @@ -222,9 +222,9 @@ class Visibility extends StatelessWidget { child: child, ); } - return Opacity( - opacity: visible ? 1.0 : 0.0, - alwaysIncludeSemantics: maintainSemantics, + return _Visibility( + visible: visible, + maintainSemantics: maintainSemantics, child: result, ); } @@ -410,8 +410,7 @@ class SliverVisibility extends StatelessWidget { /// [sliver] subtree is not trivial then it is significantly cheaper to not /// even keep the state (see [maintainState]). /// - /// If this property is true, [SliverOpacity] is used instead of - /// [SliverOffstage]. + /// If this property is false, [SliverOffstage] is used. /// /// If this property is false, then [maintainSemantics] and /// [maintainInteractivity] must also be false. @@ -460,9 +459,9 @@ class SliverVisibility extends StatelessWidget { ignoringSemantics: !visible && !maintainSemantics, ); } - return SliverOpacity( - opacity: visible ? 1.0 : 0.0, - alwaysIncludeSemantics: maintainSemantics, + return _SliverVisibility( + visible: visible, + maintainSemantics: maintainSemantics, sliver: result, ); } @@ -495,3 +494,132 @@ class SliverVisibility extends StatelessWidget { properties.add(FlagProperty('maintainInteractivity', value: maintainInteractivity, ifFalse: 'maintainInteractivity')); } } + +// A widget that conditionally hides its child, but without the forced compositing of `Opacity`. +// +// A fully opaque `Opacity` widget is required to leave its opacity layer in the layer tree. This +// forces all parent render objects to also composite, which can break a simple scene into many +// different layers. This can be significantly more expensive, so the issue is avoided by a +// specialized render object that does not ever force compositing. +class _Visibility extends SingleChildRenderObjectWidget { + const _Visibility({ required this.visible, required this.maintainSemantics, super.child }); + + final bool visible; + final bool maintainSemantics; + + @override + RenderObject createRenderObject(BuildContext context) { + return _RenderVisibility(visible, maintainSemantics); + } + + @override + void updateRenderObject(BuildContext context, covariant RenderObject renderObject) { + (renderObject as _RenderVisibility) + ..visible = visible + ..maintainSemantics = maintainSemantics; + } +} + +class _RenderVisibility extends RenderProxyBox { + _RenderVisibility(this._visible, this._maintainSemantics); + + bool get visible => _visible; + bool _visible; + set visible(bool value) { + if (value == visible) { + return; + } + _visible = value; + markNeedsPaint(); + } + + bool get maintainSemantics => _maintainSemantics; + bool _maintainSemantics; + set maintainSemantics(bool value) { + if (value == maintainSemantics) { + return; + } + _maintainSemantics = value; + markNeedsSemanticsUpdate(); + } + + @override + void visitChildrenForSemantics(RenderObjectVisitor visitor) { + if (maintainSemantics || visible) { + super.visitChildrenForSemantics(visitor); + } + } + + @override + void paint(PaintingContext context, Offset offset) { + if (!visible) { + return; + } + super.paint(context, offset); + } +} + +// A widget that conditionally hides its child, but without the forced compositing of `SliverOpacity`. +// +// A fully opaque `SliverOpacity` widget is required to leave its opacity layer in the layer tree. +// This forces all parent render objects to also composite, which can break a simple scene into many +// different layers. This can be significantly more expensive, so the issue is avoided by a +// specialized render object that does not ever force compositing. +class _SliverVisibility extends SingleChildRenderObjectWidget { + const _SliverVisibility({ required this.visible, required this.maintainSemantics, Widget? sliver }) + : super(child: sliver); + + final bool visible; + final bool maintainSemantics; + + @override + RenderObject createRenderObject(BuildContext context) { + return _RenderSliverVisibility(visible, maintainSemantics); + } + + @override + void updateRenderObject(BuildContext context, covariant RenderObject renderObject) { + (renderObject as _RenderSliverVisibility) + ..visible = visible + ..maintainSemantics = maintainSemantics; + } +} + +class _RenderSliverVisibility extends RenderProxySliver { + _RenderSliverVisibility(this._visible, this._maintainSemantics); + + bool get visible => _visible; + bool _visible; + set visible(bool value) { + if (value == visible) { + return; + } + _visible = value; + markNeedsPaint(); + } + + bool get maintainSemantics => _maintainSemantics; + bool _maintainSemantics; + set maintainSemantics(bool value) { + if (value == maintainSemantics) { + return; + } + _maintainSemantics = value; + markNeedsSemanticsUpdate(); + } + + @override + void visitChildrenForSemantics(RenderObjectVisitor visitor) { + if (maintainSemantics || visible) { + super.visitChildrenForSemantics(visitor); + } + } + + @override + void paint(PaintingContext context, Offset offset) { + if (!visible) { + return; + } + super.paint(context, offset); + } +} diff --git a/packages/flutter/test/widgets/visibility_test.dart b/packages/flutter/test/widgets/visibility_test.dart index ee4a5bf04e..6720830734 100644 --- a/packages/flutter/test/widgets/visibility_test.dart +++ b/packages/flutter/test/widgets/visibility_test.dart @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'package:flutter/rendering.dart'; import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; @@ -424,4 +425,50 @@ void main() { semantics.dispose(); }); + + testWidgets('Visibility does not force compositing when visible and maintain*', (WidgetTester tester) async { + await tester.pumpWidget( + const Visibility( + maintainSize: true, + maintainAnimation: true, + maintainState: true, + child: Text('hello', textDirection: TextDirection.ltr), + ), + ); + + // Root transform from the tester and then the picture created by the text. + expect(tester.layers, hasLength(2)); + expect(tester.layers, isNot(contains(isA()))); + expect(tester.layers.last, isA()); + }); + + testWidgets('SliverVisibility does not force compositing when visible and maintain*', (WidgetTester tester) async { + await tester.pumpWidget( + const Directionality( + textDirection: TextDirection.ltr, + child: CustomScrollView( + slivers: [ + SliverVisibility( + maintainSize: true, + maintainAnimation: true, + maintainState: true, + sliver: SliverList( + delegate: SliverChildListDelegate.fixed( + addRepaintBoundaries: false, + [ + Text('hello'), + ], + ), + )) + ] + ), + ), + ); + + // This requires a lot more layers due to including sliver lists which do manage additional + // offset layers. Just trust me this is one fewer layers than before... + expect(tester.layers, hasLength(6)); + expect(tester.layers, isNot(contains(isA()))); + expect(tester.layers.last, isA()); + }); }