[Impeller] Use waitUntilScheduled before presenting

Author: bdero

impeller/renderer/backend/metal/command_buffer_mtl.h

@@ -5,6 +5,7 @@
 #pragma once
 
 #include <Metal/Metal.h>
+#include <functional>
 
 #include "flutter/fml/macros.h"
 #include "impeller/renderer/command_buffer.h"
@@ -13,16 +14,20 @@ namespace impeller {
 
 class CommandBufferMTL final : public CommandBuffer {
  public:
+  using SubmitCallback = std::function<void(id<MTLCommandBuffer> buffer)>;
+
   // |CommandBuffer|
   ~CommandBufferMTL() override;
 
  private:
   friend class ContextMTL;
 
   id<MTLCommandBuffer> buffer_ = nullptr;
+  SubmitCallback submit_callback_;
 
   CommandBufferMTL(const std::weak_ptr<const Context>& context,
-                   id<MTLCommandQueue> queue);
+                   id<MTLCommandQueue> queue,
+                   SubmitCallback on_submit);
 
   // |CommandBuffer|
   void SetLabel(const std::string& label) const override;

impeller/renderer/backend/metal/command_buffer_mtl.mm

@@ -6,6 +6,7 @@
 
 #include "impeller/renderer/backend/metal/blit_pass_mtl.h"
 #include "impeller/renderer/backend/metal/compute_pass_mtl.h"
+#include "impeller/renderer/backend/metal/context_mtl.h"
 #include "impeller/renderer/backend/metal/render_pass_mtl.h"
 
 namespace impeller {
@@ -124,8 +125,11 @@ static bool LogMTLCommandBufferErrorIfPresent(id<MTLCommandBuffer> buffer) {
 }
 
 CommandBufferMTL::CommandBufferMTL(const std::weak_ptr<const Context>& context,
-                                   id<MTLCommandQueue> queue)
-    : CommandBuffer(context), buffer_(CreateCommandBuffer(queue)) {}
+                                   id<MTLCommandQueue> queue,
+                                   SubmitCallback on_submit)
+    : CommandBuffer(context),
+      buffer_(CreateCommandBuffer(queue)),
+      submit_callback_(std::move(on_submit)) {}
 
 CommandBufferMTL::~CommandBufferMTL() = default;
 
@@ -166,20 +170,16 @@ static bool LogMTLCommandBufferErrorIfPresent(id<MTLCommandBuffer> buffer) {
   }
 
   [buffer_ commit];
+  submit_callback_(buffer_);
 
-#if (FML_OS_MACOSX || FML_OS_IOS_SIMULATOR)
-  // We're using waitUntilScheduled on macOS and iOS simulator to force a hard
-  // barrier between the execution of different command buffers. This forces all
-  // renderable texture access to be synchronous (i.e. a write from a previous
-  // command buffer will not get scheduled to happen at the same time as a read
-  // in a future command buffer).
-  //
+#if (FML_OS_MACOSX)
   // Metal hazard tracks shared memory resources by default, and we don't need
   // to do any additional work to synchronize access to MTLTextures and
   // MTLBuffers on iOS devices with UMA. However, shared textures are disallowed
   // on macOS according to the documentation:
   // https://developer.apple.com/documentation/metal/mtlstoragemode/shared
-  // And so this is a stopgap solution that has been present in Impeller since
+  //
+  // This is a stopgap solution that has been present in Impeller since
   // multi-pass rendering/SaveLayer support was first set up.
   //
   // TODO(bdero): Remove this for all targets once a solution for resource

impeller/renderer/backend/metal/context_mtl.h

@@ -64,9 +64,21 @@ class ContextMTL final : public Context,
   // |Context|
   bool UpdateOffscreenLayerPixelFormat(PixelFormat format) override;
 
+  /// @brief  Block the current thread until all of the previously submitted
+  ///         command buffers have been scheduled. This should be called before
+  ///         using non-command Metal functions that access resources mutated by
+  ///         a previous command buffer.
+  ///         For example, when rendering a frame, this should be called
+  ///         immediately before `MTLDrawable::present()` (which should be
+  ///         called after all command buffers in the frame have already been
+  ///         submitted).
+  void WaitUntilScheduled();
+
  private:
   id<MTLDevice> device_ = nullptr;
   id<MTLCommandQueue> command_queue_ = nullptr;
+  mutable id<MTLCommandBuffer> last_committed_buffer_ = nullptr;
+  mutable std::mutex last_committed_buffer_lock_;
   std::shared_ptr<ShaderLibraryMTL> shader_library_;
   std::shared_ptr<PipelineLibraryMTL> pipeline_library_;
   std::shared_ptr<SamplerLibrary> sampler_library_;

impeller/renderer/backend/metal/context_mtl.mm

@@ -5,6 +5,7 @@
 #include "impeller/renderer/backend/metal/context_mtl.h"
 
 #include <Foundation/Foundation.h>
+#include <mutex>
 
 #include "flutter/fml/file.h"
 #include "flutter/fml/logging.h"
@@ -264,8 +265,11 @@ static bool DeviceSupportsComputeSubgroups(id<MTLDevice> device) {
     return nullptr;
   }
 
-  auto buffer = std::shared_ptr<CommandBufferMTL>(
-      new CommandBufferMTL(weak_from_this(), queue));
+  auto buffer = std::shared_ptr<CommandBufferMTL>(new CommandBufferMTL(
+      weak_from_this(), queue, [this](id<MTLCommandBuffer> buffer) mutable {
+        std::scoped_lock lock(last_committed_buffer_lock_);
+        last_committed_buffer_ = buffer;
+      }));
   if (!buffer->IsValid()) {
     return nullptr;
   }
@@ -290,4 +294,13 @@ static bool DeviceSupportsComputeSubgroups(id<MTLDevice> device) {
   return true;
 }
 
+void ContextMTL::WaitUntilScheduled() {
+  if (!last_committed_buffer_) {
+    return;
+  }
+
+  [last_committed_buffer_ waitUntilScheduled];
+  last_committed_buffer_ = nullptr;
+}
+
 }  // namespace impeller

