Skip to content

Commit 528f77d

Browse files
dnfielddnfield
authored
Opacity fix (#90017)
* Make sure Opacity widgets/layers do not drop the offset
1 parent 2470f63 commit 528f77d

File tree

8 files changed

+175
-56
lines changed

8 files changed

+175
-56
lines changed

‎packages/flutter/lib/src/rendering/layer.dart‎

Lines changed: 25 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1741,7 +1741,7 @@ class TransformLayer extends OffsetLayer {
17411741
///
17421742
/// Try to avoid an [OpacityLayer] with no children. Remove that layer if
17431743
/// possible to save some tree walks.
1744-
class OpacityLayer extends ContainerLayer {
1744+
class OpacityLayer extends OffsetLayer {
17451745
/// Creates an opacity layer.
17461746
///
17471747
/// The [alpha] property must be non-null before the compositing phase of
@@ -1750,7 +1750,7 @@ class OpacityLayer extends ContainerLayer {
17501750
int? alpha,
17511751
Offset offset = Offset.zero,
17521752
}) : _alpha = alpha,
1753-
_offset = offset;
1753+
super(offset: offset);
17541754

17551755
/// The amount to multiply into the alpha channel.
17561756
///
@@ -1764,55 +1764,53 @@ class OpacityLayer extends ContainerLayer {
17641764
set alpha(int? value) {
17651765
assert(value != null);
17661766
if (value != _alpha) {
1767+
if (value == 255 || _alpha == 255) {
1768+
engineLayer = null;
1769+
}
17671770
_alpha = value;
17681771
markNeedsAddToScene();
17691772
}
17701773
}
17711774

1772-
/// Offset from parent in the parent's coordinate system.
1773-
Offset? get offset => _offset;
1774-
Offset? _offset;
1775-
set offset(Offset? value) {
1776-
if (value != _offset) {
1777-
_offset = value;
1778-
markNeedsAddToScene();
1779-
}
1780-
}
1781-
1782-
@override
1783-
void applyTransform(Layer? child, Matrix4 transform) {
1784-
assert(child != null);
1785-
assert(transform != null);
1786-
transform.translate(offset!.dx, offset!.dy);
1787-
}
1788-
17891775
@override
17901776
void addToScene(ui.SceneBuilder builder, [ Offset layerOffset = Offset.zero ]) {
17911777
assert(alpha != null);
17921778
bool enabled = firstChild != null; // don't add this layer if there's no child
1779+
if (!enabled) {
1780+
// TODO(dnfield): Remove this if/when we can fix https://github.com/flutter/flutter/issues/90004
1781+
return;
1782+
}
17931783
assert(() {
17941784
enabled = enabled && !debugDisableOpacityLayers;
17951785
return true;
17961786
}());
17971787

1798-
if (enabled)
1788+
final int realizedAlpha = alpha!;
1789+
// The type assertions work because the [alpha] setter nulls out the
1790+
// engineLayer if it would have changed type (i.e. changed to or from 255).
1791+
if (enabled && realizedAlpha < 255) {
1792+
assert(_engineLayer is ui.OpacityEngineLayer?);
17991793
engineLayer = builder.pushOpacity(
1800-
alpha!,
1801-
offset: offset! + layerOffset,
1794+
realizedAlpha,
1795+
offset: offset + layerOffset,
18021796
oldLayer: _engineLayer as ui.OpacityEngineLayer?,
18031797
);
1804-
else
1805-
engineLayer = null;
1798+
} else {
1799+
assert(_engineLayer is ui.OffsetEngineLayer?);
1800+
engineLayer = builder.pushOffset(
1801+
layerOffset.dx + offset.dx,
1802+
layerOffset.dy + offset.dy,
1803+
oldLayer: _engineLayer as ui.OffsetEngineLayer?,
1804+
);
1805+
}
18061806
addChildrenToScene(builder);
1807-
if (enabled)
1808-
builder.pop();
1807+
builder.pop();
18091808
}
18101809

18111810
@override
18121811
void debugFillProperties(DiagnosticPropertiesBuilder properties) {
18131812
super.debugFillProperties(properties);
18141813
properties.add(IntProperty('alpha', alpha));
1815-
properties.add(DiagnosticsProperty<Offset>('offset', offset));
18161814
}
18171815
}
18181816

‎packages/flutter/lib/src/rendering/proxy_box.dart‎

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -843,7 +843,7 @@ class RenderOpacity extends RenderProxyBox {
843843
super(child);
844844

845845
@override
846-
bool get alwaysNeedsCompositing => child != null && (_alpha != 0 && _alpha != 255);
846+
bool get alwaysNeedsCompositing => child != null && (_alpha > 0);
847847

848848
int _alpha;
849849

@@ -897,12 +897,6 @@ class RenderOpacity extends RenderProxyBox {
897897
layer = null;
898898
return;
899899
}
900-
if (_alpha == 255) {
901-
// No need to keep the layer. We'll create a new one if necessary.
902-
layer = null;
903-
context.paintChild(child!, offset);
904-
return;
905-
}
906900
assert(needsCompositing);
907901
layer = context.pushOpacity(offset, _alpha, super.paint, oldLayer: layer as OpacityLayer?);
908902
}
@@ -993,7 +987,7 @@ mixin RenderAnimatedOpacityMixin<T extends RenderObject> on RenderObjectWithChil
993987
_alpha = ui.Color.getAlphaFromOpacity(opacity.value);
994988
if (oldAlpha != _alpha) {
995989
final bool? didNeedCompositing = _currentlyNeedsCompositing;
996-
_currentlyNeedsCompositing = _alpha! > 0 && _alpha! < 255;
990+
_currentlyNeedsCompositing = _alpha! > 0;
997991
if (child != null && didNeedCompositing != _currentlyNeedsCompositing)
998992
markNeedsCompositingBitsUpdate();
999993
markNeedsPaint();
@@ -1010,12 +1004,6 @@ mixin RenderAnimatedOpacityMixin<T extends RenderObject> on RenderObjectWithChil
10101004
layer = null;
10111005
return;
10121006
}
1013-
if (_alpha == 255) {
1014-
// No need to keep the layer. We'll create a new one if necessary.
1015-
layer = null;
1016-
context.paintChild(child!, offset);
1017-
return;
1018-
}
10191007
assert(needsCompositing);
10201008
layer = context.pushOpacity(offset, _alpha!, super.paint, oldLayer: layer as OpacityLayer?);
10211009
}

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

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ class RenderSliverOpacity extends RenderProxySliver {
112112
}
113113

114114
@override
115-
bool get alwaysNeedsCompositing => child != null && (_alpha != 0 && _alpha != 255);
115+
bool get alwaysNeedsCompositing => child != null && (_alpha > 0);
116116

117117
int _alpha;
118118

@@ -166,12 +166,6 @@ class RenderSliverOpacity extends RenderProxySliver {
166166
layer = null;
167167
return;
168168
}
169-
if (_alpha == 255) {
170-
// No need to keep the layer. We'll create a new one if necessary.
171-
layer = null;
172-
context.paintChild(child!, offset);
173-
return;
174-
}
175169
assert(needsCompositing);
176170
layer = context.pushOpacity(
177171
offset,

‎packages/flutter/test/rendering/debug_test.dart‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,4 +208,30 @@ void main() {
208208
expect(error, isNull);
209209
debugPaintSizeEnabled = false;
210210
});
211+
212+
test('debugDisableOpacity keeps things in the right spot', () {
213+
debugDisableOpacityLayers = true;
214+
215+
final RenderDecoratedBox blackBox = RenderDecoratedBox(
216+
decoration: const BoxDecoration(color: Color(0xff000000)),
217+
child: RenderConstrainedBox(
218+
additionalConstraints: BoxConstraints.tight(const Size.square(20.0)),
219+
),
220+
);
221+
final RenderOpacity root = RenderOpacity(
222+
opacity: .5,
223+
child: blackBox,
224+
);
225+
layout(root, phase: EnginePhase.compositingBits);
226+
227+
final OffsetLayer rootLayer = OffsetLayer(offset: Offset.zero);
228+
final PaintingContext context = PaintingContext(
229+
rootLayer,
230+
const Rect.fromLTWH(0, 0, 500, 500),
231+
);
232+
root.paint(context, const Offset(40, 40));
233+
final OpacityLayer opacityLayer = rootLayer.firstChild! as OpacityLayer;
234+
expect(opacityLayer.offset, const Offset(40, 40));
235+
debugDisableOpacityLayers = false;
236+
});
211237
}

‎packages/flutter/test/rendering/layers_test.dart‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -537,6 +537,53 @@ void main() {
537537

538538
expect(() => holder.layer = layer, throwsAssertionError);
539539
});
540+
541+
test('OpacityLayer does not push an OffsetLayer if there are no children', () {
542+
final OpacityLayer layer = OpacityLayer(alpha: 128);
543+
final FakeSceneBuilder builder = FakeSceneBuilder();
544+
layer.addToScene(builder);
545+
expect(builder.pushedOpacity, false);
546+
expect(builder.pushedOffset, false);
547+
expect(builder.addedPicture, false);
548+
expect(layer.engineLayer, null);
549+
550+
layer.append(PictureLayer(Rect.largest)..picture = FakePicture());
551+
552+
builder.reset();
553+
layer.addToScene(builder);
554+
555+
expect(builder.pushedOpacity, true);
556+
expect(builder.pushedOffset, false);
557+
expect(builder.addedPicture, true);
558+
expect(layer.engineLayer, isA<FakeOpacityEngineLayer>());
559+
560+
builder.reset();
561+
562+
layer.alpha = 200;
563+
expect(layer.engineLayer, isA<FakeOpacityEngineLayer>());
564+
565+
layer.alpha = 255;
566+
expect(layer.engineLayer, null);
567+
568+
builder.reset();
569+
layer.addToScene(builder);
570+
571+
expect(builder.pushedOpacity, false);
572+
expect(builder.pushedOffset, true);
573+
expect(builder.addedPicture, true);
574+
expect(layer.engineLayer, isA<FakeOffsetEngineLayer>());
575+
576+
layer.alpha = 200;
577+
expect(layer.engineLayer, null);
578+
579+
builder.reset();
580+
layer.addToScene(builder);
581+
582+
expect(builder.pushedOpacity, true);
583+
expect(builder.pushedOffset, false);
584+
expect(builder.addedPicture, true);
585+
expect(layer.engineLayer, isA<FakeOpacityEngineLayer>());
586+
});
540587
}
541588

