[Metal] Fixed synchronization of command buffers in-flight for Metal …

…backend (again). The semaphore in MTDirectCommandBuffer is meant to synchronize the lifetime of in-flight resources such as the staging buffer pool, not the native command buffers (Metal already takes care of those). The calls to ...wait() and ...signal() must be perfectly balanced, or the synchronization fails.
LukasBanana · Jun 6, 2024 · a335b63 · a335b63
1 parent 151580e
commit a335b63
Show file tree

Hide file tree

Showing 5 changed files with 18 additions and 13 deletions.
diff --git a/sources/Renderer/Metal/Command/MTCommandBuffer.h b/sources/Renderer/Metal/Command/MTCommandBuffer.h
@@ -60,6 +60,8 @@ class MTCommandBuffer : public CommandBuffer
 
     protected:
 
+        static constexpr NSUInteger maxNumCommandBuffersInFlight = 3;
+
         MTCommandBuffer(id<MTLDevice> device, long flags);
 
         void ResetRenderStates();
@@ -74,13 +76,11 @@ class MTCommandBuffer : public CommandBuffer
 
     private:
 
-        static constexpr NSUInteger     maxNumStagingPools      = 3;
-
         id<MTLDevice>                   device_                 = nil;
         long                            flags_                  = 0;
 
         NSUInteger                      currentStagingPool_     = 0;
-        MTStagingBufferPool             stagingBufferPools_[MTCommandBuffer::maxNumStagingPools];
+        MTStagingBufferPool             stagingBufferPools_[MTCommandBuffer::maxNumCommandBuffersInFlight];
         SmallVector<id<MTLDrawable>, 2> queuedDrawables_;
 
 };

diff --git a/sources/Renderer/Metal/Command/MTCommandBuffer.mm b/sources/Renderer/Metal/Command/MTCommandBuffer.mm
@@ -40,7 +40,7 @@
 
 void MTCommandBuffer::ResetRenderStates()
 {
-    currentStagingPool_ = (currentStagingPool_ + 1) % maxNumStagingPools;
+    currentStagingPool_ = (currentStagingPool_ + 1) % MTCommandBuffer::maxNumCommandBuffersInFlight;
 }
 
 void MTCommandBuffer::ResetStagingPool()

diff --git a/sources/Renderer/Metal/Command/MTCommandQueue.mm b/sources/Renderer/Metal/Command/MTCommandQueue.mm
@@ -46,8 +46,8 @@
         auto& directCommandBufferMT = LLGL_CAST(MTDirectCommandBuffer&, commandBufferMT);
         if (!directCommandBufferMT.IsImmediateCmdBuffer())
         {
-            SubmitCommandBuffer(directCommandBufferMT.GetNative());
             directCommandBufferMT.MarkSubmitted();
+            SubmitCommandBuffer(directCommandBufferMT.GetNative());
         }
     }
 }

diff --git a/sources/Renderer/Metal/Command/MTDirectCommandBuffer.h b/sources/Renderer/Metal/Command/MTDirectCommandBuffer.h
@@ -65,7 +65,7 @@ class MTDirectCommandBuffer final : public MTCommandBuffer
 
         id<MTLCommandBuffer>            cmdBuffer_          = nil;
         dispatch_semaphore_t            cmdBufferSemaphore_ = nil;
-        bool                            cmdBufferSubmitted_ = false;
+        bool                            cmdBufferDirty_     = false;
 
         MTCommandQueue&                 cmdQueue_;
         MTCommandContext                context_;

diff --git a/sources/Renderer/Metal/Command/MTDirectCommandBuffer.mm b/sources/Renderer/Metal/Command/MTDirectCommandBuffer.mm
@@ -49,17 +49,22 @@
     cmdQueue_          { cmdQueue           },
     context_           { device             }
 {
-    const NSUInteger maxCmdBuffers = 3;
-    cmdBufferSemaphore_ = dispatch_semaphore_create(maxCmdBuffers);
+    cmdBufferSemaphore_ = dispatch_semaphore_create(MTCommandBuffer::maxNumCommandBuffersInFlight);
 }
 
 /* ----- Encoding ----- */
 
 void MTDirectCommandBuffer::Begin()
 {
-    /* Wait until next command buffer becomes available */
-    if (cmdBufferSubmitted_)
+    /*
+    If the previous encoded command buffer was submitted to the command queue,
+    we have to wait until our in-flight resources (such as staging buffer pool) become available again.
+    */
+    if (!cmdBufferDirty_)
+    {
         dispatch_semaphore_wait(cmdBufferSemaphore_, DISPATCH_TIME_FOREVER);
+        cmdBufferDirty_ = true;
+    }
 
     /* Allocate new command buffer from command queue */
     cmdBuffer_ = [cmdQueue_.GetNative() commandBuffer];
@@ -70,7 +75,6 @@
         addCompletedHandler:^(id<MTLCommandBuffer> cmdBuffer)
         {
             dispatch_semaphore_signal(blockSemaphore);
-            this->cmdBufferSubmitted_ = false;
         }
     ];
 
@@ -87,8 +91,8 @@
     /* Commit native buffer right after encoding for immediate command buffers */
     if (IsImmediateCmdBuffer())
     {
-        cmdQueue_.SubmitCommandBuffer(GetNative());
         MarkSubmitted();
+        cmdQueue_.SubmitCommandBuffer(GetNative());
     }
 
     ResetRenderStates();
@@ -1022,7 +1026,8 @@ static void TrapIndirectPatchesNotSupported()
 
 void MTDirectCommandBuffer::MarkSubmitted()
 {
-    cmdBufferSubmitted_ = true;
+    /* Reset dirty bit now that it has been submitted to the command queue */
+    cmdBufferDirty_ = false;
 }