diff --git a/lib/ui/painting/canvas.cc b/lib/ui/painting/canvas.cc index 3b06b394947fc..2ee718cd2f2ba 100644 --- a/lib/ui/painting/canvas.cc +++ b/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(transforms.data()), reinterpret_cast(rects.data()), diff --git a/lib/ui/painting/canvas.h b/lib/ui/painting/canvas.h index e431da8bbf8d9..8f5a459ab7ef8 100644 --- a/lib/ui/painting/canvas.h +++ b/lib/ui/painting/canvas.h @@ -170,6 +170,8 @@ class Canvas : public RefCountedDartWrappable { static void RegisterNatives(tonic::DartLibraryNatives* natives); + size_t image_allocation_size() const { return image_allocation_size_; } + private: explicit Canvas(SkCanvas* canvas); @@ -177,6 +179,7 @@ class Canvas : public RefCountedDartWrappable { // 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 diff --git a/lib/ui/painting/engine_layer.cc b/lib/ui/painting/engine_layer.cc index 0868b42cc2928..4504ac278e618 100644 --- a/lib/ui/painting/engine_layer.cc +++ b/lib/ui/painting/engine_layer.cc @@ -18,7 +18,7 @@ EngineLayer::EngineLayer(std::shared_ptr 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. diff --git a/lib/ui/painting/engine_layer.h b/lib/ui/painting/engine_layer.h index 8e402ba9150f5..83f2cb16942ab 100644 --- a/lib/ui/painting/engine_layer.h +++ b/lib/ui/painting/engine_layer.h @@ -23,7 +23,7 @@ class EngineLayer : public RefCountedDartWrappable { public: ~EngineLayer() override; - size_t GetAllocationSize() override; + size_t GetAllocationSize() const override; static fml::RefPtr MakeRetained( std::shared_ptr layer) { diff --git a/lib/ui/painting/image.cc b/lib/ui/painting/image.cc index 8ee65790924bf..126205530fa4a 100644 --- a/lib/ui/painting/image.cc +++ b/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; diff --git a/lib/ui/painting/image.h b/lib/ui/painting/image.h index 516a2e5d59c49..b8bae633e6e25 100644 --- a/lib/ui/painting/image.h +++ b/lib/ui/painting/image.h @@ -39,7 +39,7 @@ class CanvasImage final : public RefCountedDartWrappable { image_ = std::move(image); } - size_t GetAllocationSize() override; + size_t GetAllocationSize() const override; static void RegisterNatives(tonic::DartLibraryNatives* natives); diff --git a/lib/ui/painting/image_decoder.cc b/lib/ui/painting/image_decoder.cc index 604b41d7dcd95..5b322e1d8deb0 100644 --- a/lib/ui/painting/image_decoder.cc +++ b/lib/ui/painting/image_decoder.cc @@ -278,7 +278,7 @@ static SkiaGPUObject UploadRasterImage( SkSafeUnref(static_cast(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 UploadRasterImage( FML_LOG(ERROR) << "Could not make x-context image."; result = {}; } else { - result = {texture_image, queue}; + result = {std::move(texture_image), queue}; } })); diff --git a/lib/ui/painting/picture.cc b/lib/ui/painting/picture.cc index 511e369d6afdc..6457d74ab4765 100644 --- a/lib/ui/painting/picture.cc +++ b/lib/ui/painting/picture.cc @@ -26,17 +26,20 @@ IMPLEMENT_WRAPPERTYPEINFO(ui, Picture); DART_BIND_ALL(Picture, FOR_EACH_BINDING) -fml::RefPtr Picture::Create( - Dart_Handle dart_handle, - flutter::SkiaGPUObject picture) { - auto canvas_picture = fml::MakeRefCounted(std::move(picture)); +fml::RefPtr Picture::Create(Dart_Handle dart_handle, + flutter::SkiaGPUObject picture, + size_t image_allocation_size) { + auto canvas_picture = + fml::MakeRefCounted(std::move(picture), image_allocation_size); canvas_picture->AssociateWithDartWrapper(dart_handle); return canvas_picture; } -Picture::Picture(flutter::SkiaGPUObject picture) - : picture_(std::move(picture)) {} +Picture::Picture(flutter::SkiaGPUObject 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); } diff --git a/lib/ui/painting/picture.h b/lib/ui/painting/picture.h index f6dd98887d264..20d21ad3ba722 100644 --- a/lib/ui/painting/picture.h +++ b/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 { public: ~Picture() override; static fml::RefPtr Create(Dart_Handle dart_handle, - flutter::SkiaGPUObject picture); + flutter::SkiaGPUObject picture, + size_t image_allocation_size); sk_sp picture() const { return picture_.get(); } @@ -34,7 +36,7 @@ class Picture : public RefCountedDartWrappable { 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 { uint32_t height, Dart_Handle raw_image_callback); + size_t image_allocation_size() const { return image_allocation_size_; } + private: - explicit Picture(flutter::SkiaGPUObject picture); + Picture(flutter::SkiaGPUObject picture, + size_t image_allocation_size_); flutter::SkiaGPUObject picture_; + size_t image_allocation_size_; }; } // namespace flutter diff --git a/lib/ui/painting/picture_recorder.cc b/lib/ui/painting/picture_recorder.cc index d86c1c0fd556f..ab044adbf3780 100644 --- a/lib/ui/painting/picture_recorder.cc +++ b/lib/ui/painting/picture_recorder.cc @@ -52,9 +52,12 @@ fml::RefPtr PictureRecorder::endRecording(Dart_Handle dart_picture) { if (!isRecording()) return nullptr; - fml::RefPtr picture = Picture::Create( - dart_picture, UIDartState::CreateGPUObject( - picture_recorder_.finishRecordingAsPicture())); + fml::RefPtr picture = + Picture::Create(dart_picture, + UIDartState::CreateGPUObject( + picture_recorder_.finishRecordingAsPicture()), + canvas_->image_allocation_size()); + canvas_->Clear(); canvas_->ClearDartWrapper(); canvas_ = nullptr; diff --git a/lib/ui/painting/single_frame_codec.cc b/lib/ui/painting/single_frame_codec.cc index 44361f583a583..7b77b03a1b5c0 100644 --- a/lib/ui/painting/single_frame_codec.cc +++ b/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()) diff --git a/lib/ui/painting/single_frame_codec.h b/lib/ui/painting/single_frame_codec.h index b01638c71ce63..c3d92946ff306 100644 --- a/lib/ui/painting/single_frame_codec.h +++ b/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 }; diff --git a/lib/ui/text/paragraph.cc b/lib/ui/text/paragraph.cc index 92800db2ebd67..25d773227fd97 100644 --- a/lib/ui/text/paragraph.cc +++ b/lib/ui/text/paragraph.cc @@ -44,7 +44,7 @@ Paragraph::Paragraph(std::unique_ptr 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. diff --git a/lib/ui/text/paragraph.h b/lib/ui/text/paragraph.h index 73c79d3c67de3..a422d847b40e0 100644 --- a/lib/ui/text/paragraph.h +++ b/lib/ui/text/paragraph.h @@ -53,7 +53,7 @@ class Paragraph : public RefCountedDartWrappable { Dart_Handle getLineBoundary(unsigned offset); tonic::Float64List computeLineMetrics(); - size_t GetAllocationSize() override; + size_t GetAllocationSize() const override; static void RegisterNatives(tonic::DartLibraryNatives* natives); diff --git a/testing/dart/canvas_test.dart b/testing/dart/canvas_test.dart index 25d1ed04dc4d6..b02a69fd3bf2b 100644 --- a/testing/dart/canvas_test.dart +++ b/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 createImage(int width, int height) { + final Completer completer = Completer(); + decodeImageFromPixels( + Uint8List.fromList(List.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, [], [], [], 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)); + }); } diff --git a/third_party/tonic/dart_wrappable.cc b/third_party/tonic/dart_wrappable.cc index d0906646f313b..3bdfe3e6e498a 100644 --- a/third_party/tonic/dart_wrappable.cc +++ b/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; } diff --git a/third_party/tonic/dart_wrappable.h b/third_party/tonic/dart_wrappable.h index 1d2e5e75bacb2..49b0a2c40baf3 100644 --- a/third_party/tonic/dart_wrappable.h +++ b/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;