From 0c2546c657e1af2c955a986b493961ede360b69e Mon Sep 17 00:00:00 2001 From: Ian Hickson Date: Fri, 9 Jun 2017 16:50:55 -0700 Subject: [PATCH] Factor out some common code in PaintingContext (#10607) ...and remove some highly specialised methods now that they can just be implemented directly by the previous callers. --- .../flutter/lib/src/rendering/object.dart | 189 +++++------------- .../flutter/lib/src/rendering/proxy_box.dart | 52 ++++- packages/flutter/test/material/page_test.dart | 4 +- .../test/rendering/recording_canvas.dart | 11 +- 4 files changed, 111 insertions(+), 145 deletions(-) diff --git a/packages/flutter/lib/src/rendering/object.dart b/packages/flutter/lib/src/rendering/object.dart index 81856d505a5..6eb23ead8b2 100644 --- a/packages/flutter/lib/src/rendering/object.dart +++ b/packages/flutter/lib/src/rendering/object.dart @@ -3,7 +3,7 @@ // found in the LICENSE file. import 'dart:developer'; -import 'dart:ui' as ui show ImageFilter, PictureRecorder; +import 'dart:ui' as ui show PictureRecorder; import 'package:flutter/foundation.dart'; import 'package:flutter/gestures.dart'; @@ -46,8 +46,8 @@ typedef void PaintingContextCallback(PaintingContext context, Offset offset); /// A place to paint. /// -/// Rather than holding a canvas directly, render objects paint using a painting -/// context. The painting context has a canvas, which receives the +/// Rather than holding a canvas directly, [RenderObject]s paint using a painting +/// context. The painting context has a [Canvas], which receives the /// individual draw operations, and also has functions for painting child /// render objects. /// @@ -242,7 +242,7 @@ class PaintingContext { _currentLayer?.willChangeHint = true; } - /// Adds a composited layer to the recording. + /// Adds a composited leaf layer to the recording. /// /// After calling this function, the [canvas] property will change to refer to /// a new [Canvas] that draws on top of the given layer. @@ -250,12 +250,46 @@ class PaintingContext { /// A [RenderObject] that uses this function is very likely to require its /// [RenderObject.alwaysNeedsCompositing] property to return true. That informs /// ancestor render objects that this render object will include a composited - /// layer, which causes them to use composited clips, for example. + /// layer, which, for example, causes them to use composited clips. + /// + /// See also: + /// + /// * [pushLayer], for adding a layer and using its canvas to paint with that + /// layer. void addLayer(Layer layer) { _stopRecordingIfNeeded(); _appendLayer(layer); } + /// Appends the given layer to the recording, and calls the `painter` callback + /// with that layer, providing the [childPaintBounds] as the paint bounds of + /// the child. Canvas recording commands are not guaranteed to be stored + /// outside of the paint bounds. + /// + /// The given layer must be an unattached orphan. (Providing a newly created + /// object, rather than reusing an existing layer, satisfies that + /// requirement.) + /// + /// The `offset` is the offset to pass to the `painter`. + /// + /// If the `childPaintBounds` are not specified then the current layer's + /// bounds are used. This is appropriate if the child layer does not apply any + /// transformation or clipping to its contents. + /// + /// See also: + /// + /// * [addLayer], for pushing a leaf layer whose canvas is not used. + void pushLayer(Layer childLayer, PaintingContextCallback painter, Offset offset, { Rect childPaintBounds }) { + assert(!childLayer.attached); + assert(childLayer.parent == null); + assert(painter != null); + _stopRecordingIfNeeded(); + _appendLayer(childLayer); + final PaintingContext childContext = new PaintingContext._(childLayer, childPaintBounds ?? _paintBounds); + painter(childContext, offset); + childContext._stopRecordingIfNeeded(); + } + /// Clip further painting using a rectangle. /// /// * `needsCompositing` is whether the child needs compositing. Typically @@ -269,12 +303,7 @@ class PaintingContext { void pushClipRect(bool needsCompositing, Offset offset, Rect clipRect, PaintingContextCallback painter) { final Rect offsetClipRect = clipRect.shift(offset); if (needsCompositing) { - _stopRecordingIfNeeded(); - final ClipRectLayer clipLayer = new ClipRectLayer(clipRect: offsetClipRect); - _appendLayer(clipLayer); - final PaintingContext childContext = new PaintingContext._(clipLayer, offsetClipRect); - painter(childContext, offset); - childContext._stopRecordingIfNeeded(); + pushLayer(new ClipRectLayer(clipRect: offsetClipRect), painter, offset, childPaintBounds: offsetClipRect); } else { canvas.save(); canvas.clipRect(offsetClipRect); @@ -299,12 +328,7 @@ class PaintingContext { final Rect offsetBounds = bounds.shift(offset); final RRect offsetClipRRect = clipRRect.shift(offset); if (needsCompositing) { - _stopRecordingIfNeeded(); - final ClipRRectLayer clipLayer = new ClipRRectLayer(clipRRect: offsetClipRRect); - _appendLayer(clipLayer); - final PaintingContext childContext = new PaintingContext._(clipLayer, offsetBounds); - painter(childContext, offset); - childContext._stopRecordingIfNeeded(); + pushLayer(new ClipRRectLayer(clipRRect: offsetClipRRect), painter, offset, childPaintBounds: offsetBounds); } else { canvas.saveLayer(offsetBounds, _defaultPaint); canvas.clipRRect(offsetClipRRect); @@ -329,12 +353,7 @@ class PaintingContext { final Rect offsetBounds = bounds.shift(offset); final Path offsetClipPath = clipPath.shift(offset); if (needsCompositing) { - _stopRecordingIfNeeded(); - final ClipPathLayer clipLayer = new ClipPathLayer(clipPath: offsetClipPath); - _appendLayer(clipLayer); - final PaintingContext childContext = new PaintingContext._(clipLayer, offsetBounds); - painter(childContext, offset); - childContext._stopRecordingIfNeeded(); + pushLayer(new ClipPathLayer(clipPath: offsetClipPath), painter, offset, childPaintBounds: offsetBounds); } else { canvas.saveLayer(bounds.shift(offset), _defaultPaint); canvas.clipPath(clipPath.shift(offset)); @@ -356,13 +375,12 @@ class PaintingContext { final Matrix4 effectiveTransform = new Matrix4.translationValues(offset.dx, offset.dy, 0.0) ..multiply(transform)..translate(-offset.dx, -offset.dy); if (needsCompositing) { - _stopRecordingIfNeeded(); - final TransformLayer transformLayer = new TransformLayer(transform: effectiveTransform); - _appendLayer(transformLayer); - final Rect transformedPaintBounds = MatrixUtils.inverseTransformRect(effectiveTransform, _paintBounds); - final PaintingContext childContext = new PaintingContext._(transformLayer, transformedPaintBounds); - painter(childContext, offset); - childContext._stopRecordingIfNeeded(); + pushLayer( + new TransformLayer(transform: effectiveTransform), + painter, + offset, + childPaintBounds: MatrixUtils.inverseTransformRect(effectiveTransform, _paintBounds), + ); } else { canvas.save(); canvas.transform(effectiveTransform.storage); @@ -384,112 +402,9 @@ class PaintingContext { /// A [RenderObject] that uses this function is very likely to require its /// [RenderObject.alwaysNeedsCompositing] property to return true. That informs /// ancestor render objects that this render object will include a composited - /// layer, which causes them to use composited clips, for example. + /// layer, which, for example, causes them to use composited clips. void pushOpacity(Offset offset, int alpha, PaintingContextCallback painter) { - _stopRecordingIfNeeded(); - final OpacityLayer opacityLayer = new OpacityLayer(alpha: alpha); - _appendLayer(opacityLayer); - final PaintingContext childContext = new PaintingContext._(opacityLayer, _paintBounds); - painter(childContext, offset); - childContext._stopRecordingIfNeeded(); - } - - /// Apply a mask derived from a shader to further painting. - /// - /// * `offset` is the offset from the origin of the canvas' coordinate system - /// to the origin of the caller's coordinate system. - /// * `shader` is the shader that will generate the mask. The shader operates - /// in the coordinate system of the caller. - /// * `maskRect` is the region of the canvas (in the coodinate system of the - /// caller) in which to apply the mask. - /// * `blendMode` is the [BlendMode] to use when applying the shader to - /// the painting done by `painter`. - /// * `painter` is a callback that will paint with the mask applied. This - /// function calls the `painter` synchronously. - /// - /// A [RenderObject] that uses this function is very likely to require its - /// [RenderObject.alwaysNeedsCompositing] property to return true. That informs - /// ancestor render objects that this render object will include a composited - /// layer, which causes them to use composited clips, for example. - void pushShaderMask(Offset offset, Shader shader, Rect maskRect, BlendMode blendMode, PaintingContextCallback painter) { - _stopRecordingIfNeeded(); - final ShaderMaskLayer shaderLayer = new ShaderMaskLayer( - shader: shader, - maskRect: maskRect.shift(offset), - blendMode: blendMode, - ); - _appendLayer(shaderLayer); - final PaintingContext childContext = new PaintingContext._(shaderLayer, _paintBounds); - painter(childContext, offset); - childContext._stopRecordingIfNeeded(); - } - - /// Push a backdrop filter. - /// - /// This function applies a filter to the existing painted content and then - /// synchronously calls the painter to paint on top of the filtered backdrop. - /// - /// A [RenderObject] that uses this function is very likely to require its - /// [RenderObject.alwaysNeedsCompositing] property to return true. That informs - /// ancestor render objects that this render object will include a composited - /// layer, which causes them to use composited clips, for example. - // TODO(abarth): I don't quite understand how this API is supposed to work. - void pushBackdropFilter(Offset offset, ui.ImageFilter filter, PaintingContextCallback painter) { - _stopRecordingIfNeeded(); - final BackdropFilterLayer backdropFilterLayer = new BackdropFilterLayer(filter: filter); - _appendLayer(backdropFilterLayer); - final PaintingContext childContext = new PaintingContext._(backdropFilterLayer, _paintBounds); - painter(childContext, offset); - childContext._stopRecordingIfNeeded(); - } - - /// Clip using a physical model layer. - /// - /// * `offset` is the offset from the origin of the canvas' coordinate system - /// to the origin of the caller's coordinate system. - /// * `bounds` is the region of the canvas (in the caller's coodinate system) - /// into which `painter` will paint in. - /// * `clipRRect` is the rounded-rectangle (in the caller's coodinate system) - /// to use to clip the painting done by `painter`. - /// * `elevation` is the z-coordinate at which to place this material. - /// * `color` is the background color. - /// * `painter` is a callback that will paint with the `clipRRect` applied. This - /// function calls the `painter` synchronously. - void pushPhysicalModel(bool needsCompositing, Offset offset, Rect bounds, RRect clipRRect, double elevation, Color color, PaintingContextCallback painter) { - final Rect offsetBounds = bounds.shift(offset); - final RRect offsetClipRRect = clipRRect.shift(offset); - if (needsCompositing) { - _stopRecordingIfNeeded(); - final PhysicalModelLayer physicalModel = new PhysicalModelLayer( - clipRRect: offsetClipRRect, - elevation: elevation, - color: color, - ); - _appendLayer(physicalModel); - final PaintingContext childContext = new PaintingContext._(physicalModel, offsetBounds); - painter(childContext, offset); - childContext._stopRecordingIfNeeded(); - } else { - if (elevation != 0) { - // The drawShadow call doesn't add the region of the shadow to the - // picture's bounds, so we draw a hardcoded amount of extra space to - // account for the maximum potential area of the shadow. - // TODO(jsimmons): remove this when Skia does it for us. - canvas.drawRect(offsetBounds.inflate(20.0), - new Paint()..color=const Color(0)); - canvas.drawShadow( - new Path()..addRRect(offsetClipRRect), - const Color(0xFF000000), - elevation.toDouble(), - color.alpha != 0xFF, - ); - } - canvas.drawRRect(offsetClipRRect, new Paint()..color=color); - canvas.saveLayer(offsetBounds, _defaultPaint); - canvas.clipRRect(offsetClipRRect); - painter(this, offset); - canvas.restore(); - } + pushLayer(new OpacityLayer(alpha: alpha), painter, offset); } } @@ -2082,8 +1997,8 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget { /// creates at least one composited layer. For example, videos should return /// true if they use hardware decoders. /// - /// You must call markNeedsCompositingBitsUpdate() if the value of this - /// getter changes. + /// You must call [markNeedsCompositingBitsUpdate] if the value of this getter + /// changes. (This is implied when [adoptChild] or [dropChild] are called.) @protected bool get alwaysNeedsCompositing => false; diff --git a/packages/flutter/lib/src/rendering/proxy_box.dart b/packages/flutter/lib/src/rendering/proxy_box.dart index 757db32661a..08f0db94775 100644 --- a/packages/flutter/lib/src/rendering/proxy_box.dart +++ b/packages/flutter/lib/src/rendering/proxy_box.dart @@ -12,6 +12,7 @@ import 'package:vector_math/vector_math_64.dart'; import 'box.dart'; import 'debug.dart'; +import 'layer.dart'; import 'object.dart'; import 'semantics.dart'; @@ -837,8 +838,15 @@ class RenderShaderMask extends RenderProxyBox { void paint(PaintingContext context, Offset offset) { if (child != null) { assert(needsCompositing); - final Shader shader = _shaderCallback(offset & size); - context.pushShaderMask(offset, shader, Offset.zero & size, _blendMode, super.paint); + context.pushLayer( + new ShaderMaskLayer( + shader: _shaderCallback(offset & size), + maskRect: offset & size, + blendMode: _blendMode, + ), + super.paint, + offset, + ); } } } @@ -878,7 +886,7 @@ class RenderBackdropFilter extends RenderProxyBox { void paint(PaintingContext context, Offset offset) { if (child != null) { assert(needsCompositing); - context.pushBackdropFilter(offset, _filter, super.paint); + context.pushLayer(new BackdropFilterLayer(filter: _filter), super.paint, offset); } } } @@ -1308,11 +1316,47 @@ class RenderPhysicalModel extends _RenderCustomClip { return super.hitTest(result, position: position); } + static final Paint _defaultPaint = new Paint(); + static final Paint _transparentPaint = new Paint()..color = const Color(0x00000000); + @override void paint(PaintingContext context, Offset offset) { if (child != null) { _updateClip(); - context.pushPhysicalModel(needsCompositing, offset, _clip.outerRect, _clip, elevation, color, super.paint); + final RRect offsetClipRRect = _clip.shift(offset); + final Rect offsetBounds = offsetClipRRect.outerRect; + if (needsCompositing) { + final PhysicalModelLayer physicalModel = new PhysicalModelLayer( + clipRRect: offsetClipRRect, + elevation: elevation, + color: color, + ); + context.pushLayer(physicalModel, super.paint, offset, childPaintBounds: offsetBounds); + } else { + final Canvas canvas = context.canvas; + if (elevation != 0.0) { + // The drawShadow call doesn't add the region of the shadow to the + // picture's bounds, so we draw a hardcoded amount of extra space to + // account for the maximum potential area of the shadow. + // TODO(jsimmons): remove this when Skia does it for us. + canvas.drawRect( + offsetBounds.inflate(20.0), + _transparentPaint, + ); + canvas.drawShadow( + new Path()..addRRect(offsetClipRRect), + const Color(0xFF000000), + elevation, + color.alpha != 0xFF, + ); + } + canvas.drawRRect(offsetClipRRect, new Paint()..color = color); + canvas.saveLayer(offsetBounds, _defaultPaint); + canvas.clipRRect(offsetClipRRect); + super.paint(context, offset); + canvas.restore(); + assert(context.canvas == canvas, 'canvas changed even though needsCompositing was false'); + } } } diff --git a/packages/flutter/test/material/page_test.dart b/packages/flutter/test/material/page_test.dart index 9dbb93ac870..4545b42ac68 100644 --- a/packages/flutter/test/material/page_test.dart +++ b/packages/flutter/test/material/page_test.dart @@ -96,10 +96,10 @@ void main() { expect(widget1InitialTopLeft.dy == widget2TopLeft.dy, true); // Page 2 is coming in from the right. expect(widget2TopLeft.dx > widget1InitialTopLeft.dx, true); - // The shadow should be drawn to one screen width to the left of where + // The shadow should be drawn to one screen width to the left of where // the page 2 box is. `paints` tests relative to the painter's given canvas // rather than relative to the screen so assert that it's one screen - // width to the left of 0 offset box rect and nothing is drawn inside the + // width to the left of 0 offset box rect and nothing is drawn inside the // box's rect. expect(box, paints..rect( rect: new Rect.fromLTWH(-800.0, 0.0, 800.0, 600.0) diff --git a/packages/flutter/test/rendering/recording_canvas.dart b/packages/flutter/test/rendering/recording_canvas.dart index 481306fe9ca..dbcef517d71 100644 --- a/packages/flutter/test/rendering/recording_canvas.dart +++ b/packages/flutter/test/rendering/recording_canvas.dart @@ -38,6 +38,12 @@ class TestRecordingCanvas implements Canvas { invocations.add(new _MethodCall(#save)); } + @override + void saveLayer(Rect bounds, Paint paint) { + _saveCount += 1; + invocations.add(new _MethodCall(#saveLayer, [bounds, paint])); + } + @override void restore() { _saveCount -= 1; @@ -77,8 +83,9 @@ class TestRecordingPaintingContext implements PaintingContext { } class _MethodCall implements Invocation { - _MethodCall(this._name); + _MethodCall(this._name, [ this._arguments = const [] ]); final Symbol _name; + final List _arguments; @override bool get isAccessor => false; @override @@ -92,5 +99,5 @@ class _MethodCall implements Invocation { @override Map get namedArguments => {}; @override - List get positionalArguments => []; + List get positionalArguments => _arguments; }