[Impeller] use sync fence for image uploads. (flutter/engine#56609)

Fixes https://github.com/flutter/flutter/issues/158963

If the GLES version is at least 3, then we can attach a sync fence to the texture gles object. If this operation succeeds, then we can use gl.Flush instad of gl.Finish. Then, when binding the texture - if a sync fence is present we wait and then remove the fence.

engine/src/flutter/impeller/renderer/backend/gles/BUILD.gn

@@ -25,6 +25,7 @@ impeller_component("gles_unittests") {
     "test/reactor_unittests.cc",
     "test/specialization_constants_unittests.cc",
     "test/surface_gles_unittests.cc",
+    "test/texture_gles_unittests.cc",
   ]
   deps = [
     ":gles",

engine/src/flutter/impeller/renderer/backend/gles/command_buffer_gles.cc

@@ -43,6 +43,11 @@ void CommandBufferGLES::OnWaitUntilCompleted() {
   reactor_->GetProcTable().Finish();
 }
 
+// |CommandBuffer|
+void CommandBufferGLES::OnWaitUntilScheduled() {
+  reactor_->GetProcTable().Flush();
+}
+
 // |CommandBuffer|
 std::shared_ptr<RenderPass> CommandBufferGLES::OnCreateRenderPass(
     RenderTarget target) {

engine/src/flutter/impeller/renderer/backend/gles/command_buffer_gles.h

@@ -37,6 +37,9 @@ class CommandBufferGLES final : public CommandBuffer {
   // |CommandBuffer|
   void OnWaitUntilCompleted() override;
 
+  // |CommandBuffer|
+  void OnWaitUntilScheduled() override;
+
   // |CommandBuffer|
   std::shared_ptr<RenderPass> OnCreateRenderPass(RenderTarget target) override;
 

engine/src/flutter/impeller/renderer/backend/gles/context_gles.cc

@@ -9,7 +9,9 @@
 #include "impeller/base/validation.h"
 #include "impeller/renderer/backend/gles/command_buffer_gles.h"
 #include "impeller/renderer/backend/gles/gpu_tracer_gles.h"
+#include "impeller/renderer/backend/gles/handle_gles.h"
 #include "impeller/renderer/backend/gles/render_pass_gles.h"
+#include "impeller/renderer/backend/gles/texture_gles.h"
 #include "impeller/renderer/command_queue.h"
 
 namespace impeller {
@@ -157,4 +159,15 @@ void ContextGLES::ResetThreadLocalState() const {
       });
 }
 
+// |Context|
+bool ContextGLES::AddTrackingFence(
+    const std::shared_ptr<Texture>& texture) const {
+  if (!reactor_->GetProcTable().FenceSync.IsAvailable()) {
+    return false;
+  }
+  HandleGLES fence = reactor_->CreateHandle(HandleType::kFence);
+  TextureGLES::Cast(*texture).SetFence(fence);
+  return true;
+}
+
 }  // namespace impeller

engine/src/flutter/impeller/renderer/backend/gles/context_gles.h

@@ -93,6 +93,9 @@ class ContextGLES final : public Context,
   // |Context|
   void Shutdown() override;
 
+  // |Context|
+  bool AddTrackingFence(const std::shared_ptr<Texture>& texture) const override;
+
   // |Context|
   void ResetThreadLocalState() const override;
 

engine/src/flutter/impeller/renderer/backend/gles/handle_gles.cc

@@ -22,6 +22,8 @@ std::string HandleTypeToString(HandleType type) {
       return "RenderBuffer";
     case HandleType::kFrameBuffer:
       return "Framebuffer";
+    case HandleType::kFence:
+      return "Fence";
   }
   FML_UNREACHABLE();
 }

engine/src/flutter/impeller/renderer/backend/gles/handle_gles.h

@@ -22,6 +22,7 @@ enum class HandleType {
   kProgram,
   kRenderBuffer,
   kFrameBuffer,
+  kFence,
 };
 
 std::string HandleTypeToString(HandleType type);

engine/src/flutter/impeller/renderer/backend/gles/proc_table_gles.cc

@@ -334,6 +334,8 @@ static std::optional<GLenum> ToDebugIdentifier(DebugResourceType type) {
       return GL_RENDERBUFFER;
     case DebugResourceType::kFrameBuffer:
       return GL_FRAMEBUFFER;
+    case DebugResourceType::kFence:
+      return GL_SYNC_FENCE;
   }
   FML_UNREACHABLE();
 }
