From 491e166798ae1c15cf53a1d426a49ced68e94d39 Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Thu, 14 Nov 2024 20:43:20 -0800
Subject: [PATCH 1/9] [Impeller] use sync fence for image uploads.

---
 impeller/renderer/backend/gles/context_gles.h | 11 +++
 impeller/renderer/backend/gles/handle_gles.cc |  2 +
 impeller/renderer/backend/gles/handle_gles.h  |  1 +
 .../renderer/backend/gles/proc_table_gles.cc  |  4 +
 .../renderer/backend/gles/proc_table_gles.h   |  7 +-
 .../renderer/backend/gles/reactor_gles.cc     | 73 ++++++++++++++-----
 impeller/renderer/backend/gles/reactor_gles.h | 14 +++-
 .../renderer/backend/gles/texture_gles.cc     | 11 +++
 impeller/renderer/backend/gles/texture_gles.h |  7 ++
 impeller/renderer/context.h                   |  4 +
 lib/ui/painting/image_decoder_impeller.cc     |  3 +-
 11 files changed, 114 insertions(+), 23 deletions(-)

diff --git a/impeller/renderer/backend/gles/context_gles.h b/impeller/renderer/backend/gles/context_gles.h
index 2a0f09aca39b1..971347615fc3b 100644
--- a/impeller/renderer/backend/gles/context_gles.h
+++ b/impeller/renderer/backend/gles/context_gles.h
@@ -9,10 +9,12 @@
 #include "impeller/renderer/backend/gles/allocator_gles.h"
 #include "impeller/renderer/backend/gles/capabilities_gles.h"
 #include "impeller/renderer/backend/gles/gpu_tracer_gles.h"
+#include "impeller/renderer/backend/gles/handle_gles.h"
 #include "impeller/renderer/backend/gles/pipeline_library_gles.h"
 #include "impeller/renderer/backend/gles/reactor_gles.h"
 #include "impeller/renderer/backend/gles/sampler_library_gles.h"
 #include "impeller/renderer/backend/gles/shader_library_gles.h"
+#include "impeller/renderer/backend/gles/texture_gles.h"
 #include "impeller/renderer/capabilities.h"
 #include "impeller/renderer/command_queue.h"
 #include "impeller/renderer/context.h"
@@ -93,6 +95,15 @@ class ContextGLES final : public Context,
   // |Context|
   void Shutdown() override;
 
+  bool AddTrackingFence(const std::shared_ptr<Texture>& texture) override {
+    if (!reactor_->GetProcTable().FenceSync.IsAvailable()) {
+      return true;
+    }
+    auto fence = reactor_->CreateHandle(HandleType::kFence);
+    TextureGLES::Cast(*texture).SetFence(fence);
+    return true;
+  }
+
   ContextGLES(const ContextGLES&) = delete;
 
   ContextGLES& operator=(const ContextGLES&) = delete;
diff --git a/impeller/renderer/backend/gles/handle_gles.cc b/impeller/renderer/backend/gles/handle_gles.cc
index 5862d8cc63859..f1ee08ac9b491 100644
--- a/impeller/renderer/backend/gles/handle_gles.cc
+++ b/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();
 }
diff --git a/impeller/renderer/backend/gles/handle_gles.h b/impeller/renderer/backend/gles/handle_gles.h
index cc52c8ceabf4a..3e7b44f0320b0 100644
--- a/impeller/renderer/backend/gles/handle_gles.h
+++ b/impeller/renderer/backend/gles/handle_gles.h
@@ -22,6 +22,7 @@ enum class HandleType {
   kProgram,
   kRenderBuffer,
   kFrameBuffer,
+  kFence,
 };
 
 std::string HandleTypeToString(HandleType type);
diff --git a/impeller/renderer/backend/gles/proc_table_gles.cc b/impeller/renderer/backend/gles/proc_table_gles.cc
index 3e9d7d70e7b43..b294e46ab0491 100644
--- a/impeller/renderer/backend/gles/proc_table_gles.cc
+++ b/impeller/renderer/backend/gles/proc_table_gles.cc
@@ -326,6 +326,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();
 }
@@ -346,6 +348,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();
 }
