Implement GetAllocationSize for Vertices (flutter#18756)

dnfield · web-flow · commit 25054fb47016 · 2020-06-03T10:35:14.000-07:00
* Implement GetAllocationSize for Vertices

* Reflect vertex buffer size in pictures
diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter
@@ -354,6 +354,7 @@ FILE: ../../../flutter/lib/ui/painting/single_frame_codec.cc
 FILE: ../../../flutter/lib/ui/painting/single_frame_codec.h
 FILE: ../../../flutter/lib/ui/painting/vertices.cc
 FILE: ../../../flutter/lib/ui/painting/vertices.h
+FILE: ../../../flutter/lib/ui/painting/vertices_unittests.cc
 FILE: ../../../flutter/lib/ui/plugins.dart
 FILE: ../../../flutter/lib/ui/plugins/callback_cache.cc
 FILE: ../../../flutter/lib/ui/plugins/callback_cache.h
diff --git a/lib/ui/BUILD.gn b/lib/ui/BUILD.gn
@@ -164,13 +164,15 @@ if (current_toolchain == host_toolchain) {
       "painting/image_decoder_test.cc",
       "painting/image_decoder_test.h",
       "painting/image_decoder_unittests.cc",
+      "painting/vertices_unittests.cc",
       "window/pointer_data_packet_converter_unittests.cc",
     ]
 
     deps = [
       ":ui",
       ":ui_unittests_fixtures",
       "//flutter/common",
+      "//flutter/shell/common:shell_test_fixture_sources",
       "//flutter/testing",
       "//flutter/testing:dart",
       "//flutter/testing:opengl",
diff --git a/lib/ui/fixtures/ui_test.dart b/lib/ui/fixtures/ui_test.dart
@@ -3,10 +3,39 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+import 'dart:typed_data';
 import 'dart:ui';
 
 void main() {}
 
+@pragma('vm:entry-point')
+void createVertices() {
+  const int uint16max = 65535;
+
+  final Int32List colors = Int32List(uint16max);
+  final Float32List coords = Float32List(uint16max * 2);
+  final Uint16List indices = Uint16List(uint16max);
+  final Float32List positions = Float32List(uint16max * 2);
+  colors[0] = const Color(0xFFFF0000).value;
+  colors[1] = const Color(0xFF00FF00).value;
+  colors[2] = const Color(0xFF0000FF).value;
+  colors[3] = const Color(0xFF00FFFF).value;
+  indices[1] = indices[3] = 1;
+  indices[2] = indices[5] = 3;
+  indices[4] = 2;
+  positions[2] = positions[4] = positions[5] = positions[7] = 250.0;
+
+  final Vertices vertices = Vertices.raw(
+    VertexMode.triangles,
+    positions,
+    textureCoordinates: coords,
+    colors: colors,
+    indices: indices,
+  );
+  _validateVertices(vertices);
+}
+void _validateVertices(Vertices vertices) native 'ValidateVertices';
+
 @pragma('vm:entry-point')
 void frameCallback(FrameInfo info) {
   print('called back');
diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart
@@ -3249,8 +3249,7 @@ class Vertices extends NativeFieldWrapperClass2 {
       ? Uint16List.fromList(indices)
       : null;
 
-    _constructor();
-    if (!_init(mode.index, encodedPositions, encodedTextureCoordinates, encodedColors, encodedIndices))
+    if (!_init(this, mode.index, encodedPositions, encodedTextureCoordinates, encodedColors, encodedIndices))
       throw ArgumentError('Invalid configuration for vertices.');
   }
 
@@ -3287,14 +3286,12 @@ class Vertices extends NativeFieldWrapperClass2 {
     if (indices != null && indices.any((int i) => i < 0 || i >= positions.length))
       throw ArgumentError('"indices" values must be valid indices in the positions list.');
 
-    _constructor();
-    if (!_init(mode.index, positions, textureCoordinates, colors, indices))
+    if (!_init(this, mode.index, positions, textureCoordinates, colors, indices))
       throw ArgumentError('Invalid configuration for vertices.');
   }
 
-  void _constructor() native 'Vertices_constructor';
-
-  bool _init(int mode,
+  bool _init(Vertices outVertices,
+             int mode,
              Float32List positions,
              Float32List textureCoordinates,
              Int32List colors,
diff --git a/lib/ui/painting/canvas.cc b/lib/ui/painting/canvas.cc
@@ -293,7 +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();
+  external_allocation_size_ += image->GetAllocationSize();
   canvas_->drawImage(image->image(), x, y, paint.paint());
 }
 
@@ -315,7 +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();
+  external_allocation_size_ += image->GetAllocationSize();
   canvas_->drawImageRect(image->image(), src, dst, paint.paint(),
                          SkCanvas::kFast_SrcRectConstraint);
 }
@@ -341,7 +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();
+  external_allocation_size_ += image->GetAllocationSize();
   canvas_->drawImageNine(image->image(), icenter, dst, paint.paint());
 }
 
@@ -351,7 +351,7 @@ void Canvas::drawPicture(Picture* picture) {
   if (!picture)
     Dart_ThrowException(
         ToDart("Canvas.drawPicture called with non-genuine Picture."));
-  image_allocation_size_ += picture->GetAllocationSize();
+  external_allocation_size_ += picture->GetAllocationSize();
   canvas_->drawPicture(picture->picture().get());
 }
 
@@ -380,7 +380,7 @@ void Canvas::drawVertices(const Vertices* vertices,
   if (!vertices)
     Dart_ThrowException(
         ToDart("Canvas.drawVertices called with non-genuine Vertices."));
-
+  external_allocation_size_ += vertices->GetAllocationSize();
   canvas_->drawVertices(vertices->vertices(), blend_mode, *paint.paint());
 }
 
@@ -406,7 +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();
+  external_allocation_size_ += atlas->GetAllocationSize();
   canvas_->drawAtlas(
       skImage.get(), reinterpret_cast<const SkRSXform*>(transforms.data()),
       reinterpret_cast<const SkRect*>(rects.data()),
diff --git a/lib/ui/painting/canvas.h b/lib/ui/painting/canvas.h
@@ -170,7 +170,7 @@ class Canvas : public RefCountedDartWrappable<Canvas> {
 
   static void RegisterNatives(tonic::DartLibraryNatives* natives);
 
-  size_t image_allocation_size() const { return image_allocation_size_; }
+  size_t external_allocation_size() const { return external_allocation_size_; }
 
  private:
   explicit Canvas(SkCanvas* canvas);
@@ -179,7 +179,7 @@ class Canvas : public RefCountedDartWrappable<Canvas> {
   // 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;
+  size_t external_allocation_size_ = 0;
 };
 
 }  // namespace flutter
diff --git a/lib/ui/painting/picture.cc b/lib/ui/painting/picture.cc
@@ -28,18 +28,18 @@ DART_BIND_ALL(Picture, FOR_EACH_BINDING)
 
 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);
+                                     size_t external_allocation_size) {
+  auto canvas_picture = fml::MakeRefCounted<Picture>(std::move(picture),
+                                                     external_allocation_size);
 
   canvas_picture->AssociateWithDartWrapper(dart_handle);
   return canvas_picture;
 }
 
 Picture::Picture(flutter::SkiaGPUObject<SkPicture> picture,
-                 size_t image_allocation_size)
+                 size_t external_allocation_size)
     : picture_(std::move(picture)),