@@ -354,6 +356,8 @@ static bool ResourceIsLive(const ProcTableGLES& gl,
       return gl.IsRenderbuffer(name);
     case DebugResourceType::kFrameBuffer:
       return gl.IsFramebuffer(name);
+    case DebugResourceType::kFence:
+      return true;
   }
   FML_UNREACHABLE();
 }

engine/src/flutter/impeller/renderer/backend/gles/proc_table_gles.h

@@ -258,7 +258,11 @@ void(glDepthRange)(GLdouble n, GLdouble f);
   PROC(ClearDepth);                               \
   PROC(DepthRange);
 
-#define FOR_EACH_IMPELLER_GLES3_PROC(PROC) PROC(BlitFramebuffer);
+#define FOR_EACH_IMPELLER_GLES3_PROC(PROC) \
+  PROC(FenceSync);                         \
+  PROC(DeleteSync);                        \
+  PROC(WaitSync);                          \
+  PROC(BlitFramebuffer);
 
 #define FOR_EACH_IMPELLER_EXT_PROC(PROC)    \
   PROC(DebugMessageControlKHR);             \
@@ -282,6 +286,7 @@ enum class DebugResourceType {
   kShader,
   kRenderBuffer,
   kFrameBuffer,
+  kFence,
 };
 
 class ProcTableGLES {

engine/src/flutter/impeller/renderer/backend/gles/reactor_gles.cc

@@ -13,51 +13,59 @@
 
 namespace impeller {
 
-static std::optional<GLuint> CreateGLHandle(const ProcTableGLES& gl,
+// static
-                                            HandleType type) {
+std::optional<ReactorGLES::GLStorage> ReactorGLES::CreateGLHandle(
-  GLuint handle = GL_NONE;
+    const ProcTableGLES& gl,
+    HandleType type) {
+  GLStorage handle = GLStorage{.handle = GL_NONE};
   switch (type) {
     case HandleType::kUnknown:
       return std::nullopt;
     case HandleType::kTexture:
-      gl.GenTextures(1u, &handle);
+      gl.GenTextures(1u, &handle.handle);
       return handle;
     case HandleType::kBuffer:
-      gl.GenBuffers(1u, &handle);
+      gl.GenBuffers(1u, &handle.handle);
       return handle;
     case HandleType::kProgram:
-      return gl.CreateProgram();
+      return GLStorage{.handle = gl.CreateProgram()};
     case HandleType::kRenderBuffer:
-      gl.GenRenderbuffers(1u, &handle);
+      gl.GenRenderbuffers(1u, &handle.handle);
       return handle;
     case HandleType::kFrameBuffer:
-      gl.GenFramebuffers(1u, &handle);
+      gl.GenFramebuffers(1u, &handle.handle);
       return handle;
+    case HandleType::kFence:
+      return GLStorage{.sync = gl.FenceSync(GL_SYNC_GPU_COMMANDS_COMPLETE, 0)};
   }
   return std::nullopt;
 }
 
-static bool CollectGLHandle(const ProcTableGLES& gl,
+// static
-                            HandleType type,
+bool ReactorGLES::CollectGLHandle(const ProcTableGLES& gl,
-                            GLuint handle) {
+                                  HandleType type,
+                                  ReactorGLES::GLStorage handle) {
   switch (type) {
     case HandleType::kUnknown:
       return false;
     case HandleType::kTexture:
-      gl.DeleteTextures(1u, &handle);
+      gl.DeleteTextures(1u, &handle.handle);
       return true;
     case HandleType::kBuffer:
-      gl.DeleteBuffers(1u, &handle);
+      gl.DeleteBuffers(1u, &handle.handle);
       return true;
     case HandleType::kProgram:
-      gl.DeleteProgram(handle);
+      gl.DeleteProgram(handle.handle);
       return true;
     case HandleType::kRenderBuffer:
-      gl.DeleteRenderbuffers(1u, &handle);
+      gl.DeleteRenderbuffers(1u, &handle.handle);
       return true;
     case HandleType::kFrameBuffer:
-      gl.DeleteFramebuffers(1u, &handle);
+      gl.DeleteFramebuffers(1u, &handle.handle);
       return true;
+    case HandleType::kFence:
+      gl.DeleteSync(handle.sync);
+      break;
   }
   return false;
 }
@@ -116,7 +124,8 @@ const ProcTableGLES& ReactorGLES::GetProcTable() const {
   return *proc_table_;
 }
 
-std::optional<GLuint> ReactorGLES::GetGLHandle(const HandleGLES& handle) const {
+std::optional<ReactorGLES::GLStorage> ReactorGLES::GetHandle(
+    const HandleGLES& handle) const {
   ReaderLock handles_lock(handles_mutex_);
   if (auto found = handles_.find(handle); found != handles_.end()) {
     if (found->second.pending_collection) {
@@ -124,16 +133,39 @@ std::optional<GLuint> ReactorGLES::GetGLHandle(const HandleGLES& handle) const {
           << "Attempted to acquire a handle that was pending collection.";
       return std::nullopt;
     }
-    if (!found->second.name.has_value()) {
+    std::optional<ReactorGLES::GLStorage> name = found->second.name;
+    if (!name.has_value()) {
       VALIDATION_LOG << "Attempt to acquire a handle outside of an operation.";
       return std::nullopt;
     }
-    return found->second.name;
+    return name;
   }
   VALIDATION_LOG << "Attempted to acquire an invalid GL handle.";
   return std::nullopt;
 }
 
+std::optional<GLuint> ReactorGLES::GetGLHandle(const HandleGLES& handle) const {
+  if (handle.type == HandleType::kFence) {
+    return std::nullopt;
+  }
+  std::optional<ReactorGLES::GLStorage> gl_handle = GetHandle(handle);
+  if (gl_handle.has_value()) {
+    return gl_handle->handle;
+  }
+  return std::nullopt;
+}
+
+std::optional<GLsync> ReactorGLES::GetGLFence(const HandleGLES& handle) const {
+  if (handle.type != HandleType::kFence) {
+    return std::nullopt;
+  }
+  std::optional<ReactorGLES::GLStorage> gl_handle = GetHandle(handle);
+  if (gl_handle.has_value()) {
+    return gl_handle->sync;
+  }
+  return std::nullopt;
+}
+
 bool ReactorGLES::AddOperation(Operation operation) {
   if (!operation) {
     return false;
@@ -171,9 +203,9 @@ HandleGLES ReactorGLES::CreateHandle(HandleType type, GLuint external_handle) {
   }
   WriterLock handles_lock(handles_mutex_);
 
-  std::optional<GLuint> gl_handle;
+  std::optional<ReactorGLES::GLStorage> gl_handle;
   if (external_handle != GL_NONE) {
-    gl_handle = external_handle;
+    gl_handle = ReactorGLES::GLStorage{.handle = external_handle};
   } else if (CanReactOnCurrentThread()) {
     gl_handle = CreateGLHandle(GetProcTable(), type);
   }
@@ -215,6 +247,8 @@ static DebugResourceType ToDebugResourceType(HandleType type) {
       return DebugResourceType::kRenderBuffer;
     case HandleType::kFrameBuffer:
       return DebugResourceType::kFrameBuffer;
+    case HandleType::kFence:
+      return DebugResourceType::kFence;
   }
   FML_UNREACHABLE();
 }
@@ -253,9 +287,10 @@ bool ReactorGLES::ConsolidateHandles() {
       handle.second.name = gl_handle;
     }
     // Set pending debug labels.
-    if (handle.second.pending_debug_label.has_value()) {
+    if (handle.second.pending_debug_label.has_value() &&
+        handle.first.type != HandleType::kFence) {
       if (gl.SetDebugLabel(ToDebugResourceType(handle.first.type),
-                           handle.second.name.value(),
+                           handle.second.name.value().handle,
                            handle.second.pending_debug_label.value())) {
         handle.second.pending_debug_label = std::nullopt;
       }

engine/src/flutter/impeller/renderer/backend/gles/reactor_gles.h

@@ -158,6 +158,8 @@ class ReactorGLES {
   ///
   std::optional<GLuint> GetGLHandle(const HandleGLES& handle) const;
 
+  std::optional<GLsync> GetGLFence(const HandleGLES& handle) const;
+
   //----------------------------------------------------------------------------
   /// @brief      Create a reactor handle.
   ///
@@ -245,15 +247,23 @@ class ReactorGLES {
   [[nodiscard]] bool React();
 
  private:
+  /// @brief Storage for either a GL handle or sync fence.
+  struct GLStorage {
+    union {
+      GLuint handle;
+      GLsync sync;
+    };
+  };
+
   struct LiveHandle {
-    std::optional<GLuint> name;
+    std::optional<GLStorage> name;
     std::optional<std::string> pending_debug_label;
     bool pending_collection = false;
     fml::ScopedCleanupClosure callback = {};
 
     LiveHandle() = default;
 
-    explicit LiveHandle(std::optional<GLuint> p_name) : name(p_name) {}
+    explicit LiveHandle(std::optional<GLStorage> p_name) : name(p_name) {}
 
     constexpr bool IsLive() const { return name.has_value(); }
   };
@@ -292,6 +302,15 @@ class ReactorGLES {
 
   void SetupDebugGroups();
 
+  std::optional<GLStorage> GetHandle(const HandleGLES& handle) const;
+
+  static std::optional<GLStorage> CreateGLHandle(const ProcTableGLES& gl,
+                                                 HandleType type);
+
+  static bool CollectGLHandle(const ProcTableGLES& gl,
+                              HandleType type,
+                              GLStorage handle);
+
   ReactorGLES(const ReactorGLES&) = delete;
 
   ReactorGLES& operator=(const ReactorGLES&) = delete;

engine/src/flutter/impeller/renderer/backend/gles/test/texture_gles_unittests.cc

@@ -0,0 +1,68 @@
+// 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/impeller/playground/playground_test.h"
+#include "flutter/impeller/renderer/backend/gles/context_gles.h"
+#include "flutter/impeller/renderer/backend/gles/texture_gles.h"
+#include "flutter/testing/testing.h"
+#include "gtest/gtest.h"
+#include "impeller/core/formats.h"
+#include "impeller/core/texture_descriptor.h"
+#include "impeller/renderer/backend/gles/handle_gles.h"
+#include "impeller/renderer/backend/gles/proc_table_gles.h"
+
+namespace impeller::testing {
+
+using TextureGLESTest = PlaygroundTest;
+INSTANTIATE_OPENGLES_PLAYGROUND_SUITE(TextureGLESTest);
+
+TEST_P(TextureGLESTest, CanSetSyncFence) {
+  ContextGLES& context_gles = ContextGLES::Cast(*GetContext());
+  if (!context_gles.GetReactor()
+           ->GetProcTable()
+           .GetDescription()
+           ->GetGlVersion()
+           .IsAtLeast(Version{3, 0, 0})) {
+    GTEST_SKIP() << "GL Version too low to test sync fence.";
+  }
+
+  TextureDescriptor desc;
+  desc.storage_mode = StorageMode::kDevicePrivate;
+  desc.size = {100, 100};
+  desc.format = PixelFormat::kR8G8B8A8UNormInt;
+
+  auto texture = GetContext()->GetResourceAllocator()->CreateTexture(desc);
+  ASSERT_TRUE(!!texture);
+
+  EXPECT_TRUE(GetContext()->AddTrackingFence(texture));
+  EXPECT_TRUE(context_gles.GetReactor()->React());
+
+  std::optional<HandleGLES> sync_fence =
+      TextureGLES::Cast(*texture).GetSyncFence();
+  ASSERT_TRUE(sync_fence.has_value());
+  if (!sync_fence.has_value()) {
+    return;
+  }
+  EXPECT_EQ(sync_fence.value().type, HandleType::kFence);
+
+  std::optional<GLsync> sync =
+      context_gles.GetReactor()->GetGLFence(sync_fence.value());
+  ASSERT_TRUE(sync.has_value());
+  if (!sync.has_value()) {
+    return;
+  }
+
+  // Now queue up operation that binds texture to verify that sync fence is
+  // waited and removed.
+
+  EXPECT_TRUE(
+      context_gles.GetReactor()->AddOperation([&](const ReactorGLES& reactor) {
+        return TextureGLES::Cast(*texture).Bind();
+      }));
+
+  sync_fence = TextureGLES::Cast(*texture).GetSyncFence();
+  ASSERT_FALSE(sync_fence.has_value());
+}
+
+}  // namespace impeller::testing

engine/src/flutter/impeller/renderer/backend/gles/texture_gles.cc

@@ -478,6 +478,16 @@ bool TextureGLES::Bind() const {
     return false;
   }
   const auto& gl = reactor_->GetProcTable();
+
+  if (fence_.has_value()) {
+    std::optional<GLsync> fence = reactor_->GetGLFence(fence_.value());
+    if (fence.has_value()) {
+      gl.WaitSync(fence.value(), 0, GL_TIMEOUT_IGNORED);
+    }
+    reactor_->CollectHandle(fence_.value());
+    fence_ = std::nullopt;
+  }
+
   switch (type_) {
     case Type::kTexture:
     case Type::kTextureMultisampled: {
@@ -625,4 +635,14 @@ std::optional<GLuint> TextureGLES::GetFBO() const {
   return wrapped_fbo_;
 }
 
+void TextureGLES::SetFence(HandleGLES fence) {
+  FML_DCHECK(!fence_.has_value());
+  fence_ = fence;
+}
+
+// Visible for testing.
+std::optional<HandleGLES> TextureGLES::GetSyncFence() const {
+  return fence_;
+}
+
 }  // namespace impeller

engine/src/flutter/impeller/renderer/backend/gles/texture_gles.h

@@ -7,6 +7,7 @@
 
 #include <bitset>
 
+#include "fml/logging.h"
 #include "impeller/base/backend_cast.h"
 #include "impeller/core/texture.h"
 #include "impeller/renderer/backend/gles/handle_gles.h"
@@ -121,10 +122,22 @@ class TextureGLES final : public Texture,
 
   bool IsSliceInitialized(size_t slice) const;
 
+  //----------------------------------------------------------------------------
+  /// @brief      Attach a sync fence to this texture that will be waited on
+  ///             before encoding a rendering operation that references it.
+  ///
+  /// @param[in]  fence  A handle to a sync fence.
+  ///
+  void SetFence(HandleGLES fence);
+
+  // Visible for testing.
+  std::optional<HandleGLES> GetSyncFence() const;
+
  private:
   ReactorGLES::Ref reactor_;
   const Type type_;
   HandleGLES handle_;
+  mutable std::optional<HandleGLES> fence_ = std::nullopt;
   mutable std::bitset<6> slices_initialized_ = 0;
   const bool is_wrapped_;
   const std::optional<GLuint> wrapped_fbo_;

engine/src/flutter/impeller/renderer/backend/metal/command_buffer_mtl.h

@@ -39,6 +39,9 @@ class CommandBufferMTL final : public CommandBuffer {
   // |CommandBuffer|
   void OnWaitUntilCompleted() override;
 
+  // |CommandBuffer|
+  void OnWaitUntilScheduled() override;
+
   // |CommandBuffer|
   std::shared_ptr<RenderPass> OnCreateRenderPass(RenderTarget target) override;
 

engine/src/flutter/impeller/renderer/backend/metal/command_buffer_mtl.mm

@@ -189,6 +189,8 @@ bool CommandBufferMTL::OnSubmitCommands(CompletionCallback callback) {
 
 void CommandBufferMTL::OnWaitUntilCompleted() {}
 
+void CommandBufferMTL::OnWaitUntilScheduled() {}
+
 std::shared_ptr<RenderPass> CommandBufferMTL::OnCreateRenderPass(
     RenderTarget target) {
   if (!buffer_) {

engine/src/flutter/impeller/renderer/backend/vulkan/command_buffer_vk.cc

@@ -49,6 +49,8 @@ bool CommandBufferVK::OnSubmitCommands(CompletionCallback callback) {
 
 void CommandBufferVK::OnWaitUntilCompleted() {}
 
+void CommandBufferVK::OnWaitUntilScheduled() {}
+
 std::shared_ptr<RenderPass> CommandBufferVK::OnCreateRenderPass(
     RenderTarget target) {
   auto context = context_.lock();

engine/src/flutter/impeller/renderer/backend/vulkan/command_buffer_vk.h

@@ -102,6 +102,9 @@ class CommandBufferVK final
   // |CommandBuffer|
   void OnWaitUntilCompleted() override;
 
+  // |CommandBuffer|
+  void OnWaitUntilScheduled() override;
+
   // |CommandBuffer|
   std::shared_ptr<RenderPass> OnCreateRenderPass(RenderTarget target) override;
 

engine/src/flutter/impeller/renderer/command_buffer.cc

@@ -34,6 +34,10 @@ void CommandBuffer::WaitUntilCompleted() {
   return OnWaitUntilCompleted();
 }
 
+void CommandBuffer::WaitUntilScheduled() {
+  return OnWaitUntilScheduled();
+}
+
 std::shared_ptr<RenderPass> CommandBuffer::CreateRenderPass(
     const RenderTarget& render_target) {
   auto pass = OnCreateRenderPass(render_target);

engine/src/flutter/impeller/renderer/command_buffer.h

@@ -66,6 +66,12 @@ class CommandBuffer {
   ///
   void WaitUntilCompleted();
 
+  //----------------------------------------------------------------------------
+  /// @brief      Block the current thread until the GPU has completed
+  ///             scheduling execution of the commands.
+  ///
+  void WaitUntilScheduled();
+
   //----------------------------------------------------------------------------
   /// @brief      Create a render pass to record render commands into.
   ///
@@ -105,6 +111,8 @@ class CommandBuffer {
 
   virtual void OnWaitUntilCompleted() = 0;
 
+  virtual void OnWaitUntilScheduled() = 0;
+
   virtual std::shared_ptr<ComputePass> OnCreateComputePass() = 0;
 
  private:

engine/src/flutter/impeller/renderer/context.cc

@@ -33,4 +33,8 @@ void Context::ResetThreadLocalState() const {
   // Nothing to do.
 }
 
+bool Context::AddTrackingFence(const std::shared_ptr<Texture>& texture) const {
+  return false;
+}
+
 }  // namespace impeller

engine/src/flutter/impeller/renderer/context.h

@@ -222,6 +222,8 @@ class Context {
   /// rendering a 2D workload.
   [[nodiscard]] virtual bool FlushCommandBuffers();
 
+  virtual bool AddTrackingFence(const std::shared_ptr<Texture>& texture) const;
+
   virtual std::shared_ptr<const IdleWaiter> GetIdleWaiter() const;
 
   //----------------------------------------------------------------------------

engine/src/flutter/impeller/renderer/testing/mocks.h

@@ -129,6 +129,7 @@ class MockCommandBuffer : public CommandBuffer {
               (CompletionCallback callback),
               (override));
   MOCK_METHOD(void, OnWaitUntilCompleted, (), (override));
+  MOCK_METHOD(void, OnWaitUntilScheduled, (), (override));
   MOCK_METHOD(std::shared_ptr<ComputePass>,
               OnCreateComputePass,
               (),

engine/src/flutter/lib/ui/painting/image_decoder_impeller.cc

@@ -359,7 +359,6 @@ ImageDecoderImpeller::UnsafeUploadTextureToPrivate(
       resize_desc.usage |= impeller::TextureUsage::kShaderWrite;
       resize_desc.compression_type = impeller::CompressionType::kLossless;
     }
-
     auto resize_texture =
         context->GetResourceAllocator()->CreateTexture(resize_desc);
     if (!resize_texture) {
@@ -386,7 +385,11 @@ ImageDecoderImpeller::UnsafeUploadTextureToPrivate(
 
   // Flush the pending command buffer to ensure that its output becomes visible
   // to the raster thread.
-  command_buffer->WaitUntilCompleted();
+  if (context->AddTrackingFence(result_texture)) {
+    command_buffer->WaitUntilScheduled();
+  } else {
+    command_buffer->WaitUntilCompleted();
+  }
 
   context->DisposeThreadLocalCachedResources();