From be72a43166d56af37d7e0d4b092818a9a577095a Mon Sep 17 00:00:00 2001 From: ferhatb Date: Thu, 20 Feb 2020 15:58:25 -0800 Subject: [PATCH 1/3] Fix canvas leak when dpi changes. Evict from BitmapCanvas cache under memory pressure. --- .../lib/src/engine/surface/picture.dart | 43 +++++++++++++------ .../src/engine/surface/recording_canvas.dart | 25 +++++++++-- 2 files changed, 52 insertions(+), 16 deletions(-) diff --git a/lib/web_ui/lib/src/engine/surface/picture.dart b/lib/web_ui/lib/src/engine/surface/picture.dart index 2138dd2653327..b8b4be1c411d9 100644 --- a/lib/web_ui/lib/src/engine/surface/picture.dart +++ b/lib/web_ui/lib/src/engine/surface/picture.dart @@ -19,6 +19,15 @@ const int _kCanvasCacheSize = 30; /// Canvases available for reuse, capped at [_kCanvasCacheSize]. final List _recycledCanvases = []; +/// Reduces recycled canvas list by 50% to reduce bitmap canvas memory use. +void _reduceCanvasMemoryUsage() { + final int canvasCount = _recycledCanvases.length; + for (int i = 0; i < canvasCount; i++) { + final BitmapCanvas removedCanvas = _recycledCanvases.removeAt(0); + removedCanvas.dispose(); + } +} + /// A request to repaint a canvas. /// /// Paint requests are prioritized such that the larger pictures go first. This @@ -42,22 +51,27 @@ class _PaintRequest { List<_PaintRequest> _paintQueue = <_PaintRequest>[]; void _recycleCanvas(EngineCanvas canvas) { - if (canvas is BitmapCanvas && canvas.isReusable()) { - _recycledCanvases.add(canvas); - if (_recycledCanvases.length > _kCanvasCacheSize) { - final BitmapCanvas removedCanvas = _recycledCanvases.removeAt(0); - removedCanvas.dispose(); + if (canvas is BitmapCanvas) { + if (canvas.isReusable()) { + _recycledCanvases.add(canvas); + if (_recycledCanvases.length > _kCanvasCacheSize) { + final BitmapCanvas removedCanvas = _recycledCanvases.removeAt(0); + removedCanvas.dispose(); + if (_debugShowCanvasReuseStats) { + DebugCanvasReuseOverlay.instance.disposedCount++; + } + } if (_debugShowCanvasReuseStats) { - DebugCanvasReuseOverlay.instance.disposedCount++; + DebugCanvasReuseOverlay.instance.inRecycleCount = + _recycledCanvases.length; } - } - if (_debugShowCanvasReuseStats) { - DebugCanvasReuseOverlay.instance.inRecycleCount = - _recycledCanvases.length; + } else { + canvas.dispose(); } } } + /// Signature of a function that instantiates a [PersistedPicture]. typedef PersistedPictureFactory = PersistedPicture Function( double dx, @@ -272,7 +286,7 @@ class PersistedStandardPicture extends PersistedPicture { final ui.Size canvasSize = bounds.size; BitmapCanvas bestRecycledCanvas; double lastPixelCount = double.infinity; - + final double requestedPixelCount = bounds.width * bounds.height; for (int i = 0; i < _recycledCanvases.length; i++) { final BitmapCanvas candidate = _recycledCanvases[i]; if (!candidate.isReusable()) { @@ -285,7 +299,12 @@ class PersistedStandardPicture extends PersistedPicture { final bool fits = candidate.doesFitBounds(bounds); final bool isSmaller = candidatePixelCount < lastPixelCount; - if (fits && isSmaller) { + // If we are about to reuse a huge canvas compared to what we need, + // skip candidate since we will be waiting + final bool isTooSmall = isSmaller && + requestedPixelCount > 1 && + (candidatePixelCount / requestedPixelCount) > 10; + if (fits && isSmaller && !isTooSmall) { bestRecycledCanvas = candidate; lastPixelCount = candidatePixelCount; final bool fitsExactly = candidateSize.width == canvasSize.width && 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 4209efe638e80..2c503a89df627 100644 --- a/lib/web_ui/lib/src/engine/surface/recording_canvas.dart +++ b/lib/web_ui/lib/src/engine/surface/recording_canvas.dart @@ -183,10 +183,12 @@ class RecordingCanvas { } void drawColor(ui.Color color, ui.BlendMode blendMode) { - _hasArbitraryPaint = true; - _didDraw = true; - _paintBounds.grow(_paintBounds.maxPaintBounds); - _commands.add(PaintDrawColor(color, blendMode)); + drawRect( + _paintBounds.maxPaintBounds, + ui.Paint() + ..color = color + ..style = ui.PaintingStyle.fill + ..blendMode = blendMode); } void drawLine(ui.Offset p1, ui.Offset p2, SurfacePaint paint) { @@ -315,6 +317,21 @@ class RecordingCanvas { } void drawPath(ui.Path path, SurfacePaint paint) { + if (paint.shader == null) { + // For Rect/RoundedRect paths use drawRect/drawRRect code paths for + // DomCanvas optimization. + SurfacePath sPath = path; + final ui.Rect rect = sPath.webOnlyPathAsRect; + if (rect != null) { + drawRect(rect, paint); + return; + } + final ui.RRect rrect = sPath.webOnlyPathAsRoundedRect; + if (rrect != null) { + drawRRect(rrect, paint); + return; + } + } _hasArbitraryPaint = true; _didDraw = true; ui.Rect pathBounds = path.getBounds(); From dc3ede37d4ebce556467e31b1c36dd6cb3b5f1f6 Mon Sep 17 00:00:00 2001 From: ferhatb Date: Fri, 21 Feb 2020 10:00:58 -0800 Subject: [PATCH 2/3] optimized _reduceCanvasMemoryUsage --- lib/web_ui/lib/src/engine/surface/picture.dart | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/web_ui/lib/src/engine/surface/picture.dart b/lib/web_ui/lib/src/engine/surface/picture.dart index b8b4be1c411d9..db95cf5bd1040 100644 --- a/lib/web_ui/lib/src/engine/surface/picture.dart +++ b/lib/web_ui/lib/src/engine/surface/picture.dart @@ -23,9 +23,9 @@ final List _recycledCanvases = []; void _reduceCanvasMemoryUsage() { final int canvasCount = _recycledCanvases.length; for (int i = 0; i < canvasCount; i++) { - final BitmapCanvas removedCanvas = _recycledCanvases.removeAt(0); - removedCanvas.dispose(); + _recycledCanvases[i].dispose(); } + _recycledCanvases.clear(); } /// A request to repaint a canvas. @@ -299,8 +299,8 @@ class PersistedStandardPicture extends PersistedPicture { final bool fits = candidate.doesFitBounds(bounds); final bool isSmaller = candidatePixelCount < lastPixelCount; - // If we are about to reuse a huge canvas compared to what we need, - // skip candidate since we will be waiting + // [isTooSmall] is used to make sure that a small picture doesn't + // reuse and hold onto memory of a large canvas. final bool isTooSmall = isSmaller && requestedPixelCount > 1 && (candidatePixelCount / requestedPixelCount) > 10; From 4d938aa55a38c7d526f4639e22121786aedda535 Mon Sep 17 00:00:00 2001 From: ferhatb Date: Fri, 21 Feb 2020 11:03:43 -0800 Subject: [PATCH 3/3] Addressed review comments --- .../lib/src/engine/surface/picture.dart | 30 ++++++++++--------- .../src/engine/surface/recording_canvas.dart | 8 ++--- 2 files changed, 18 insertions(+), 20 deletions(-) diff --git a/lib/web_ui/lib/src/engine/surface/picture.dart b/lib/web_ui/lib/src/engine/surface/picture.dart index db95cf5bd1040..c95559691be61 100644 --- a/lib/web_ui/lib/src/engine/surface/picture.dart +++ b/lib/web_ui/lib/src/engine/surface/picture.dart @@ -286,7 +286,6 @@ class PersistedStandardPicture extends PersistedPicture { final ui.Size canvasSize = bounds.size; BitmapCanvas bestRecycledCanvas; double lastPixelCount = double.infinity; - final double requestedPixelCount = bounds.width * bounds.height; for (int i = 0; i < _recycledCanvases.length; i++) { final BitmapCanvas candidate = _recycledCanvases[i]; if (!candidate.isReusable()) { @@ -299,19 +298,22 @@ class PersistedStandardPicture extends PersistedPicture { final bool fits = candidate.doesFitBounds(bounds); final bool isSmaller = candidatePixelCount < lastPixelCount; - // [isTooSmall] is used to make sure that a small picture doesn't - // reuse and hold onto memory of a large canvas. - final bool isTooSmall = isSmaller && - requestedPixelCount > 1 && - (candidatePixelCount / requestedPixelCount) > 10; - if (fits && isSmaller && !isTooSmall) { - bestRecycledCanvas = candidate; - lastPixelCount = candidatePixelCount; - final bool fitsExactly = candidateSize.width == canvasSize.width && - candidateSize.height == canvasSize.height; - if (fitsExactly) { - // No need to keep looking any more. - break; + if (fits && isSmaller) { + // [isTooSmall] is used to make sure that a small picture doesn't + // reuse and hold onto memory of a large canvas. + final double requestedPixelCount = bounds.width * bounds.height; + final bool isTooSmall = isSmaller && + requestedPixelCount > 1 && + (candidatePixelCount / requestedPixelCount) > 4; + if (!isTooSmall) { + bestRecycledCanvas = candidate; + lastPixelCount = candidatePixelCount; + final bool fitsExactly = candidateSize.width == canvasSize.width && + candidateSize.height == canvasSize.height; + if (fitsExactly) { + // No need to keep looking any more. + break; + } } } } 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 2c503a89df627..92ce62b93e303 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,8 @@ class RecordingCanvas { } void drawColor(ui.Color color, ui.BlendMode blendMode) { - drawRect( - _paintBounds.maxPaintBounds, - ui.Paint() - ..color = color - ..style = ui.PaintingStyle.fill - ..blendMode = blendMode); + _paintBounds.grow(_paintBounds.maxPaintBounds); + _commands.add(PaintDrawColor(color, blendMode)); } void drawLine(ui.Offset p1, ui.Offset p2, SurfacePaint paint) {