-      image_allocation_size_(image_allocation_size) {}
+      external_allocation_size_(external_allocation_size) {}
 
 Picture::~Picture() = default;
 
@@ -61,7 +61,7 @@ void Picture::dispose() {
 size_t Picture::GetAllocationSize() const {
   if (auto picture = picture_.get()) {
     return picture->approximateBytesUsed() + sizeof(Picture) +
-           image_allocation_size_;
+           external_allocation_size_;
   } else {
     return sizeof(Picture);
   }
diff --git a/lib/ui/painting/picture.h b/lib/ui/painting/picture.h
@@ -26,7 +26,7 @@ class Picture : public RefCountedDartWrappable<Picture> {
   ~Picture() override;
   static fml::RefPtr<Picture> Create(Dart_Handle dart_handle,
                                      flutter::SkiaGPUObject<SkPicture> picture,
-                                     size_t image_allocation_size);
+                                     size_t external_allocation_size);
 
   sk_sp<SkPicture> picture() const { return picture_.get(); }
 
@@ -45,14 +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_; }
+  size_t external_allocation_size() const { return external_allocation_size_; }
 
  private:
   Picture(flutter::SkiaGPUObject<SkPicture> picture,
-          size_t image_allocation_size_);
+          size_t external_allocation_size_);
 
   flutter::SkiaGPUObject<SkPicture> picture_;