542589
class FakeEngineLayer extends Fake implements EngineLayer {
@@ -568,3 +615,38 @@ class _TestAlwaysNeedsAddToSceneLayer extends ContainerLayer {
568615
@override
569616
bool get alwaysNeedsAddToScene => true;
570617
}
618+
619+
class FakeSceneBuilder extends Fake implements SceneBuilder {
620+
void reset() {
621+
pushedOpacity = false;
622+
pushedOffset = false;
623+
addedPicture = false;
624+
}
625+
626+
bool pushedOpacity = false;
627+
@override
628+
OpacityEngineLayer pushOpacity(int alpha, {Offset? offset = Offset.zero, OpacityEngineLayer? oldLayer}) {
629+
pushedOpacity = true;
630+
return FakeOpacityEngineLayer();
631+
}
632+
633+
bool pushedOffset = false;
634+
@override
635+
OffsetEngineLayer pushOffset(double x, double y, {OffsetEngineLayer? oldLayer}) {
636+
pushedOffset = true;
637+
return FakeOffsetEngineLayer();
638+
}
639+
640+
bool addedPicture = false;
641+
@override
642+
void addPicture(Offset offset, Picture picture, {bool isComplexHint = false, bool willChangeHint = false}) {
643+
addedPicture = true;
644+
}
645+
646+
@override
647+
void pop() {}
648+
}
649+
650+
class FakeOpacityEngineLayer extends FakeEngineLayer implements OpacityEngineLayer {}
651+
652+
class FakeOffsetEngineLayer extends FakeEngineLayer implements OffsetEngineLayer {}

‎packages/flutter/test/rendering/proxy_box_test.dart‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -274,14 +274,14 @@ void main() {
274274
expect(renderOpacity.needsCompositing, false);
275275
});
276276