diff --git a/impeller/renderer/backend/gles/proc_table_gles.h b/impeller/renderer/backend/gles/proc_table_gles.h
index eadcb4f333057..0ff5f323978f7 100644
--- a/impeller/renderer/backend/gles/proc_table_gles.h
+++ b/impeller/renderer/backend/gles/proc_table_gles.h
@@ -237,7 +237,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);             \
@@ -261,6 +265,7 @@ enum class DebugResourceType {
   kShader,
   kRenderBuffer,
   kFrameBuffer,
+  kFence,
 };
 
 class ProcTableGLES {
diff --git a/impeller/renderer/backend/gles/reactor_gles.cc b/impeller/renderer/backend/gles/reactor_gles.cc
index 4ccbb47891b38..58a47242e1011 100644
--- a/impeller/renderer/backend/gles/reactor_gles.cc
+++ b/impeller/renderer/backend/gles/reactor_gles.cc
@@ -6,58 +6,66 @@
 
 #include <algorithm>
 
+#include "GLES3/gl3.h"
 #include "flutter/fml/trace_event.h"
 #include "fml/closure.h"
 #include "fml/logging.h"
 #include "impeller/base/validation.h"
+#include "impeller/renderer/backend/gles/handle_gles.h"
 
 namespace impeller {
 
-static std::optional<GLuint> CreateGLHandle(const ProcTableGLES& gl,
-                                            HandleType type) {
-  GLuint handle = GL_NONE;
+static std::optional<GLHandle> CreateGLHandle(const ProcTableGLES& gl,
+                                              HandleType type) {
+  GLHandle handle = GLHandle{.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 GLHandle{.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 GLHandle{.sync = gl.FenceSync(GL_SYNC_GPU_COMMANDS_COMPLETE, 0)};
+      break;
   }
   return std::nullopt;
 }
 
 static bool CollectGLHandle(const ProcTableGLES& gl,
                             HandleType type,
-                            GLuint handle) {
+                            GLHandle 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;
 }
@@ -115,6 +123,30 @@ const ProcTableGLES& ReactorGLES::GetProcTable() const {
 }
 
 std::optional<GLuint> ReactorGLES::GetGLHandle(const HandleGLES& handle) const {
+  if (handle.type == HandleType::kFence) {
+    return std::nullopt;
+  }
+  ReaderLock handles_lock(handles_mutex_);
+  if (auto found = handles_.find(handle); found != handles_.end()) {
+    if (found->second.pending_collection) {
+      VALIDATION_LOG
+          << "Attempted to acquire a handle that was pending collection.";
+      return std::nullopt;
+    }
+    if (!found->second.name.has_value()) {
+      VALIDATION_LOG << "Attempt to acquire a handle outside of an operation.";
+      return std::nullopt;
+    }
+    return found->second.name->handle;
+  }
+  VALIDATION_LOG << "Attempted to acquire an invalid GL handle.";
+  return std::nullopt;
+}
+
+std::optional<GLsync> ReactorGLES::GetGLFence(const HandleGLES& handle) const {
+  if (handle.type != HandleType::kFence) {
+    return std::nullopt;
+  }
   ReaderLock handles_lock(handles_mutex_);
   if (auto found = handles_.find(handle); found != handles_.end()) {
     if (found->second.pending_collection) {
@@ -126,7 +158,7 @@ std::optional<GLuint> ReactorGLES::GetGLHandle(const HandleGLES& handle) const {
       VALIDATION_LOG << "Attempt to acquire a handle outside of an operation.";
       return std::nullopt;
     }
-    return found->second.name;
+    return found->second.name->sync;
   }
   VALIDATION_LOG << "Attempted to acquire an invalid GL handle.";
   return std::nullopt;
@@ -168,9 +200,9 @@ HandleGLES ReactorGLES::CreateHandle(HandleType type, GLuint external_handle) {
   }
   WriterLock handles_lock(handles_mutex_);
 
-  std::optional<GLuint> gl_handle;
+  std::optional<GLHandle> gl_handle;
   if (external_handle != GL_NONE) {
-    gl_handle = external_handle;
+    gl_handle = GLHandle{.handle = external_handle};
   } else if (CanReactOnCurrentThread()) {
     gl_handle = CreateGLHandle(GetProcTable(), type);
   }
@@ -216,6 +248,8 @@ static DebugResourceType ToDebugResourceType(HandleType type) {
       return DebugResourceType::kRenderBuffer;
     case HandleType::kFrameBuffer:
       return DebugResourceType::kFrameBuffer;
+    case HandleType::kFence:
+      return DebugResourceType::kFence;
   }
   FML_UNREACHABLE();
 }
@@ -254,9 +288,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;
       }
diff --git a/impeller/renderer/backend/gles/reactor_gles.h b/impeller/renderer/backend/gles/reactor_gles.h
index 80fb6877365bb..be9e3d68d9ad9 100644
--- a/impeller/renderer/backend/gles/reactor_gles.h
+++ b/impeller/renderer/backend/gles/reactor_gles.h
@@ -7,6 +7,7 @@
 
 #include <functional>
 #include <memory>
+#include <unordered_map>
 #include <vector>
 
 #include "fml/closure.h"
@@ -16,6 +17,13 @@
 
 namespace impeller {
 
+struct GLHandle {
+  union {
+    GLuint handle;
+    GLsync sync;
+  };
+};
+
 //------------------------------------------------------------------------------
 /// @brief      The reactor attempts to make thread-safe usage of OpenGL ES
 ///             easier to reason about.
@@ -158,6 +166,8 @@ class ReactorGLES {
   ///
   std::optional<GLuint> GetGLHandle(const HandleGLES& handle) const;
 
+  std::optional<GLsync> GetGLFence(const HandleGLES& handle) const;
+
   //----------------------------------------------------------------------------
   /// @brief      Create a reactor handle.
   ///
@@ -246,14 +256,14 @@ class ReactorGLES {
 
  private:
   struct LiveHandle {
-    std::optional<GLuint> name;
+    std::optional<GLHandle> 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<GLHandle> p_name) : name(p_name) {}
 
     constexpr bool IsLive() const { return name.has_value(); }
   };
diff --git a/impeller/renderer/backend/gles/texture_gles.cc b/impeller/renderer/backend/gles/texture_gles.cc
index 1902fd22ae7c7..56f4eb9b2c7ed 100644
--- a/impeller/renderer/backend/gles/texture_gles.cc
+++ b/impeller/renderer/backend/gles/texture_gles.cc
@@ -7,6 +7,7 @@
 #include <optional>
 #include <utility>
 
+#include "GLES3/gl3.h"
 #include "flutter/fml/logging.h"
 #include "flutter/fml/mapping.h"
 #include "flutter/fml/trace_event.h"
@@ -478,6 +479,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: {
diff --git a/impeller/renderer/backend/gles/texture_gles.h b/impeller/renderer/backend/gles/texture_gles.h
index 2f5a9d176104e..2ffc550bf9e47 100644
--- a/impeller/renderer/backend/gles/texture_gles.h
+++ b/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,16 @@ class TextureGLES final : public Texture,
 
   bool IsSliceInitialized(size_t slice) const;
 
+  void SetFence(HandleGLES fence) {
+    FML_DCHECK(!fence_.has_value());
+    fence_ = fence;
+  }
+
  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_;
diff --git a/impeller/renderer/context.h b/impeller/renderer/context.h
index 657083c04bf87..2907220fb6614 100644
--- a/impeller/renderer/context.h
+++ b/impeller/renderer/context.h
@@ -222,6 +222,10 @@ class Context {
   /// rendering a 2D workload.
   [[nodiscard]] virtual bool FlushCommandBuffers();
 
+  virtual bool AddTrackingFence(const std::shared_ptr<Texture>& texture) {
+    return true;
+  }
+
   virtual std::shared_ptr<const IdleWaiter> GetIdleWaiter() const {
     return nullptr;
   }
diff --git a/lib/ui/painting/image_decoder_impeller.cc b/lib/ui/painting/image_decoder_impeller.cc
index 1355653d59a7e..67dad1fbb39f8 100644
--- a/lib/ui/painting/image_decoder_impeller.cc
+++ b/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) {
@@ -378,6 +377,8 @@ ImageDecoderImpeller::UnsafeUploadTextureToPrivate(
   }
   blit_pass->EncodeCommands(context->GetResourceAllocator());
 
+  context->AddTrackingFence(result_texture);
+
   if (!context->GetCommandQueue()->Submit({command_buffer}).ok()) {
     std::string decode_error("Failed to submit image decoding command buffer.");
     FML_DLOG(ERROR) << decode_error;

From 75e37d6d6e956102c9b4a80378a2efd4c51c1c4e Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Fri, 15 Nov 2024 09:18:14 -0800
Subject: [PATCH 2/9] ++

---
 impeller/renderer/backend/gles/capabilities_gles.cc   |  1 +
 impeller/renderer/backend/gles/command_buffer_gles.cc |  5 +++++
 impeller/renderer/backend/gles/command_buffer_gles.h  |  3 +++
 impeller/renderer/backend/gles/context_gles.cc        | 11 +++++++++++
 impeller/renderer/backend/gles/context_gles.h         |  9 +--------
 impeller/renderer/backend/metal/command_buffer_mtl.h  |  3 +++
 impeller/renderer/backend/metal/command_buffer_mtl.mm |  2 ++
 impeller/renderer/backend/vulkan/command_buffer_vk.cc |  2 ++
 impeller/renderer/backend/vulkan/command_buffer_vk.h  |  3 +++
 impeller/renderer/command_buffer.cc                   |  4 ++++
 impeller/renderer/command_buffer.h                    |  9 +++++++++
 impeller/renderer/context.cc                          |  4 ++++
 impeller/renderer/context.h                           |  4 +---
 impeller/renderer/testing/mocks.h                     |  1 +
 lib/ui/painting/image_decoder_impeller.cc             |  8 ++++++--
 15 files changed, 56 insertions(+), 13 deletions(-)

diff --git a/impeller/renderer/backend/gles/capabilities_gles.cc b/impeller/renderer/backend/gles/capabilities_gles.cc
index 18a5e1accb9e6..195206aa33eee 100644
--- a/impeller/renderer/backend/gles/capabilities_gles.cc
+++ b/impeller/renderer/backend/gles/capabilities_gles.cc
@@ -4,6 +4,7 @@
 
 #include "impeller/renderer/backend/gles/capabilities_gles.h"
 
+#include "impeller/base/version.h"
 #include "impeller/core/formats.h"
 #include "impeller/renderer/backend/gles/proc_table_gles.h"
 
diff --git a/impeller/renderer/backend/gles/command_buffer_gles.cc b/impeller/renderer/backend/gles/command_buffer_gles.cc
index 2ade0db37f821..4e0831f9e3a10 100644
--- a/impeller/renderer/backend/gles/command_buffer_gles.cc
+++ b/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) {
diff --git a/impeller/renderer/backend/gles/command_buffer_gles.h b/impeller/renderer/backend/gles/command_buffer_gles.h
index a1536d9433271..09570b19a955b 100644
--- a/impeller/renderer/backend/gles/command_buffer_gles.h
+++ b/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;
 
diff --git a/impeller/renderer/backend/gles/context_gles.cc b/impeller/renderer/backend/gles/context_gles.cc
index c4070a5393cce..c24a98fb9fb93 100644
--- a/impeller/renderer/backend/gles/context_gles.cc
+++ b/impeller/renderer/backend/gles/context_gles.cc
@@ -157,4 +157,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
diff --git a/impeller/renderer/backend/gles/context_gles.h b/impeller/renderer/backend/gles/context_gles.h
index 39c751dbe0e57..b8b3362e37f54 100644
--- a/impeller/renderer/backend/gles/context_gles.h
+++ b/impeller/renderer/backend/gles/context_gles.h
@@ -96,14 +96,7 @@ class ContextGLES final : public Context,
   void Shutdown() override;
 
   // |Context|
-  bool AddTrackingFence(const std::shared_ptr<Texture>& texture) override {
-    if (!reactor_->GetProcTable().FenceSync.IsAvailable()) {
-      return true;
-    }
-    auto fence = reactor_->CreateHandle(HandleType::kFence);
-    TextureGLES::Cast(*texture).SetFence(fence);
-    return true;
-  }
+  bool AddTrackingFence(const std::shared_ptr<Texture>& texture) const override;
 
   // |Context|
   void ResetThreadLocalState() const override;
diff --git a/impeller/renderer/backend/metal/command_buffer_mtl.h b/impeller/renderer/backend/metal/command_buffer_mtl.h
index c644ea114f457..ed8247c6d846e 100644
--- a/impeller/renderer/backend/metal/command_buffer_mtl.h
+++ b/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;
 
diff --git a/impeller/renderer/backend/metal/command_buffer_mtl.mm b/impeller/renderer/backend/metal/command_buffer_mtl.mm
index f6b907da54eff..6eb9f2ea050ee 100644
--- a/impeller/renderer/backend/metal/command_buffer_mtl.mm
+++ b/impeller/renderer/backend/metal/command_buffer_mtl.mm
@@ -189,6 +189,8 @@ static bool LogMTLCommandBufferErrorIfPresent(id<MTLCommandBuffer> buffer) {
 
 void CommandBufferMTL::OnWaitUntilCompleted() {}
 
+void CommandBufferMTL::OnWaitUntilScheduled() {}
+
 std::shared_ptr<RenderPass> CommandBufferMTL::OnCreateRenderPass(
     RenderTarget target) {
   if (!buffer_) {
diff --git a/impeller/renderer/backend/vulkan/command_buffer_vk.cc b/impeller/renderer/backend/vulkan/command_buffer_vk.cc
index cbce517d435e9..780b423b54a41 100644
--- a/impeller/renderer/backend/vulkan/command_buffer_vk.cc
+++ b/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();
diff --git a/impeller/renderer/backend/vulkan/command_buffer_vk.h b/impeller/renderer/backend/vulkan/command_buffer_vk.h
index e4ac9138859b1..47ded4867d47a 100644
--- a/impeller/renderer/backend/vulkan/command_buffer_vk.h
+++ b/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;
 
diff --git a/impeller/renderer/command_buffer.cc b/impeller/renderer/command_buffer.cc
index 918e590da2beb..12a79eacaa5fd 100644
--- a/impeller/renderer/command_buffer.cc
+++ b/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);
diff --git a/impeller/renderer/command_buffer.h b/impeller/renderer/command_buffer.h
index cbfd406895c5f..604368106d117 100644
--- a/impeller/renderer/command_buffer.h
+++ b/impeller/renderer/command_buffer.h
@@ -66,6 +66,13 @@ 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 +112,8 @@ class CommandBuffer {
 
   virtual void OnWaitUntilCompleted() = 0;
 
+  virtual void OnWaitUntilScheduled() = 0;
+
   virtual std::shared_ptr<ComputePass> OnCreateComputePass() = 0;
 
  private:
diff --git a/impeller/renderer/context.cc b/impeller/renderer/context.cc
index e563ac7a2e345..b7922caf213e9 100644
--- a/impeller/renderer/context.cc
+++ b/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
diff --git a/impeller/renderer/context.h b/impeller/renderer/context.h
index dab4dfc4512df..62254504b9fa9 100644
--- a/impeller/renderer/context.h
+++ b/impeller/renderer/context.h
@@ -222,9 +222,7 @@ class Context {
   /// rendering a 2D workload.
   [[nodiscard]] virtual bool FlushCommandBuffers();
 
-  virtual bool AddTrackingFence(const std::shared_ptr<Texture>& texture) {
-    return true;
-  }
+  virtual bool AddTrackingFence(const std::shared_ptr<Texture>& texture) const;
 
   virtual std::shared_ptr<const IdleWaiter> GetIdleWaiter() const;
 
diff --git a/impeller/renderer/testing/mocks.h b/impeller/renderer/testing/mocks.h
index 99464fa9e7597..c020daacfe4d2 100644
--- a/impeller/renderer/testing/mocks.h
+++ b/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,
               (),
diff --git a/lib/ui/painting/image_decoder_impeller.cc b/lib/ui/painting/image_decoder_impeller.cc
index 49e0287c9e896..5aa0d6e66f7b7 100644
--- a/lib/ui/painting/image_decoder_impeller.cc
+++ b/lib/ui/painting/image_decoder_impeller.cc
@@ -377,7 +377,7 @@ ImageDecoderImpeller::UnsafeUploadTextureToPrivate(
   }
   blit_pass->EncodeCommands(context->GetResourceAllocator());
 
-  context->AddTrackingFence(result_texture);
+  bool did_add_fence = context->AddTrackingFence(result_texture);
 
   if (!context->GetCommandQueue()->Submit({command_buffer}).ok()) {
     std::string decode_error("Failed to submit image decoding command buffer.");
@@ -387,7 +387,11 @@ ImageDecoderImpeller::UnsafeUploadTextureToPrivate(
 
   // Flush the pending command buffer to ensure that its output becomes visible
   // to the raster thread.
-  command_buffer->WaitUntilCompleted();
+  if (did_add_fence) {
+    command_buffer->WaitUntilScheduled();
+  } else {
+    command_buffer->WaitUntilCompleted();
+  }
 
   context->DisposeThreadLocalCachedResources();
 

From c67779a4ea9a120b6246ef441f6dd8ef37d1e969 Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Fri, 15 Nov 2024 09:26:45 -0800
Subject: [PATCH 3/9] fix importss.

---
 impeller/renderer/backend/gles/context_gles.cc |  2 ++
 impeller/renderer/backend/gles/context_gles.h  |  2 --
 impeller/renderer/backend/gles/reactor_gles.cc |  2 --
 impeller/renderer/backend/gles/reactor_gles.h  |  2 +-
 impeller/renderer/backend/gles/texture_gles.cc |  6 +++++-
 impeller/renderer/backend/gles/texture_gles.h  | 11 +++++++----
 impeller/renderer/command_buffer.h             |  3 +--
 7 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/impeller/renderer/backend/gles/context_gles.cc b/impeller/renderer/backend/gles/context_gles.cc
index c24a98fb9fb93..640f36e3ce550 100644
--- a/impeller/renderer/backend/gles/context_gles.cc
+++ b/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 {
diff --git a/impeller/renderer/backend/gles/context_gles.h b/impeller/renderer/backend/gles/context_gles.h
index b8b3362e37f54..138494429bd18 100644
--- a/impeller/renderer/backend/gles/context_gles.h
+++ b/impeller/renderer/backend/gles/context_gles.h
@@ -9,12 +9,10 @@
 #include "impeller/renderer/backend/gles/allocator_gles.h"
 #include "impeller/renderer/backend/gles/capabilities_gles.h"
 #include "impeller/renderer/backend/gles/gpu_tracer_gles.h"
-#include "impeller/renderer/backend/gles/handle_gles.h"
 #include "impeller/renderer/backend/gles/pipeline_library_gles.h"
 #include "impeller/renderer/backend/gles/reactor_gles.h"
 #include "impeller/renderer/backend/gles/sampler_library_gles.h"
 #include "impeller/renderer/backend/gles/shader_library_gles.h"
-#include "impeller/renderer/backend/gles/texture_gles.h"
 #include "impeller/renderer/capabilities.h"
 #include "impeller/renderer/command_queue.h"
 #include "impeller/renderer/context.h"
diff --git a/impeller/renderer/backend/gles/reactor_gles.cc b/impeller/renderer/backend/gles/reactor_gles.cc
index f5096d36a8e6d..9e1a54c3730f6 100644
--- a/impeller/renderer/backend/gles/reactor_gles.cc
+++ b/impeller/renderer/backend/gles/reactor_gles.cc
@@ -6,12 +6,10 @@
 
 #include <algorithm>
 
-#include "GLES3/gl3.h"
 #include "flutter/fml/trace_event.h"
 #include "fml/closure.h"
 #include "fml/logging.h"
 #include "impeller/base/validation.h"
-#include "impeller/renderer/backend/gles/handle_gles.h"
 
 namespace impeller {
 
diff --git a/impeller/renderer/backend/gles/reactor_gles.h b/impeller/renderer/backend/gles/reactor_gles.h
index 929330cc2f580..9bda419e535e7 100644
--- a/impeller/renderer/backend/gles/reactor_gles.h
+++ b/impeller/renderer/backend/gles/reactor_gles.h
@@ -7,7 +7,6 @@
 
 #include <functional>
 #include <memory>
-#include <unordered_map>
 #include <vector>
 
 #include "fml/closure.h"
@@ -17,6 +16,7 @@
 
 namespace impeller {
 
+/// @brief Storage for either a GL handle or sync fence.
 struct GLHandle {
   union {
     GLuint handle;
diff --git a/impeller/renderer/backend/gles/texture_gles.cc b/impeller/renderer/backend/gles/texture_gles.cc
index 56f4eb9b2c7ed..53746e9578419 100644
--- a/impeller/renderer/backend/gles/texture_gles.cc
+++ b/impeller/renderer/backend/gles/texture_gles.cc
@@ -7,7 +7,6 @@
 #include <optional>
 #include <utility>
 
-#include "GLES3/gl3.h"
 #include "flutter/fml/logging.h"
 #include "flutter/fml/mapping.h"
 #include "flutter/fml/trace_event.h"
@@ -636,4 +635,9 @@ std::optional<GLuint> TextureGLES::GetFBO() const {
   return wrapped_fbo_;
 }
 
+void TextureGLES::SetFence(HandleGLES fence) {
+  FML_DCHECK(!fence_.has_value());
+  fence_ = fence;
+}
+
 }  // namespace impeller
diff --git a/impeller/renderer/backend/gles/texture_gles.h b/impeller/renderer/backend/gles/texture_gles.h
index 2ffc550bf9e47..a80f3bbcc3a08 100644
--- a/impeller/renderer/backend/gles/texture_gles.h
+++ b/impeller/renderer/backend/gles/texture_gles.h
@@ -122,10 +122,13 @@ class TextureGLES final : public Texture,
 
   bool IsSliceInitialized(size_t slice) const;
 
-  void SetFence(HandleGLES fence) {
-    FML_DCHECK(!fence_.has_value());
-    fence_ = fence;
-  }
+  //----------------------------------------------------------------------------
+  /// @brief      Attach a sync fence to this texture that will be waited on
+  ///             before encoding a rendering operatio that references it.
+  ///
+  /// @param[in]  fence  A handle to a sync fence.
+  ///
+  void SetFence(HandleGLES fence);
 
  private:
   ReactorGLES::Ref reactor_;
diff --git a/impeller/renderer/command_buffer.h b/impeller/renderer/command_buffer.h
index 604368106d117..578bf003ac845 100644
--- a/impeller/renderer/command_buffer.h
+++ b/impeller/renderer/command_buffer.h
@@ -68,8 +68,7 @@ class CommandBuffer {
 
   //----------------------------------------------------------------------------
   /// @brief      Block the current thread until the GPU has completed
-  /// scheduling
-  ///             execution of the commands.
+  ///             scheduling execution of the commands.
   ///
   void WaitUntilScheduled();
 

From 6b199eaf510305b65fea65ec2345a86a6f71da35 Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Fri, 15 Nov 2024 09:31:26 -0800
Subject: [PATCH 4/9] remove one more include.

---
 impeller/renderer/backend/gles/capabilities_gles.cc | 1 -
 1 file changed, 1 deletion(-)

diff --git a/impeller/renderer/backend/gles/capabilities_gles.cc b/impeller/renderer/backend/gles/capabilities_gles.cc
index 195206aa33eee..18a5e1accb9e6 100644
--- a/impeller/renderer/backend/gles/capabilities_gles.cc
+++ b/impeller/renderer/backend/gles/capabilities_gles.cc
@@ -4,7 +4,6 @@
 
 #include "impeller/renderer/backend/gles/capabilities_gles.h"
 
-#include "impeller/base/version.h"
 #include "impeller/core/formats.h"
 #include "impeller/renderer/backend/gles/proc_table_gles.h"
 

From 6232416846d9f9eac73b2df8076aa06c152f74e3 Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Fri, 15 Nov 2024 09:52:21 -0800
Subject: [PATCH 5/9] add unit test.

---
 impeller/renderer/backend/gles/BUILD.gn       |  1 +
 .../gles/test/texture_gles_unittests.cc       | 67 +++++++++++++++++++
 .../renderer/backend/gles/texture_gles.cc     |  5 ++
 impeller/renderer/backend/gles/texture_gles.h |  3 +
 4 files changed, 76 insertions(+)
 create mode 100644 impeller/renderer/backend/gles/test/texture_gles_unittests.cc

diff --git a/impeller/renderer/backend/gles/BUILD.gn b/impeller/renderer/backend/gles/BUILD.gn
index b3e54aefae8b7..bede7318a32a0 100644
--- a/impeller/renderer/backend/gles/BUILD.gn
+++ b/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",
diff --git a/impeller/renderer/backend/gles/test/texture_gles_unittests.cc b/impeller/renderer/backend/gles/test/texture_gles_unittests.cc
new file mode 100644
index 0000000000000..2008f90d7e2fa
--- /dev/null
+++ b/impeller/renderer/backend/gles/test/texture_gles_unittests.cc
@@ -0,0 +1,67 @@
+// 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};
+
+  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
diff --git a/impeller/renderer/backend/gles/texture_gles.cc b/impeller/renderer/backend/gles/texture_gles.cc
index 53746e9578419..1b4b47f74b373 100644
--- a/impeller/renderer/backend/gles/texture_gles.cc
+++ b/impeller/renderer/backend/gles/texture_gles.cc
@@ -640,4 +640,9 @@ void TextureGLES::SetFence(HandleGLES fence) {
   fence_ = fence;
 }
 
+// Visible for testing.
+std::optional<HandleGLES> TextureGLES::GetSyncFence() const {
+  return fence_;
+}
+
 }  // namespace impeller
diff --git a/impeller/renderer/backend/gles/texture_gles.h b/impeller/renderer/backend/gles/texture_gles.h
index a80f3bbcc3a08..55b2784ea17c1 100644
--- a/impeller/renderer/backend/gles/texture_gles.h
+++ b/impeller/renderer/backend/gles/texture_gles.h
@@ -130,6 +130,9 @@ class TextureGLES final : public Texture,
   ///
   void SetFence(HandleGLES fence);
 
+  // Visible for testing.
+  std::optional<HandleGLES> GetSyncFence() const;
+
  private:
   ReactorGLES::Ref reactor_;
   const Type type_;

From b6bf09e23d270979c04ee307a38595f74ca5a8e3 Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Fri, 15 Nov 2024 10:17:34 -0800
Subject: [PATCH 6/9] fix texture format.

---
 impeller/renderer/backend/gles/test/texture_gles_unittests.cc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/impeller/renderer/backend/gles/test/texture_gles_unittests.cc b/impeller/renderer/backend/gles/test/texture_gles_unittests.cc
index 2008f90d7e2fa..ceafa3a2197e3 100644
--- a/impeller/renderer/backend/gles/test/texture_gles_unittests.cc
+++ b/impeller/renderer/backend/gles/test/texture_gles_unittests.cc
@@ -30,6 +30,7 @@ TEST_P(TextureGLESTest, CanSetSyncFence) {
   TextureDescriptor desc;
   desc.storage_mode = StorageMode::kDevicePrivate;
   desc.size = {100, 100};
+  desc.format = PixelFormat::kR8G8B8A8UNormInt;
 
   auto texture = GetContext()->GetResourceAllocator()->CreateTexture(desc);
   ASSERT_TRUE(!!texture);

From 1d29ad2e65f2ed443e8c1af51741507be393e23e Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Fri, 15 Nov 2024 11:17:46 -0800
Subject: [PATCH 7/9] clang tidy.

---
 impeller/renderer/backend/gles/reactor_gles.cc | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/impeller/renderer/backend/gles/reactor_gles.cc b/impeller/renderer/backend/gles/reactor_gles.cc
index 9e1a54c3730f6..496443b7d127d 100644
--- a/impeller/renderer/backend/gles/reactor_gles.cc
+++ b/impeller/renderer/backend/gles/reactor_gles.cc
@@ -133,11 +133,12 @@ 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<GLHandle> 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->handle;
+    return name->handle;
   }
   VALIDATION_LOG << "Attempted to acquire an invalid GL handle.";
   return std::nullopt;
@@ -154,11 +155,12 @@ std::optional<GLsync> ReactorGLES::GetGLFence(const HandleGLES& handle) const {
           << "Attempted to acquire a handle that was pending collection.";
       return std::nullopt;
     }
-    if (!found->second.name.has_value()) {
+    std::optional<GLHandle> 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->sync;
+    return name->sync;
   }
   VALIDATION_LOG << "Attempted to acquire an invalid GL handle.";
   return std::nullopt;

From e0d15ad952d218729247e7ebc46985e35f7301e8 Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Fri, 15 Nov 2024 12:58:58 -0800
Subject: [PATCH 8/9] jason review.

---
 .../renderer/backend/gles/reactor_gles.cc     | 36 +++++++++----------
 impeller/renderer/backend/gles/reactor_gles.h |  2 ++
 impeller/renderer/backend/gles/texture_gles.h |  2 +-
 lib/ui/painting/image_decoder_impeller.cc     |  4 +--
 4 files changed, 20 insertions(+), 24 deletions(-)

diff --git a/impeller/renderer/backend/gles/reactor_gles.cc b/impeller/renderer/backend/gles/reactor_gles.cc
index 496443b7d127d..07bb5b116f99b 100644
--- a/impeller/renderer/backend/gles/reactor_gles.cc
+++ b/impeller/renderer/backend/gles/reactor_gles.cc
@@ -35,7 +35,6 @@ static std::optional<GLHandle> CreateGLHandle(const ProcTableGLES& gl,
       return handle;
     case HandleType::kFence:
       return GLHandle{.sync = gl.FenceSync(GL_SYNC_GPU_COMMANDS_COMPLETE, 0)};
-      break;
   }
   return std::nullopt;
 }
@@ -122,10 +121,7 @@ const ProcTableGLES& ReactorGLES::GetProcTable() const {
   return *proc_table_;
 }
 
-std::optional<GLuint> ReactorGLES::GetGLHandle(const HandleGLES& handle) const {
-  if (handle.type == HandleType::kFence) {
-    return std::nullopt;
-  }
+std::optional<GLHandle> 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) {
@@ -138,31 +134,31 @@ std::optional<GLuint> ReactorGLES::GetGLHandle(const HandleGLES& handle) const {
       VALIDATION_LOG << "Attempt to acquire a handle outside of an operation.";
       return std::nullopt;
     }
-    return name->handle;
+    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<GLHandle> 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;
   }
-  ReaderLock handles_lock(handles_mutex_);
-  if (auto found = handles_.find(handle); found != handles_.end()) {
-    if (found->second.pending_collection) {
-      VALIDATION_LOG
-          << "Attempted to acquire a handle that was pending collection.";
-      return std::nullopt;
-    }
-    std::optional<GLHandle> name = found->second.name;
-    if (!name.has_value()) {
-      VALIDATION_LOG << "Attempt to acquire a handle outside of an operation.";
-      return std::nullopt;
-    }
-    return name->sync;
+  std::optional<GLHandle> gl_handle = GetHandle(handle);
+  if (gl_handle.has_value()) {
+    return gl_handle->sync;
   }
-  VALIDATION_LOG << "Attempted to acquire an invalid GL handle.";
   return std::nullopt;
 }
 
diff --git a/impeller/renderer/backend/gles/reactor_gles.h b/impeller/renderer/backend/gles/reactor_gles.h
index 9bda419e535e7..e9cb92a3073fc 100644
--- a/impeller/renderer/backend/gles/reactor_gles.h
+++ b/impeller/renderer/backend/gles/reactor_gles.h
@@ -302,6 +302,8 @@ class ReactorGLES {
 
   void SetupDebugGroups();
 
+  std::optional<GLHandle> GetHandle(const HandleGLES& handle) const;
+
   ReactorGLES(const ReactorGLES&) = delete;
 
   ReactorGLES& operator=(const ReactorGLES&) = delete;
diff --git a/impeller/renderer/backend/gles/texture_gles.h b/impeller/renderer/backend/gles/texture_gles.h
index 55b2784ea17c1..f5fbed558ea1f 100644
--- a/impeller/renderer/backend/gles/texture_gles.h
+++ b/impeller/renderer/backend/gles/texture_gles.h
@@ -124,7 +124,7 @@ class TextureGLES final : public Texture,
 
   //----------------------------------------------------------------------------
   /// @brief      Attach a sync fence to this texture that will be waited on
-  ///             before encoding a rendering operatio that references it.
+  ///             before encoding a rendering operation that references it.
   ///
   /// @param[in]  fence  A handle to a sync fence.
   ///
diff --git a/lib/ui/painting/image_decoder_impeller.cc b/lib/ui/painting/image_decoder_impeller.cc
index 5aa0d6e66f7b7..f57e21dfc9642 100644
--- a/lib/ui/painting/image_decoder_impeller.cc
+++ b/lib/ui/painting/image_decoder_impeller.cc
@@ -377,8 +377,6 @@ ImageDecoderImpeller::UnsafeUploadTextureToPrivate(
   }
   blit_pass->EncodeCommands(context->GetResourceAllocator());
 
-  bool did_add_fence = context->AddTrackingFence(result_texture);
-
   if (!context->GetCommandQueue()->Submit({command_buffer}).ok()) {
     std::string decode_error("Failed to submit image decoding command buffer.");
     FML_DLOG(ERROR) << decode_error;
@@ -387,7 +385,7 @@ ImageDecoderImpeller::UnsafeUploadTextureToPrivate(
 
   // Flush the pending command buffer to ensure that its output becomes visible
   // to the raster thread.
-  if (did_add_fence) {
+  if (context->AddTrackingFence(result_texture)) {
     command_buffer->WaitUntilScheduled();
   } else {
     command_buffer->WaitUntilCompleted();

From 7ef2efd56a20bd767d2baff875cbc1cf7a85f659 Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Fri, 15 Nov 2024 13:24:04 -0800
Subject: [PATCH 9/9] rename to GLStorage, make private.

---
 .../renderer/backend/gles/reactor_gles.cc     | 32 +++++++++++--------
 impeller/renderer/backend/gles/reactor_gles.h | 29 ++++++++++-------
 2 files changed, 36 insertions(+), 25 deletions(-)

diff --git a/impeller/renderer/backend/gles/reactor_gles.cc b/impeller/renderer/backend/gles/reactor_gles.cc
index 07bb5b116f99b..945e2205febc6 100644
--- a/impeller/renderer/backend/gles/reactor_gles.cc
+++ b/impeller/renderer/backend/gles/reactor_gles.cc
@@ -13,9 +13,11 @@
 
 namespace impeller {
 
-static std::optional<GLHandle> CreateGLHandle(const ProcTableGLES& gl,
-                                              HandleType type) {
-  GLHandle handle = GLHandle{.handle = GL_NONE};
+// static
+std::optional<ReactorGLES::GLStorage> ReactorGLES::CreateGLHandle(
+    const ProcTableGLES& gl,
+    HandleType type) {
+  GLStorage handle = GLStorage{.handle = GL_NONE};
   switch (type) {
     case HandleType::kUnknown:
       return std::nullopt;
@@ -26,7 +28,7 @@ static std::optional<GLHandle> CreateGLHandle(const ProcTableGLES& gl,
       gl.GenBuffers(1u, &handle.handle);
       return handle;
     case HandleType::kProgram:
-      return GLHandle{.handle = gl.CreateProgram()};
+      return GLStorage{.handle = gl.CreateProgram()};
     case HandleType::kRenderBuffer:
       gl.GenRenderbuffers(1u, &handle.handle);
       return handle;
@@ -34,14 +36,15 @@ static std::optional<GLHandle> CreateGLHandle(const ProcTableGLES& gl,
       gl.GenFramebuffers(1u, &handle.handle);
       return handle;
     case HandleType::kFence:
-      return GLHandle{.sync = gl.FenceSync(GL_SYNC_GPU_COMMANDS_COMPLETE, 0)};
+      return GLStorage{.sync = gl.FenceSync(GL_SYNC_GPU_COMMANDS_COMPLETE, 0)};
   }
   return std::nullopt;
 }
 
-static bool CollectGLHandle(const ProcTableGLES& gl,
-                            HandleType type,
-                            GLHandle handle) {
+// static
+bool ReactorGLES::CollectGLHandle(const ProcTableGLES& gl,
+                                  HandleType type,
+                                  ReactorGLES::GLStorage handle) {
   switch (type) {
     case HandleType::kUnknown:
       return false;
@@ -121,7 +124,8 @@ const ProcTableGLES& ReactorGLES::GetProcTable() const {
   return *proc_table_;
 }
 
-std::optional<GLHandle> ReactorGLES::GetHandle(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) {
@@ -129,7 +133,7 @@ std::optional<GLHandle> ReactorGLES::GetHandle(const HandleGLES& handle) const {
           << "Attempted to acquire a handle that was pending collection.";
       return std::nullopt;
     }
-    std::optional<GLHandle> name = found->second.name;
+    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;
@@ -144,7 +148,7 @@ std::optional<GLuint> ReactorGLES::GetGLHandle(const HandleGLES& handle) const {
   if (handle.type == HandleType::kFence) {
     return std::nullopt;
   }
-  std::optional<GLHandle> gl_handle = GetHandle(handle);
+  std::optional<ReactorGLES::GLStorage> gl_handle = GetHandle(handle);
   if (gl_handle.has_value()) {
     return gl_handle->handle;
   }
@@ -155,7 +159,7 @@ std::optional<GLsync> ReactorGLES::GetGLFence(const HandleGLES& handle) const {
   if (handle.type != HandleType::kFence) {
     return std::nullopt;
   }
-  std::optional<GLHandle> gl_handle = GetHandle(handle);
+  std::optional<ReactorGLES::GLStorage> gl_handle = GetHandle(handle);
   if (gl_handle.has_value()) {
     return gl_handle->sync;
   }
@@ -199,9 +203,9 @@ HandleGLES ReactorGLES::CreateHandle(HandleType type, GLuint external_handle) {
   }
   WriterLock handles_lock(handles_mutex_);
 
-  std::optional<GLHandle> gl_handle;
+  std::optional<ReactorGLES::GLStorage> gl_handle;
   if (external_handle != GL_NONE) {
-    gl_handle = GLHandle{.handle = external_handle};
+    gl_handle = ReactorGLES::GLStorage{.handle = external_handle};
   } else if (CanReactOnCurrentThread()) {
     gl_handle = CreateGLHandle(GetProcTable(), type);
   }
diff --git a/impeller/renderer/backend/gles/reactor_gles.h b/impeller/renderer/backend/gles/reactor_gles.h
index e9cb92a3073fc..dc1396314e969 100644
--- a/impeller/renderer/backend/gles/reactor_gles.h
+++ b/impeller/renderer/backend/gles/reactor_gles.h
@@ -16,14 +16,6 @@
 
 namespace impeller {
 
-/// @brief Storage for either a GL handle or sync fence.
-struct GLHandle {
-  union {
-    GLuint handle;
-    GLsync sync;
-  };
-};
-
 //------------------------------------------------------------------------------
 /// @brief      The reactor attempts to make thread-safe usage of OpenGL ES
 ///             easier to reason about.
@@ -255,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<GLHandle> 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<GLHandle> p_name) : name(p_name) {}
+    explicit LiveHandle(std::optional<GLStorage> p_name) : name(p_name) {}
 
     constexpr bool IsLive() const { return name.has_value(); }
   };
@@ -302,7 +302,14 @@ class ReactorGLES {
 
   void SetupDebugGroups();
 
-  std::optional<GLHandle> GetHandle(const HandleGLES& handle) const;
+  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;