-  size_t image_allocation_size_;
+  size_t external_allocation_size_;
 };
 
 }  // namespace flutter
diff --git a/lib/ui/painting/picture_recorder.cc b/lib/ui/painting/picture_recorder.cc
@@ -56,7 +56,7 @@ fml::RefPtr<Picture> PictureRecorder::endRecording(Dart_Handle dart_picture) {
       Picture::Create(dart_picture,
                       UIDartState::CreateGPUObject(
                           picture_recorder_.finishRecordingAsPicture()),
-                      canvas_->image_allocation_size());
+                      canvas_->external_allocation_size());
 
   canvas_->Clear();
   canvas_->ClearDartWrapper();
diff --git a/lib/ui/painting/vertices.cc b/lib/ui/painting/vertices.cc
@@ -27,11 +27,6 @@ void DecodeInts(const tonic::Int32List& ints, T* out) {
 
 }  // namespace
 
-static void Vertices_constructor(Dart_NativeArguments args) {
-  UIDartState::ThrowIfUIOperationsProhibited();
-  DartCallConstructor(&Vertices::Create, args);
-}
-
 IMPLEMENT_WRAPPERTYPEINFO(ui, Vertices);
 
 #define FOR_EACH_BINDING(V) V(Vertices, init)
@@ -43,19 +38,16 @@ Vertices::Vertices() {}
 Vertices::~Vertices() {}
 
 void Vertices::RegisterNatives(tonic::DartLibraryNatives* natives) {
-  natives->Register({{"Vertices_constructor", Vertices_constructor, 1, true},
-                     FOR_EACH_BINDING(DART_REGISTER_NATIVE)});
-}
-
-fml::RefPtr<Vertices> Vertices::Create() {
-  return fml::MakeRefCounted<Vertices>();
+  natives->Register({FOR_EACH_BINDING(DART_REGISTER_NATIVE)});
 }
 
-bool Vertices::init(SkVertices::VertexMode vertex_mode,
+bool Vertices::init(Dart_Handle vertices_handle,
+                    SkVertices::VertexMode vertex_mode,
                     const tonic::Float32List& positions,
                     const tonic::Float32List& texture_coordinates,
                     const tonic::Int32List& colors,
                     const tonic::Uint16List& indices) {
+  UIDartState::ThrowIfUIOperationsProhibited();
   uint32_t builderFlags = 0;
   if (texture_coordinates.data())
     builderFlags |= SkVertices::kHasTexCoords_BuilderFlag;
@@ -89,9 +81,15 @@ bool Vertices::init(SkVertices::VertexMode vertex_mode,
               builder.indices());
   }
 
-  vertices_ = builder.detach();
+  auto vertices = fml::MakeRefCounted<Vertices>();
+  vertices->vertices_ = builder.detach();
+  vertices->AssociateWithDartWrapper(vertices_handle);
 
   return true;
 }
 