impeller/renderer/backend/metal/surface_mtl.h

@@ -5,6 +5,7 @@
 #pragma once
 
 #include <QuartzCore/CAMetalLayer.h>
+#include <memory>
 
 #include "flutter/fml/macros.h"
 #include "impeller/renderer/context.h"
@@ -43,9 +44,12 @@ class SurfaceMTL final : public Surface {
   id<MTLDrawable> drawable() const { return drawable_; }
 
  private:
+  std::weak_ptr<Context> context_;
   id<MTLDrawable> drawable_ = nil;
 
-  SurfaceMTL(const RenderTarget& target, id<MTLDrawable> drawable);
+  SurfaceMTL(const std::weak_ptr<Context>& context,
+             const RenderTarget& target,
+             id<MTLDrawable> drawable);
 
   // |Surface|
   bool Present() const override;

impeller/renderer/backend/metal/surface_mtl.mm

@@ -6,6 +6,7 @@
 
 #include "flutter/fml/trace_event.h"
 #include "impeller/base/validation.h"
+#include "impeller/renderer/backend/metal/context_mtl.h"
 #include "impeller/renderer/backend/metal/formats_mtl.h"
 #include "impeller/renderer/backend/metal/texture_mtl.h"
 #include "impeller/renderer/render_target.h"
@@ -111,12 +112,14 @@
   render_target_desc.SetStencilAttachment(stencil0);
 
   // The constructor is private. So make_unique may not be used.
-  return std::unique_ptr<SurfaceMTL>(
-      new SurfaceMTL(render_target_desc, current_drawable));
+  return std::unique_ptr<SurfaceMTL>(new SurfaceMTL(
+      context->weak_from_this(), render_target_desc, current_drawable));
 }
 
-SurfaceMTL::SurfaceMTL(const RenderTarget& target, id<MTLDrawable> drawable)
-    : Surface(target), drawable_(drawable) {}
+SurfaceMTL::SurfaceMTL(const std::weak_ptr<Context>& context,
+                       const RenderTarget& target,
+                       id<MTLDrawable> drawable)
+    : Surface(target), context_(context), drawable_(drawable) {}
 
 // |Surface|
 SurfaceMTL::~SurfaceMTL() = default;
@@ -127,6 +130,8 @@
     return false;
   }
 
+  ContextMTL::Cast(context_.lock().get())->WaitUntilScheduled();
+
   [drawable_ present];
   return true;
 }