From 17cd7d8ae82021fac7fe9acceb825f156f670fe0 Mon Sep 17 00:00:00 2001 From: Jason Simmons Date: Thu, 20 Aug 2020 14:24:19 -0700 Subject: [PATCH] Remove image sizes from Picture::GetAllocationSize Including images in the size of a Picture may overstate the picture's memory usage. The picture may not be the only object holding a reference to the image (possibly because the image is owned by the framework's image cache). If Dart thinks that the size of the picture includes the image's size, then Dart will more aggressively garbage collect the picture. But deleting the picture will not delete the image, and the extra GCs may cause jank. Fixes https://github.com/flutter/flutter/issues/63876 --- lib/ui/painting/canvas.cc | 3 --- testing/dart/canvas_test.dart | 28 ---------------------------- 2 files changed, 31 deletions(-) diff --git a/lib/ui/painting/canvas.cc b/lib/ui/painting/canvas.cc index 01e6f5cf96a6f..c68f0d6001ad6 100644 --- a/lib/ui/painting/canvas.cc +++ b/lib/ui/painting/canvas.cc @@ -327,7 +327,6 @@ void Canvas::drawImage(const CanvasImage* image, ToDart("Canvas.drawImage called with non-genuine Image.")); return; } - external_allocation_size_ += image->GetAllocationSize(); canvas_->drawImage(image->image(), x, y, paint.paint()); } @@ -352,7 +351,6 @@ void Canvas::drawImageRect(const CanvasImage* image, } SkRect src = SkRect::MakeLTRB(src_left, src_top, src_right, src_bottom); SkRect dst = SkRect::MakeLTRB(dst_left, dst_top, dst_right, dst_bottom); - external_allocation_size_ += image->GetAllocationSize(); canvas_->drawImageRect(image->image(), src, dst, paint.paint(), SkCanvas::kFast_SrcRectConstraint); } @@ -381,7 +379,6 @@ void Canvas::drawImageNine(const CanvasImage* image, SkIRect icenter; center.round(&icenter); SkRect dst = SkRect::MakeLTRB(dst_left, dst_top, dst_right, dst_bottom); - external_allocation_size_ += image->GetAllocationSize(); canvas_->drawImageNine(image->image(), icenter, dst, paint.paint()); } diff --git a/testing/dart/canvas_test.dart b/testing/dart/canvas_test.dart index 09255ab86a6d8..8640cc86bf63d 100644 --- a/testing/dart/canvas_test.dart +++ b/testing/dart/canvas_test.dart @@ -297,34 +297,6 @@ void main() { expectArgumentError(() => canvas.drawRawAtlas(image, Float32List(4), Float32List(4), Int32List(2), BlendMode.src, rect, paint)); }); - test('Image size reflected in picture size for image*, drawAtlas, and drawPicture methods', () async { - final Image image = await createImage(100, 100); - final PictureRecorder recorder = PictureRecorder(); - final Canvas canvas = Canvas(recorder); - const Rect rect = Rect.fromLTWH(0, 0, 100, 100); - canvas.drawImage(image, Offset.zero, Paint()); - canvas.drawImageRect(image, rect, rect, Paint()); - canvas.drawImageNine(image, rect, rect, Paint()); - canvas.drawAtlas(image, [], [], [], BlendMode.src, rect, Paint()); - final Picture picture = recorder.endRecording(); - - // Some of the numbers here appear to utilize sharing/reuse of common items, - // e.g. of the Paint() or same `Rect` usage, etc. - // The raw utilization of a 100x100 picture here should be 53333: - // 100 * 100 * 4 * (4/3) = 53333.333333.... - // To avoid platform specific idiosyncrasies and brittleness against changes - // to Skia, we just assert this is _at least_ 4x the image size. - const int minimumExpected = 53333 * 4; - expect(picture.approximateBytesUsed, greaterThan(minimumExpected)); - - final PictureRecorder recorder2 = PictureRecorder(); - final Canvas canvas2 = Canvas(recorder2); - canvas2.drawPicture(picture); - final Picture picture2 = recorder2.endRecording(); - - expect(picture2.approximateBytesUsed, greaterThan(minimumExpected)); - }); - test('Vertex buffer size reflected in picture size for drawVertices', () async { final PictureRecorder recorder = PictureRecorder(); final Canvas canvas = Canvas(recorder);