Reland: Avoid a copy in EncodeImage (#20003)

Author: zanderso

ci/licenses_golden/licenses_flutter

@@ -339,6 +339,7 @@ FILE: ../../../flutter/lib/ui/painting/image_descriptor.cc
 FILE: ../../../flutter/lib/ui/painting/image_descriptor.h
 FILE: ../../../flutter/lib/ui/painting/image_encoding.cc
 FILE: ../../../flutter/lib/ui/painting/image_encoding.h
+FILE: ../../../flutter/lib/ui/painting/image_encoding_unittests.cc
 FILE: ../../../flutter/lib/ui/painting/image_filter.cc
 FILE: ../../../flutter/lib/ui/painting/image_filter.h
 FILE: ../../../flutter/lib/ui/painting/image_shader.cc

lib/ui/BUILD.gn

@@ -178,6 +178,7 @@ if (enable_unittests) {
     public_configs = [ "//flutter:export_dynamic_symbols" ]
 
     sources = [
+      "painting/image_encoding_unittests.cc",
       "painting/vertices_unittests.cc",
       "window/pointer_data_packet_converter_unittests.cc",
     ]

lib/ui/fixtures/ui_test.dart

@@ -44,3 +44,29 @@ void frameCallback(FrameInfo info) {
 @pragma('vm:entry-point')
 void messageCallback(dynamic data) {
 }
+
+
+// Draw a circle on a Canvas that has a PictureRecorder. Take the image from
+// the PictureRecorder, and encode it as png. Check that the png data is
+// backed by an external Uint8List.
+@pragma('vm:entry-point')
+Future<void> encodeImageProducesExternalUint8List() async {
+  final PictureRecorder pictureRecorder = PictureRecorder();
+  final Canvas canvas = Canvas(pictureRecorder);
+  final Paint paint = Paint()
+    ..color = Color.fromRGBO(255, 255, 255, 1.0)
+    ..style = PaintingStyle.fill;
+  final Offset c = Offset(50.0, 50.0);
+  canvas.drawCircle(c, 25.0, paint);
+  final Picture picture = pictureRecorder.endRecording();
+  final Image image = await picture.toImage(100, 100);
+  _encodeImage(image, ImageByteFormat.png.index, (Uint8List result) {
+    // The buffer should be non-null and writable.
+    result[0] = 0;
+    // The buffer should be external typed data.
+    _validateExternal(result);
+  });
+}
+void _encodeImage(Image i, int format, void Function(Uint8List result))
+  native 'EncodeImage';
+void _validateExternal(Uint8List result) native 'ValidateExternal';

lib/ui/painting/image_encoding.cc

@@ -35,6 +35,13 @@ enum ImageByteFormat {
   kPNG,
 };
 
+void FinalizeSkData(void* isolate_callback_data,
+                    Dart_WeakPersistentHandle handle,
+                    void* peer) {
+  SkData* buffer = reinterpret_cast<SkData*>(peer);
+  buffer->unref();
+}
+
 void InvokeDataCallback(std::unique_ptr<DartPersistentValue> callback,
                         sk_sp<SkData> buffer) {
   std::shared_ptr<tonic::DartState> dart_state = callback->dart_state().lock();
@@ -44,11 +51,17 @@ void InvokeDataCallback(std::unique_ptr<DartPersistentValue> callback,
   tonic::DartState::Scope scope(dart_state);
   if (!buffer) {
     DartInvoke(callback->value(), {Dart_Null()});
-  } else {
-    Dart_Handle dart_data = tonic::DartConverter<tonic::Uint8List>::ToDart(
-        buffer->bytes(), buffer->size());
-    DartInvoke(callback->value(), {dart_data});
+    return;
   }
+  // Skia will not modify the buffer, and it is backed by memory that is
+  // read/write, so Dart can be given direct access to the buffer through an
+  // external Uint8List.
+  void* bytes = const_cast<void*>(buffer->data());
+  const intptr_t length = buffer->size();
+  void* peer = reinterpret_cast<void*>(buffer.release());
+  Dart_Handle dart_data = Dart_NewExternalTypedDataWithFinalizer(
+      Dart_TypedData_kUint8, bytes, length, peer, length, FinalizeSkData);
+  DartInvoke(callback->value(), {dart_data});
 }
 
 sk_sp<SkImage> ConvertToRasterUsingResourceContext(
@@ -222,9 +235,10 @@ void EncodeImageAndInvokeDataCallback(
   auto encode_task = [callback_task = std::move(callback_task), format,
                       ui_task_runner](sk_sp<SkImage> raster_image) {
     sk_sp<SkData> encoded = EncodeImage(std::move(raster_image), format);
-    ui_task_runner->PostTask(
-        [callback_task = std::move(callback_task),
-         encoded = std::move(encoded)] { callback_task(encoded); });
+    ui_task_runner->PostTask([callback_task = std::move(callback_task),
+                              encoded = std::move(encoded)]() mutable {
+      callback_task(std::move(encoded));
+    });
   };
 
   ConvertImageToRaster(std::move(image), encode_task, raster_task_runner,

lib/ui/painting/image_encoding_unittests.cc

@@ -0,0 +1,77 @@
+// 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/image_encoding.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 {
+
+namespace {
+fml::AutoResetWaitableEvent message_latch;
+};
+
+TEST_F(ShellTest, EncodeImageGivesExternalTypedData) {
+  auto nativeEncodeImage = [&](Dart_NativeArguments args) {
+    auto image_handle = Dart_GetNativeArgument(args, 0);
+    auto format_handle = Dart_GetNativeArgument(args, 1);
+    auto callback_handle = Dart_GetNativeArgument(args, 2);
+
+    intptr_t peer = 0;
+    Dart_Handle result = Dart_GetNativeInstanceField(
+        image_handle, tonic::DartWrappable::kPeerIndex, &peer);
+    ASSERT_FALSE(Dart_IsError(result));
+    CanvasImage* canvas_image = reinterpret_cast<CanvasImage*>(peer);
+
+    int64_t format = -1;
+    result = Dart_IntegerToInt64(format_handle, &format);
+    ASSERT_FALSE(Dart_IsError(result));
+
+    result = EncodeImage(canvas_image, format, callback_handle);
+    ASSERT_TRUE(Dart_IsNull(result));
+  };
+
+  auto nativeValidateExternal = [&](Dart_NativeArguments args) {
+    auto handle = Dart_GetNativeArgument(args, 0);
+
+    auto typed_data_type = Dart_GetTypeOfExternalTypedData(handle);
+    EXPECT_EQ(typed_data_type, Dart_TypedData_kUint8);
+
+    message_latch.Signal();
+  };
+
+  Settings settings = CreateSettingsForFixture();
+  TaskRunners task_runners("test",                  // label
+                           GetCurrentTaskRunner(),  // platform
+                           CreateNewThread(),       // raster
+                           CreateNewThread(),       // ui
+                           CreateNewThread()        // io
+  );
+
+  AddNativeCallback("EncodeImage", CREATE_NATIVE_ENTRY(nativeEncodeImage));
+  AddNativeCallback("ValidateExternal",
+                    CREATE_NATIVE_ENTRY(nativeValidateExternal));
+
+  std::unique_ptr<Shell> shell =
+      CreateShell(std::move(settings), std::move(task_runners));
+
+  ASSERT_TRUE(shell->IsSetup());
+  auto configuration = RunConfiguration::InferFromSettings(settings);
+  configuration.SetEntrypoint("encodeImageProducesExternalUint8List");
+
+  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