Commit d4beaf54 authored by Ian Hickson's avatar Ian Hickson Committed by GitHub

Audit use of saveLayer (#11442)

It has recently come to light that we may have been ordering saveLayer
calls and clip calls incorrectly.
parent abe1e252
...@@ -429,7 +429,7 @@ class _DialPainter extends CustomPainter { ...@@ -429,7 +429,7 @@ class _DialPainter extends CustomPainter {
center: focusedPoint, radius: focusedRadius center: focusedPoint, radius: focusedRadius
); );
canvas canvas
..saveLayer(focusedRect, new Paint()) ..save()
..clipPath(new Path()..addOval(focusedRect)); ..clipPath(new Path()..addOval(focusedRect));
paintLabels(secondaryLabels); paintLabels(secondaryLabels);
canvas.restore(); canvas.restore();
......
...@@ -466,6 +466,7 @@ class _FlutterLogoPainter extends BoxPainter { ...@@ -466,6 +466,7 @@ class _FlutterLogoPainter extends BoxPainter {
final double fontSize = 0.35 * logoTargetSquare.height * (1 - (10.4 * 2.0) / 202.0); final double fontSize = 0.35 * logoTargetSquare.height * (1 - (10.4 * 2.0) / 202.0);
final double scale = fontSize / 100.0; final double scale = fontSize / 100.0;
if (_config._position > -1.0) { if (_config._position > -1.0) {
// This limits what the drawRect call below is going to blend with.
canvas.saveLayer(_textBoundingRect, new Paint()); canvas.saveLayer(_textBoundingRect, new Paint());
} else { } else {
canvas.save(); canvas.save();
......
...@@ -330,10 +330,12 @@ class PaintingContext { ...@@ -330,10 +330,12 @@ class PaintingContext {
if (needsCompositing) { if (needsCompositing) {
pushLayer(new ClipRectLayer(clipRect: offsetClipRect), painter, offset, childPaintBounds: offsetClipRect); pushLayer(new ClipRectLayer(clipRect: offsetClipRect), painter, offset, childPaintBounds: offsetClipRect);
} else { } else {
canvas.save(); canvas
canvas.clipRect(offsetClipRect); ..save()
..clipRect(offsetClipRect);
painter(this, offset); painter(this, offset);
canvas.restore(); canvas
..restore();
} }
} }
...@@ -355,10 +357,14 @@ class PaintingContext { ...@@ -355,10 +357,14 @@ class PaintingContext {
if (needsCompositing) { if (needsCompositing) {
pushLayer(new ClipRRectLayer(clipRRect: offsetClipRRect), painter, offset, childPaintBounds: offsetBounds); pushLayer(new ClipRRectLayer(clipRRect: offsetClipRRect), painter, offset, childPaintBounds: offsetBounds);
} else { } else {
canvas.saveLayer(offsetBounds, _defaultPaint); canvas
canvas.clipRRect(offsetClipRRect); ..save()
..clipRRect(offsetClipRRect)
..saveLayer(offsetBounds, _defaultPaint);
painter(this, offset); painter(this, offset);
canvas.restore(); canvas
..restore()
..restore();
} }
} }
...@@ -380,10 +386,14 @@ class PaintingContext { ...@@ -380,10 +386,14 @@ class PaintingContext {
if (needsCompositing) { if (needsCompositing) {
pushLayer(new ClipPathLayer(clipPath: offsetClipPath), painter, offset, childPaintBounds: offsetBounds); pushLayer(new ClipPathLayer(clipPath: offsetClipPath), painter, offset, childPaintBounds: offsetBounds);
} else { } else {
canvas.saveLayer(bounds.shift(offset), _defaultPaint); canvas
canvas.clipPath(clipPath.shift(offset)); ..save()
..clipPath(clipPath.shift(offset))
..saveLayer(bounds.shift(offset), _defaultPaint);
painter(this, offset); painter(this, offset);
canvas.restore(); canvas
..restore()
..restore();
} }
} }
...@@ -407,10 +417,12 @@ class PaintingContext { ...@@ -407,10 +417,12 @@ class PaintingContext {
childPaintBounds: MatrixUtils.inverseTransformRect(effectiveTransform, canvasBounds), childPaintBounds: MatrixUtils.inverseTransformRect(effectiveTransform, canvasBounds),
); );
} else { } else {
canvas.save(); canvas
canvas.transform(effectiveTransform.storage); ..save()
..transform(effectiveTransform.storage);
painter(this, offset); painter(this, offset);
canvas.restore(); canvas
..restore();
} }
} }
......
...@@ -297,10 +297,13 @@ class RenderParagraph extends RenderBox { ...@@ -297,10 +297,13 @@ class RenderParagraph extends RenderBox {
if (_hasVisualOverflow) { if (_hasVisualOverflow) {
final Rect bounds = offset & size; final Rect bounds = offset & size;
if (_overflowShader != null) if (_overflowShader != null) {
// This layer limits what the shader below blends with to be just the text
// (as opposed to the text and its background).
canvas.saveLayer(bounds, new Paint()); canvas.saveLayer(bounds, new Paint());
else } else {
canvas.save(); canvas.save();
}
canvas.clipRect(bounds); canvas.clipRect(bounds);
} }
_textPainter.paint(canvas, offset); _textPainter.paint(canvas, offset);
......
...@@ -1351,13 +1351,19 @@ class RenderPhysicalModel extends _RenderCustomClip<RRect> { ...@@ -1351,13 +1351,19 @@ class RenderPhysicalModel extends _RenderCustomClip<RRect> {
); );
} }
canvas.drawRRect(offsetClipRRect, new Paint()..color = color); canvas.drawRRect(offsetClipRRect, new Paint()..color = color);
if (offsetClipRRect.isRect) {
canvas.save(); canvas.save();
} else {
canvas.saveLayer(offsetBounds, _defaultPaint);
}
canvas.clipRRect(offsetClipRRect); canvas.clipRRect(offsetClipRRect);
// We only use a new layer for non-rectangular clips, on the basis that
// rectangular clips won't need antialiasing. This is not really
// correct, because if we're e.g. rotated, rectangles will also be
// aliased. Unfortunately, it's too much of a performance win to err on
// the side of correctness here.
// TODO(ianh): Find a better solution.
if (!offsetClipRRect.isRect)
canvas.saveLayer(offsetBounds, _defaultPaint);
super.paint(context, offset); super.paint(context, offset);
if (!offsetClipRRect.isRect)
canvas.restore();
canvas.restore(); canvas.restore();
assert(context.canvas == canvas, 'canvas changed even though needsCompositing was false'); assert(context.canvas == canvas, 'canvas changed even though needsCompositing was 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