Skip to content

Commit

Permalink
remove forced compositing from opacity (#105334)
Browse files Browse the repository at this point in the history
  • Loading branch information
jonahwilliams authored Jun 6, 2022
1 parent d5c6243 commit febc6a1
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 30 deletions.
52 changes: 30 additions & 22 deletions packages/flutter/lib/src/rendering/proxy_box.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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.
Expand All @@ -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();
Expand Down Expand Up @@ -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
Expand Down
12 changes: 6 additions & 6 deletions packages/flutter/test/rendering/proxy_box_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand All @@ -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));

Expand All @@ -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
Expand All @@ -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', () {
Expand Down
4 changes: 2 additions & 2 deletions packages/flutter/test/widgets/debug_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down

0 comments on commit febc6a1

Please sign in to comment.