Opacity fix (#90017) (#90143)
* Opacity fix (#90017)
* Make sure Opacity widgets/layers do not drop the offset
* Update ci.yaml to enable the tests
* renew cirrus key (#89988)
Co-authored-by: Dan Field <dnfield@google.com>
Co-authored-by: keyonghan <54558023+keyonghan@users.noreply.github.com>
diff --git a/.ci.yaml b/.ci.yaml
index b80197a..b7c9a7c 100755
--- a/.ci.yaml
+++ b/.ci.yaml
@@ -10,6 +10,7 @@
- dev
- beta
- stable
+ - flutter-2.6-candidate.10
platform_properties:
linux:
diff --git a/.cirrus.yml b/.cirrus.yml
index fe14d80..f73e1dc 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -12,7 +12,7 @@
# dependency on precisely how Cirrus is detected by our tools.
BOT: "true"
-gcp_credentials: ENCRYPTED[!0e63b52bd7e4fda1cd7b7bf2b4fe515a27fadbeaced01f5ad8b699b81d3611ed64c5d3271bcd8426dd914ef41cba48a0!]
+gcp_credentials: ENCRYPTED[!48cff44dd32e9cc412d4d381c7fe68d373ca04cf2639f8192d21cb1a9ab5e21129651423a1cf88f3fd7fe2125c1cabd9!]
# LINUX SHARDS
task:
diff --git a/packages/flutter/lib/src/rendering/layer.dart b/packages/flutter/lib/src/rendering/layer.dart
index 085ee1b..5ae73b0 100644
--- a/packages/flutter/lib/src/rendering/layer.dart
+++ b/packages/flutter/lib/src/rendering/layer.dart
@@ -1741,7 +1741,7 @@
///
/// 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 @@
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 @@
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));
}
}
diff --git a/packages/flutter/lib/src/rendering/proxy_box.dart b/packages/flutter/lib/src/rendering/proxy_box.dart
index 8c39dea..adadf52 100644
--- a/packages/flutter/lib/src/rendering/proxy_box.dart
+++ b/packages/flutter/lib/src/rendering/proxy_box.dart
@@ -843,7 +843,7 @@
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 @@
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 @@
_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 @@
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 00cf7fd..520e0d1 100644
--- a/packages/flutter/lib/src/rendering/proxy_sliver.dart
+++ b/packages/flutter/lib/src/rendering/proxy_sliver.dart
@@ -112,7 +112,7 @@
}
@override
- bool get alwaysNeedsCompositing => child != null && (_alpha != 0 && _alpha != 255);
+ bool get alwaysNeedsCompositing => child != null && (_alpha > 0);
int _alpha;
@@ -166,12 +166,6 @@
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 3bc4058..dce60cc 100644
--- a/packages/flutter/test/rendering/debug_test.dart
+++ b/packages/flutter/test/rendering/debug_test.dart
@@ -208,4 +208,30 @@
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/layers_test.dart b/packages/flutter/test/rendering/layers_test.dart
index f8845a6..bade2a5 100644
--- a/packages/flutter/test/rendering/layers_test.dart
+++ b/packages/flutter/test/rendering/layers_test.dart
@@ -537,6 +537,53 @@
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 @@
@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
diff --git a/packages/flutter/test/rendering/proxy_box_test.dart b/packages/flutter/test/rendering/proxy_box_test.dart
index 6693af1..6733cce 100644
--- a/packages/flutter/test/rendering/proxy_box_test.dart
+++ b/packages/flutter/test/rendering/proxy_box_test.dart
@@ -274,14 +274,14 @@
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 @@
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 @@
);
layout(renderAnimatedOpacity, phase: EnginePhase.composite);
- expect(renderAnimatedOpacity.needsCompositing, false);
+ expect(renderAnimatedOpacity.needsCompositing, true);
});
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 85169d2..a197f82 100644
--- a/packages/flutter/test/rendering/proxy_sliver_test.dart
+++ b/packages/flutter/test/rendering/proxy_sliver_test.dart
@@ -29,7 +29,7 @@
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 @@
);
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 @@
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 @@
);
layout(root, phase: EnginePhase.composite);
- expect(renderSliverAnimatedOpacity.needsCompositing, false);
+ expect(renderSliverAnimatedOpacity.needsCompositing, true);
});
test('RenderSliverAnimatedOpacity reuses its layer', () {
diff --git a/packages/flutter/test/widgets/opacity_test.dart b/packages/flutter/test/widgets/opacity_test.dart
index f3225c9..23af74d 100644
--- a/packages/flutter/test/widgets/opacity_test.dart
+++ b/packages/flutter/test/widgets/opacity_test.dart
@@ -195,4 +195,35 @@
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;
+ });
}
