Opacity fix (#90017)

* Make sure Opacity widgets/layers do not drop the offset

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 ContainerLayer {
+class OpacityLayer extends OffsetLayer {
   /// Creates an opacity layer.
   ///
   /// The [alpha] property must be non-null before the compositing phase of
@@ -1750,7 +1750,7 @@ class OpacityLayer extends ContainerLayer {
     int? alpha,
     Offset offset = Offset.zero,
   }) : _alpha = alpha,
-       _offset = offset;
+       super(offset: offset);
 
   /// The amount to multiply into the alpha channel.
   ///
@@ -1764,55 +1764,53 @@ class OpacityLayer extends ContainerLayer {
   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);
     bool enabled = firstChild != null;  // don't add this layer if there's no child
+    if (!enabled) {
+      // TODO(dnfield): Remove this if/when we can fix https://github.com/flutter/flutter/issues/90004
+      return;
+    }
     assert(() {
       enabled = enabled && !debugDisableOpacityLayers;
       return true;
     }());
 
-    if (enabled)
+    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?);
       engineLayer = builder.pushOpacity(
-        alpha!,
-        offset: offset! + layerOffset,
+        realizedAlpha,
+        offset: offset + layerOffset,
         oldLayer: _engineLayer as ui.OpacityEngineLayer?,
       );
-    else
-      engineLayer = null;
+    } else {
+      assert(_engineLayer is ui.OffsetEngineLayer?);
+      engineLayer = builder.pushOffset(
+        layerOffset.dx + offset.dx,
+        layerOffset.dy + offset.dy,
+        oldLayer: _engineLayer as ui.OffsetEngineLayer?,
+      );
+    }
     addChildrenToScene(builder);
-    if (enabled)
-      builder.pop();
+    builder.pop();
   }
 
   @override
   void debugFillProperties(DiagnosticPropertiesBuilder properties) {
     super.debugFillProperties(properties);
     properties.add(IntProperty('alpha', alpha));
-    properties.add(DiagnosticsProperty<Offset>('offset', offset));
   }
 }
 

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 && _alpha != 255);
+  bool get alwaysNeedsCompositing => child != null && (_alpha > 0);
 
   int _alpha;
 
@@ -897,12 +897,6 @@ 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?);
     }
@@ -993,7 +987,7 @@ mixin RenderAnimatedOpacityMixin<T extends RenderObject> on RenderObjectWithChil
     _alpha = ui.Color.getAlphaFromOpacity(opacity.value);
     if (oldAlpha != _alpha) {
       final bool? didNeedCompositing = _currentlyNeedsCompositing;
-      _currentlyNeedsCompositing = _alpha! > 0 && _alpha! < 255;
+      _currentlyNeedsCompositing = _alpha! > 0;
       if (child != null && didNeedCompositing != _currentlyNeedsCompositing)
         markNeedsCompositingBitsUpdate();
       markNeedsPaint();
@@ -1010,12 +1004,6 @@ mixin RenderAnimatedOpacityMixin<T extends RenderObject> 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?);
     }

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

@@ -112,7 +112,7 @@ class RenderSliverOpacity extends RenderProxySliver {
   }
 
   @override
-  bool get alwaysNeedsCompositing => child != null && (_alpha != 0 && _alpha != 255);
+  bool get alwaysNeedsCompositing => child != null && (_alpha > 0);
 
   int _alpha;
 
@@ -166,12 +166,6 @@ 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,

packages/flutter/test/rendering/debug_test.dart

@@ -208,4 +208,30 @@ 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;
+  });
 }

packages/flutter/test/rendering/layers_test.dart

@@ -537,6 +537,53 @@ void main() {
 
     expect(() => holder.layer = layer, throwsAssertionError);
   });
