Make images contribute to Picture allocation size, more aggressively release references when dispose is called (#18706)

SkImage references get held by both our Image and Picture objects. The GC has no idea about this, and so sometimes pictures look very small when they're actually holding a reference to a large chunk of memory. This patch helps make sure that the GC can more adequately catch the real size impact of Picture objects, and combined with an upstream patch in Dart allows for much more aggressive collection of unused image related memory.

Author: dnfield

lib/ui/painting/canvas.cc

@@ -293,6 +293,7 @@ void Canvas::drawImage(const CanvasImage* image,
   if (!image)
     Dart_ThrowException(
         ToDart("Canvas.drawImage called with non-genuine Image."));
+  image_allocation_size_ += image->GetAllocationSize();
   canvas_->drawImage(image->image(), x, y, paint.paint());
 }
 
@@ -314,6 +315,7 @@ void Canvas::drawImageRect(const CanvasImage* image,
         ToDart("Canvas.drawImageRect called with non-genuine 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);
+  image_allocation_size_ += image->GetAllocationSize();
   canvas_->drawImageRect(image->image(), src, dst, paint.paint(),
                          SkCanvas::kFast_SrcRectConstraint);
 }
@@ -339,6 +341,7 @@ void Canvas::drawImageNine(const CanvasImage* image,
   SkIRect icenter;
   center.round(&icenter);
   SkRect dst = SkRect::MakeLTRB(dst_left, dst_top, dst_right, dst_bottom);
+  image_allocation_size_ += image->GetAllocationSize();
   canvas_->drawImageNine(image->image(), icenter, dst, paint.paint());
 }
 
@@ -348,6 +351,7 @@ void Canvas::drawPicture(Picture* picture) {
   if (!picture)
     Dart_ThrowException(
         ToDart("Canvas.drawPicture called with non-genuine Picture."));
+  image_allocation_size_ += picture->GetAllocationSize();
   canvas_->drawPicture(picture->picture().get());
 }
 
@@ -402,6 +406,7 @@ void Canvas::drawAtlas(const Paint& paint,
   static_assert(sizeof(SkRect) == sizeof(float) * 4,
                 "SkRect doesn't use floats.");
 
+  image_allocation_size_ += atlas->GetAllocationSize();
   canvas_->drawAtlas(
       skImage.get(), reinterpret_cast<const SkRSXform*>(transforms.data()),
       reinterpret_cast<const SkRect*>(rects.data()),

lib/ui/painting/canvas.h

@@ -170,13 +170,16 @@ class Canvas : public RefCountedDartWrappable<Canvas> {
 
   static void RegisterNatives(tonic::DartLibraryNatives* natives);
 
+  size_t image_allocation_size() const { return image_allocation_size_; }
+
  private:
   explicit Canvas(SkCanvas* canvas);
 
   // The SkCanvas is supplied by a call to SkPictureRecorder::beginRecording,
   // which does not transfer ownership.  For this reason, we hold a raw
   // pointer and manually set to null in Clear.
   SkCanvas* canvas_;
+  size_t image_allocation_size_ = 0;
 };
 
 }  // namespace flutter

lib/ui/painting/image.cc

@@ -38,6 +38,7 @@ Dart_Handle CanvasImage::toByteData(int format, Dart_Handle callback) {
 
 void CanvasImage::dispose() {
   ClearDartWrapper();
+  image_.reset();
 }
 
 size_t CanvasImage::GetAllocationSize() const {

lib/ui/painting/image_decoder.cc

@@ -278,7 +278,7 @@ static SkiaGPUObject<SkImage> UploadRasterImage(
                   SkSafeUnref(static_cast<SkImage*>(context));
                 },
                 image.get());
-            result = {texture_image, nullptr};
+            result = {std::move(texture_image), nullptr};
           })
           .SetIfFalse([&result, context = io_manager->GetResourceContext(),
                        &pixmap, queue = io_manager->GetSkiaUnrefQueue()] {
@@ -293,7 +293,7 @@ static SkiaGPUObject<SkImage> UploadRasterImage(
               FML_LOG(ERROR) << "Could not make x-context image.";
               result = {};
             } else {
-              result = {texture_image, queue};
+              result = {std::move(texture_image), queue};
             }
           }));
 

lib/ui/painting/picture.cc

@@ -26,17 +26,20 @@ IMPLEMENT_WRAPPERTYPEINFO(ui, Picture);
 
 DART_BIND_ALL(Picture, FOR_EACH_BINDING)
 
-fml::RefPtr<Picture> Picture::Create(
-    Dart_Handle dart_handle,
-    flutter::SkiaGPUObject<SkPicture> picture) {
-  auto canvas_picture = fml::MakeRefCounted<Picture>(std::move(picture));
+fml::RefPtr<Picture> Picture::Create(Dart_Handle dart_handle,
+                                     flutter::SkiaGPUObject<SkPicture> picture,
+                                     size_t image_allocation_size) {
+  auto canvas_picture =
+      fml::MakeRefCounted<Picture>(std::move(picture), image_allocation_size);
 
   canvas_picture->AssociateWithDartWrapper(dart_handle);
   return canvas_picture;
 }
 
-Picture::Picture(flutter::SkiaGPUObject<SkPicture> picture)
-    : picture_(std::move(picture)) {}
+Picture::Picture(flutter::SkiaGPUObject<SkPicture> picture,
+                 size_t image_allocation_size)
+    : picture_(std::move(picture)),
+      image_allocation_size_(image_allocation_size) {}
 
 Picture::~Picture() = default;
 
@@ -52,11 +55,13 @@ Dart_Handle Picture::toImage(uint32_t width,
 
 void Picture::dispose() {
   ClearDartWrapper();
+  picture_.reset();
 }
 
 size_t Picture::GetAllocationSize() const {
   if (auto picture = picture_.get()) {
-    return picture->approximateBytesUsed();
+    return picture->approximateBytesUsed() + sizeof(Picture) +
+           image_allocation_size_;
   } else {
     return sizeof(Picture);
   }

lib/ui/painting/picture.h

@@ -8,6 +8,7 @@
 #include "flutter/flow/skia_gpu_object.h"
 #include "flutter/lib/ui/dart_wrapper.h"
 #include "flutter/lib/ui/painting/image.h"
+#include "flutter/lib/ui/ui_dart_state.h"
 #include "third_party/skia/include/core/SkPicture.h"
 
 namespace tonic {
@@ -24,7 +25,8 @@ class Picture : public RefCountedDartWrappable<Picture> {
  public:
   ~Picture() override;
   static fml::RefPtr<Picture> Create(Dart_Handle dart_handle,
-                                     flutter::SkiaGPUObject<SkPicture> picture);
+                                     flutter::SkiaGPUObject<SkPicture> picture,
+                                     size_t image_allocation_size);
 
   sk_sp<SkPicture> picture() const { return picture_.get(); }
 
@@ -43,10 +45,14 @@ class Picture : public RefCountedDartWrappable<Picture> {
                                       uint32_t height,
                                       Dart_Handle raw_image_callback);
 
+  size_t image_allocation_size() const { return image_allocation_size_; }
+
  private:
-  explicit Picture(flutter::SkiaGPUObject<SkPicture> picture);
+  Picture(flutter::SkiaGPUObject<SkPicture> picture,
+          size_t image_allocation_size_);
 
   flutter::SkiaGPUObject<SkPicture> picture_;
+  size_t image_allocation_size_;
 };
 
 }  // namespace flutter

lib/ui/painting/picture_recorder.cc

@@ -52,9 +52,12 @@ fml::RefPtr<Picture> PictureRecorder::endRecording(Dart_Handle dart_picture) {
   if (!isRecording())
     return nullptr;
 
-  fml::RefPtr<Picture> picture = Picture::Create(
-      dart_picture, UIDartState::CreateGPUObject(
-                        picture_recorder_.finishRecordingAsPicture()));
+  fml::RefPtr<Picture> picture =
+      Picture::Create(dart_picture,
+                      UIDartState::CreateGPUObject(
+                          picture_recorder_.finishRecordingAsPicture()),
+                      canvas_->image_allocation_size());
+
   canvas_->Clear();
   canvas_->ClearDartWrapper();
   canvas_ = nullptr;

testing/dart/canvas_test.dart

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 // @dart = 2.6
+import 'dart:async';
 import 'dart:io';
 import 'dart:typed_data';
 import 'dart:ui';
@@ -13,6 +14,24 @@ import 'package:test/test.dart';
 
 typedef CanvasCallback = void Function(Canvas canvas);
 
+Future<Image> createImage(int width, int height) {
+  final Completer<Image> completer = Completer<Image>();
+  decodeImageFromPixels(
+    Uint8List.fromList(List<int>.generate(
+      width * height * 4,
+      (int pixel) => pixel % 255,
+    )),
+    width,
+    height,
+    PixelFormat.rgba8888,
+    (Image image) {
+      completer.complete(image);
+    },
+  );
+
+  return completer.future;
+}
+
 void testCanvas(CanvasCallback callback) {
   try {
     callback(Canvas(PictureRecorder(), const Rect.fromLTRB(0.0, 0.0, 0.0, 0.0)));
@@ -198,4 +217,32 @@ void main() {
         await fuzzyGoldenImageCompare(image, 'canvas_test_dithered_gradient.png');
     expect(areEqual, true);
   }, skip: !Platform.isLinux); // https://github.com/flutter/flutter/issues/53784
+
+  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, <RSTransform>[], <Rect>[], <Color>[], 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));
+  });
 }