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

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/engine_layer.cc

@@ -18,7 +18,7 @@ EngineLayer::EngineLayer(std::shared_ptr<flutter::ContainerLayer> layer)
 
 EngineLayer::~EngineLayer() = default;
 
-size_t EngineLayer::GetAllocationSize() {
+size_t EngineLayer::GetAllocationSize() const {
   // Provide an approximation of the total memory impact of this object to the
   // Dart GC.  The ContainerLayer may hold references to a tree of other layers,
   // which in turn may contain Skia objects.

lib/ui/painting/engine_layer.h

@@ -23,7 +23,7 @@ class EngineLayer : public RefCountedDartWrappable<EngineLayer> {
  public:
   ~EngineLayer() override;
 
-  size_t GetAllocationSize() override;
+  size_t GetAllocationSize() const override;
 
   static fml::RefPtr<EngineLayer> MakeRetained(
       std::shared_ptr<flutter::ContainerLayer> layer) {

lib/ui/painting/image.cc

@@ -38,9 +38,10 @@ Dart_Handle CanvasImage::toByteData(int format, Dart_Handle callback) {
 
 void CanvasImage::dispose() {
   ClearDartWrapper();
+  image_.reset();
 }
 
-size_t CanvasImage::GetAllocationSize() {
+size_t CanvasImage::GetAllocationSize() const {
   if (auto image = image_.get()) {
     const auto& info = image->imageInfo();
     const auto kMipmapOverhead = 4.0 / 3.0;

lib/ui/painting/image.h

@@ -39,7 +39,7 @@ class CanvasImage final : public RefCountedDartWrappable<CanvasImage> {
     image_ = std::move(image);
   }
 
-  size_t GetAllocationSize() override;
+  size_t GetAllocationSize() const override;
 
   static void RegisterNatives(tonic::DartLibraryNatives* natives);
 

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() {
+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(); }
 
@@ -34,7 +36,7 @@ class Picture : public RefCountedDartWrappable<Picture> {
 
   void dispose();
 
-  size_t GetAllocationSize() override;
+  size_t GetAllocationSize() const override;
 
   static void RegisterNatives(tonic::DartLibraryNatives* natives);
 
@@ -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;

lib/ui/painting/single_frame_codec.cc

@@ -101,7 +101,7 @@ Dart_Handle SingleFrameCodec::getNextFrame(Dart_Handle callback_handle) {
   return Dart_Null();
 }
 
-size_t SingleFrameCodec::GetAllocationSize() {
+size_t SingleFrameCodec::GetAllocationSize() const {
   const auto& data = descriptor_.data;
   const auto data_byte_size = data ? data->size() : 0;
   const auto frame_byte_size = (cached_frame_ && cached_frame_->image())

lib/ui/painting/single_frame_codec.h

@@ -28,7 +28,7 @@ class SingleFrameCodec : public Codec {
   Dart_Handle getNextFrame(Dart_Handle args) override;
 
   // |DartWrappable|
-  size_t GetAllocationSize() override;
+  size_t GetAllocationSize() const override;
 
  private:
   enum class Status { kNew, kInProgress, kComplete };

lib/ui/text/paragraph.cc

@@ -44,7 +44,7 @@ Paragraph::Paragraph(std::unique_ptr<txt::Paragraph> paragraph)
 
 Paragraph::~Paragraph() = default;
 
-size_t Paragraph::GetAllocationSize() {
+size_t Paragraph::GetAllocationSize() const {
   // We don't have an accurate accounting of the paragraph's memory consumption,
   // so return a fixed size to indicate that its impact is more than the size
   // of the Paragraph class.

lib/ui/text/paragraph.h

@@ -53,7 +53,7 @@ class Paragraph : public RefCountedDartWrappable<Paragraph> {
   Dart_Handle getLineBoundary(unsigned offset);
   tonic::Float64List computeLineMetrics();
 
-  size_t GetAllocationSize() override;
+  size_t GetAllocationSize() const override;
 
   static void RegisterNatives(tonic::DartLibraryNatives* natives);
 

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));
+  });
 }

third_party/tonic/dart_wrappable.cc

@@ -77,7 +77,7 @@ void DartWrappable::FinalizeDartWrapper(void* isolate_callback_data,
   wrappable->ReleaseDartWrappableReference();  // Balanced in CreateDartWrapper.
 }
 
-size_t DartWrappable::GetAllocationSize() {
+size_t DartWrappable::GetAllocationSize() const {
   return GetDartWrapperInfo().size_in_bytes;
 }
 

third_party/tonic/dart_wrappable.h

@@ -37,7 +37,7 @@ class DartWrappable {
   // Override this to customize the object size reported to the Dart garbage
   // collector.
   // Implement using IMPLEMENT_WRAPPERTYPEINFO macro
-  virtual size_t GetAllocationSize();
+  virtual size_t GetAllocationSize() const;
 
   virtual void RetainDartWrappableReference() const = 0;