277-
test('RenderOpacity does not composite if it is opaque', () {
277+
test('RenderOpacity does composite if it is opaque', () {
278278
final RenderOpacity renderOpacity = RenderOpacity(
279279
opacity: 1.0,
280280
child: RenderSizedBox(const Size(1.0, 1.0)), // size doesn't matter
281281
);
282282

283283
layout(renderOpacity, phase: EnginePhase.composite);
284-
expect(renderOpacity.needsCompositing, false);
284+
expect(renderOpacity.needsCompositing, true);
285285
});
286286

287287
test('RenderOpacity reuses its layer', () {
@@ -306,7 +306,7 @@ void main() {
306306
expect(renderAnimatedOpacity.needsCompositing, false);
307307
});
308308

309-
test('RenderAnimatedOpacity does not composite if it is opaque', () {
309+
test('RenderAnimatedOpacity does composite if it is opaque', () {
310310
final Animation<double> opacityAnimation = AnimationController(
311311
vsync: FakeTickerProvider(),
312312
)..value = 1.0;
@@ -318,7 +318,7 @@ void main() {
318318
);
319319

320320
layout(renderAnimatedOpacity, phase: EnginePhase.composite);
321-
expect(renderAnimatedOpacity.needsCompositing, false);
321+
expect(renderAnimatedOpacity.needsCompositing, true);
322322
});
323323

324324
test('RenderAnimatedOpacity reuses its layer', () {

‎packages/flutter/test/rendering/proxy_sliver_test.dart‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ void main() {
2929
expect(renderSliverOpacity.needsCompositing, false);
3030
});
3131

32-
test('RenderSliverOpacity does not composite if it is opaque', () {
32+
test('RenderSliverOpacity does composite if it is opaque', () {
3333
final RenderSliverOpacity renderSliverOpacity = RenderSliverOpacity(
3434
opacity: 1.0,
3535
sliver: RenderSliverToBoxAdapter(
@@ -46,7 +46,7 @@ void main() {
4646
);
4747

4848
layout(root, phase: EnginePhase.composite);
49-
expect(renderSliverOpacity.needsCompositing, false);
49+
expect(renderSliverOpacity.needsCompositing, true);
5050
});
5151
test('RenderSliverOpacity reuses its layer', () {
5252
final RenderSliverOpacity renderSliverOpacity = RenderSliverOpacity(
@@ -102,7 +102,7 @@ void main() {
102102
expect(renderSliverAnimatedOpacity.needsCompositing, false);
103103
});
104104

105-
test('RenderSliverAnimatedOpacity does not composite if it is opaque', () {
105+
test('RenderSliverAnimatedOpacity does composite if it is opaque', () {
106106
final Animation<double> opacityAnimation = AnimationController(
107107
vsync: FakeTickerProvider(),
108108
)..value = 1.0;
@@ -124,7 +124,7 @@ void main() {
124124
);
125125

126126
layout(root, phase: EnginePhase.composite);
127-
expect(renderSliverAnimatedOpacity.needsCompositing, false);
127+
expect(renderSliverAnimatedOpacity.needsCompositing, true);
128128
});
129129

130130
test('RenderSliverAnimatedOpacity reuses its layer', () {

‎packages/flutter/test/widgets/opacity_test.dart‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,4 +195,35 @@ void main() {
195195
final OffsetLayer offsetLayer = element.renderObject!.debugLayer! as OffsetLayer;
196196
await offsetLayer.toImage(const Rect.fromLTRB(0.0, 0.0, 1.0, 1.0));
197197
}, skip: isBrowser); // https://github.com/flutter/flutter/issues/49857
198+
199+
testWidgets('Child shows up in the right spot when opacity is disabled', (WidgetTester tester) async {
200+
debugDisableOpacityLayers = true;
201+
final GlobalKey key = GlobalKey();
202+
await tester.pumpWidget(
203+
RepaintBoundary(
204+
key: key,
205+
child: Directionality(
206+
textDirection: TextDirection.ltr,
207+
child: Stack(
208+
children: <Widget>[
209+
Positioned(
210+
top: 40,
211+
left: 140,
212+
child: Opacity(
213+
opacity: .5,
214+
child: Container(height: 100, width: 100, color: Colors.red),
215+
),
216+
),
217+
],
218+
),
219+
),
220+
),
221+
);
222+
223+
await expectLater(
224+
find.byKey(key),
225+
matchesGoldenFile('opacity_disabled_with_child.png'),
226+
);
227+
debugDisableOpacityLayers = false;
228+
});
198229
}

0 commit comments

Comments
 (0)