+size_t Vertices::GetAllocationSize() const {
+  return sizeof(Vertices) + vertices_->approximateSize();
+}
+
 }  // namespace flutter
diff --git a/lib/ui/painting/vertices.h b/lib/ui/painting/vertices.h
@@ -24,16 +24,17 @@ class Vertices : public RefCountedDartWrappable<Vertices> {
 
   static void RegisterNatives(tonic::DartLibraryNatives* natives);
 
-  static fml::RefPtr<Vertices> Create();
-
-  bool init(SkVertices::VertexMode vertex_mode,
-            const tonic::Float32List& positions,
-            const tonic::Float32List& texture_coordinates,
-            const tonic::Int32List& colors,
-            const tonic::Uint16List& indices);
+  static bool init(Dart_Handle vertices_handle,
+                   SkVertices::VertexMode vertex_mode,
+                   const tonic::Float32List& positions,
+                   const tonic::Float32List& texture_coordinates,
+                   const tonic::Int32List& colors,
+                   const tonic::Uint16List& indices);
 
   const sk_sp<SkVertices>& vertices() const { return vertices_; }
 
+  size_t GetAllocationSize() const override;
+
  private:
   Vertices();
 
diff --git a/lib/ui/painting/vertices_unittests.cc b/lib/ui/painting/vertices_unittests.cc
@@ -0,0 +1,61 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "flutter/common/task_runners.h"
+#include "flutter/fml/synchronization/waitable_event.h"
+#include "flutter/lib/ui/painting/vertices.h"
+#include "flutter/runtime/dart_vm.h"
+#include "flutter/shell/common/shell_test.h"
+#include "flutter/shell/common/thread_host.h"
+#include "flutter/testing/testing.h"
+
+namespace flutter {
+namespace testing {
+
+TEST_F(ShellTest, VerticesAccuratelyReportsSize) {
+  fml::AutoResetWaitableEvent message_latch;
+
+  auto nativeValidateVertices = [&](Dart_NativeArguments args) {
+    auto handle = Dart_GetNativeArgument(args, 0);
+    intptr_t peer = 0;
+    Dart_Handle result = Dart_GetNativeInstanceField(
+        handle, tonic::DartWrappable::kPeerIndex, &peer);
+    ASSERT_FALSE(Dart_IsError(result));
+    Vertices* vertices = reinterpret_cast<Vertices*>(peer);
+    // GT because we don't want to be too dependent on platform specific
+    // differences, or minor changes to Skia. The actual value of this on
+    // macOS as of the test writing is 1441890ul. Just need to assert it's
+    // big enough to get the Dart GC's attention.
+    ASSERT_GT(vertices->GetAllocationSize(), 1300000ul);
+    message_latch.Signal();
+  };
+
+  Settings settings = CreateSettingsForFixture();
+  TaskRunners task_runners("test",                  // label
+                           GetCurrentTaskRunner(),  // platform
+                           CreateNewThread(),       // raster
+                           CreateNewThread(),       // ui
+                           CreateNewThread()        // io
+  );
+
+  AddNativeCallback("ValidateVertices",
+                    CREATE_NATIVE_ENTRY(nativeValidateVertices));
+
+  std::unique_ptr<Shell> shell =
+      CreateShell(std::move(settings), std::move(task_runners));
+
+  ASSERT_TRUE(shell->IsSetup());
+  auto configuration = RunConfiguration::InferFromSettings(settings);
+  configuration.SetEntrypoint("createVertices");
+
+  shell->RunEngine(std::move(configuration), [&](auto result) {
+    ASSERT_EQ(result, Engine::RunStatus::Success);
+  });
+
+  message_latch.Wait();
+  DestroyShell(std::move(shell), std::move(task_runners));
+}
+
+}  // namespace testing
+}  // namespace flutter
diff --git a/shell/common/BUILD.gn b/shell/common/BUILD.gn
diff --git a/testing/dart/canvas_test.dart b/testing/dart/canvas_test.dart
