From 61c54d506d8e8c72813d4c10f8d15f57fcb99f1a Mon Sep 17 00:00:00 2001 From: Yegor Jbanov Date: Thu, 26 Mar 2020 20:44:14 -0700 Subject: [PATCH 01/11] store paint command bounds --- .../src/engine/surface/recording_canvas.dart | 136 +++++++++++------- 1 file changed, 85 insertions(+), 51 deletions(-) diff --git a/lib/web_ui/lib/src/engine/surface/recording_canvas.dart b/lib/web_ui/lib/src/engine/surface/recording_canvas.dart index 5b26210ae2af2..bf0ea2d208bff 100644 --- a/lib/web_ui/lib/src/engine/surface/recording_canvas.dart +++ b/lib/web_ui/lib/src/engine/surface/recording_canvas.dart @@ -183,12 +183,14 @@ class RecordingCanvas { } void drawColor(ui.Color color, ui.BlendMode blendMode) { - _paintBounds.grow(_paintBounds.maxPaintBounds); - _commands.add(PaintDrawColor(color, blendMode)); + final PaintDrawColor command = PaintDrawColor(color, blendMode); + _commands.add(command); + _paintBounds.grow(_paintBounds.maxPaintBounds, command.bounds); } void drawLine(ui.Offset p1, ui.Offset p2, SurfacePaint paint) { final double paintSpread = math.max(_getPaintSpread(paint), 1.0); + final PaintDrawLine command = PaintDrawLine(p1, p2, paint.paintData); // TODO(yjbanov): This can be optimized. Currently we create a box around // the line and then apply the transform on the box to get // the bounding box. If you have a 45-degree line and a @@ -201,17 +203,19 @@ class RecordingCanvas { math.min(p1.dy, p2.dy) - paintSpread, math.max(p1.dx, p2.dx) + paintSpread, math.max(p1.dy, p2.dy) + paintSpread, + command.bounds, ); _hasArbitraryPaint = true; _didDraw = true; - _commands.add(PaintDrawLine(p1, p2, paint.paintData)); + _commands.add(command); } void drawPaint(SurfacePaint paint) { _hasArbitraryPaint = true; _didDraw = true; - _paintBounds.grow(_paintBounds.maxPaintBounds); - _commands.add(PaintDrawPaint(paint.paintData)); + final PaintDrawPaint command = PaintDrawPaint(paint.paintData); + _paintBounds.grow(_paintBounds.maxPaintBounds, command.bounds); + _commands.add(command); } void drawRect(ui.Rect rect, SurfacePaint paint) { @@ -220,12 +224,13 @@ class RecordingCanvas { } _didDraw = true; final double paintSpread = _getPaintSpread(paint); + final PaintDrawRect command = PaintDrawRect(rect, paint.paintData); if (paintSpread != 0.0) { - _paintBounds.grow(rect.inflate(paintSpread)); + _paintBounds.grow(rect.inflate(paintSpread), command.bounds); } else { - _paintBounds.grow(rect); + _paintBounds.grow(rect, command.bounds); } - _commands.add(PaintDrawRect(rect, paint.paintData)); + _commands.add(command); } void drawRRect(ui.RRect rrect, SurfacePaint paint) { @@ -238,8 +243,9 @@ class RecordingCanvas { final double top = math.min(rrect.top, rrect.bottom) - paintSpread; final double right = math.max(rrect.left, rrect.right) + paintSpread; final double bottom = math.max(rrect.top, rrect.bottom) + paintSpread; - _paintBounds.growLTRB(left, top, right, bottom); - _commands.add(PaintDrawRRect(rrect, paint.paintData)); + final PaintDrawRRect command = PaintDrawRRect(rrect, paint.paintData); + _paintBounds.growLTRB(left, top, right, bottom, command.bounds); + _commands.add(command); } void drawDRRect(ui.RRect outer, ui.RRect inner, SurfacePaint paint) { @@ -283,38 +289,43 @@ class RecordingCanvas { _hasArbitraryPaint = true; _didDraw = true; final double paintSpread = _getPaintSpread(paint); + final PaintDrawDRRect command = PaintDrawDRRect(outer, inner, paint.paintData); _paintBounds.growLTRB( outer.left - paintSpread, outer.top - paintSpread, outer.right + paintSpread, outer.bottom + paintSpread, + command.bounds, ); - _commands.add(PaintDrawDRRect(outer, inner, paint.paintData)); + _commands.add(command); } void drawOval(ui.Rect rect, SurfacePaint paint) { _hasArbitraryPaint = true; _didDraw = true; final double paintSpread = _getPaintSpread(paint); + final PaintDrawOval command = PaintDrawOval(rect, paint.paintData); if (paintSpread != 0.0) { - _paintBounds.grow(rect.inflate(paintSpread)); + _paintBounds.grow(rect.inflate(paintSpread), command.bounds); } else { - _paintBounds.grow(rect); + _paintBounds.grow(rect, command.bounds); } - _commands.add(PaintDrawOval(rect, paint.paintData)); + _commands.add(command); } void drawCircle(ui.Offset c, double radius, SurfacePaint paint) { _hasArbitraryPaint = true; _didDraw = true; final double paintSpread = _getPaintSpread(paint); + final PaintDrawCircle command = PaintDrawCircle(c, radius, paint.paintData); _paintBounds.growLTRB( c.dx - radius - paintSpread, c.dy - radius - paintSpread, c.dx + radius + paintSpread, c.dy + radius + paintSpread, + command.bounds, ); - _commands.add(PaintDrawCircle(c, radius, paint.paintData)); + _commands.add(command); } void drawPath(ui.Path path, SurfacePaint paint) { @@ -340,11 +351,12 @@ class RecordingCanvas { if (paintSpread != 0.0) { pathBounds = pathBounds.inflate(paintSpread); } - _paintBounds.grow(pathBounds); // Clone path so it can be reused for subsequent draw calls. final ui.Path clone = SurfacePath._shallowCopy(path); + final PaintDrawPath command = PaintDrawPath(clone, paint.paintData); + _paintBounds.grow(pathBounds, command.bounds); clone.fillType = path.fillType; - _commands.add(PaintDrawPath(clone, paint.paintData)); + _commands.add(command); } void drawImage(ui.Image image, ui.Offset offset, SurfacePaint paint) { @@ -352,16 +364,18 @@ class RecordingCanvas { _didDraw = true; final double left = offset.dx; final double top = offset.dy; - _paintBounds.growLTRB(left, top, left + image.width, top + image.height); - _commands.add(PaintDrawImage(image, offset, paint.paintData)); + final command = PaintDrawImage(image, offset, paint.paintData); + _paintBounds.growLTRB(left, top, left + image.width, top + image.height, command.bounds); + _commands.add(command); } void drawImageRect( ui.Image image, ui.Rect src, ui.Rect dst, SurfacePaint paint) { _hasArbitraryPaint = true; _didDraw = true; - _paintBounds.grow(dst); - _commands.add(PaintDrawImageRect(image, src, dst, paint.paintData)); + final PaintDrawImageRect command = PaintDrawImageRect(image, src, dst, paint.paintData); + _paintBounds.grow(dst, command.bounds); + _commands.add(command); } void drawParagraph(ui.Paragraph paragraph, ui.Offset offset) { @@ -377,9 +391,15 @@ class RecordingCanvas { } final double left = offset.dx; final double top = offset.dy; + final PaintDrawParagraph command = PaintDrawParagraph(engineParagraph, offset); _paintBounds.growLTRB( - left, top, left + engineParagraph.width, top + engineParagraph.height); - _commands.add(PaintDrawParagraph(engineParagraph, offset)); + left, + top, + left + engineParagraph.width, + top + engineParagraph.height, + command.bounds, + ); + _commands.add(command); } void drawShadow(ui.Path path, ui.Color color, double elevation, @@ -388,16 +408,18 @@ class RecordingCanvas { _didDraw = true; final ui.Rect shadowRect = computePenumbraBounds(path.getBounds(), elevation); - _paintBounds.grow(shadowRect); - _commands.add(PaintDrawShadow(path, color, elevation, transparentOccluder)); + final PaintDrawShadow command = PaintDrawShadow(path, color, elevation, transparentOccluder); + _paintBounds.grow(shadowRect, command.bounds); + _commands.add(command); } void drawVertices( ui.Vertices vertices, ui.BlendMode blendMode, SurfacePaint paint) { _hasArbitraryPaint = true; _didDraw = true; - _growPaintBoundsByPoints(vertices.positions, 0, paint); - _commands.add(PaintVertices(vertices, blendMode, paint.paintData)); + final PaintDrawVertices command = PaintDrawVertices(vertices, blendMode, paint.paintData); + _growPaintBoundsByPoints(vertices.positions, 0, paint, command.bounds); + _commands.add(command); } void drawRawPoints( @@ -407,12 +429,12 @@ class RecordingCanvas { } _hasArbitraryPaint = true; _didDraw = true; - _growPaintBoundsByPoints(points, paint.strokeWidth, paint); - _commands - .add(PaintPoints(pointMode, points, paint.strokeWidth, paint.color)); + final PaintDrawPoints command = PaintDrawPoints(pointMode, points, paint.strokeWidth, paint.color); + _growPaintBoundsByPoints(points, paint.strokeWidth, paint, command.bounds); + _commands.add(command); } - void _growPaintBoundsByPoints(Float32List points, double thickness, SurfacePaint paint) { + void _growPaintBoundsByPoints(Float32List points, double thickness, SurfacePaint paint, Float32List bounds) { double minValueX, maxValueX, minValueY, maxValueY; minValueX = maxValueX = points[0]; minValueY = maxValueY = points[1]; @@ -436,6 +458,7 @@ class RecordingCanvas { minValueY - distance - paintSpread, maxValueX + distance + paintSpread, maxValueY + distance + paintSpread, + bounds, ); } @@ -458,6 +481,11 @@ abstract class PaintCommand { void serializeToCssPaint(List> serializedCommands); } +/// A [PaintCommand] that puts pixels on the screen (unlike [SaveCommand]). +abstract class DrawCommand extends PaintCommand { + final Float32List bounds = Float32List(4); +} + class PaintSave extends PaintCommand { const PaintSave(); @@ -710,7 +738,7 @@ class PaintClipPath extends PaintCommand { } } -class PaintDrawColor extends PaintCommand { +class PaintDrawColor extends DrawCommand { final ui.Color color; final ui.BlendMode blendMode; @@ -737,7 +765,7 @@ class PaintDrawColor extends PaintCommand { } } -class PaintDrawLine extends PaintCommand { +class PaintDrawLine extends DrawCommand { final ui.Offset p1; final ui.Offset p2; final SurfacePaintData paint; @@ -771,7 +799,7 @@ class PaintDrawLine extends PaintCommand { } } -class PaintDrawPaint extends PaintCommand { +class PaintDrawPaint extends DrawCommand { final SurfacePaintData paint; PaintDrawPaint(this.paint); @@ -796,11 +824,11 @@ class PaintDrawPaint extends PaintCommand { } } -class PaintVertices extends PaintCommand { +class PaintDrawVertices extends DrawCommand { final ui.Vertices vertices; final ui.BlendMode blendMode; final SurfacePaintData paint; - PaintVertices(this.vertices, this.blendMode, this.paint); + PaintDrawVertices(this.vertices, this.blendMode, this.paint); @override void apply(EngineCanvas canvas) { @@ -822,12 +850,12 @@ class PaintVertices extends PaintCommand { } } -class PaintPoints extends PaintCommand { +class PaintDrawPoints extends DrawCommand { final Float32List points; final ui.PointMode pointMode; final double strokeWidth; final ui.Color color; - PaintPoints(this.pointMode, this.points, this.strokeWidth, this.color); + PaintDrawPoints(this.pointMode, this.points, this.strokeWidth, this.color); @override void apply(EngineCanvas canvas) { @@ -849,7 +877,7 @@ class PaintPoints extends PaintCommand { } } -class PaintDrawRect extends PaintCommand { +class PaintDrawRect extends DrawCommand { final ui.Rect rect; final SurfacePaintData paint; @@ -879,7 +907,7 @@ class PaintDrawRect extends PaintCommand { } } -class PaintDrawRRect extends PaintCommand { +class PaintDrawRRect extends DrawCommand { final ui.RRect rrect; final SurfacePaintData paint; @@ -909,7 +937,7 @@ class PaintDrawRRect extends PaintCommand { } } -class PaintDrawDRRect extends PaintCommand { +class PaintDrawDRRect extends DrawCommand { final ui.RRect outer; final ui.RRect inner; final SurfacePaintData paint; @@ -941,7 +969,7 @@ class PaintDrawDRRect extends PaintCommand { } } -class PaintDrawOval extends PaintCommand { +class PaintDrawOval extends DrawCommand { final ui.Rect rect; final SurfacePaintData paint; @@ -971,7 +999,7 @@ class PaintDrawOval extends PaintCommand { } } -class PaintDrawCircle extends PaintCommand { +class PaintDrawCircle extends DrawCommand { final ui.Offset c; final double radius; final SurfacePaintData paint; @@ -1004,7 +1032,7 @@ class PaintDrawCircle extends PaintCommand { } } -class PaintDrawPath extends PaintCommand { +class PaintDrawPath extends DrawCommand { final SurfacePath path; final SurfacePaintData paint; @@ -1034,7 +1062,7 @@ class PaintDrawPath extends PaintCommand { } } -class PaintDrawShadow extends PaintCommand { +class PaintDrawShadow extends DrawCommand { PaintDrawShadow( this.path, this.color, this.elevation, this.transparentOccluder); @@ -1074,7 +1102,7 @@ class PaintDrawShadow extends PaintCommand { } } -class PaintDrawImage extends PaintCommand { +class PaintDrawImage extends DrawCommand { final ui.Image image; final ui.Offset offset; final SurfacePaintData paint; @@ -1103,7 +1131,7 @@ class PaintDrawImage extends PaintCommand { } } -class PaintDrawImageRect extends PaintCommand { +class PaintDrawImageRect extends DrawCommand { final ui.Image image; final ui.Rect src; final ui.Rect dst; @@ -1133,7 +1161,7 @@ class PaintDrawImageRect extends PaintCommand { } } -class PaintDrawParagraph extends PaintCommand { +class PaintDrawParagraph extends DrawCommand { final EngineParagraph paragraph; final ui.Offset offset; @@ -1810,12 +1838,13 @@ class _PaintBounds { } /// Grow painted area to include given rectangle. - void grow(ui.Rect r) { - growLTRB(r.left, r.top, r.right, r.bottom); + void grow(ui.Rect r, Float32List bounds) { + growLTRB(r.left, r.top, r.right, r.bottom, bounds); } /// Grow painted area to include given rectangle. - void growLTRB(double left, double top, double right, double bottom) { + void growLTRB(double left, double top, double right, double bottom, Float32List bounds) { + assert(bounds.length == 4); if (left == right || top == bottom) { return; } @@ -1861,6 +1890,11 @@ class _PaintBounds { } } + bounds[0] = transformedPointLeft; + bounds[1] = transformedPointTop; + bounds[2] = transformedPointRight; + bounds[3] = transformedPointBottom; + if (_didPaintInsideClipArea) { _left = math.min( math.min(_left, transformedPointLeft), transformedPointRight); From d6bfb2b4d90e5db2a052a219e63c1f5c82958bc9 Mon Sep 17 00:00:00 2001 From: Yegor Jbanov Date: Fri, 27 Mar 2020 11:07:56 -0700 Subject: [PATCH 02/11] do not apply commands outside the clip region --- lib/web_ui/lib/src/engine/bitmap_canvas.dart | 2 +- lib/web_ui/lib/src/engine/picture.dart | 5 +-- .../lib/src/engine/surface/picture.dart | 8 ++--- .../src/engine/surface/recording_canvas.dart | 31 +++++++++++++++++-- 4 files changed, 37 insertions(+), 9 deletions(-) diff --git a/lib/web_ui/lib/src/engine/bitmap_canvas.dart b/lib/web_ui/lib/src/engine/bitmap_canvas.dart index 943d93d755e17..d9bd8fa25aef6 100644 --- a/lib/web_ui/lib/src/engine/bitmap_canvas.dart +++ b/lib/web_ui/lib/src/engine/bitmap_canvas.dart @@ -545,7 +545,7 @@ class BitmapCanvas extends EngineCanvas { /// Paints the [picture] into this canvas. void drawPicture(ui.Picture picture) { final EnginePicture enginePicture = picture; - enginePicture.recordingCanvas.apply(this); + enginePicture.recordingCanvas.apply(this, bounds); } /// Draws vertices on a gl context. diff --git a/lib/web_ui/lib/src/engine/picture.dart b/lib/web_ui/lib/src/engine/picture.dart index d88272cefc9ea..1b6a9d7a00eb5 100644 --- a/lib/web_ui/lib/src/engine/picture.dart +++ b/lib/web_ui/lib/src/engine/picture.dart @@ -46,8 +46,9 @@ class EnginePicture implements ui.Picture { @override Future toImage(int width, int height) async { - final BitmapCanvas canvas = BitmapCanvas(ui.Rect.fromLTRB(0, 0, width.toDouble(), height.toDouble())); - recordingCanvas.apply(canvas); + final ui.Rect imageRect = ui.Rect.fromLTRB(0, 0, width.toDouble(), height.toDouble()); + final BitmapCanvas canvas = BitmapCanvas(imageRect); + recordingCanvas.apply(canvas, imageRect); final String imageDataUrl = canvas.toDataUrl(); final html.ImageElement imageElement = html.ImageElement() ..src = imageDataUrl diff --git a/lib/web_ui/lib/src/engine/surface/picture.dart b/lib/web_ui/lib/src/engine/surface/picture.dart index c0a5f2198cc75..1a0de2859af97 100644 --- a/lib/web_ui/lib/src/engine/surface/picture.dart +++ b/lib/web_ui/lib/src/engine/surface/picture.dart @@ -145,7 +145,7 @@ class PersistedHoudiniPicture extends PersistedPicture { _canvas = canvas; domRenderer.clearDom(rootElement); rootElement.append(_canvas.rootElement); - picture.recordingCanvas.apply(_canvas); + picture.recordingCanvas.apply(_canvas, _optimalLocalCullRect); canvas.commit(); } } @@ -231,7 +231,7 @@ class PersistedStandardPicture extends PersistedPicture { _canvas = DomCanvas(); domRenderer.clearDom(rootElement); rootElement.append(_canvas.rootElement); - picture.recordingCanvas.apply(_canvas); + picture.recordingCanvas.apply(_canvas, _optimalLocalCullRect); } void _applyBitmapPaint(EngineCanvas oldCanvas) { @@ -244,7 +244,7 @@ class PersistedStandardPicture extends PersistedPicture { oldCanvas.bounds = _optimalLocalCullRect; _canvas = oldCanvas; _canvas.clear(); - picture.recordingCanvas.apply(_canvas); + picture.recordingCanvas.apply(_canvas, _optimalLocalCullRect); } else { // We can't use the old canvas because the size has changed, so we put // it in a cache for later reuse. @@ -265,7 +265,7 @@ class PersistedStandardPicture extends PersistedPicture { domRenderer.clearDom(rootElement); rootElement.append(_canvas.rootElement); _canvas.clear(); - picture.recordingCanvas.apply(_canvas); + picture.recordingCanvas.apply(_canvas, _optimalLocalCullRect); }, )); } diff --git a/lib/web_ui/lib/src/engine/surface/recording_canvas.dart b/lib/web_ui/lib/src/engine/surface/recording_canvas.dart index bf0ea2d208bff..9a861e92f2bd4 100644 --- a/lib/web_ui/lib/src/engine/surface/recording_canvas.dart +++ b/lib/web_ui/lib/src/engine/surface/recording_canvas.dart @@ -59,7 +59,9 @@ class RecordingCanvas { } /// Applies the recorded commands onto an [engineCanvas]. - void apply(EngineCanvas engineCanvas) { + /// + /// The [clipRect] specifies the clip applied to the picture (screen clip at a minimum). + void apply(EngineCanvas engineCanvas, ui.Rect clipRect) { if (_debugDumpPaintCommands) { final StringBuffer debugBuf = StringBuffer(); debugBuf.writeln( @@ -67,6 +69,13 @@ class RecordingCanvas { 'with bounds $_paintBounds'); for (int i = 0; i < _commands.length; i++) { final PaintCommand command = _commands[i]; + if (command is DrawCommand) { + if (_isOutsideClipRegion(command, clipRect)) { + // The drawing command is outside the clip region. No need to apply. + debugBuf.writeln('SKIPPED: ctx.$command;'); + continue; + } + } debugBuf.writeln('ctx.$command;'); command.apply(engineCanvas); } @@ -75,7 +84,13 @@ class RecordingCanvas { } else { try { for (int i = 0, len = _commands.length; i < len; i++) { - PaintCommand command = _commands[i]; + final PaintCommand command = _commands[i]; + if (command is DrawCommand) { + if (_isOutsideClipRegion(command, clipRect)) { + // The drawing command is outside the clip region. No need to apply. + continue; + } + } command.apply(engineCanvas); } } catch (e) { @@ -89,6 +104,13 @@ class RecordingCanvas { engineCanvas.endOfPaint(); } + static bool _isOutsideClipRegion(DrawCommand command, ui.Rect clipRect) { + return command.rightBound < clipRect.left || + command.bottomBound < clipRect.top || + command.leftBound > clipRect.right || + command.topBound > clipRect.bottom; + } + /// Prints recorded commands. String debugPrintCommands() { if (assertionsEnabled) { @@ -484,6 +506,11 @@ abstract class PaintCommand { /// A [PaintCommand] that puts pixels on the screen (unlike [SaveCommand]). abstract class DrawCommand extends PaintCommand { final Float32List bounds = Float32List(4); + + double get leftBound => bounds[0]; + double get topBound => bounds[1]; + double get rightBound => bounds[2]; + double get bottomBound => bounds[3]; } class PaintSave extends PaintCommand { From 1afd9eec70281a44db4e9be3682bc2ab57dbbc7b Mon Sep 17 00:00:00 2001 From: Yegor Jbanov Date: Sun, 29 Mar 2020 12:01:46 -0700 Subject: [PATCH 03/11] better cull rect prediction --- .../lib/src/engine/surface/picture.dart | 41 +++++++++++-------- .../src/engine/surface/recording_canvas.dart | 7 +++- 2 files changed, 31 insertions(+), 17 deletions(-) diff --git a/lib/web_ui/lib/src/engine/surface/picture.dart b/lib/web_ui/lib/src/engine/surface/picture.dart index 1a0de2859af97..2aaedc6bfd14c 100644 --- a/lib/web_ui/lib/src/engine/surface/picture.dart +++ b/lib/web_ui/lib/src/engine/surface/picture.dart @@ -501,24 +501,16 @@ abstract class PersistedPicture extends PersistedLeafSurface { // within the cull rect we compute now. // If any of the borders moved. - // TODO(yjbanov): consider switching to Mouad's snap-to-10px strategy. It - // might be sufficient, if not more effective. - const double kPredictedGrowthFactor = 3.0; - final double leftwardTrend = kPredictedGrowthFactor * - math.max(oldOptimalLocalCullRect.left - _exactLocalCullRect.left, 0); - final double upwardTrend = kPredictedGrowthFactor * - math.max(oldOptimalLocalCullRect.top - _exactLocalCullRect.top, 0); - final double rightwardTrend = kPredictedGrowthFactor * - math.max(_exactLocalCullRect.right - oldOptimalLocalCullRect.right, 0); - final double bottomwardTrend = kPredictedGrowthFactor * - math.max( - _exactLocalCullRect.bottom - oldOptimalLocalCullRect.bottom, 0); + final double leftwardDelta = oldOptimalLocalCullRect.left - _exactLocalCullRect.left; + final double upwardDelta = oldOptimalLocalCullRect.top - _exactLocalCullRect.top; + final double rightwardDelta = _exactLocalCullRect.right - oldOptimalLocalCullRect.right; + final double bottomwardDelta = _exactLocalCullRect.bottom - oldOptimalLocalCullRect.bottom; final ui.Rect newLocalCullRect = ui.Rect.fromLTRB( - oldOptimalLocalCullRect.left - leftwardTrend, - oldOptimalLocalCullRect.top - upwardTrend, - oldOptimalLocalCullRect.right + rightwardTrend, - oldOptimalLocalCullRect.bottom + bottomwardTrend, + _exactLocalCullRect.left - _predictTrend(leftwardDelta, _exactLocalCullRect.width), + _exactLocalCullRect.top - _predictTrend(upwardDelta, _exactLocalCullRect.height), + _exactLocalCullRect.right + _predictTrend(rightwardDelta, _exactLocalCullRect.width), + _exactLocalCullRect.bottom + _predictTrend(bottomwardDelta, _exactLocalCullRect.height), ).intersect(localPaintBounds); final bool localCullRectChanged = _optimalLocalCullRect != newLocalCullRect; @@ -526,6 +518,23 @@ abstract class PersistedPicture extends PersistedLeafSurface { return localCullRectChanged; } + static double _predictTrend(double delta, double extent) { + if (delta <= 0.0) { + // Shrinking. Give it 10% of the extent in case the trend is reversed. + return extent * 0.1; + } else { + print('>>> _predictTrend(extent = $extent, delta = $delta)'); + // Growing. Predict 10 more frames of similar deltas. Give it at least + // 50% of the extent (protect from extremely slow growth trend such as + // slow scrolling). Give no more than the full extent (protects from + // fast scrolling that could lead to overallocation). + return math.max( + math.min(extent * 0.5, delta * 10.0), + extent, + ); + } + } + /// Number of bitmap pixel painted by this picture. /// /// If the implementation does not paint onto a bitmap canvas, it should diff --git a/lib/web_ui/lib/src/engine/surface/recording_canvas.dart b/lib/web_ui/lib/src/engine/surface/recording_canvas.dart index 9a861e92f2bd4..c570164f84337 100644 --- a/lib/web_ui/lib/src/engine/surface/recording_canvas.dart +++ b/lib/web_ui/lib/src/engine/surface/recording_canvas.dart @@ -64,21 +64,26 @@ class RecordingCanvas { void apply(EngineCanvas engineCanvas, ui.Rect clipRect) { if (_debugDumpPaintCommands) { final StringBuffer debugBuf = StringBuffer(); + int skips = 0; debugBuf.writeln( '--- Applying RecordingCanvas to ${engineCanvas.runtimeType} ' - 'with bounds $_paintBounds'); + 'with bounds $_paintBounds and clip $clipRect (w = ${clipRect.width}, h = ${clipRect.height})'); for (int i = 0; i < _commands.length; i++) { final PaintCommand command = _commands[i]; if (command is DrawCommand) { if (_isOutsideClipRegion(command, clipRect)) { // The drawing command is outside the clip region. No need to apply. debugBuf.writeln('SKIPPED: ctx.$command;'); + skips += 1; continue; } } debugBuf.writeln('ctx.$command;'); command.apply(engineCanvas); } + if (skips > 0) { + debugBuf.writeln('Total commands skipped: $skips'); + } debugBuf.writeln('--- End of command stream'); print(debugBuf); } else { From e23cee4a4089c872b9ece98add72facfe1ef522c Mon Sep 17 00:00:00 2001 From: Yegor Jbanov Date: Wed, 8 Apr 2020 08:55:05 -0700 Subject: [PATCH 04/11] implement paint ops culling, better cull rect prediction --- lib/web_ui/lib/src/engine/picture.dart | 1 + .../lib/src/engine/surface/picture.dart | 2 +- .../src/engine/surface/recording_canvas.dart | 106 ++++++++++-------- 3 files changed, 64 insertions(+), 45 deletions(-) diff --git a/lib/web_ui/lib/src/engine/picture.dart b/lib/web_ui/lib/src/engine/picture.dart index 1b6a9d7a00eb5..04010a28f27f4 100644 --- a/lib/web_ui/lib/src/engine/picture.dart +++ b/lib/web_ui/lib/src/engine/picture.dart @@ -32,6 +32,7 @@ class EnginePictureRecorder implements ui.PictureRecorder { return null; } _isRecording = false; + _canvas.endRecording(); return EnginePicture(_canvas, cullRect); } } diff --git a/lib/web_ui/lib/src/engine/surface/picture.dart b/lib/web_ui/lib/src/engine/surface/picture.dart index 2aaedc6bfd14c..6091d7c8de080 100644 --- a/lib/web_ui/lib/src/engine/surface/picture.dart +++ b/lib/web_ui/lib/src/engine/surface/picture.dart @@ -352,7 +352,7 @@ class PersistedStandardPicture extends PersistedPicture { /// to draw shapes and text. abstract class PersistedPicture extends PersistedLeafSurface { PersistedPicture(this.dx, this.dy, this.picture, this.hints) - : localPaintBounds = picture.recordingCanvas.computePaintBounds(); + : localPaintBounds = picture.recordingCanvas.pictureBounds; EngineCanvas _canvas; diff --git a/lib/web_ui/lib/src/engine/surface/recording_canvas.dart b/lib/web_ui/lib/src/engine/surface/recording_canvas.dart index c570164f84337..99424052283e3 100644 --- a/lib/web_ui/lib/src/engine/surface/recording_canvas.dart +++ b/lib/web_ui/lib/src/engine/surface/recording_canvas.dart @@ -23,8 +23,19 @@ double _measureBorderRadius(double x, double y) { /// /// See [Canvas] for docs for these methods. class RecordingCanvas { - /// Maximum paintable bounds for this canvas. + /// Computes [_pictureBounds]. final _PaintBounds _paintBounds; + + /// Maximum paintable bounds for this canvas. + ui.Rect _pictureBounds; + ui.Rect get pictureBounds { + assert( + _pictureBounds != null, + 'Picture bounds not available yet. Call [endRecording] before accessing picture bounds.', + ); + return _pictureBounds; + } + final List _commands = []; RecordingCanvas(ui.Rect bounds) : _paintBounds = _PaintBounds(bounds); @@ -53,9 +64,9 @@ class RecordingCanvas { bool get didDraw => _didDraw; bool _didDraw = false; - /// Computes paint bounds based on estimated [bounds] and transforms. - ui.Rect computePaintBounds() { - return _paintBounds.computeBounds(); + /// Stops recording drawing commands and computes paint bounds. + void endRecording() { + _pictureBounds = _paintBounds.computeBounds(); } /// Applies the recorded commands onto an [engineCanvas]. @@ -88,15 +99,25 @@ class RecordingCanvas { print(debugBuf); } else { try { - for (int i = 0, len = _commands.length; i < len; i++) { - final PaintCommand command = _commands[i]; - if (command is DrawCommand) { - if (_isOutsideClipRegion(command, clipRect)) { - // The drawing command is outside the clip region. No need to apply. - continue; + if (rectContainsOther(clipRect, _pictureBounds)) { + // No need to check if commands fit in the clip rect if we already + // know that the entire picture fits it. + for (int i = 0, len = _commands.length; i < len; i++) { + _commands[i].apply(engineCanvas); + } + } else { + // The picture doesn't fit the clip rect. Check that drawing commands + // fit before applying them. + for (int i = 0, len = _commands.length; i < len; i++) { + final PaintCommand command = _commands[i]; + if (command is DrawCommand) { + if (_isOutsideClipRegion(command, clipRect)) { + // The drawing command is outside the clip region. No need to apply. + continue; + } } + command.apply(engineCanvas); } - command.apply(engineCanvas); } } catch (e) { // commands should never fail, but... @@ -212,7 +233,7 @@ class RecordingCanvas { void drawColor(ui.Color color, ui.BlendMode blendMode) { final PaintDrawColor command = PaintDrawColor(color, blendMode); _commands.add(command); - _paintBounds.grow(_paintBounds.maxPaintBounds, command.bounds); + _paintBounds.grow(_paintBounds.maxPaintBounds, command); } void drawLine(ui.Offset p1, ui.Offset p2, SurfacePaint paint) { @@ -230,7 +251,7 @@ class RecordingCanvas { math.min(p1.dy, p2.dy) - paintSpread, math.max(p1.dx, p2.dx) + paintSpread, math.max(p1.dy, p2.dy) + paintSpread, - command.bounds, + command, ); _hasArbitraryPaint = true; _didDraw = true; @@ -241,7 +262,7 @@ class RecordingCanvas { _hasArbitraryPaint = true; _didDraw = true; final PaintDrawPaint command = PaintDrawPaint(paint.paintData); - _paintBounds.grow(_paintBounds.maxPaintBounds, command.bounds); + _paintBounds.grow(_paintBounds.maxPaintBounds, command); _commands.add(command); } @@ -253,9 +274,9 @@ class RecordingCanvas { final double paintSpread = _getPaintSpread(paint); final PaintDrawRect command = PaintDrawRect(rect, paint.paintData); if (paintSpread != 0.0) { - _paintBounds.grow(rect.inflate(paintSpread), command.bounds); + _paintBounds.grow(rect.inflate(paintSpread), command); } else { - _paintBounds.grow(rect, command.bounds); + _paintBounds.grow(rect, command); } _commands.add(command); } @@ -271,7 +292,7 @@ class RecordingCanvas { final double right = math.max(rrect.left, rrect.right) + paintSpread; final double bottom = math.max(rrect.top, rrect.bottom) + paintSpread; final PaintDrawRRect command = PaintDrawRRect(rrect, paint.paintData); - _paintBounds.growLTRB(left, top, right, bottom, command.bounds); + _paintBounds.growLTRB(left, top, right, bottom, command); _commands.add(command); } @@ -322,7 +343,7 @@ class RecordingCanvas { outer.top - paintSpread, outer.right + paintSpread, outer.bottom + paintSpread, - command.bounds, + command, ); _commands.add(command); } @@ -333,9 +354,9 @@ class RecordingCanvas { final double paintSpread = _getPaintSpread(paint); final PaintDrawOval command = PaintDrawOval(rect, paint.paintData); if (paintSpread != 0.0) { - _paintBounds.grow(rect.inflate(paintSpread), command.bounds); + _paintBounds.grow(rect.inflate(paintSpread), command); } else { - _paintBounds.grow(rect, command.bounds); + _paintBounds.grow(rect, command); } _commands.add(command); } @@ -350,7 +371,7 @@ class RecordingCanvas { c.dy - radius - paintSpread, c.dx + radius + paintSpread, c.dy + radius + paintSpread, - command.bounds, + command, ); _commands.add(command); } @@ -381,7 +402,7 @@ class RecordingCanvas { // Clone path so it can be reused for subsequent draw calls. final ui.Path clone = SurfacePath._shallowCopy(path); final PaintDrawPath command = PaintDrawPath(clone, paint.paintData); - _paintBounds.grow(pathBounds, command.bounds); + _paintBounds.grow(pathBounds, command); clone.fillType = path.fillType; _commands.add(command); } @@ -392,7 +413,7 @@ class RecordingCanvas { final double left = offset.dx; final double top = offset.dy; final command = PaintDrawImage(image, offset, paint.paintData); - _paintBounds.growLTRB(left, top, left + image.width, top + image.height, command.bounds); + _paintBounds.growLTRB(left, top, left + image.width, top + image.height, command); _commands.add(command); } @@ -401,7 +422,7 @@ class RecordingCanvas { _hasArbitraryPaint = true; _didDraw = true; final PaintDrawImageRect command = PaintDrawImageRect(image, src, dst, paint.paintData); - _paintBounds.grow(dst, command.bounds); + _paintBounds.grow(dst, command); _commands.add(command); } @@ -424,7 +445,7 @@ class RecordingCanvas { top, left + engineParagraph.width, top + engineParagraph.height, - command.bounds, + command, ); _commands.add(command); } @@ -436,7 +457,7 @@ class RecordingCanvas { final ui.Rect shadowRect = computePenumbraBounds(path.getBounds(), elevation); final PaintDrawShadow command = PaintDrawShadow(path, color, elevation, transparentOccluder); - _paintBounds.grow(shadowRect, command.bounds); + _paintBounds.grow(shadowRect, command); _commands.add(command); } @@ -445,7 +466,7 @@ class RecordingCanvas { _hasArbitraryPaint = true; _didDraw = true; final PaintDrawVertices command = PaintDrawVertices(vertices, blendMode, paint.paintData); - _growPaintBoundsByPoints(vertices.positions, 0, paint, command.bounds); + _growPaintBoundsByPoints(vertices.positions, 0, paint, command); _commands.add(command); } @@ -457,11 +478,11 @@ class RecordingCanvas { _hasArbitraryPaint = true; _didDraw = true; final PaintDrawPoints command = PaintDrawPoints(pointMode, points, paint.strokeWidth, paint.color); - _growPaintBoundsByPoints(points, paint.strokeWidth, paint, command.bounds); + _growPaintBoundsByPoints(points, paint.strokeWidth, paint, command); _commands.add(command); } - void _growPaintBoundsByPoints(Float32List points, double thickness, SurfacePaint paint, Float32List bounds) { + void _growPaintBoundsByPoints(Float32List points, double thickness, SurfacePaint paint, DrawCommand command) { double minValueX, maxValueX, minValueY, maxValueY; minValueX = maxValueX = points[0]; minValueY = maxValueY = points[1]; @@ -485,7 +506,7 @@ class RecordingCanvas { minValueY - distance - paintSpread, maxValueX + distance + paintSpread, maxValueY + distance + paintSpread, - bounds, + command, ); } @@ -510,12 +531,10 @@ abstract class PaintCommand { /// A [PaintCommand] that puts pixels on the screen (unlike [SaveCommand]). abstract class DrawCommand extends PaintCommand { - final Float32List bounds = Float32List(4); - - double get leftBound => bounds[0]; - double get topBound => bounds[1]; - double get rightBound => bounds[2]; - double get bottomBound => bounds[3]; + double leftBound = double.negativeInfinity; + double topBound = double.negativeInfinity; + double rightBound = double.infinity; + double bottomBound = double.infinity; } class PaintSave extends PaintCommand { @@ -1870,13 +1889,12 @@ class _PaintBounds { } /// Grow painted area to include given rectangle. - void grow(ui.Rect r, Float32List bounds) { - growLTRB(r.left, r.top, r.right, r.bottom, bounds); + void grow(ui.Rect r, DrawCommand command) { + growLTRB(r.left, r.top, r.right, r.bottom, command); } /// Grow painted area to include given rectangle. - void growLTRB(double left, double top, double right, double bottom, Float32List bounds) { - assert(bounds.length == 4); + void growLTRB(double left, double top, double right, double bottom, DrawCommand command) { if (left == right || top == bottom) { return; } @@ -1922,10 +1940,10 @@ class _PaintBounds { } } - bounds[0] = transformedPointLeft; - bounds[1] = transformedPointTop; - bounds[2] = transformedPointRight; - bounds[3] = transformedPointBottom; + command.leftBound = transformedPointLeft; + command.topBound = transformedPointTop; + command.rightBound = transformedPointRight; + command.bottomBound = transformedPointBottom; if (_didPaintInsideClipArea) { _left = math.min( From 190db2956e1bd2402217727461e930d89a9b04f4 Mon Sep 17 00:00:00 2001 From: Yegor Jbanov Date: Wed, 8 Apr 2020 09:31:57 -0700 Subject: [PATCH 05/11] remove stray print statement --- lib/web_ui/lib/src/engine/surface/picture.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/web_ui/lib/src/engine/surface/picture.dart b/lib/web_ui/lib/src/engine/surface/picture.dart index 6091d7c8de080..af832c68b76e4 100644 --- a/lib/web_ui/lib/src/engine/surface/picture.dart +++ b/lib/web_ui/lib/src/engine/surface/picture.dart @@ -523,7 +523,6 @@ abstract class PersistedPicture extends PersistedLeafSurface { // Shrinking. Give it 10% of the extent in case the trend is reversed. return extent * 0.1; } else { - print('>>> _predictTrend(extent = $extent, delta = $delta)'); // Growing. Predict 10 more frames of similar deltas. Give it at least // 50% of the extent (protect from extremely slow growth trend such as // slow scrolling). Give no more than the full extent (protects from From a9886a668ec7a69415a2ff15e26ac3bc29c83f89 Mon Sep 17 00:00:00 2001 From: Yegor Jbanov Date: Thu, 9 Apr 2020 10:59:22 -0700 Subject: [PATCH 06/11] enforce endRecording --- .../src/engine/surface/recording_canvas.dart | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/lib/web_ui/lib/src/engine/surface/recording_canvas.dart b/lib/web_ui/lib/src/engine/surface/recording_canvas.dart index 99424052283e3..50e326233c72a 100644 --- a/lib/web_ui/lib/src/engine/surface/recording_canvas.dart +++ b/lib/web_ui/lib/src/engine/surface/recording_canvas.dart @@ -64,9 +64,18 @@ class RecordingCanvas { bool get didDraw => _didDraw; bool _didDraw = false; + bool _debugRecordingEnded = false; + /// Stops recording drawing commands and computes paint bounds. + /// + /// This must be called prior to passing the picture to the [SceneBuilder] + /// for rendering. In a production app, this is done automatically by + /// [PictureRecorder] when the framework calls [PictureRecorder.endRecording]. + /// However, if you are writing a unit-test and using [RecordingCanvas] + /// directly it is up to you to call this method explicitly. void endRecording() { _pictureBounds = _paintBounds.computeBounds(); + _debugRecordingEnded = true; } /// Applies the recorded commands onto an [engineCanvas]. @@ -151,12 +160,14 @@ class RecordingCanvas { } void save() { + assert(!_debugRecordingEnded); _paintBounds.saveTransformsAndClip(); _commands.add(const PaintSave()); _saveCount++; } void saveLayerWithoutBounds(SurfacePaint paint) { + assert(!_debugRecordingEnded); _hasArbitraryPaint = true; // TODO(het): Implement this correctly using another canvas. _commands.add(const PaintSave()); @@ -165,6 +176,7 @@ class RecordingCanvas { } void saveLayer(ui.Rect bounds, SurfacePaint paint) { + assert(!_debugRecordingEnded); _hasArbitraryPaint = true; // TODO(het): Implement this correctly using another canvas. _commands.add(const PaintSave()); @@ -173,6 +185,7 @@ class RecordingCanvas { } void restore() { + assert(!_debugRecordingEnded); _paintBounds.restoreTransformsAndClip(); if (_commands.isNotEmpty && _commands.last is PaintSave) { // A restore followed a save without any drawing operations in between. @@ -187,56 +200,66 @@ class RecordingCanvas { } void translate(double dx, double dy) { + assert(!_debugRecordingEnded); _paintBounds.translate(dx, dy); _commands.add(PaintTranslate(dx, dy)); } void scale(double sx, double sy) { + assert(!_debugRecordingEnded); _paintBounds.scale(sx, sy); _commands.add(PaintScale(sx, sy)); } void rotate(double radians) { + assert(!_debugRecordingEnded); _paintBounds.rotateZ(radians); _commands.add(PaintRotate(radians)); } void transform(Float64List matrix4) { + assert(!_debugRecordingEnded); _paintBounds.transform(matrix4); _commands.add(PaintTransform(matrix4)); } void skew(double sx, double sy) { + assert(!_debugRecordingEnded); _hasArbitraryPaint = true; _paintBounds.skew(sx, sy); _commands.add(PaintSkew(sx, sy)); } void clipRect(ui.Rect rect) { + assert(!_debugRecordingEnded); _paintBounds.clipRect(rect); _hasArbitraryPaint = true; _commands.add(PaintClipRect(rect)); } void clipRRect(ui.RRect rrect) { + assert(!_debugRecordingEnded); _paintBounds.clipRect(rrect.outerRect); _hasArbitraryPaint = true; _commands.add(PaintClipRRect(rrect)); } void clipPath(ui.Path path, {bool doAntiAlias = true}) { + assert(!_debugRecordingEnded); _paintBounds.clipRect(path.getBounds()); _hasArbitraryPaint = true; _commands.add(PaintClipPath(path)); } void drawColor(ui.Color color, ui.BlendMode blendMode) { + assert(!_debugRecordingEnded); final PaintDrawColor command = PaintDrawColor(color, blendMode); _commands.add(command); _paintBounds.grow(_paintBounds.maxPaintBounds, command); } void drawLine(ui.Offset p1, ui.Offset p2, SurfacePaint paint) { + assert(!_debugRecordingEnded); final double paintSpread = math.max(_getPaintSpread(paint), 1.0); final PaintDrawLine command = PaintDrawLine(p1, p2, paint.paintData); // TODO(yjbanov): This can be optimized. Currently we create a box around @@ -259,6 +282,7 @@ class RecordingCanvas { } void drawPaint(SurfacePaint paint) { + assert(!_debugRecordingEnded); _hasArbitraryPaint = true; _didDraw = true; final PaintDrawPaint command = PaintDrawPaint(paint.paintData); @@ -267,6 +291,7 @@ class RecordingCanvas { } void drawRect(ui.Rect rect, SurfacePaint paint) { + assert(!_debugRecordingEnded); if (paint.shader != null) { _hasArbitraryPaint = true; } @@ -282,6 +307,7 @@ class RecordingCanvas { } void drawRRect(ui.RRect rrect, SurfacePaint paint) { + assert(!_debugRecordingEnded); if (paint.shader != null || !rrect.webOnlyUniformRadii) { _hasArbitraryPaint = true; } @@ -297,6 +323,7 @@ class RecordingCanvas { } void drawDRRect(ui.RRect outer, ui.RRect inner, SurfacePaint paint) { + assert(!_debugRecordingEnded); // Check the inner bounds are contained within the outer bounds // see: https://cs.chromium.org/chromium/src/third_party/skia/src/core/SkCanvas.cpp?l=1787-1789 ui.Rect innerRect = inner.outerRect; @@ -349,6 +376,7 @@ class RecordingCanvas { } void drawOval(ui.Rect rect, SurfacePaint paint) { + assert(!_debugRecordingEnded); _hasArbitraryPaint = true; _didDraw = true; final double paintSpread = _getPaintSpread(paint); @@ -362,6 +390,7 @@ class RecordingCanvas { } void drawCircle(ui.Offset c, double radius, SurfacePaint paint) { + assert(!_debugRecordingEnded); _hasArbitraryPaint = true; _didDraw = true; final double paintSpread = _getPaintSpread(paint); @@ -377,6 +406,7 @@ class RecordingCanvas { } void drawPath(ui.Path path, SurfacePaint paint) { + assert(!_debugRecordingEnded); if (paint.shader == null) { // For Rect/RoundedRect paths use drawRect/drawRRect code paths for // DomCanvas optimization. @@ -408,6 +438,7 @@ class RecordingCanvas { } void drawImage(ui.Image image, ui.Offset offset, SurfacePaint paint) { + assert(!_debugRecordingEnded); _hasArbitraryPaint = true; _didDraw = true; final double left = offset.dx; @@ -419,6 +450,7 @@ class RecordingCanvas { void drawImageRect( ui.Image image, ui.Rect src, ui.Rect dst, SurfacePaint paint) { + assert(!_debugRecordingEnded); _hasArbitraryPaint = true; _didDraw = true; final PaintDrawImageRect command = PaintDrawImageRect(image, src, dst, paint.paintData); @@ -427,6 +459,7 @@ class RecordingCanvas { } void drawParagraph(ui.Paragraph paragraph, ui.Offset offset) { + assert(!_debugRecordingEnded); final EngineParagraph engineParagraph = paragraph; if (!engineParagraph._isLaidOut) { // Ignore non-laid out paragraphs. This matches Flutter's behavior. @@ -452,6 +485,7 @@ class RecordingCanvas { void drawShadow(ui.Path path, ui.Color color, double elevation, bool transparentOccluder) { + assert(!_debugRecordingEnded); _hasArbitraryPaint = true; _didDraw = true; final ui.Rect shadowRect = @@ -463,6 +497,7 @@ class RecordingCanvas { void drawVertices( ui.Vertices vertices, ui.BlendMode blendMode, SurfacePaint paint) { + assert(!_debugRecordingEnded); _hasArbitraryPaint = true; _didDraw = true; final PaintDrawVertices command = PaintDrawVertices(vertices, blendMode, paint.paintData); @@ -472,6 +507,7 @@ class RecordingCanvas { void drawRawPoints( ui.PointMode pointMode, Float32List points, SurfacePaint paint) { + assert(!_debugRecordingEnded); if (paint.strokeWidth == null) { return; } From ec7fa26561071fd5ffade7515ee3ce0989ea273d Mon Sep 17 00:00:00 2001 From: Yegor Jbanov Date: Thu, 9 Apr 2020 10:59:32 -0700 Subject: [PATCH 07/11] fix some tests --- .../engine/canvas_draw_image_golden_test.dart | 2 +- .../engine/recording_canvas_golden_test.dart | 136 ++++++++++++------ 2 files changed, 94 insertions(+), 44 deletions(-) diff --git a/lib/web_ui/test/golden_tests/engine/canvas_draw_image_golden_test.dart b/lib/web_ui/test/golden_tests/engine/canvas_draw_image_golden_test.dart index dcaf6c93b59bd..8243868dae997 100644 --- a/lib/web_ui/test/golden_tests/engine/canvas_draw_image_golden_test.dart +++ b/lib/web_ui/test/golden_tests/engine/canvas_draw_image_golden_test.dart @@ -26,7 +26,7 @@ void main() async { double maxDiffRatePercent = 0.0}) async { final EngineCanvas engineCanvas = BitmapCanvas(screenRect); - rc.apply(engineCanvas); + rc.apply(engineCanvas, screenRect); // Wrap in so that our CSS selectors kick in. final html.Element sceneElement = html.Element.tag('flt-scene'); diff --git a/lib/web_ui/test/golden_tests/engine/recording_canvas_golden_test.dart b/lib/web_ui/test/golden_tests/engine/recording_canvas_golden_test.dart index c015a1ea4ec45..6b842b5267c61 100644 --- a/lib/web_ui/test/golden_tests/engine/recording_canvas_golden_test.dart +++ b/lib/web_ui/test/golden_tests/engine/recording_canvas_golden_test.dart @@ -30,7 +30,7 @@ void main() async { engineCanvas ..save() ..drawRect( - rc.computePaintBounds(), + rc.pictureBounds, SurfacePaintData() ..color = const Color.fromRGBO(0, 0, 255, 1.0) ..style = PaintingStyle.stroke @@ -38,7 +38,7 @@ void main() async { ) ..restore(); - rc.apply(engineCanvas); + rc.apply(engineCanvas, screenRect); // Wrap in so that our CSS selectors kick in. final html.Element sceneElement = html.Element.tag('flt-scene'); @@ -63,15 +63,17 @@ void main() async { test('Empty canvas reports correct paint bounds', () async { final RecordingCanvas rc = RecordingCanvas(const Rect.fromLTWH(1, 2, 300, 400)); - expect(rc.computePaintBounds(), Rect.zero); + rc.endRecording(); + expect(rc.pictureBounds, Rect.zero); await _checkScreenshot(rc, 'empty_canvas'); }); test('Computes paint bounds for draw line', () async { final RecordingCanvas rc = RecordingCanvas(screenRect); rc.drawLine(const Offset(50, 100), const Offset(120, 140), testPaint); + rc.endRecording(); // The off by one is due to the minimum stroke width of 1. - expect(rc.computePaintBounds(), const Rect.fromLTRB(49, 99, 121, 141)); + expect(rc.pictureBounds, const Rect.fromLTRB(49, 99, 121, 141)); await _checkScreenshot(rc, 'draw_line'); }); @@ -81,8 +83,9 @@ void main() async { final RecordingCanvas rc = RecordingCanvas(screenRect); rc.drawLine(const Offset(50, 100), const Offset(screenWidth + 100.0, 140), testPaint); + rc.endRecording(); // The off by one is due to the minimum stroke width of 1. - expect(rc.computePaintBounds(), + expect(rc.pictureBounds, const Rect.fromLTRB(49.0, 99.0, screenWidth, 141.0)); await _checkScreenshot(rc, 'draw_line_exceeding_limits'); }); @@ -90,7 +93,8 @@ void main() async { test('Computes paint bounds for draw rect', () async { final RecordingCanvas rc = RecordingCanvas(screenRect); rc.drawRect(const Rect.fromLTRB(10, 20, 30, 40), testPaint); - expect(rc.computePaintBounds(), const Rect.fromLTRB(10, 20, 30, 40)); + rc.endRecording(); + expect(rc.pictureBounds, const Rect.fromLTRB(10, 20, 30, 40)); await _checkScreenshot(rc, 'draw_rect'); }); @@ -100,12 +104,14 @@ void main() async { rc.drawRect( const Rect.fromLTRB(10, 20, 30 + screenWidth, 40 + screenHeight), testPaint); - expect(rc.computePaintBounds(), + rc.endRecording(); + expect(rc.pictureBounds, const Rect.fromLTRB(10, 20, screenWidth, screenHeight)); rc = RecordingCanvas(screenRect); rc.drawRect(const Rect.fromLTRB(-200, -100, 30, 40), testPaint); - expect(rc.computePaintBounds(), const Rect.fromLTRB(0, 0, 30, 40)); + rc.endRecording(); + expect(rc.pictureBounds, const Rect.fromLTRB(0, 0, 30, 40)); await _checkScreenshot(rc, 'draw_rect_exceeding_limits'); }); @@ -113,7 +119,8 @@ void main() async { final RecordingCanvas rc = RecordingCanvas(screenRect); rc.translate(5, 7); rc.drawRect(const Rect.fromLTRB(10, 20, 30, 40), testPaint); - expect(rc.computePaintBounds(), const Rect.fromLTRB(15, 27, 35, 47)); + rc.endRecording(); + expect(rc.pictureBounds, const Rect.fromLTRB(15, 27, 35, 47)); await _checkScreenshot(rc, 'translate'); }); @@ -121,7 +128,8 @@ void main() async { final RecordingCanvas rc = RecordingCanvas(screenRect); rc.scale(2, 2); rc.drawRect(const Rect.fromLTRB(10, 20, 30, 40), testPaint); - expect(rc.computePaintBounds(), const Rect.fromLTRB(20, 40, 60, 80)); + rc.endRecording(); + expect(rc.pictureBounds, const Rect.fromLTRB(20, 40, 60, 80)); await _checkScreenshot(rc, 'scale'); }); @@ -130,8 +138,9 @@ void main() async { rc.rotate(math.pi / 4.0); rc.drawLine( const Offset(1, 0), Offset(50 * math.sqrt(2) - 1, 0), testPaint); + rc.endRecording(); // The extra 0.7 is due to stroke width of 1 rotated by 45 degrees. - expect(rc.computePaintBounds(), + expect(rc.pictureBounds, within(distance: 0.1, from: const Rect.fromLTRB(0, 0, 50.7, 50.7))); await _checkScreenshot(rc, 'rotate'); }); @@ -140,8 +149,9 @@ void main() async { final RecordingCanvas rc = RecordingCanvas(screenRect); rc.skew(1.0, 0.0); rc.drawRect(const Rect.fromLTRB(20, 20, 40, 40), testPaint); + rc.endRecording(); expect( - rc.computePaintBounds(), + rc.pictureBounds, within( distance: 0.1, from: const Rect.fromLTRB(40.0, 20.0, 80.0, 40.0))); await _checkScreenshot(rc, 'skew_horizontally'); @@ -151,8 +161,9 @@ void main() async { final RecordingCanvas rc = RecordingCanvas(screenRect); rc.skew(0.0, 1.0); rc.drawRect(const Rect.fromLTRB(20, 20, 40, 40), testPaint); + rc.endRecording(); expect( - rc.computePaintBounds(), + rc.pictureBounds, within( distance: 0.1, from: const Rect.fromLTRB(20.0, 40.0, 40.0, 80.0))); await _checkScreenshot(rc, 'skew_vertically'); @@ -180,7 +191,8 @@ void main() async { matrix[15] = 1.0; rc.transform(matrix); rc.drawRect(const Rect.fromLTRB(10, 20, 30, 40), testPaint); - expect(rc.computePaintBounds(), + rc.endRecording(); + expect(rc.pictureBounds, const Rect.fromLTRB(168.0, 283.6, 224.0, 368.4)); await _checkScreenshot(rc, 'complex_transform'); }); @@ -189,7 +201,8 @@ void main() async { final RecordingCanvas rc = RecordingCanvas(screenRect); rc.drawPaint(testPaint); rc.drawRect(const Rect.fromLTRB(10, 20, 30, 40), testPaint); - expect(rc.computePaintBounds(), screenRect); + rc.endRecording(); + expect(rc.pictureBounds, screenRect); await _checkScreenshot(rc, 'draw_paint'); }); @@ -199,14 +212,16 @@ void main() async { rc.drawRect(const Rect.fromLTRB(10, 20, 30, 40), testPaint); rc.drawColor(const Color(0xFFFF0000), BlendMode.multiply); rc.drawRect(const Rect.fromLTRB(10, 60, 30, 80), testPaint); - expect(rc.computePaintBounds(), screenRect); + rc.endRecording(); + expect(rc.pictureBounds, screenRect); await _checkScreenshot(rc, 'draw_color'); }); test('Computes paint bounds for draw oval', () async { final RecordingCanvas rc = RecordingCanvas(screenRect); rc.drawOval(const Rect.fromLTRB(10, 20, 30, 40), testPaint); - expect(rc.computePaintBounds(), const Rect.fromLTRB(10, 20, 30, 40)); + rc.endRecording(); + expect(rc.pictureBounds, const Rect.fromLTRB(10, 20, 30, 40)); await _checkScreenshot(rc, 'draw_oval'); }); @@ -216,7 +231,8 @@ void main() async { RRect.fromRectAndRadius( const Rect.fromLTRB(10, 20, 30, 40), const Radius.circular(5.0)), testPaint); - expect(rc.computePaintBounds(), const Rect.fromLTRB(10, 20, 30, 40)); + rc.endRecording(); + expect(rc.pictureBounds, const Rect.fromLTRB(10, 20, 30, 40)); await _checkScreenshot(rc, 'draw_round_rect'); }); @@ -226,7 +242,8 @@ void main() async { final RecordingCanvas rc = RecordingCanvas(screenRect); rc.drawDRRect(RRect.fromRectAndCorners(const Rect.fromLTRB(10, 20, 30, 40)), RRect.fromRectAndCorners(const Rect.fromLTRB(1, 2, 3, 4)), testPaint); - expect(rc.computePaintBounds(), const Rect.fromLTRB(0, 0, 0, 0)); + rc.endRecording(); + expect(rc.pictureBounds, const Rect.fromLTRB(0, 0, 0, 0)); await _checkScreenshot(rc, 'draw_drrect_empty'); }); @@ -236,34 +253,44 @@ void main() async { RRect.fromRectAndCorners(const Rect.fromLTRB(10, 20, 30, 40)), RRect.fromRectAndCorners(const Rect.fromLTRB(12, 22, 28, 38)), testPaint); - expect(rc.computePaintBounds(), const Rect.fromLTRB(10, 20, 30, 40)); + rc.endRecording(); + expect(rc.pictureBounds, const Rect.fromLTRB(10, 20, 30, 40)); await _checkScreenshot(rc, 'draw_drrect'); }); test('Computes paint bounds for draw circle', () async { - final RecordingCanvas rc = RecordingCanvas(screenRect); + // Paint bounds of one circle. + RecordingCanvas rc = RecordingCanvas(screenRect); rc.drawCircle(const Offset(20, 20), 10.0, testPaint); + rc.endRecording(); expect( - rc.computePaintBounds(), const Rect.fromLTRB(10.0, 10.0, 30.0, 30.0)); + rc.pictureBounds, const Rect.fromLTRB(10.0, 10.0, 30.0, 30.0)); + + // Paint bounds of a union of two circles. + rc = RecordingCanvas(screenRect); + rc.drawCircle(const Offset(20, 20), 10.0, testPaint); rc.drawCircle(const Offset(200, 300), 100.0, testPaint); + rc.endRecording(); expect( - rc.computePaintBounds(), const Rect.fromLTRB(10.0, 10.0, 300.0, 400.0)); + rc.pictureBounds, const Rect.fromLTRB(10.0, 10.0, 300.0, 400.0)); await _checkScreenshot(rc, 'draw_circle'); }); test('Computes paint bounds for draw image', () { final RecordingCanvas rc = RecordingCanvas(screenRect); rc.drawImage(TestImage(), const Offset(50, 100), Paint()); + rc.endRecording(); expect( - rc.computePaintBounds(), const Rect.fromLTRB(50.0, 100.0, 70.0, 110.0)); + rc.pictureBounds, const Rect.fromLTRB(50.0, 100.0, 70.0, 110.0)); }); test('Computes paint bounds for draw image rect', () { final RecordingCanvas rc = RecordingCanvas(screenRect); rc.drawImageRect(TestImage(), const Rect.fromLTRB(1, 1, 20, 10), const Rect.fromLTRB(5, 6, 400, 500), Paint()); + rc.endRecording(); expect( - rc.computePaintBounds(), const Rect.fromLTRB(5.0, 6.0, 400.0, 500.0)); + rc.pictureBounds, const Rect.fromLTRB(5.0, 6.0, 400.0, 500.0)); }); test('Computes paint bounds for single-line draw paragraph', () async { @@ -274,8 +301,9 @@ void main() async { const double widthConstraint = 300.0; paragraph.layout(const ParagraphConstraints(width: widthConstraint)); rc.drawParagraph(paragraph, const Offset(textLeft, textTop)); + rc.endRecording(); expect( - rc.computePaintBounds(), + rc.pictureBounds, const Rect.fromLTRB(textLeft, textTop, textLeft + widthConstraint, 21.0), ); await _checkScreenshot(rc, 'draw_paragraph'); @@ -286,27 +314,35 @@ void main() async { final Paragraph paragraph = createTestParagraph(); const double textLeft = 5.0; const double textTop = 7.0; - const double widthConstraint = - 130.0; // do not go lower than the shortest word. + // Do not go lower than the shortest word. + const double widthConstraint = 130.0; paragraph.layout(const ParagraphConstraints(width: widthConstraint)); rc.drawParagraph(paragraph, const Offset(textLeft, textTop)); + rc.endRecording(); expect( - rc.computePaintBounds(), + rc.pictureBounds, const Rect.fromLTRB(textLeft, textTop, textLeft + widthConstraint, 35.0), ); await _checkScreenshot(rc, 'draw_paragraph_multi_line'); }); test('Should exclude painting outside simple clipRect', () async { - final RecordingCanvas rc = RecordingCanvas(screenRect); + // One clipped line. + RecordingCanvas rc = RecordingCanvas(screenRect); rc.clipRect(const Rect.fromLTRB(50, 50, 100, 100)); rc.drawLine(const Offset(10, 11), const Offset(20, 21), testPaint); + rc.endRecording(); + expect(rc.pictureBounds, Rect.zero); - expect(rc.computePaintBounds(), Rect.zero); + // Two clipped lines. + rc = RecordingCanvas(screenRect); + rc.clipRect(const Rect.fromLTRB(50, 50, 100, 100)); + rc.drawLine(const Offset(10, 11), const Offset(20, 21), testPaint); rc.drawLine(const Offset(52, 53), const Offset(55, 56), testPaint); + rc.endRecording(); // Extra pixel due to default line length - expect(rc.computePaintBounds(), const Rect.fromLTRB(51, 52, 56, 57)); + expect(rc.pictureBounds, const Rect.fromLTRB(51, 52, 56, 57)); await _checkScreenshot(rc, 'clip_rect_simple'); }); @@ -314,13 +350,15 @@ void main() async { RecordingCanvas rc = RecordingCanvas(screenRect); rc.clipRect(const Rect.fromLTRB(50, 50, 100, 100)); rc.drawRect(const Rect.fromLTRB(20, 60, 120, 70), testPaint); - expect(rc.computePaintBounds(), const Rect.fromLTRB(50, 60, 100, 70)); + rc.endRecording(); + expect(rc.pictureBounds, const Rect.fromLTRB(50, 60, 100, 70)); await _checkScreenshot(rc, 'clip_rect_intersects_paint_left_to_right'); rc = RecordingCanvas(screenRect); rc.clipRect(const Rect.fromLTRB(50, 50, 100, 100)); rc.drawRect(const Rect.fromLTRB(60, 20, 70, 200), testPaint); - expect(rc.computePaintBounds(), const Rect.fromLTRB(60, 50, 70, 100)); + rc.endRecording(); + expect(rc.pictureBounds, const Rect.fromLTRB(60, 50, 70, 100)); await _checkScreenshot(rc, 'clip_rect_intersects_paint_top_to_bottom'); }); @@ -330,7 +368,9 @@ void main() async { rc.scale(2.0, 2.0); rc.clipRect(const Rect.fromLTRB(30, 30, 45, 45)); rc.drawRect(const Rect.fromLTRB(10, 30, 60, 35), testPaint); - expect(rc.computePaintBounds(), const Rect.fromLTRB(60, 60, 90, 70)); + rc.endRecording(); + + expect(rc.pictureBounds, const Rect.fromLTRB(60, 60, 90, 70)); await _checkScreenshot(rc, 'clip_rects_intersect'); }); @@ -340,8 +380,10 @@ void main() async { final Path path = Path(); path.addRect(const Rect.fromLTRB(20, 30, 100, 110)); rc.drawShadow(path, const Color(0xFFFF0000), 2.0, true); + rc.endRecording(); + expect( - rc.computePaintBounds(), + rc.pictureBounds, within(distance: 0.05, from: const Rect.fromLTRB(17.9, 28.5, 103.5, 114.1)), ); await _checkScreenshot(rc, 'path_with_shadow'); @@ -361,8 +403,10 @@ void main() async { ..scale(1, -1) ..clipRect(const Rect.fromLTRB(0, 0, 100, 50)) ..drawRect(const Rect.fromLTRB(0, 0, 100, 100), Paint()); + rc.endRecording(); + expect( - rc.computePaintBounds(), const Rect.fromLTRB(0.0, 50.0, 100.0, 100.0)); + rc.pictureBounds, const Rect.fromLTRB(0.0, 50.0, 100.0, 100.0)); await _checkScreenshot(rc, 'scale_negative'); }); @@ -374,8 +418,10 @@ void main() async { ..rotate(math.pi / 4.0) ..clipRect(const Rect.fromLTWH(-20, -20, 40, 40)) ..drawRect(const Rect.fromLTWH(-80, -80, 160, 160), Paint()); + rc.endRecording(); + expect( - rc.computePaintBounds(), + rc.pictureBounds, Rect.fromCircle(center: const Offset(50, 50), radius: 20 * math.sqrt(2)), ); await _checkScreenshot(rc, 'clip_rect_rotated'); @@ -388,8 +434,10 @@ void main() async { ..translate(50, 50) ..rotate(math.pi / 4.0) ..drawLine(const Offset(0, 0), const Offset(20, 20), Paint()); + rc.endRecording(); + expect( - rc.computePaintBounds(), + rc.pictureBounds, within(distance: 0.1, from: const Rect.fromLTRB(34.4, 48.6, 65.6, 79.7)), ); await _checkScreenshot(rc, 'line_rotated'); @@ -418,6 +466,7 @@ void main() async { ..style = PaintingStyle.stroke ..strokeWidth = 2.0 ..color = const Color(0xFF00FF00)); + rc.endRecording(); await _checkScreenshot(rc, 'reuse_path'); }); @@ -440,6 +489,7 @@ void main() async { ..strokeWidth = 2.0 ..color = const Color(0xFF404000)); rc.restore(); + rc.endRecording(); await _checkScreenshot(rc, 'path_with_line_and_roundrect'); }); @@ -536,7 +586,7 @@ void main() async { final SurfacePaint zeroSpreadPaint = SurfacePaint(); painter(canvas, zeroSpreadPaint); sb.addPicture(Offset.zero, recorder.endRecording()); - sb.addPicture(Offset.zero, drawBounds(canvas.computePaintBounds())); + sb.addPicture(Offset.zero, drawBounds(canvas.pictureBounds)); sb.pop(); } @@ -550,7 +600,7 @@ void main() async { ..strokeWidth = 5.0; painter(canvas, thickStrokePaint); sb.addPicture(Offset.zero, recorder.endRecording()); - sb.addPicture(Offset.zero, drawBounds(canvas.computePaintBounds())); + sb.addPicture(Offset.zero, drawBounds(canvas.pictureBounds)); sb.pop(); } @@ -563,7 +613,7 @@ void main() async { ..maskFilter = const MaskFilter.blur(BlurStyle.normal, 5.0); painter(canvas, maskFilterBlurPaint); sb.addPicture(Offset.zero, recorder.endRecording()); - sb.addPicture(Offset.zero, drawBounds(canvas.computePaintBounds())); + sb.addPicture(Offset.zero, drawBounds(canvas.pictureBounds)); sb.pop(); } @@ -578,7 +628,7 @@ void main() async { ..maskFilter = const MaskFilter.blur(BlurStyle.normal, 5.0); painter(canvas, thickStrokeAndBlurPaint); sb.addPicture(Offset.zero, recorder.endRecording()); - sb.addPicture(Offset.zero, drawBounds(canvas.computePaintBounds())); + sb.addPicture(Offset.zero, drawBounds(canvas.pictureBounds)); sb.pop(); } From 348997bf6da153c31c68eea01ce1ebae88b91df0 Mon Sep 17 00:00:00 2001 From: Yegor Jbanov Date: Thu, 9 Apr 2020 14:30:13 -0700 Subject: [PATCH 08/11] update tests --- .../src/engine/surface/recording_canvas.dart | 1 + lib/web_ui/test/canvas_test.dart | 14 +++++++------ .../test/engine/recording_canvas_test.dart | 21 ++++++++++++------- .../engine/canvas_blend_golden_test.dart | 3 ++- .../engine/canvas_clip_path_test.dart | 3 ++- .../engine/canvas_context_test.dart | 3 ++- .../engine/canvas_draw_image_golden_test.dart | 1 + .../engine/canvas_reuse_test.dart | 6 ++++-- .../engine/conic_golden_test.dart | 3 ++- .../engine/draw_vertices_golden_test.dart | 3 ++- .../engine/linear_gradient_golden_test.dart | 3 ++- .../multiline_text_clipping_golden_test.dart | 7 ++++--- .../engine/path_metrics_test.dart | 3 ++- .../engine/path_to_svg_golden_test.dart | 3 ++- .../engine/path_transform_test.dart | 3 ++- .../engine/radial_gradient_golden_test.dart | 3 ++- 16 files changed, 52 insertions(+), 28 deletions(-) diff --git a/lib/web_ui/lib/src/engine/surface/recording_canvas.dart b/lib/web_ui/lib/src/engine/surface/recording_canvas.dart index 50e326233c72a..0a72b7a5f068b 100644 --- a/lib/web_ui/lib/src/engine/surface/recording_canvas.dart +++ b/lib/web_ui/lib/src/engine/surface/recording_canvas.dart @@ -82,6 +82,7 @@ class RecordingCanvas { /// /// The [clipRect] specifies the clip applied to the picture (screen clip at a minimum). void apply(EngineCanvas engineCanvas, ui.Rect clipRect) { + assert(_debugRecordingEnded); if (_debugDumpPaintCommands) { final StringBuffer debugBuf = StringBuffer(); int skips = 0; diff --git a/lib/web_ui/test/canvas_test.dart b/lib/web_ui/test/canvas_test.dart index ce9d1499caca2..cb7bcd2fbcf57 100644 --- a/lib/web_ui/test/canvas_test.dart +++ b/lib/web_ui/test/canvas_test.dart @@ -36,16 +36,17 @@ void main() { } testCanvas('draws laid out paragraph', (EngineCanvas canvas) { - final RecordingCanvas recordingCanvas = - RecordingCanvas(const ui.Rect.fromLTWH(0, 0, 100, 100)); + final ui.Rect screenRect = const ui.Rect.fromLTWH(0, 0, 100, 100); + final RecordingCanvas recordingCanvas = RecordingCanvas(screenRect); final ui.ParagraphBuilder builder = ui.ParagraphBuilder(ui.ParagraphStyle()); builder.addText('sample'); paragraph = builder.build(); paragraph.layout(const ui.ParagraphConstraints(width: 100)); recordingCanvas.drawParagraph(paragraph, const ui.Offset(10, 10)); + recordingCanvas.endRecording(); canvas.clear(); - recordingCanvas.apply(canvas); + recordingCanvas.apply(canvas, screenRect); }, whenDone: () { expect(mockCanvas.methodCallLog, hasLength(3)); @@ -60,15 +61,16 @@ void main() { testCanvas('ignores paragraphs that were not laid out', (EngineCanvas canvas) { - final RecordingCanvas recordingCanvas = - RecordingCanvas(const ui.Rect.fromLTWH(0, 0, 100, 100)); + final ui.Rect screenRect = const ui.Rect.fromLTWH(0, 0, 100, 100); + final RecordingCanvas recordingCanvas = RecordingCanvas(screenRect); final ui.ParagraphBuilder builder = ui.ParagraphBuilder(ui.ParagraphStyle()); builder.addText('sample'); final ui.Paragraph paragraph = builder.build(); recordingCanvas.drawParagraph(paragraph, const ui.Offset(10, 10)); + recordingCanvas.endRecording(); canvas.clear(); - recordingCanvas.apply(canvas); + recordingCanvas.apply(canvas, screenRect); }, whenDone: () { expect(mockCanvas.methodCallLog, hasLength(2)); expect(mockCanvas.methodCallLog[0].methodName, 'clear'); diff --git a/lib/web_ui/test/engine/recording_canvas_test.dart b/lib/web_ui/test/engine/recording_canvas_test.dart index 2757ba96fd4a4..de3fa3c9c3c92 100644 --- a/lib/web_ui/test/engine/recording_canvas_test.dart +++ b/lib/web_ui/test/engine/recording_canvas_test.dart @@ -12,9 +12,10 @@ import '../mock_engine_canvas.dart'; void main() { RecordingCanvas underTest; MockEngineCanvas mockCanvas; + final Rect screenRect = Rect.largest; setUp(() { - underTest = RecordingCanvas(Rect.largest); + underTest = RecordingCanvas(screenRect); mockCanvas = MockEngineCanvas(); }); @@ -25,7 +26,8 @@ void main() { test('Happy case', () { underTest.drawDRRect(rrect, rrect.deflate(1), somePaint); - underTest.apply(mockCanvas); + underTest.endRecording(); + underTest.apply(mockCanvas, screenRect); _expectDrawCall(mockCanvas, { 'outer': rrect, @@ -36,7 +38,8 @@ void main() { test('Inner RRect > Outer RRect', () { underTest.drawDRRect(rrect, rrect.inflate(1), somePaint); - underTest.apply(mockCanvas); + underTest.endRecording(); + underTest.apply(mockCanvas, screenRect); // Expect nothing to be called expect(mockCanvas.methodCallLog.length, equals(1)); expect(mockCanvas.methodCallLog.single.methodName, 'endOfPaint'); @@ -45,7 +48,8 @@ void main() { test('Inner RRect not completely inside Outer RRect', () { underTest.drawDRRect( rrect, rrect.deflate(1).shift(const Offset(0.0, 10)), somePaint); - underTest.apply(mockCanvas); + underTest.endRecording(); + underTest.apply(mockCanvas, screenRect); // Expect nothing to be called expect(mockCanvas.methodCallLog.length, equals(1)); expect(mockCanvas.methodCallLog.single.methodName, 'endOfPaint'); @@ -53,7 +57,8 @@ void main() { test('Inner RRect same as Outer RRect', () { underTest.drawDRRect(rrect, rrect, somePaint); - underTest.apply(mockCanvas); + underTest.endRecording(); + underTest.apply(mockCanvas, screenRect); // Expect nothing to be called expect(mockCanvas.methodCallLog.length, equals(1)); expect(mockCanvas.methodCallLog.single.methodName, 'endOfPaint'); @@ -72,7 +77,8 @@ void main() { expect(inner.trRadius, equals(Radius.circular(-1))); underTest.drawDRRect(outer, inner, somePaint); - underTest.apply(mockCanvas); + underTest.endRecording(); + underTest.apply(mockCanvas, screenRect); // Expect to draw, even when inner has negative radii (which get ignored by canvas) _expectDrawCall(mockCanvas, { @@ -89,7 +95,8 @@ void main() { RRect.fromRectAndCorners(const Rect.fromLTRB(12, 22, 28, 38)); underTest.drawDRRect(outer, inner, somePaint); - underTest.apply(mockCanvas); + underTest.endRecording(); + underTest.apply(mockCanvas, screenRect); _expectDrawCall(mockCanvas, { 'outer': outer, diff --git a/lib/web_ui/test/golden_tests/engine/canvas_blend_golden_test.dart b/lib/web_ui/test/golden_tests/engine/canvas_blend_golden_test.dart index 694274bfe516e..8407e48766b33 100644 --- a/lib/web_ui/test/golden_tests/engine/canvas_blend_golden_test.dart +++ b/lib/web_ui/test/golden_tests/engine/canvas_blend_golden_test.dart @@ -23,7 +23,8 @@ void main() async { double maxDiffRatePercent = 0.0}) async { final EngineCanvas engineCanvas = BitmapCanvas(screenRect); - rc.apply(engineCanvas); + rc.endRecording(); + rc.apply(engineCanvas, screenRect); // Wrap in so that our CSS selectors kick in. final html.Element sceneElement = html.Element.tag('flt-scene'); diff --git a/lib/web_ui/test/golden_tests/engine/canvas_clip_path_test.dart b/lib/web_ui/test/golden_tests/engine/canvas_clip_path_test.dart index 316b6b3467fde..d01620921c1d7 100644 --- a/lib/web_ui/test/golden_tests/engine/canvas_clip_path_test.dart +++ b/lib/web_ui/test/golden_tests/engine/canvas_clip_path_test.dart @@ -22,7 +22,8 @@ void main() async { {Rect region = const Rect.fromLTWH(0, 0, 500, 500)}) async { final engine.EngineCanvas engineCanvas = engine.BitmapCanvas(screenRect); - rc.apply(engineCanvas); + rc.endRecording(); + rc.apply(engineCanvas, screenRect); // Wrap in so that our CSS selectors kick in. final html.Element sceneElement = html.Element.tag('flt-scene'); diff --git a/lib/web_ui/test/golden_tests/engine/canvas_context_test.dart b/lib/web_ui/test/golden_tests/engine/canvas_context_test.dart index f0d2f15a08a77..927bf2a300fbd 100644 --- a/lib/web_ui/test/golden_tests/engine/canvas_context_test.dart +++ b/lib/web_ui/test/golden_tests/engine/canvas_context_test.dart @@ -22,7 +22,8 @@ void main() async { {Rect region = const Rect.fromLTWH(0, 0, 500, 500)}) async { final engine.EngineCanvas engineCanvas = engine.BitmapCanvas(screenRect); - rc.apply(engineCanvas); + rc.endRecording(); + rc.apply(engineCanvas, screenRect); // Wrap in so that our CSS selectors kick in. final html.Element sceneElement = html.Element.tag('flt-scene'); diff --git a/lib/web_ui/test/golden_tests/engine/canvas_draw_image_golden_test.dart b/lib/web_ui/test/golden_tests/engine/canvas_draw_image_golden_test.dart index 8243868dae997..6a61f401b25bd 100644 --- a/lib/web_ui/test/golden_tests/engine/canvas_draw_image_golden_test.dart +++ b/lib/web_ui/test/golden_tests/engine/canvas_draw_image_golden_test.dart @@ -26,6 +26,7 @@ void main() async { double maxDiffRatePercent = 0.0}) async { final EngineCanvas engineCanvas = BitmapCanvas(screenRect); + rc.endRecording(); rc.apply(engineCanvas, screenRect); // Wrap in so that our CSS selectors kick in. diff --git a/lib/web_ui/test/golden_tests/engine/canvas_reuse_test.dart b/lib/web_ui/test/golden_tests/engine/canvas_reuse_test.dart index e92f55287a252..469cc2f88faf8 100644 --- a/lib/web_ui/test/golden_tests/engine/canvas_reuse_test.dart +++ b/lib/web_ui/test/golden_tests/engine/canvas_reuse_test.dart @@ -39,7 +39,8 @@ void main() async { ..moveTo(3, 0) ..lineTo(100, 97); rc.drawPath(path, testPaint); - rc.apply(engineCanvas); + rc.endRecording(); + rc.apply(engineCanvas, screenRect); engineCanvas.endOfPaint(); html.Element sceneElement = html.Element.tag('flt-scene'); @@ -69,7 +70,8 @@ void main() async { ..quadraticBezierTo(100, 0, 100, 100); rc2.drawImage(_createRealTestImage(), Offset(0, 0), Paint()); rc2.drawPath(path2, testPaint); - rc2.apply(engineCanvas); + rc2.endRecording(); + rc2.apply(engineCanvas, screenRect); sceneElement = html.Element.tag('flt-scene'); sceneElement.append(engineCanvas.rootElement); diff --git a/lib/web_ui/test/golden_tests/engine/conic_golden_test.dart b/lib/web_ui/test/golden_tests/engine/conic_golden_test.dart index 0ec9770603d40..bcf69c155e212 100644 --- a/lib/web_ui/test/golden_tests/engine/conic_golden_test.dart +++ b/lib/web_ui/test/golden_tests/engine/conic_golden_test.dart @@ -31,9 +31,10 @@ void main() async { ..style = PaintingStyle.stroke; canvas.drawPath(path, paint); + canvas.endRecording(); html.document.body.append(bitmapCanvas.rootElement); - canvas.apply(bitmapCanvas); + canvas.apply(bitmapCanvas, canvasBounds); await matchGoldenFile('$scubaFileName.png', region: region); bitmapCanvas.rootElement.remove(); } diff --git a/lib/web_ui/test/golden_tests/engine/draw_vertices_golden_test.dart b/lib/web_ui/test/golden_tests/engine/draw_vertices_golden_test.dart index dc77dbc781ae8..765c614ae4cfc 100644 --- a/lib/web_ui/test/golden_tests/engine/draw_vertices_golden_test.dart +++ b/lib/web_ui/test/golden_tests/engine/draw_vertices_golden_test.dart @@ -22,7 +22,8 @@ void main() async { {Rect region = const Rect.fromLTWH(0, 0, 500, 500), bool write = false}) async { final EngineCanvas engineCanvas = BitmapCanvas(screenRect); - rc.apply(engineCanvas); + rc.endRecording(); + rc.apply(engineCanvas, screenRect); // Wrap in so that our CSS selectors kick in. final html.Element sceneElement = html.Element.tag('flt-scene'); diff --git a/lib/web_ui/test/golden_tests/engine/linear_gradient_golden_test.dart b/lib/web_ui/test/golden_tests/engine/linear_gradient_golden_test.dart index c535719597045..35b7cae8f9ecf 100644 --- a/lib/web_ui/test/golden_tests/engine/linear_gradient_golden_test.dart +++ b/lib/web_ui/test/golden_tests/engine/linear_gradient_golden_test.dart @@ -21,7 +21,8 @@ void main() async { {Rect region = const Rect.fromLTWH(0, 0, 500, 500), bool write = false}) async { final EngineCanvas engineCanvas = BitmapCanvas(screenRect); - rc.apply(engineCanvas); + rc.endRecording(); + rc.apply(engineCanvas, screenRect); // Wrap in so that our CSS selectors kick in. final html.Element sceneElement = html.Element.tag('flt-scene'); diff --git a/lib/web_ui/test/golden_tests/engine/multiline_text_clipping_golden_test.dart b/lib/web_ui/test/golden_tests/engine/multiline_text_clipping_golden_test.dart index cec631431b49d..cd3378b569896 100644 --- a/lib/web_ui/test/golden_tests/engine/multiline_text_clipping_golden_test.dart +++ b/lib/web_ui/test/golden_tests/engine/multiline_text_clipping_golden_test.dart @@ -21,10 +21,11 @@ void main() async { setUpStableTestFonts(); void paintTest(EngineCanvas canvas, PaintTest painter) { - final RecordingCanvas recordingCanvas = - RecordingCanvas(const Rect.fromLTWH(0, 0, 600, 600)); + final Rect screenRect = const Rect.fromLTWH(0, 0, 600, 600); + final RecordingCanvas recordingCanvas = RecordingCanvas(screenRect); painter(recordingCanvas); - recordingCanvas.apply(canvas); + recordingCanvas.endRecording(); + recordingCanvas.apply(canvas, screenRect); } testEachCanvas( diff --git a/lib/web_ui/test/golden_tests/engine/path_metrics_test.dart b/lib/web_ui/test/golden_tests/engine/path_metrics_test.dart index 38379087867e2..b5609c72a43fd 100644 --- a/lib/web_ui/test/golden_tests/engine/path_metrics_test.dart +++ b/lib/web_ui/test/golden_tests/engine/path_metrics_test.dart @@ -25,7 +25,8 @@ void main() async { {Rect region = const Rect.fromLTWH(0, 0, 500, 500), bool write = false}) async { final EngineCanvas engineCanvas = BitmapCanvas(screenRect); - rc.apply(engineCanvas); + rc.endRecording(); + rc.apply(engineCanvas, screenRect); // Wrap in so that our CSS selectors kick in. final html.Element sceneElement = html.Element.tag('flt-scene'); diff --git a/lib/web_ui/test/golden_tests/engine/path_to_svg_golden_test.dart b/lib/web_ui/test/golden_tests/engine/path_to_svg_golden_test.dart index c4a1d07a76e9b..bbfd9df35fc6b 100644 --- a/lib/web_ui/test/golden_tests/engine/path_to_svg_golden_test.dart +++ b/lib/web_ui/test/golden_tests/engine/path_to_svg_golden_test.dart @@ -37,7 +37,8 @@ void main() async { html.document.body.append(bitmapCanvas.rootElement); html.document.body.append(svgElement); - canvas.apply(bitmapCanvas); + canvas.endRecording(); + canvas.apply(bitmapCanvas, canvasBounds); await matchGoldenFile('$scubaFileName.png', region: region); diff --git a/lib/web_ui/test/golden_tests/engine/path_transform_test.dart b/lib/web_ui/test/golden_tests/engine/path_transform_test.dart index dd53ae3153a17..342ecfd20da68 100644 --- a/lib/web_ui/test/golden_tests/engine/path_transform_test.dart +++ b/lib/web_ui/test/golden_tests/engine/path_transform_test.dart @@ -22,7 +22,8 @@ void main() async { {Rect region = const Rect.fromLTWH(0, 0, 500, 500), bool write = false}) async { final EngineCanvas engineCanvas = BitmapCanvas(screenRect); - rc.apply(engineCanvas); + rc.endRecording(); + rc.apply(engineCanvas, screenRect); // Wrap in so that our CSS selectors kick in. final html.Element sceneElement = html.Element.tag('flt-scene'); diff --git a/lib/web_ui/test/golden_tests/engine/radial_gradient_golden_test.dart b/lib/web_ui/test/golden_tests/engine/radial_gradient_golden_test.dart index 3e40f57aa37c5..4fb58ebc3df94 100644 --- a/lib/web_ui/test/golden_tests/engine/radial_gradient_golden_test.dart +++ b/lib/web_ui/test/golden_tests/engine/radial_gradient_golden_test.dart @@ -21,7 +21,8 @@ void main() async { {Rect region = const Rect.fromLTWH(0, 0, 500, 500), bool write = false}) async { final EngineCanvas engineCanvas = BitmapCanvas(screenRect); - rc.apply(engineCanvas); + rc.endRecording(); + rc.apply(engineCanvas, screenRect); // Wrap in so that our CSS selectors kick in. final html.Element sceneElement = html.Element.tag('flt-scene'); From e86cf4acdb84f9764d539346fa448b2b502721f0 Mon Sep 17 00:00:00 2001 From: Yegor Jbanov Date: Thu, 9 Apr 2020 17:04:34 -0700 Subject: [PATCH 09/11] tweaks and tests --- .../lib/src/engine/surface/picture.dart | 9 +- .../src/engine/surface/recording_canvas.dart | 100 ++++++++++++++---- .../test/engine/recording_canvas_test.dart | 93 +++++++++++++++- lib/web_ui/test/mock_engine_canvas.dart | 5 +- 4 files changed, 177 insertions(+), 30 deletions(-) diff --git a/lib/web_ui/lib/src/engine/surface/picture.dart b/lib/web_ui/lib/src/engine/surface/picture.dart index af832c68b76e4..aa73958395266 100644 --- a/lib/web_ui/lib/src/engine/surface/picture.dart +++ b/lib/web_ui/lib/src/engine/surface/picture.dart @@ -491,7 +491,7 @@ abstract class PersistedPicture extends PersistedLeafSurface { // The new cull rect contains area not covered by a previous rect. Perhaps // the clip is growing, moving around the picture, or both. In this case - // a part of the picture may not been painted. We will need to + // a part of the picture may not have been painted. We will need to // request a new canvas and paint the picture on it. However, this is also // a strong signal that the clip will continue growing as typically // Flutter uses animated transitions. So instead of allocating the canvas @@ -500,12 +500,14 @@ abstract class PersistedPicture extends PersistedLeafSurface { // will hit the above case where the new cull rect is fully contained // within the cull rect we compute now. - // If any of the borders moved. + // Compute the delta, by which each of the side of the clip rect has "moved" + // since the last time we updated the cull rect. final double leftwardDelta = oldOptimalLocalCullRect.left - _exactLocalCullRect.left; final double upwardDelta = oldOptimalLocalCullRect.top - _exactLocalCullRect.top; final double rightwardDelta = _exactLocalCullRect.right - oldOptimalLocalCullRect.right; final double bottomwardDelta = _exactLocalCullRect.bottom - oldOptimalLocalCullRect.bottom; + // Compute the new optimal rect to paint into. final ui.Rect newLocalCullRect = ui.Rect.fromLTRB( _exactLocalCullRect.left - _predictTrend(leftwardDelta, _exactLocalCullRect.width), _exactLocalCullRect.top - _predictTrend(upwardDelta, _exactLocalCullRect.height), @@ -518,6 +520,9 @@ abstract class PersistedPicture extends PersistedLeafSurface { return localCullRectChanged; } + /// Predicts the delta a particular side of a clip rect will move given the + /// [delta] it moved by last, and the respective [extent] (width or height) + /// of the clip. static double _predictTrend(double delta, double extent) { if (delta <= 0.0) { // Shrinking. Give it 10% of the extent in case the trend is reversed. diff --git a/lib/web_ui/lib/src/engine/surface/recording_canvas.dart b/lib/web_ui/lib/src/engine/surface/recording_canvas.dart index 0a72b7a5f068b..110ec9ca3d2ba 100644 --- a/lib/web_ui/lib/src/engine/surface/recording_canvas.dart +++ b/lib/web_ui/lib/src/engine/surface/recording_canvas.dart @@ -27,17 +27,25 @@ class RecordingCanvas { final _PaintBounds _paintBounds; /// Maximum paintable bounds for this canvas. - ui.Rect _pictureBounds; ui.Rect get pictureBounds { assert( - _pictureBounds != null, + !_debugRecordingEnded, 'Picture bounds not available yet. Call [endRecording] before accessing picture bounds.', ); return _pictureBounds; } + ui.Rect _pictureBounds; final List _commands = []; + /// In debug mode returns the list of recorded paint commands for testing. + List get debugPaintCommands { + if (assertionsEnabled) { + return _commands; + } + throw UnsupportedError('For debugging only.'); + } + RecordingCanvas(ui.Rect bounds) : _paintBounds = _PaintBounds(bounds); /// Whether this canvas is doing arbitrary paint operations not expressible @@ -64,6 +72,8 @@ class RecordingCanvas { bool get didDraw => _didDraw; bool _didDraw = false; + /// When assertions are enabled used to ensure that [endRecording] is called + /// before calling [apply] or [pictureBounds]. bool _debugRecordingEnded = false; /// Stops recording drawing commands and computes paint bounds. @@ -75,12 +85,17 @@ class RecordingCanvas { /// directly it is up to you to call this method explicitly. void endRecording() { _pictureBounds = _paintBounds.computeBounds(); - _debugRecordingEnded = true; + if (assertionsEnabled) { + _debugRecordingEnded = true; + } } /// Applies the recorded commands onto an [engineCanvas]. /// - /// The [clipRect] specifies the clip applied to the picture (screen clip at a minimum). + /// The [clipRect] specifies the clip applied to the picture (screen clip at + /// a minimum). The commands that fall outside the clip are skipped and are + /// not applied to the [engineCanvas]. A command must have a non-zero + /// intersection with the clip in order to be applied. void apply(EngineCanvas engineCanvas, ui.Rect clipRect) { assert(_debugRecordingEnded); if (_debugDumpPaintCommands) { @@ -88,11 +103,12 @@ class RecordingCanvas { int skips = 0; debugBuf.writeln( '--- Applying RecordingCanvas to ${engineCanvas.runtimeType} ' - 'with bounds $_paintBounds and clip $clipRect (w = ${clipRect.width}, h = ${clipRect.height})'); + 'with bounds $_paintBounds and clip $clipRect (w = ${clipRect.width},' + ' h = ${clipRect.height})'); for (int i = 0; i < _commands.length; i++) { final PaintCommand command = _commands[i]; if (command is DrawCommand) { - if (_isOutsideClipRegion(command, clipRect)) { + if (_isInvisible(command, clipRect)) { // The drawing command is outside the clip region. No need to apply. debugBuf.writeln('SKIPPED: ctx.$command;'); skips += 1; @@ -121,7 +137,7 @@ class RecordingCanvas { for (int i = 0, len = _commands.length; i < len; i++) { final PaintCommand command = _commands[i]; if (command is DrawCommand) { - if (_isOutsideClipRegion(command, clipRect)) { + if (_isInvisible(command, clipRect)) { // The drawing command is outside the clip region. No need to apply. continue; } @@ -140,11 +156,18 @@ class RecordingCanvas { engineCanvas.endOfPaint(); } - static bool _isOutsideClipRegion(DrawCommand command, ui.Rect clipRect) { - return command.rightBound < clipRect.left || - command.bottomBound < clipRect.top || - command.leftBound > clipRect.right || - command.topBound > clipRect.bottom; + /// Return true is the [command] is invisible because it is clipped out. + static bool _isInvisible(DrawCommand command, ui.Rect clipRect) { + if (command.isClippedOut) { + return true; + } + + // Check top and bottom first because vertical scrolling is more common + // than horizontal scrolling. + return command.bottomBound < clipRect.top || + command.topBound > clipRect.bottom || + command.rightBound < clipRect.left || + command.leftBound > clipRect.right; } /// Prints recorded commands. @@ -233,23 +256,26 @@ class RecordingCanvas { void clipRect(ui.Rect rect) { assert(!_debugRecordingEnded); - _paintBounds.clipRect(rect); + final PaintClipRect command = PaintClipRect(rect); + _paintBounds.clipRect(rect, command); _hasArbitraryPaint = true; - _commands.add(PaintClipRect(rect)); + _commands.add(command); } void clipRRect(ui.RRect rrect) { assert(!_debugRecordingEnded); - _paintBounds.clipRect(rrect.outerRect); + final PaintClipRRect command = PaintClipRRect(rrect); + _paintBounds.clipRect(rrect.outerRect, command); _hasArbitraryPaint = true; - _commands.add(PaintClipRRect(rrect)); + _commands.add(command); } void clipPath(ui.Path path, {bool doAntiAlias = true}) { assert(!_debugRecordingEnded); - _paintBounds.clipRect(path.getBounds()); + final PaintClipPath command = PaintClipPath(path); + _paintBounds.clipRect(path.getBounds(), command); _hasArbitraryPaint = true; - _commands.add(PaintClipPath(path)); + _commands.add(command); } void drawColor(ui.Color color, ui.BlendMode blendMode) { @@ -566,11 +592,26 @@ abstract class PaintCommand { void serializeToCssPaint(List> serializedCommands); } -/// A [PaintCommand] that puts pixels on the screen (unlike [SaveCommand]). +/// A [PaintCommand] that affect pixels on the screen (unlike, for example, the +/// [SaveCommand]). abstract class DrawCommand extends PaintCommand { + /// Whether the command is completely clipped out of the picture. + bool isClippedOut = false; + + /// The left bound of the graphic produced by this command in picture-global + /// coordinates. double leftBound = double.negativeInfinity; + + /// The top bound of the graphic produced by this command in picture-global + /// coordinates. double topBound = double.negativeInfinity; + + /// The right bound of the graphic produced by this command in picture-global + /// coordinates. double rightBound = double.infinity; + + /// The bottom bound of the graphic produced by this command in + /// picture-global coordinates. double bottomBound = double.infinity; } @@ -748,7 +789,7 @@ class PaintSkew extends PaintCommand { } } -class PaintClipRect extends PaintCommand { +class PaintClipRect extends DrawCommand { final ui.Rect rect; PaintClipRect(this.rect); @@ -773,7 +814,7 @@ class PaintClipRect extends PaintCommand { } } -class PaintClipRRect extends PaintCommand { +class PaintClipRRect extends DrawCommand { final ui.RRect rrect; PaintClipRRect(this.rrect); @@ -801,7 +842,7 @@ class PaintClipRRect extends PaintCommand { } } -class PaintClipPath extends PaintCommand { +class PaintClipPath extends DrawCommand { final SurfacePath path; PaintClipPath(this.path); @@ -1881,7 +1922,7 @@ class _PaintBounds { _currentMatrix.multiply(skewMatrix); } - void clipRect(ui.Rect rect) { + void clipRect(ui.Rect rect, DrawCommand command) { // If we have an active transform, calculate screen relative clipping // rectangle and union with current clipping rectangle. if (!_currentMatrixIsIdentity) { @@ -1923,6 +1964,14 @@ class _PaintBounds { _currentClipBottom = rect.bottom; } } + if (_currentClipLeft >= _currentClipRight || _currentClipTop >= _currentClipBottom) { + command.isClippedOut = true; + } else { + command.leftBound = _currentClipLeft; + command.topBound = _currentClipTop; + command.rightBound = _currentClipRight; + command.bottomBound = _currentClipBottom; + } } /// Grow painted area to include given rectangle. @@ -1933,6 +1982,7 @@ class _PaintBounds { /// Grow painted area to include given rectangle. void growLTRB(double left, double top, double right, double bottom, DrawCommand command) { if (left == right || top == bottom) { + command.isClippedOut = true; return; } @@ -1952,15 +2002,19 @@ class _PaintBounds { if (_clipRectInitialized) { if (transformedPointLeft > _currentClipRight) { + command.isClippedOut = true; return; } if (transformedPointRight < _currentClipLeft) { + command.isClippedOut = true; return; } if (transformedPointTop > _currentClipBottom) { + command.isClippedOut = true; return; } if (transformedPointBottom < _currentClipTop) { + command.isClippedOut = true; return; } if (transformedPointLeft < _currentClipLeft) { diff --git a/lib/web_ui/test/engine/recording_canvas_test.dart b/lib/web_ui/test/engine/recording_canvas_test.dart index de3fa3c9c3c92..5610c9c1f4de8 100644 --- a/lib/web_ui/test/engine/recording_canvas_test.dart +++ b/lib/web_ui/test/engine/recording_canvas_test.dart @@ -29,7 +29,7 @@ void main() { underTest.endRecording(); underTest.apply(mockCanvas, screenRect); - _expectDrawCall(mockCanvas, { + _expectDrawDRRectCall(mockCanvas, { 'outer': rrect, 'inner': rrect.deflate(1), 'paint': somePaint.paintData, @@ -81,7 +81,7 @@ void main() { underTest.apply(mockCanvas, screenRect); // Expect to draw, even when inner has negative radii (which get ignored by canvas) - _expectDrawCall(mockCanvas, { + _expectDrawDRRectCall(mockCanvas, { 'outer': outer, 'inner': inner, 'paint': somePaint.paintData, @@ -98,17 +98,102 @@ void main() { underTest.endRecording(); underTest.apply(mockCanvas, screenRect); - _expectDrawCall(mockCanvas, { + _expectDrawDRRectCall(mockCanvas, { 'outer': outer, 'inner': inner, 'paint': somePaint.paintData, }); }); }); + + test('Filters out paint commands outside the clip rect', () { + // Outside to the left + underTest.drawRect(Rect.fromLTWH(0.0, 20.0, 10.0, 10.0), Paint()); + + // Outside above + underTest.drawRect(Rect.fromLTWH(20.0, 0.0, 10.0, 10.0), Paint()); + + // Visible + underTest.drawRect(Rect.fromLTWH(20.0, 20.0, 10.0, 10.0), Paint()); + + // Inside the layer clip rect but zero-size + underTest.drawRect(Rect.fromLTRB(20.0, 20.0, 30.0, 20.0), Paint()); + + // Inside the layer clip but clipped out by a canvas clip + underTest.save(); + underTest.clipRect(Rect.fromLTWH(0, 0, 10, 10)); + underTest.drawRect(Rect.fromLTWH(20.0, 20.0, 10.0, 10.0), Paint()); + underTest.restore(); + + // Outside to the right + underTest.drawRect(Rect.fromLTWH(40.0, 20.0, 10.0, 10.0), Paint()); + + // Outside below + underTest.drawRect(Rect.fromLTWH(20.0, 40.0, 10.0, 10.0), Paint()); + + underTest.endRecording(); + + expect(underTest.debugPaintCommands, hasLength(10)); + final PaintDrawRect outsideLeft = underTest.debugPaintCommands[0]; + expect(outsideLeft.isClippedOut, false); + expect(outsideLeft.leftBound, 0); + expect(outsideLeft.topBound, 20); + expect(outsideLeft.rightBound, 10); + expect(outsideLeft.bottomBound, 30); + + final PaintDrawRect outsideAbove = underTest.debugPaintCommands[1]; + expect(outsideAbove.isClippedOut, false); + + final PaintDrawRect visible = underTest.debugPaintCommands[2]; + expect(visible.isClippedOut, false); + + final PaintDrawRect zeroSize = underTest.debugPaintCommands[3]; + expect(zeroSize.isClippedOut, true); + + expect(underTest.debugPaintCommands[4], isA()); + + final PaintClipRect clip = underTest.debugPaintCommands[5]; + expect(clip.isClippedOut, false); + + final PaintDrawRect clippedOut = underTest.debugPaintCommands[6]; + expect(clippedOut.isClippedOut, true); + + expect(underTest.debugPaintCommands[7], isA()); + + final PaintDrawRect outsideRight = underTest.debugPaintCommands[8]; + expect(outsideRight.isClippedOut, false); + + final PaintDrawRect outsideBelow = underTest.debugPaintCommands[9]; + expect(outsideBelow.isClippedOut, false); + + // Give it the entire screen so everything paints. + underTest.apply(mockCanvas, screenRect); + expect(mockCanvas.methodCallLog, hasLength(11)); + expect(mockCanvas.methodCallLog[0].methodName, 'drawRect'); + expect(mockCanvas.methodCallLog[1].methodName, 'drawRect'); + expect(mockCanvas.methodCallLog[2].methodName, 'drawRect'); + expect(mockCanvas.methodCallLog[3].methodName, 'drawRect'); + expect(mockCanvas.methodCallLog[4].methodName, 'save'); + expect(mockCanvas.methodCallLog[5].methodName, 'clipRect'); + expect(mockCanvas.methodCallLog[6].methodName, 'drawRect'); + expect(mockCanvas.methodCallLog[7].methodName, 'restore'); + expect(mockCanvas.methodCallLog[8].methodName, 'drawRect'); + expect(mockCanvas.methodCallLog[9].methodName, 'drawRect'); + expect(mockCanvas.methodCallLog[10].methodName, 'endOfPaint'); + + // Clip out a middle region that only contains 'drawRect' + mockCanvas.methodCallLog.clear(); + underTest.apply(mockCanvas, Rect.fromLTRB(15, 15, 35, 35)); + expect(mockCanvas.methodCallLog, hasLength(4)); + expect(mockCanvas.methodCallLog[0].methodName, 'drawRect'); + expect(mockCanvas.methodCallLog[1].methodName, 'save'); + expect(mockCanvas.methodCallLog[2].methodName, 'restore'); + expect(mockCanvas.methodCallLog[3].methodName, 'endOfPaint'); + }); } // Expect a drawDRRect call to be registered in the mock call log, with the expectedArguments -void _expectDrawCall( +void _expectDrawDRRectCall( MockEngineCanvas mock, Map expectedArguments) { expect(mock.methodCallLog.length, equals(2)); MockCanvasCall mockCall = mock.methodCallLog[0]; diff --git a/lib/web_ui/test/mock_engine_canvas.dart b/lib/web_ui/test/mock_engine_canvas.dart index 81f17bab46236..b0253225dc7e8 100644 --- a/lib/web_ui/test/mock_engine_canvas.dart +++ b/lib/web_ui/test/mock_engine_canvas.dart @@ -137,7 +137,10 @@ class MockEngineCanvas implements EngineCanvas { @override void drawRect(Rect rect, SurfacePaintData paint) { - _called('drawRect', arguments: paint); + _called('drawRect', arguments: { + 'rect': rect, + 'paint': paint, + }); } @override From fc2411eaab6561d2eb868149b4aabf20aacddbaf Mon Sep 17 00:00:00 2001 From: Yegor Jbanov Date: Mon, 13 Apr 2020 20:28:05 -0700 Subject: [PATCH 10/11] invert assert expression --- lib/web_ui/lib/src/engine/surface/recording_canvas.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/web_ui/lib/src/engine/surface/recording_canvas.dart b/lib/web_ui/lib/src/engine/surface/recording_canvas.dart index 110ec9ca3d2ba..a428d65d2d367 100644 --- a/lib/web_ui/lib/src/engine/surface/recording_canvas.dart +++ b/lib/web_ui/lib/src/engine/surface/recording_canvas.dart @@ -29,7 +29,7 @@ class RecordingCanvas { /// Maximum paintable bounds for this canvas. ui.Rect get pictureBounds { assert( - !_debugRecordingEnded, + _debugRecordingEnded, 'Picture bounds not available yet. Call [endRecording] before accessing picture bounds.', ); return _pictureBounds; From 22e526f0f3bed26a7e0d9295d099f59fee1eb609 Mon Sep 17 00:00:00 2001 From: Yegor Jbanov Date: Tue, 14 Apr 2020 11:29:50 -0700 Subject: [PATCH 11/11] address comments --- .../lib/src/engine/surface/picture.dart | 4 +- .../src/engine/surface/recording_canvas.dart | 47 ++++++++++--------- 2 files changed, 28 insertions(+), 23 deletions(-) diff --git a/lib/web_ui/lib/src/engine/surface/picture.dart b/lib/web_ui/lib/src/engine/surface/picture.dart index aa73958395266..50d77b0560cc2 100644 --- a/lib/web_ui/lib/src/engine/surface/picture.dart +++ b/lib/web_ui/lib/src/engine/surface/picture.dart @@ -532,8 +532,8 @@ abstract class PersistedPicture extends PersistedLeafSurface { // 50% of the extent (protect from extremely slow growth trend such as // slow scrolling). Give no more than the full extent (protects from // fast scrolling that could lead to overallocation). - return math.max( - math.min(extent * 0.5, delta * 10.0), + return math.min( + math.max(extent * 0.5, delta * 10.0), extent, ); } diff --git a/lib/web_ui/lib/src/engine/surface/recording_canvas.dart b/lib/web_ui/lib/src/engine/surface/recording_canvas.dart index a428d65d2d367..bb3f570450e94 100644 --- a/lib/web_ui/lib/src/engine/surface/recording_canvas.dart +++ b/lib/web_ui/lib/src/engine/surface/recording_canvas.dart @@ -26,7 +26,11 @@ class RecordingCanvas { /// Computes [_pictureBounds]. final _PaintBounds _paintBounds; - /// Maximum paintable bounds for this canvas. + /// Maximum paintable bounds for the picture painted by this recording. + /// + /// The bounds contain the full picture. The commands recorded for the picture + /// are later pruned based on the clip applied to the picture. See the [apply] + /// method for more details. ui.Rect get pictureBounds { assert( _debugRecordingEnded, @@ -108,7 +112,7 @@ class RecordingCanvas { for (int i = 0; i < _commands.length; i++) { final PaintCommand command = _commands[i]; if (command is DrawCommand) { - if (_isInvisible(command, clipRect)) { + if (command.isInvisible(clipRect)) { // The drawing command is outside the clip region. No need to apply. debugBuf.writeln('SKIPPED: ctx.$command;'); skips += 1; @@ -137,7 +141,7 @@ class RecordingCanvas { for (int i = 0, len = _commands.length; i < len; i++) { final PaintCommand command = _commands[i]; if (command is DrawCommand) { - if (_isInvisible(command, clipRect)) { + if (command.isInvisible(clipRect)) { // The drawing command is outside the clip region. No need to apply. continue; } @@ -156,20 +160,6 @@ class RecordingCanvas { engineCanvas.endOfPaint(); } - /// Return true is the [command] is invisible because it is clipped out. - static bool _isInvisible(DrawCommand command, ui.Rect clipRect) { - if (command.isClippedOut) { - return true; - } - - // Check top and bottom first because vertical scrolling is more common - // than horizontal scrolling. - return command.bottomBound < clipRect.top || - command.topBound > clipRect.bottom || - command.rightBound < clipRect.left || - command.leftBound > clipRect.right; - } - /// Prints recorded commands. String debugPrintCommands() { if (assertionsEnabled) { @@ -422,11 +412,12 @@ class RecordingCanvas { _didDraw = true; final double paintSpread = _getPaintSpread(paint); final PaintDrawCircle command = PaintDrawCircle(c, radius, paint.paintData); + final double distance = radius + paintSpread; _paintBounds.growLTRB( - c.dx - radius - paintSpread, - c.dy - radius - paintSpread, - c.dx + radius + paintSpread, - c.dy + radius + paintSpread, + c.dx - distance, + c.dy - distance, + c.dx + distance, + c.dy + distance, command, ); _commands.add(command); @@ -613,6 +604,20 @@ abstract class DrawCommand extends PaintCommand { /// The bottom bound of the graphic produced by this command in /// picture-global coordinates. double bottomBound = double.infinity; + + /// Whether this command intersects with the [clipRect]. + bool isInvisible(ui.Rect clipRect) { + if (isClippedOut) { + return true; + } + + // Check top and bottom first because vertical scrolling is more common + // than horizontal scrolling. + return bottomBound < clipRect.top || + topBound > clipRect.bottom || + rightBound < clipRect.left || + leftBound > clipRect.right; + } } class PaintSave extends PaintCommand {