+
+  test('OpacityLayer does not push an OffsetLayer if there are no children', () {
+    final OpacityLayer layer = OpacityLayer(alpha: 128);
+    final FakeSceneBuilder builder = FakeSceneBuilder();
+    layer.addToScene(builder);
+    expect(builder.pushedOpacity, false);
+    expect(builder.pushedOffset, false);
+    expect(builder.addedPicture, false);
+    expect(layer.engineLayer, null);
+
+    layer.append(PictureLayer(Rect.largest)..picture = FakePicture());
+
+    builder.reset();
+    layer.addToScene(builder);
+
+    expect(builder.pushedOpacity, true);
+    expect(builder.pushedOffset, false);
+    expect(builder.addedPicture, true);
+    expect(layer.engineLayer, isA<FakeOpacityEngineLayer>());
+
+    builder.reset();
+
+    layer.alpha = 200;
+    expect(layer.engineLayer, isA<FakeOpacityEngineLayer>());
+
+    layer.alpha = 255;
+    expect(layer.engineLayer, null);
+
+    builder.reset();
+    layer.addToScene(builder);
+
+    expect(builder.pushedOpacity, false);
+    expect(builder.pushedOffset, true);
+    expect(builder.addedPicture, true);
+    expect(layer.engineLayer, isA<FakeOffsetEngineLayer>());
+
+    layer.alpha = 200;
+    expect(layer.engineLayer, null);
+
+    builder.reset();
+    layer.addToScene(builder);
+
+    expect(builder.pushedOpacity, true);
+    expect(builder.pushedOffset, false);
+    expect(builder.addedPicture, true);
+    expect(layer.engineLayer, isA<FakeOpacityEngineLayer>());
+  });
 }
 
 class FakeEngineLayer extends Fake implements EngineLayer {
@@ -568,3 +615,38 @@ class _TestAlwaysNeedsAddToSceneLayer extends ContainerLayer {
   @override
   bool get alwaysNeedsAddToScene => true;
 }
+
+class FakeSceneBuilder extends Fake implements SceneBuilder {
+  void reset() {
+    pushedOpacity = false;
+    pushedOffset = false;
+    addedPicture = false;
+  }
+
+  bool pushedOpacity = false;
+  @override
+  OpacityEngineLayer pushOpacity(int alpha, {Offset? offset = Offset.zero, OpacityEngineLayer? oldLayer}) {
+    pushedOpacity = true;
+    return FakeOpacityEngineLayer();
+  }
+
+  bool pushedOffset = false;
+  @override
+  OffsetEngineLayer pushOffset(double x, double y, {OffsetEngineLayer? oldLayer}) {
+    pushedOffset = true;
+    return FakeOffsetEngineLayer();
+  }
+
+  bool addedPicture = false;
+  @override
+  void addPicture(Offset offset, Picture picture, {bool isComplexHint = false, bool willChangeHint = false}) {
+    addedPicture = true;
+  }
+
+  @override
+  void pop() {}
+}
+
+class FakeOpacityEngineLayer extends FakeEngineLayer implements OpacityEngineLayer {}
+
+class FakeOffsetEngineLayer extends FakeEngineLayer implements OffsetEngineLayer {}
\ No newline at end of file

packages/flutter/test/rendering/proxy_box_test.dart

@@ -274,14 +274,14 @@ void main() {
     expect(renderOpacity.needsCompositing, false);
   });
 
-  test('RenderOpacity does not composite if it is opaque', () {
+  test('RenderOpacity does 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, false);
+    expect(renderOpacity.needsCompositing, true);
   });
 
   test('RenderOpacity reuses its layer', () {
@@ -306,7 +306,7 @@ void main() {
     expect(renderAnimatedOpacity.needsCompositing, false);
   });
 
-  test('RenderAnimatedOpacity does not composite if it is opaque', () {
+  test('RenderAnimatedOpacity does composite if it is opaque', () {
     final Animation<double> opacityAnimation = AnimationController(
       vsync: FakeTickerProvider(),
     )..value = 1.0;
@@ -318,7 +318,7 @@ void main() {
     );
 
     layout(renderAnimatedOpacity, phase: EnginePhase.composite);
-    expect(renderAnimatedOpacity.needsCompositing, false);
+    expect(renderAnimatedOpacity.needsCompositing, true);
   });
 
   test('RenderAnimatedOpacity reuses its layer', () {

packages/flutter/test/rendering/proxy_sliver_test.dart

@@ -29,7 +29,7 @@ void main() {
     expect(renderSliverOpacity.needsCompositing, false);
   });
 
-  test('RenderSliverOpacity does not composite if it is opaque', () {
+  test('RenderSliverOpacity does 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, false);
+    expect(renderSliverOpacity.needsCompositing, true);
   });
   test('RenderSliverOpacity reuses its layer', () {
     final RenderSliverOpacity renderSliverOpacity = RenderSliverOpacity(
@@ -102,7 +102,7 @@ void main() {
     expect(renderSliverAnimatedOpacity.needsCompositing, false);
   });
 
-  test('RenderSliverAnimatedOpacity does not composite if it is opaque', () {
+  test('RenderSliverAnimatedOpacity does composite if it is opaque', () {
     final Animation<double> opacityAnimation = AnimationController(
       vsync: FakeTickerProvider(),
     )..value = 1.0;
@@ -124,7 +124,7 @@ void main() {
     );
 
     layout(root, phase: EnginePhase.composite);
-    expect(renderSliverAnimatedOpacity.needsCompositing, false);
+    expect(renderSliverAnimatedOpacity.needsCompositing, true);
   });
 
   test('RenderSliverAnimatedOpacity reuses its layer', () {

packages/flutter/test/widgets/opacity_test.dart

@@ -195,4 +195,35 @@ 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: <Widget>[
+              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;
+  });
 }