From febc6a14f57a23e4605067283903bca45d6856e5 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Mon, 6 Jun 2022 12:13:07 -0700 Subject: [PATCH] remove forced compositing from opacity (#105334) --- .../flutter/lib/src/rendering/proxy_box.dart | 52 +++++++++++-------- .../test/rendering/proxy_box_test.dart | 12 ++--- packages/flutter/test/widgets/debug_test.dart | 4 +- 3 files changed, 38 insertions(+), 30 deletions(-) diff --git a/packages/flutter/lib/src/rendering/proxy_box.dart b/packages/flutter/lib/src/rendering/proxy_box.dart index 22f138dd0c39..25ae87ddb9e4 100644 --- a/packages/flutter/lib/src/rendering/proxy_box.dart +++ b/packages/flutter/lib/src/rendering/proxy_box.dart @@ -13,6 +13,7 @@ import 'package:flutter/services.dart'; import 'package:vector_math/vector_math_64.dart'; import 'box.dart'; +import 'debug.dart'; import 'layer.dart'; import 'layout_helper.dart'; import 'object.dart'; @@ -884,16 +885,6 @@ class RenderOpacity extends RenderProxyBox { _alpha = ui.Color.getAlphaFromOpacity(opacity), super(child); - @override - bool get alwaysNeedsCompositing => child != null && (_alpha > 0); - - @override - OffsetLayer updateCompositedLayer({required covariant OpacityLayer? oldLayer}) { - final OpacityLayer updatedLayer = oldLayer ?? OpacityLayer(); - updatedLayer.alpha = _alpha; - return updatedLayer; - } - int _alpha; /// The fraction to scale the child's alpha value. @@ -914,13 +905,9 @@ class RenderOpacity extends RenderProxyBox { if (_opacity == value) { return; } - final bool didNeedCompositing = alwaysNeedsCompositing; final bool wasVisible = _alpha != 0; _opacity = value; _alpha = ui.Color.getAlphaFromOpacity(_opacity); - if (didNeedCompositing != alwaysNeedsCompositing) { - markNeedsCompositingBitsUpdate(); - } markNeedsPaint(); if (wasVisible != (_alpha != 0) && !alwaysIncludeSemantics) { markNeedsSemanticsUpdate(); @@ -950,19 +937,40 @@ class RenderOpacity extends RenderProxyBox { @override void paint(PaintingContext context, Offset offset) { - if (child != null) { - if (_alpha == 0) { - // No need to keep the layer. We'll create a new one if necessary. - layer = null; - return; - } - assert(needsCompositing); + if (child == null) { + return; + } + if (_alpha == 0) { + // No need to keep the layer. We'll create a new one if necessary. + layer = null; + return; + } + if (_alpha == 255) { + layer = null; + return super.paint(context, offset); + } + // Due to https://github.com/flutter/flutter/issues/48417 this will always need to be + // composited on the web. + if (needsCompositing || kIsWeb) { layer = context.pushOpacity(offset, _alpha, super.paint, oldLayer: layer as OpacityLayer?); assert(() { - layer!.debugCreator = debugCreator; + layer?.debugCreator = debugCreator; return true; }()); + return; + } + + // debugDisableOpacityLayers is used by the SceneBuilder to remove opacity layers, but + // if the framework is not asked to composite will also need to remove the opacity here. + if (kDebugMode && debugDisableOpacityLayers) { + super.paint(context, offset); + return; } + final Color color = Color.fromRGBO(0, 0, 0, opacity); + final Canvas canvas = context.canvas; + canvas.saveLayer(size != null ? offset & size : null, Paint()..color = color); + super.paint(context, offset); + canvas.restore(); } @override diff --git a/packages/flutter/test/rendering/proxy_box_test.dart b/packages/flutter/test/rendering/proxy_box_test.dart index 12831d9671b7..1fcacd946632 100644 --- a/packages/flutter/test/rendering/proxy_box_test.dart +++ b/packages/flutter/test/rendering/proxy_box_test.dart @@ -197,7 +197,7 @@ void main() { expect(data.lengthInBytes, equals(20 * 20 * 4)); expect(data.elementSizeInBytes, equals(1)); - expect(getPixel(0, 0), equals(0x00000080)); + expect(getPixel(0, 0), equals(0x0000007F)); expect(getPixel(image.width - 1, 0 ), equals(0xffffffff)); final OffsetLayer layer = boundary.debugLayer! as OffsetLayer; @@ -206,7 +206,7 @@ void main() { expect(image.width, equals(20)); expect(image.height, equals(20)); data = (await image.toByteData())!; - expect(getPixel(0, 0), equals(0x00000080)); + expect(getPixel(0, 0), equals(0x0000007F)); expect(getPixel(image.width - 1, 0 ), equals(0xffffffff)); // non-zero offsets. @@ -215,7 +215,7 @@ void main() { expect(image.height, equals(30)); data = (await image.toByteData())!; expect(getPixel(0, 0), equals(0x00000000)); - expect(getPixel(10, 10), equals(0x00000080)); + expect(getPixel(10, 10), equals(0x0000007F)); expect(getPixel(image.width - 1, 0), equals(0x00000000)); expect(getPixel(image.width - 1, 10), equals(0xffffffff)); @@ -225,7 +225,7 @@ void main() { expect(image.height, equals(60)); data = (await image.toByteData())!; expect(getPixel(0, 0), equals(0x00000000)); - expect(getPixel(20, 20), equals(0x00000080)); + expect(getPixel(20, 20), equals(0x0000007F)); expect(getPixel(image.width - 1, 0), equals(0x00000000)); expect(getPixel(image.width - 1, 20), equals(0xffffffff)); }, skip: isBrowser); // https://github.com/flutter/flutter/issues/49857 @@ -240,13 +240,13 @@ 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( 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', () { diff --git a/packages/flutter/test/widgets/debug_test.dart b/packages/flutter/test/widgets/debug_test.dart index ce1eabb770fb..a3de7586c720 100644 --- a/packages/flutter/test/widgets/debug_test.dart +++ b/packages/flutter/test/widgets/debug_test.dart @@ -285,8 +285,8 @@ void main() { child: Placeholder(), ), const Opacity( - opacity: 1.0, - child: Placeholder(), + opacity: 0.9, // ensure compositing is used. + child: RepaintBoundary(child: Placeholder()), ), ImageFiltered( imageFilter: ImageFilter.blur(sigmaX: 10.0, sigmaY: 10.0),