From 1150bc2e7b0616163cf213d102db7ee70c889f43 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Wed, 27 May 2020 14:51:10 -0700 Subject: [PATCH 1/8] start --- lib/ui/painting/canvas.cc | 10 +++++++--- lib/ui/painting/canvas.h | 9 ++++++--- lib/ui/painting/image.cc | 1 + lib/ui/painting/image.h | 1 + lib/ui/painting/image_decoder.cc | 4 ++-- lib/ui/painting/picture.cc | 19 ++++++++++++------- lib/ui/painting/picture.h | 10 ++++++++-- lib/ui/painting/picture_recorder.cc | 9 ++++++--- 8 files changed, 43 insertions(+), 20 deletions(-) diff --git a/lib/ui/painting/canvas.cc b/lib/ui/painting/canvas.cc index 3b06b394947fc..b53f9618500d4 100644 --- a/lib/ui/painting/canvas.cc +++ b/lib/ui/painting/canvas.cc @@ -283,7 +283,7 @@ void Canvas::drawPath(const CanvasPath* path, canvas_->drawPath(path->path(), *paint.paint()); } -void Canvas::drawImage(const CanvasImage* image, +void Canvas::drawImage(CanvasImage* image, double x, double y, const Paint& paint, @@ -293,10 +293,11 @@ 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()); } -void Canvas::drawImageRect(const CanvasImage* image, +void Canvas::drawImageRect(CanvasImage* image, double src_left, double src_top, double src_right, @@ -314,11 +315,12 @@ 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); } -void Canvas::drawImageNine(const CanvasImage* image, +void Canvas::drawImageNine(CanvasImage* image, double center_left, double center_top, double center_right, @@ -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()); } diff --git a/lib/ui/painting/canvas.h b/lib/ui/painting/canvas.h index e431da8bbf8d9..110874b168b9f 100644 --- a/lib/ui/painting/canvas.h +++ b/lib/ui/painting/canvas.h @@ -106,12 +106,12 @@ class Canvas : public RefCountedDartWrappable { void drawPath(const CanvasPath* path, const Paint& paint, const PaintData& paint_data); - void drawImage(const CanvasImage* image, + void drawImage(CanvasImage* image, double x, double y, const Paint& paint, const PaintData& paint_data); - void drawImageRect(const CanvasImage* image, + void drawImageRect(CanvasImage* image, double src_left, double src_top, double src_right, @@ -122,7 +122,7 @@ class Canvas : public RefCountedDartWrappable { double dst_bottom, const Paint& paint, const PaintData& paint_data); - void drawImageNine(const CanvasImage* image, + void drawImageNine(CanvasImage* image, double center_left, double center_top, double center_right, @@ -170,6 +170,8 @@ class Canvas : public RefCountedDartWrappable { static void RegisterNatives(tonic::DartLibraryNatives* natives); + size_t ImageAllocationSize() { 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_; }; } // namespace flutter diff --git a/lib/ui/painting/image.cc b/lib/ui/painting/image.cc index 8ee65790924bf..69da908a2d76c 100644 --- a/lib/ui/painting/image.cc +++ b/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() { diff --git a/lib/ui/painting/image.h b/lib/ui/painting/image.h index 516a2e5d59c49..e4592066f8a7d 100644 --- a/lib/ui/painting/image.h +++ b/lib/ui/painting/image.h @@ -5,6 +5,7 @@ #ifndef FLUTTER_LIB_UI_PAINTING_IMAGE_H_ #define FLUTTER_LIB_UI_PAINTING_IMAGE_H_ + #include "flutter/flow/skia_gpu_object.h" #include "flutter/lib/ui/dart_wrapper.h" #include "flutter/lib/ui/ui_dart_state.h" 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..a0aa4efa66b41 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() { 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..e0d9c06e12a94 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(); } @@ -43,10 +45,14 @@ class Picture : public RefCountedDartWrappable { uint32_t height, Dart_Handle raw_image_callback); + size_t ImageAllocationSize() { 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..d99f16259b2a1 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_->ImageAllocationSize()); + canvas_->Clear(); canvas_->ClearDartWrapper(); canvas_ = nullptr; From 5398e9b6bf26500613313ad7652de2c2db3d7ba3 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Fri, 29 May 2020 00:18:59 -0700 Subject: [PATCH 2/8] Fix issue with initialization of image size --- lib/ui/painting/canvas.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ui/painting/canvas.cc b/lib/ui/painting/canvas.cc index b53f9618500d4..0c1fab5d25b83 100644 --- a/lib/ui/painting/canvas.cc +++ b/lib/ui/painting/canvas.cc @@ -86,7 +86,7 @@ fml::RefPtr Canvas::Create(PictureRecorder* recorder, return canvas; } -Canvas::Canvas(SkCanvas* canvas) : canvas_(canvas) {} +Canvas::Canvas(SkCanvas* canvas) : canvas_(canvas), image_allocation_size_(0) {} Canvas::~Canvas() {} From fd87388c4eeab119bcc41837a3844f6122f7b4bb Mon Sep 17 00:00:00 2001 From: Dan Field Date: Mon, 1 Jun 2020 10:27:30 -0700 Subject: [PATCH 3/8] test --- testing/dart/canvas_test.dart | 37 +++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/testing/dart/canvas_test.dart b/testing/dart/canvas_test.dart index 25d1ed04dc4d6..f15b21bc175d6 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,22 @@ 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* 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()); + final Picture picture = recorder.endRecording(); + expect(picture.approximateBytesUsed, 161235); + + final PictureRecorder recorder2 = PictureRecorder(); + final Canvas canvas2 = Canvas(recorder2); + canvas2.drawPicture(picture); + final Picture picture2 = recorder2.endRecording(); + expect(picture2.approximateBytesUsed, 162999); + }); } From 3d7a601782e726e44943186d1f72c06c3b81dece Mon Sep 17 00:00:00 2001 From: Dan Field Date: Mon, 1 Jun 2020 10:29:08 -0700 Subject: [PATCH 4/8] stray newline --- lib/ui/painting/image.h | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/ui/painting/image.h b/lib/ui/painting/image.h index e4592066f8a7d..516a2e5d59c49 100644 --- a/lib/ui/painting/image.h +++ b/lib/ui/painting/image.h @@ -5,7 +5,6 @@ #ifndef FLUTTER_LIB_UI_PAINTING_IMAGE_H_ #define FLUTTER_LIB_UI_PAINTING_IMAGE_H_ - #include "flutter/flow/skia_gpu_object.h" #include "flutter/lib/ui/dart_wrapper.h" #include "flutter/lib/ui/ui_dart_state.h" From e445fd718ebc0901fa9e27a457867dc7cca6bf02 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Mon, 1 Jun 2020 13:44:04 -0700 Subject: [PATCH 5/8] Make GetAllocationSize const --- lib/ui/painting/engine_layer.cc | 2 +- lib/ui/painting/engine_layer.h | 2 +- lib/ui/painting/image.cc | 2 +- lib/ui/painting/image.h | 2 +- lib/ui/painting/picture.cc | 2 +- lib/ui/painting/picture.h | 2 +- lib/ui/painting/single_frame_codec.cc | 2 +- lib/ui/painting/single_frame_codec.h | 2 +- lib/ui/text/paragraph.cc | 2 +- lib/ui/text/paragraph.h | 2 +- third_party/tonic/dart_wrappable.cc | 2 +- third_party/tonic/dart_wrappable.h | 2 +- 12 files changed, 12 insertions(+), 12 deletions(-) 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..151d76ec6a647 100644 --- a/lib/ui/painting/image.cc +++ b/lib/ui/painting/image.cc @@ -40,7 +40,7 @@ void CanvasImage::dispose() { ClearDartWrapper(); } -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/picture.cc b/lib/ui/painting/picture.cc index 511e369d6afdc..051dd27dc1f29 100644 --- a/lib/ui/painting/picture.cc +++ b/lib/ui/painting/picture.cc @@ -54,7 +54,7 @@ void Picture::dispose() { ClearDartWrapper(); } -size_t Picture::GetAllocationSize() { +size_t Picture::GetAllocationSize() const { if (auto picture = picture_.get()) { return picture->approximateBytesUsed(); } else { diff --git a/lib/ui/painting/picture.h b/lib/ui/painting/picture.h index f6dd98887d264..093a13d2dca4d 100644 --- a/lib/ui/painting/picture.h +++ b/lib/ui/painting/picture.h @@ -34,7 +34,7 @@ class Picture : public RefCountedDartWrappable { void dispose(); - size_t GetAllocationSize() override; + size_t GetAllocationSize() const override; static void RegisterNatives(tonic::DartLibraryNatives* natives); 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/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; From b9b06ba50b8cb2ee961099b3189ac325ebf1b6d4 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Mon, 1 Jun 2020 14:20:44 -0700 Subject: [PATCH 6/8] review --- lib/ui/painting/canvas.cc | 9 +++++---- lib/ui/painting/canvas.h | 10 +++++----- lib/ui/painting/picture.h | 2 +- lib/ui/painting/picture_recorder.cc | 2 +- testing/dart/canvas_test.dart | 7 ++++--- 5 files changed, 16 insertions(+), 14 deletions(-) diff --git a/lib/ui/painting/canvas.cc b/lib/ui/painting/canvas.cc index 0c1fab5d25b83..2ee718cd2f2ba 100644 --- a/lib/ui/painting/canvas.cc +++ b/lib/ui/painting/canvas.cc @@ -86,7 +86,7 @@ fml::RefPtr Canvas::Create(PictureRecorder* recorder, return canvas; } -Canvas::Canvas(SkCanvas* canvas) : canvas_(canvas), image_allocation_size_(0) {} +Canvas::Canvas(SkCanvas* canvas) : canvas_(canvas) {} Canvas::~Canvas() {} @@ -283,7 +283,7 @@ void Canvas::drawPath(const CanvasPath* path, canvas_->drawPath(path->path(), *paint.paint()); } -void Canvas::drawImage(CanvasImage* image, +void Canvas::drawImage(const CanvasImage* image, double x, double y, const Paint& paint, @@ -297,7 +297,7 @@ void Canvas::drawImage(CanvasImage* image, canvas_->drawImage(image->image(), x, y, paint.paint()); } -void Canvas::drawImageRect(CanvasImage* image, +void Canvas::drawImageRect(const CanvasImage* image, double src_left, double src_top, double src_right, @@ -320,7 +320,7 @@ void Canvas::drawImageRect(CanvasImage* image, SkCanvas::kFast_SrcRectConstraint); } -void Canvas::drawImageNine(CanvasImage* image, +void Canvas::drawImageNine(const CanvasImage* image, double center_left, double center_top, double center_right, @@ -406,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 110874b168b9f..8f5a459ab7ef8 100644 --- a/lib/ui/painting/canvas.h +++ b/lib/ui/painting/canvas.h @@ -106,12 +106,12 @@ class Canvas : public RefCountedDartWrappable { void drawPath(const CanvasPath* path, const Paint& paint, const PaintData& paint_data); - void drawImage(CanvasImage* image, + void drawImage(const CanvasImage* image, double x, double y, const Paint& paint, const PaintData& paint_data); - void drawImageRect(CanvasImage* image, + void drawImageRect(const CanvasImage* image, double src_left, double src_top, double src_right, @@ -122,7 +122,7 @@ class Canvas : public RefCountedDartWrappable { double dst_bottom, const Paint& paint, const PaintData& paint_data); - void drawImageNine(CanvasImage* image, + void drawImageNine(const CanvasImage* image, double center_left, double center_top, double center_right, @@ -170,7 +170,7 @@ class Canvas : public RefCountedDartWrappable { static void RegisterNatives(tonic::DartLibraryNatives* natives); - size_t ImageAllocationSize() { return image_allocation_size_; } + size_t image_allocation_size() const { return image_allocation_size_; } private: explicit Canvas(SkCanvas* canvas); @@ -179,7 +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_; + size_t image_allocation_size_ = 0; }; } // namespace flutter diff --git a/lib/ui/painting/picture.h b/lib/ui/painting/picture.h index a87be0901411a..20d21ad3ba722 100644 --- a/lib/ui/painting/picture.h +++ b/lib/ui/painting/picture.h @@ -45,7 +45,7 @@ class Picture : public RefCountedDartWrappable { uint32_t height, Dart_Handle raw_image_callback); - size_t ImageAllocationSize() { return image_allocation_size_; } + size_t image_allocation_size() const { return image_allocation_size_; } private: Picture(flutter::SkiaGPUObject picture, diff --git a/lib/ui/painting/picture_recorder.cc b/lib/ui/painting/picture_recorder.cc index d99f16259b2a1..ab044adbf3780 100644 --- a/lib/ui/painting/picture_recorder.cc +++ b/lib/ui/painting/picture_recorder.cc @@ -56,7 +56,7 @@ fml::RefPtr PictureRecorder::endRecording(Dart_Handle dart_picture) { Picture::Create(dart_picture, UIDartState::CreateGPUObject( picture_recorder_.finishRecordingAsPicture()), - canvas_->ImageAllocationSize()); + canvas_->image_allocation_size()); canvas_->Clear(); canvas_->ClearDartWrapper(); diff --git a/testing/dart/canvas_test.dart b/testing/dart/canvas_test.dart index f15b21bc175d6..b34ba3850126a 100644 --- a/testing/dart/canvas_test.dart +++ b/testing/dart/canvas_test.dart @@ -218,7 +218,7 @@ void main() { expect(areEqual, true); }, skip: !Platform.isLinux); // https://github.com/flutter/flutter/issues/53784 - test('Image size reflected in picture size for image* and drawPicture methods', () async { + 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); @@ -226,13 +226,14 @@ void main() { 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(); - expect(picture.approximateBytesUsed, 161235); + expect(picture.approximateBytesUsed, 214576); final PictureRecorder recorder2 = PictureRecorder(); final Canvas canvas2 = Canvas(recorder2); canvas2.drawPicture(picture); final Picture picture2 = recorder2.endRecording(); - expect(picture2.approximateBytesUsed, 162999); + expect(picture2.approximateBytesUsed, 216340); }); } From 15bf5daa58e2a98a010d6bbade230c489b33bbff Mon Sep 17 00:00:00 2001 From: Dan Field Date: Mon, 1 Jun 2020 14:32:48 -0700 Subject: [PATCH 7/8] explain test --- testing/dart/canvas_test.dart | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/testing/dart/canvas_test.dart b/testing/dart/canvas_test.dart index b34ba3850126a..ca8dc3e102ff8 100644 --- a/testing/dart/canvas_test.dart +++ b/testing/dart/canvas_test.dart @@ -228,12 +228,25 @@ void main() { canvas.drawImageNine(image, rect, rect, Paint()); canvas.drawAtlas(image, [], [], [], BlendMode.src, rect, Paint()); final Picture picture = recorder.endRecording(); - expect(picture.approximateBytesUsed, 214576); + + // 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.... + const int expectedSize = 64 // base Picture size + + 53925 // drawImage overhead + + 53769 // drawImageRect overhead + + 53477 // drawImageNine overhead + + 53341; // drawAtlas overhead + expect(picture.approximateBytesUsed, expectedSize); final PictureRecorder recorder2 = PictureRecorder(); final Canvas canvas2 = Canvas(recorder2); canvas2.drawPicture(picture); final Picture picture2 = recorder2.endRecording(); - expect(picture2.approximateBytesUsed, 216340); + + const int expectedSize2 = 1764 // base Picture size (2) + + expectedSize; + expect(picture2.approximateBytesUsed, expectedSize2); }); } From 7ccaede35f8d5da9fb40fe41a9e87b8d46f5c2c7 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Mon, 1 Jun 2020 16:04:03 -0700 Subject: [PATCH 8/8] make test more lenient --- testing/dart/canvas_test.dart | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/testing/dart/canvas_test.dart b/testing/dart/canvas_test.dart index ca8dc3e102ff8..b02a69fd3bf2b 100644 --- a/testing/dart/canvas_test.dart +++ b/testing/dart/canvas_test.dart @@ -233,20 +233,16 @@ void main() { // 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.... - const int expectedSize = 64 // base Picture size - + 53925 // drawImage overhead - + 53769 // drawImageRect overhead - + 53477 // drawImageNine overhead - + 53341; // drawAtlas overhead - expect(picture.approximateBytesUsed, expectedSize); + // 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(); - const int expectedSize2 = 1764 // base Picture size (2) - + expectedSize; - expect(picture2.approximateBytesUsed, expectedSize2); + expect(picture2.approximateBytesUsed, greaterThan(minimumExpected)); }); }