From 9db9256b38ea33641b7bc02b537501ccde3a9957 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Mon, 13 Sep 2021 13:15:19 -0700 Subject: [PATCH] Revert "Make sure Opacity widgets/layers do not drop the offset (#89264)" (#89999) This reverts commit 0d0f7a4fb065da5de8640f1d6757cd5b9bc5534b. --- packages/flutter/lib/src/rendering/layer.dart | 48 +++++++++++-------- .../flutter/lib/src/rendering/proxy_box.dart | 16 ++++++- .../lib/src/rendering/proxy_sliver.dart | 8 +++- .../flutter/test/rendering/debug_test.dart | 26 ---------- .../test/rendering/proxy_box_test.dart | 8 ++-- .../test/rendering/proxy_sliver_test.dart | 8 ++-- .../flutter/test/widgets/opacity_test.dart | 31 ------------ 7 files changed, 56 insertions(+), 89 deletions(-) diff --git a/packages/flutter/lib/src/rendering/layer.dart b/packages/flutter/lib/src/rendering/layer.dart index 3cc7bffee7..085ee1bfbb 100644 --- a/packages/flutter/lib/src/rendering/layer.dart +++ b/packages/flutter/lib/src/rendering/layer.dart @@ -1741,7 +1741,7 @@ class TransformLayer extends OffsetLayer { /// /// Try to avoid an [OpacityLayer] with no children. Remove that layer if /// possible to save some tree walks. -class OpacityLayer extends OffsetLayer { +class OpacityLayer extends ContainerLayer { /// Creates an opacity layer. /// /// The [alpha] property must be non-null before the compositing phase of @@ -1750,7 +1750,7 @@ class OpacityLayer extends OffsetLayer { int? alpha, Offset offset = Offset.zero, }) : _alpha = alpha, - super(offset: offset); + _offset = offset; /// The amount to multiply into the alpha channel. /// @@ -1764,14 +1764,28 @@ class OpacityLayer extends OffsetLayer { set alpha(int? value) { assert(value != null); if (value != _alpha) { - if (value == 255 || _alpha == 255) { - engineLayer = null; - } _alpha = value; markNeedsAddToScene(); } } + /// Offset from parent in the parent's coordinate system. + Offset? get offset => _offset; + Offset? _offset; + set offset(Offset? value) { + if (value != _offset) { + _offset = value; + markNeedsAddToScene(); + } + } + + @override + void applyTransform(Layer? child, Matrix4 transform) { + assert(child != null); + assert(transform != null); + transform.translate(offset!.dx, offset!.dy); + } + @override void addToScene(ui.SceneBuilder builder, [ Offset layerOffset = Offset.zero ]) { assert(alpha != null); @@ -1781,32 +1795,24 @@ class OpacityLayer extends OffsetLayer { return true; }()); - final int realizedAlpha = alpha!; - // The type assertions work because the [alpha] setter nulls out the - // engineLayer if it would have changed type (i.e. changed to or from 255). - if (enabled && realizedAlpha < 255) { - assert(_engineLayer is ui.OpacityEngineLayer?); + if (enabled) engineLayer = builder.pushOpacity( - realizedAlpha, - offset: offset + layerOffset, + alpha!, + offset: offset! + layerOffset, oldLayer: _engineLayer as ui.OpacityEngineLayer?, ); - } else { - assert(_engineLayer is ui.OffsetEngineLayer?); - engineLayer = builder.pushOffset( - layerOffset.dx + offset.dx, - layerOffset.dy + offset.dy, - oldLayer: _engineLayer as ui.OffsetEngineLayer?, - ); - } + else + engineLayer = null; addChildrenToScene(builder); - builder.pop(); + if (enabled) + builder.pop(); } @override void debugFillProperties(DiagnosticPropertiesBuilder properties) { super.debugFillProperties(properties); properties.add(IntProperty('alpha', alpha)); + properties.add(DiagnosticsProperty('offset', offset)); } } diff --git a/packages/flutter/lib/src/rendering/proxy_box.dart b/packages/flutter/lib/src/rendering/proxy_box.dart index adadf52fab..8c39deae41 100644 --- a/packages/flutter/lib/src/rendering/proxy_box.dart +++ b/packages/flutter/lib/src/rendering/proxy_box.dart @@ -843,7 +843,7 @@ class RenderOpacity extends RenderProxyBox { super(child); @override - bool get alwaysNeedsCompositing => child != null && (_alpha > 0); + bool get alwaysNeedsCompositing => child != null && (_alpha != 0 && _alpha != 255); int _alpha; @@ -897,6 +897,12 @@ class RenderOpacity extends RenderProxyBox { layer = null; return; } + if (_alpha == 255) { + // No need to keep the layer. We'll create a new one if necessary. + layer = null; + context.paintChild(child!, offset); + return; + } assert(needsCompositing); layer = context.pushOpacity(offset, _alpha, super.paint, oldLayer: layer as OpacityLayer?); } @@ -987,7 +993,7 @@ mixin RenderAnimatedOpacityMixin on RenderObjectWithChil _alpha = ui.Color.getAlphaFromOpacity(opacity.value); if (oldAlpha != _alpha) { final bool? didNeedCompositing = _currentlyNeedsCompositing; - _currentlyNeedsCompositing = _alpha! > 0; + _currentlyNeedsCompositing = _alpha! > 0 && _alpha! < 255; if (child != null && didNeedCompositing != _currentlyNeedsCompositing) markNeedsCompositingBitsUpdate(); markNeedsPaint(); @@ -1004,6 +1010,12 @@ mixin RenderAnimatedOpacityMixin on RenderObjectWithChil layer = null; return; } + if (_alpha == 255) { + // No need to keep the layer. We'll create a new one if necessary. + layer = null; + context.paintChild(child!, offset); + return; + } assert(needsCompositing); layer = context.pushOpacity(offset, _alpha!, super.paint, oldLayer: layer as OpacityLayer?); } diff --git a/packages/flutter/lib/src/rendering/proxy_sliver.dart b/packages/flutter/lib/src/rendering/proxy_sliver.dart index 520e0d1bba..00cf7fd6f7 100644 --- a/packages/flutter/lib/src/rendering/proxy_sliver.dart +++ b/packages/flutter/lib/src/rendering/proxy_sliver.dart @@ -112,7 +112,7 @@ class RenderSliverOpacity extends RenderProxySliver { } @override - bool get alwaysNeedsCompositing => child != null && (_alpha > 0); + bool get alwaysNeedsCompositing => child != null && (_alpha != 0 && _alpha != 255); int _alpha; @@ -166,6 +166,12 @@ class RenderSliverOpacity extends RenderProxySliver { layer = null; return; } + if (_alpha == 255) { + // No need to keep the layer. We'll create a new one if necessary. + layer = null; + context.paintChild(child!, offset); + return; + } assert(needsCompositing); layer = context.pushOpacity( offset, diff --git a/packages/flutter/test/rendering/debug_test.dart b/packages/flutter/test/rendering/debug_test.dart index dce60cc9f2..3bc405840e 100644 --- a/packages/flutter/test/rendering/debug_test.dart +++ b/packages/flutter/test/rendering/debug_test.dart @@ -208,30 +208,4 @@ void main() { expect(error, isNull); debugPaintSizeEnabled = false; }); - - test('debugDisableOpacity keeps things in the right spot', () { - debugDisableOpacityLayers = true; - - final RenderDecoratedBox blackBox = RenderDecoratedBox( - decoration: const BoxDecoration(color: Color(0xff000000)), - child: RenderConstrainedBox( - additionalConstraints: BoxConstraints.tight(const Size.square(20.0)), - ), - ); - final RenderOpacity root = RenderOpacity( - opacity: .5, - child: blackBox, - ); - layout(root, phase: EnginePhase.compositingBits); - - final OffsetLayer rootLayer = OffsetLayer(offset: Offset.zero); - final PaintingContext context = PaintingContext( - rootLayer, - const Rect.fromLTWH(0, 0, 500, 500), - ); - root.paint(context, const Offset(40, 40)); - final OpacityLayer opacityLayer = rootLayer.firstChild! as OpacityLayer; - expect(opacityLayer.offset, const Offset(40, 40)); - debugDisableOpacityLayers = false; - }); } diff --git a/packages/flutter/test/rendering/proxy_box_test.dart b/packages/flutter/test/rendering/proxy_box_test.dart index 6733cce245..6693af182b 100644 --- a/packages/flutter/test/rendering/proxy_box_test.dart +++ b/packages/flutter/test/rendering/proxy_box_test.dart @@ -274,14 +274,14 @@ void main() { expect(renderOpacity.needsCompositing, false); }); - test('RenderOpacity does composite if it is opaque', () { + test('RenderOpacity does not composite if it is opaque', () { final RenderOpacity renderOpacity = RenderOpacity( opacity: 1.0, child: RenderSizedBox(const Size(1.0, 1.0)), // size doesn't matter ); layout(renderOpacity, phase: EnginePhase.composite); - expect(renderOpacity.needsCompositing, true); + expect(renderOpacity.needsCompositing, false); }); test('RenderOpacity reuses its layer', () { @@ -306,7 +306,7 @@ void main() { expect(renderAnimatedOpacity.needsCompositing, false); }); - test('RenderAnimatedOpacity does composite if it is opaque', () { + test('RenderAnimatedOpacity does not composite if it is opaque', () { final Animation opacityAnimation = AnimationController( vsync: FakeTickerProvider(), )..value = 1.0; @@ -318,7 +318,7 @@ void main() { ); layout(renderAnimatedOpacity, phase: EnginePhase.composite); - expect(renderAnimatedOpacity.needsCompositing, true); + expect(renderAnimatedOpacity.needsCompositing, false); }); test('RenderAnimatedOpacity reuses its layer', () { diff --git a/packages/flutter/test/rendering/proxy_sliver_test.dart b/packages/flutter/test/rendering/proxy_sliver_test.dart index a197f8292f..85169d2d5d 100644 --- a/packages/flutter/test/rendering/proxy_sliver_test.dart +++ b/packages/flutter/test/rendering/proxy_sliver_test.dart @@ -29,7 +29,7 @@ void main() { expect(renderSliverOpacity.needsCompositing, false); }); - test('RenderSliverOpacity does composite if it is opaque', () { + test('RenderSliverOpacity does not composite if it is opaque', () { final RenderSliverOpacity renderSliverOpacity = RenderSliverOpacity( opacity: 1.0, sliver: RenderSliverToBoxAdapter( @@ -46,7 +46,7 @@ void main() { ); layout(root, phase: EnginePhase.composite); - expect(renderSliverOpacity.needsCompositing, true); + expect(renderSliverOpacity.needsCompositing, false); }); test('RenderSliverOpacity reuses its layer', () { final RenderSliverOpacity renderSliverOpacity = RenderSliverOpacity( @@ -102,7 +102,7 @@ void main() { expect(renderSliverAnimatedOpacity.needsCompositing, false); }); - test('RenderSliverAnimatedOpacity does composite if it is opaque', () { + test('RenderSliverAnimatedOpacity does not composite if it is opaque', () { final Animation opacityAnimation = AnimationController( vsync: FakeTickerProvider(), )..value = 1.0; @@ -124,7 +124,7 @@ void main() { ); layout(root, phase: EnginePhase.composite); - expect(renderSliverAnimatedOpacity.needsCompositing, true); + expect(renderSliverAnimatedOpacity.needsCompositing, false); }); test('RenderSliverAnimatedOpacity reuses its layer', () { diff --git a/packages/flutter/test/widgets/opacity_test.dart b/packages/flutter/test/widgets/opacity_test.dart index 23af74d472..f3225c92a4 100644 --- a/packages/flutter/test/widgets/opacity_test.dart +++ b/packages/flutter/test/widgets/opacity_test.dart @@ -195,35 +195,4 @@ void main() { final OffsetLayer offsetLayer = element.renderObject!.debugLayer! as OffsetLayer; await offsetLayer.toImage(const Rect.fromLTRB(0.0, 0.0, 1.0, 1.0)); }, skip: isBrowser); // https://github.com/flutter/flutter/issues/49857 - - testWidgets('Child shows up in the right spot when opacity is disabled', (WidgetTester tester) async { - debugDisableOpacityLayers = true; - final GlobalKey key = GlobalKey(); - await tester.pumpWidget( - RepaintBoundary( - key: key, - child: Directionality( - textDirection: TextDirection.ltr, - child: Stack( - children: [ - Positioned( - top: 40, - left: 140, - child: Opacity( - opacity: .5, - child: Container(height: 100, width: 100, color: Colors.red), - ), - ), - ], - ), - ), - ), - ); - - await expectLater( - find.byKey(key), - matchesGoldenFile('opacity_disabled_with_child.png'), - ); - debugDisableOpacityLayers = false; - }); }