Unverified Commit 9db9256b authored by Dan Field's avatar Dan Field Committed by GitHub

Revert "Make sure Opacity widgets/layers do not drop the offset (#89264)" (#89999)

This reverts commit 0d0f7a4f.
parent 00f78cf2
...@@ -1741,7 +1741,7 @@ class TransformLayer extends OffsetLayer { ...@@ -1741,7 +1741,7 @@ class TransformLayer extends OffsetLayer {
/// ///
/// Try to avoid an [OpacityLayer] with no children. Remove that layer if /// Try to avoid an [OpacityLayer] with no children. Remove that layer if
/// possible to save some tree walks. /// possible to save some tree walks.
class OpacityLayer extends OffsetLayer { class OpacityLayer extends ContainerLayer {
/// Creates an opacity layer. /// Creates an opacity layer.
/// ///
/// The [alpha] property must be non-null before the compositing phase of /// The [alpha] property must be non-null before the compositing phase of
...@@ -1750,7 +1750,7 @@ class OpacityLayer extends OffsetLayer { ...@@ -1750,7 +1750,7 @@ class OpacityLayer extends OffsetLayer {
int? alpha, int? alpha,
Offset offset = Offset.zero, Offset offset = Offset.zero,
}) : _alpha = alpha, }) : _alpha = alpha,
super(offset: offset); _offset = offset;
/// The amount to multiply into the alpha channel. /// The amount to multiply into the alpha channel.
/// ///
...@@ -1764,14 +1764,28 @@ class OpacityLayer extends OffsetLayer { ...@@ -1764,14 +1764,28 @@ class OpacityLayer extends OffsetLayer {
set alpha(int? value) { set alpha(int? value) {
assert(value != null); assert(value != null);
if (value != _alpha) { if (value != _alpha) {
if (value == 255 || _alpha == 255) {
engineLayer = null;
}
_alpha = value; _alpha = value;
markNeedsAddToScene(); 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 @override
void addToScene(ui.SceneBuilder builder, [ Offset layerOffset = Offset.zero ]) { void addToScene(ui.SceneBuilder builder, [ Offset layerOffset = Offset.zero ]) {
assert(alpha != null); assert(alpha != null);
...@@ -1781,32 +1795,24 @@ class OpacityLayer extends OffsetLayer { ...@@ -1781,32 +1795,24 @@ class OpacityLayer extends OffsetLayer {
return true; return true;
}()); }());
final int realizedAlpha = alpha!; if (enabled)
// 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( engineLayer = builder.pushOpacity(
realizedAlpha, alpha!,
offset: offset + layerOffset, offset: offset! + layerOffset,
oldLayer: _engineLayer as ui.OpacityEngineLayer?, oldLayer: _engineLayer as ui.OpacityEngineLayer?,
); );
} else { else
assert(_engineLayer is ui.OffsetEngineLayer?); engineLayer = null;
engineLayer = builder.pushOffset(
layerOffset.dx + offset.dx,
layerOffset.dy + offset.dy,
oldLayer: _engineLayer as ui.OffsetEngineLayer?,
);
}
addChildrenToScene(builder); addChildrenToScene(builder);
builder.pop(); if (enabled)
builder.pop();
} }
@override @override
void debugFillProperties(DiagnosticPropertiesBuilder properties) { void debugFillProperties(DiagnosticPropertiesBuilder properties) {
super.debugFillProperties(properties); super.debugFillProperties(properties);
properties.add(IntProperty('alpha', alpha)); properties.add(IntProperty('alpha', alpha));
properties.add(DiagnosticsProperty<Offset>('offset', offset));
} }
} }
......
...@@ -843,7 +843,7 @@ class RenderOpacity extends RenderProxyBox { ...@@ -843,7 +843,7 @@ class RenderOpacity extends RenderProxyBox {
super(child); super(child);
@override @override
bool get alwaysNeedsCompositing => child != null && (_alpha > 0); bool get alwaysNeedsCompositing => child != null && (_alpha != 0 && _alpha != 255);
int _alpha; int _alpha;
...@@ -897,6 +897,12 @@ class RenderOpacity extends RenderProxyBox { ...@@ -897,6 +897,12 @@ class RenderOpacity extends RenderProxyBox {
layer = null; layer = null;
return; 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); assert(needsCompositing);
layer = context.pushOpacity(offset, _alpha, super.paint, oldLayer: layer as OpacityLayer?); layer = context.pushOpacity(offset, _alpha, super.paint, oldLayer: layer as OpacityLayer?);
} }
...@@ -987,7 +993,7 @@ mixin RenderAnimatedOpacityMixin<T extends RenderObject> on RenderObjectWithChil ...@@ -987,7 +993,7 @@ mixin RenderAnimatedOpacityMixin<T extends RenderObject> on RenderObjectWithChil
_alpha = ui.Color.getAlphaFromOpacity(opacity.value); _alpha = ui.Color.getAlphaFromOpacity(opacity.value);
if (oldAlpha != _alpha) { if (oldAlpha != _alpha) {
final bool? didNeedCompositing = _currentlyNeedsCompositing; final bool? didNeedCompositing = _currentlyNeedsCompositing;
_currentlyNeedsCompositing = _alpha! > 0; _currentlyNeedsCompositing = _alpha! > 0 && _alpha! < 255;
if (child != null && didNeedCompositing != _currentlyNeedsCompositing) if (child != null && didNeedCompositing != _currentlyNeedsCompositing)
markNeedsCompositingBitsUpdate(); markNeedsCompositingBitsUpdate();
markNeedsPaint(); markNeedsPaint();
...@@ -1004,6 +1010,12 @@ mixin RenderAnimatedOpacityMixin<T extends RenderObject> on RenderObjectWithChil ...@@ -1004,6 +1010,12 @@ mixin RenderAnimatedOpacityMixin<T extends RenderObject> on RenderObjectWithChil
layer = null; layer = null;
return; 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); assert(needsCompositing);
layer = context.pushOpacity(offset, _alpha!, super.paint, oldLayer: layer as OpacityLayer?); layer = context.pushOpacity(offset, _alpha!, super.paint, oldLayer: layer as OpacityLayer?);
} }
......
...@@ -112,7 +112,7 @@ class RenderSliverOpacity extends RenderProxySliver { ...@@ -112,7 +112,7 @@ class RenderSliverOpacity extends RenderProxySliver {
} }
@override @override
bool get alwaysNeedsCompositing => child != null && (_alpha > 0); bool get alwaysNeedsCompositing => child != null && (_alpha != 0 && _alpha != 255);
int _alpha; int _alpha;
...@@ -166,6 +166,12 @@ class RenderSliverOpacity extends RenderProxySliver { ...@@ -166,6 +166,12 @@ class RenderSliverOpacity extends RenderProxySliver {
layer = null; layer = null;
return; 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); assert(needsCompositing);
layer = context.pushOpacity( layer = context.pushOpacity(
offset, offset,
......
...@@ -208,30 +208,4 @@ void main() { ...@@ -208,30 +208,4 @@ void main() {
expect(error, isNull); expect(error, isNull);
debugPaintSizeEnabled = false; 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;
});
} }
...@@ -274,14 +274,14 @@ void main() { ...@@ -274,14 +274,14 @@ void main() {
expect(renderOpacity.needsCompositing, false); 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( final RenderOpacity renderOpacity = RenderOpacity(
opacity: 1.0, opacity: 1.0,
child: RenderSizedBox(const Size(1.0, 1.0)), // size doesn't matter child: RenderSizedBox(const Size(1.0, 1.0)), // size doesn't matter
); );
layout(renderOpacity, phase: EnginePhase.composite); layout(renderOpacity, phase: EnginePhase.composite);
expect(renderOpacity.needsCompositing, true); expect(renderOpacity.needsCompositing, false);
}); });
test('RenderOpacity reuses its layer', () { test('RenderOpacity reuses its layer', () {
...@@ -306,7 +306,7 @@ void main() { ...@@ -306,7 +306,7 @@ void main() {
expect(renderAnimatedOpacity.needsCompositing, false); expect(renderAnimatedOpacity.needsCompositing, false);
}); });
test('RenderAnimatedOpacity does composite if it is opaque', () { test('RenderAnimatedOpacity does not composite if it is opaque', () {
final Animation<double> opacityAnimation = AnimationController( final Animation<double> opacityAnimation = AnimationController(
vsync: FakeTickerProvider(), vsync: FakeTickerProvider(),
)..value = 1.0; )..value = 1.0;
...@@ -318,7 +318,7 @@ void main() { ...@@ -318,7 +318,7 @@ void main() {
); );
layout(renderAnimatedOpacity, phase: EnginePhase.composite); layout(renderAnimatedOpacity, phase: EnginePhase.composite);
expect(renderAnimatedOpacity.needsCompositing, true); expect(renderAnimatedOpacity.needsCompositing, false);
}); });
test('RenderAnimatedOpacity reuses its layer', () { test('RenderAnimatedOpacity reuses its layer', () {
......
...@@ -29,7 +29,7 @@ void main() { ...@@ -29,7 +29,7 @@ void main() {
expect(renderSliverOpacity.needsCompositing, false); expect(renderSliverOpacity.needsCompositing, false);
}); });
test('RenderSliverOpacity does composite if it is opaque', () { test('RenderSliverOpacity does not composite if it is opaque', () {
final RenderSliverOpacity renderSliverOpacity = RenderSliverOpacity( final RenderSliverOpacity renderSliverOpacity = RenderSliverOpacity(
opacity: 1.0, opacity: 1.0,
sliver: RenderSliverToBoxAdapter( sliver: RenderSliverToBoxAdapter(
...@@ -46,7 +46,7 @@ void main() { ...@@ -46,7 +46,7 @@ void main() {
); );
layout(root, phase: EnginePhase.composite); layout(root, phase: EnginePhase.composite);
expect(renderSliverOpacity.needsCompositing, true); expect(renderSliverOpacity.needsCompositing, false);
}); });
test('RenderSliverOpacity reuses its layer', () { test('RenderSliverOpacity reuses its layer', () {
final RenderSliverOpacity renderSliverOpacity = RenderSliverOpacity( final RenderSliverOpacity renderSliverOpacity = RenderSliverOpacity(
...@@ -102,7 +102,7 @@ void main() { ...@@ -102,7 +102,7 @@ void main() {
expect(renderSliverAnimatedOpacity.needsCompositing, false); expect(renderSliverAnimatedOpacity.needsCompositing, false);
}); });
test('RenderSliverAnimatedOpacity does composite if it is opaque', () { test('RenderSliverAnimatedOpacity does not composite if it is opaque', () {
final Animation<double> opacityAnimation = AnimationController( final Animation<double> opacityAnimation = AnimationController(
vsync: FakeTickerProvider(), vsync: FakeTickerProvider(),
)..value = 1.0; )..value = 1.0;
...@@ -124,7 +124,7 @@ void main() { ...@@ -124,7 +124,7 @@ void main() {
); );
layout(root, phase: EnginePhase.composite); layout(root, phase: EnginePhase.composite);
expect(renderSliverAnimatedOpacity.needsCompositing, true); expect(renderSliverAnimatedOpacity.needsCompositing, false);
}); });
test('RenderSliverAnimatedOpacity reuses its layer', () { test('RenderSliverAnimatedOpacity reuses its layer', () {
......
...@@ -195,35 +195,4 @@ void main() { ...@@ -195,35 +195,4 @@ void main() {
final OffsetLayer offsetLayer = element.renderObject!.debugLayer! as OffsetLayer; final OffsetLayer offsetLayer = element.renderObject!.debugLayer! as OffsetLayer;
await offsetLayer.toImage(const Rect.fromLTRB(0.0, 0.0, 1.0, 1.0)); await offsetLayer.toImage(const Rect.fromLTRB(0.0, 0.0, 1.0, 1.0));
}, skip: isBrowser); // https://github.com/flutter/flutter/issues/49857 }, 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;
});
} }
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment