Fix Metal frame-synchronization issue (sporadic crash on wrong comman…

…d list state), disable VSync by default
MethanePowered · Dec 25, 2024 · cfae0f6 · cfae0f6
1 parent 42cf85e
commit cfae0f6
Show file tree

Hide file tree

Showing 14 changed files with 43 additions and 22 deletions.
diff --git a/Apps/Common/Sources/Methane/Tutorials/AppSettings.cpp b/Apps/Common/Sources/Methane/Tutorials/AppSettings.cpp
@@ -33,12 +33,9 @@ AppOptions::Mask AppOptions::GetDefaultWithColorOnly() noexcept
 {
     AppOptions::Mask options;
     options |= AppOptions::Bit::ClearColor;
-#ifdef __APPLE__
-    options |= AppOptions::Bit::VSync;
-#ifndef APPLE_MACOS // iOS
+#if defined(__APPLE__) && !defined(APPLE_MACOS) // iOS
     options |= AppOptions::Bit::FullScreen;
     options |= AppOptions::Bit::HudVisible;
-#endif
 #endif
     return options;
 }

diff --git a/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/CommandListSet.h b/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/CommandListSet.h
@@ -52,7 +52,7 @@ class CommandListSet
 
     // CommandListSet interface
     virtual void Execute(const Rhi::ICommandList::CompletedCallback& completed_callback);
-    virtual void WaitUntilCompleted() = 0;
+    virtual void WaitUntilCompleted(uint32_t timeout_ms = 0U) = 0;
 
     bool IsExecuting() const noexcept { return m_is_executing; }
     void Complete() const;

diff --git a/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/CommandQueueTracking.h b/Modules/Graphics/RHI/Base/Include/Methane/Graphics/Base/CommandQueueTracking.h
@@ -60,7 +60,9 @@ class CommandQueueTracking // NOSONAR - destructor is required
     // IObject interface
     bool SetName(std::string_view name) override;
 
+    // CommandQueueTracking interface
     virtual void CompleteExecution(const Opt<Data::Index>& frame_index = { });
+    virtual void WaitUntilCompleted(const Opt<Data::Index>& frame_index = { }, uint32_t timeout_ms = 0U);
 
     Ptr<CommandListSet> GetLastExecutingCommandListSet() const;
     const Ptr<Rhi::ITimestampQueryPool>& GetTimestampQueryPoolPtr() final;

diff --git a/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/CommandList.cpp b/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/CommandList.cpp
@@ -270,13 +270,17 @@ void CommandList::SetCommandListState(State state)
 void CommandList::SetCommandListStateNoLock(State state)
 {
     META_FUNCTION_TASK();
-    if (m_state == state)
-        return;
+    {
+        std::lock_guard state_change_lock(m_state_change_mutex);
+        if (m_state == state)
+            return;
 
-    META_LOG("{} Command list '{}' change state from {} to {}",
-             magic_enum::enum_name(m_type), GetName(), magic_enum::enum_name(m_state), magic_enum::enum_name(state));
+        META_LOG("{} Command list '{}' change state from {} to {}",
+                 magic_enum::enum_name(m_type), GetName(), magic_enum::enum_name(m_state), magic_enum::enum_name(state));
+
+        m_state = state;
+    }
 
-    m_state = state;
     m_state_change_condition_var.notify_one();
 
     Data::Emitter<Rhi::ICommandListCallback>::Emit(&Rhi::ICommandListCallback::OnCommandListStateChanged, *this);

diff --git a/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/CommandQueueTracking.cpp b/Modules/Graphics/RHI/Base/Sources/Methane/Graphics/Base/CommandQueueTracking.cpp
@@ -124,6 +124,19 @@ void CommandQueueTracking::CompleteExecution(const Opt<Data::Index>& frame_index
     m_execution_waiting_condition_var.notify_one();
 }
 
+void CommandQueueTracking::WaitUntilCompleted(const Opt<Data::Index>& frame_index, uint32_t timeout_ms)
+{
+    META_FUNCTION_TASK();
+    std::scoped_lock lock_guard(m_executing_command_lists_mutex);
+    while (!m_executing_command_lists.empty() &&
+           m_executing_command_lists.front()->GetFrameIndex() == frame_index)
+    {
+        m_executing_command_lists.front()->WaitUntilCompleted(timeout_ms);
+        m_executing_command_lists.pop();
+    }
+    m_execution_waiting_condition_var.notify_one();
+}
+
 void CommandQueueTracking::WaitForExecution() noexcept
 {
     try

diff --git a/Modules/Graphics/RHI/DirectX/Include/Methane/Graphics/DirectX/CommandListSet.h b/Modules/Graphics/RHI/DirectX/Include/Methane/Graphics/DirectX/CommandListSet.h
@@ -45,7 +45,7 @@ class CommandListSet final
 
     // Base::CommandListSet interface
     void Execute(const Rhi::ICommandList::CompletedCallback& completed_callback) override;
-    void WaitUntilCompleted() override;
+    void WaitUntilCompleted(uint32_t timeout_ms) override;
 
     using NativeCommandLists = std::vector<ID3D12CommandList*>;
     const NativeCommandLists& GetNativeCommandLists() const noexcept { return m_native_command_lists; }

diff --git a/Modules/Graphics/RHI/DirectX/Sources/Methane/Graphics/DirectX/CommandListSet.cpp b/Modules/Graphics/RHI/DirectX/Sources/Methane/Graphics/DirectX/CommandListSet.cpp
@@ -78,7 +78,7 @@ void CommandListSet::Execute(const Rhi::ICommandList::CompletedCallback& complet
     m_execution_completed_fence.Signal();
 }
 
-void CommandListSet::WaitUntilCompleted()
+void CommandListSet::WaitUntilCompleted(uint32_t /*timeout_ms*/)
 {
     META_FUNCTION_TASK();
     m_execution_completed_fence.WaitOnCpu();

diff --git a/Modules/Graphics/RHI/Metal/Include/Methane/Graphics/Metal/CommandListSet.hh b/Modules/Graphics/RHI/Metal/Include/Methane/Graphics/Metal/CommandListSet.hh
@@ -34,11 +34,7 @@ class CommandListSet final
 public:
     explicit CommandListSet(const Refs<Rhi::ICommandList>& command_list_refs, Opt<Data::Index> frame_index_opt);
 
-    virtual void WaitUntilCompleted()
-    {
-        // Command list execution tracking is not needed in Metal,
-        // because native API has command list wait mechanism used directly in CommandList::Execute(...)
-    }
+    void WaitUntilCompleted(uint32_t timeout_ms) override;
 };
 
 } // namespace Methane::Graphics::Metal
diff --git a/Modules/Graphics/RHI/Metal/Sources/Methane/Graphics/Metal/CommandListSet.mm b/Modules/Graphics/RHI/Metal/Sources/Methane/Graphics/Metal/CommandListSet.mm
@@ -43,4 +43,13 @@
     : Base::CommandListSet(command_list_refs, frame_index_opt)
 { }
 
+void CommandListSet::WaitUntilCompleted(uint32_t timeout_ms)
+{
+    META_FUNCTION_TASK();
+    for(const Ref<Base::CommandList>& command_list_ref : GetBaseRefs())
+    {
+        command_list_ref.get().WaitUntilCompleted(timeout_ms);
+    }
+}
+
 } // namespace Methane::Graphics::Metal
diff --git a/Modules/Graphics/RHI/Metal/Sources/Methane/Graphics/Metal/RenderContext.mm b/Modules/Graphics/RHI/Metal/Sources/Methane/Graphics/Metal/RenderContext.mm
@@ -144,7 +144,7 @@
     default: META_UNEXPECTED(wait_for);
     }
 
-    GetMetalDefaultCommandQueue(cl_type).CompleteExecution(frame_buffer_index);
+    GetMetalDefaultCommandQueue(cl_type).WaitUntilCompleted(frame_buffer_index, 16);
 }
 
 void RenderContext::Resize(const FrameSize& frame_size)

diff --git a/Modules/Graphics/RHI/Null/Include/Methane/Graphics/Null/CommandListSet.h b/Modules/Graphics/RHI/Null/Include/Methane/Graphics/Null/CommandListSet.h
@@ -39,7 +39,7 @@ class CommandListSet final
     using Base::CommandListSet::Complete;
 
     // Base::CommandListSet interface
-    void WaitUntilCompleted() override;
+    void WaitUntilCompleted(uint32_t timeout_ms) override;
 };
 
 } // namespace Methane::Graphics::Null
diff --git a/Modules/Graphics/RHI/Null/Sources/Methane/Graphics/Null/CommandListSet.cpp b/Modules/Graphics/RHI/Null/Sources/Methane/Graphics/Null/CommandListSet.cpp
@@ -43,7 +43,7 @@ CommandListSet::CommandListSet(const Refs<Rhi::ICommandList>& command_list_refs,
 {
 }
 
-void CommandListSet::WaitUntilCompleted()
+void CommandListSet::WaitUntilCompleted(uint32_t /*timeout_ms*/)
 {
     Complete();
 }

diff --git a/Modules/Graphics/RHI/Vulkan/Include/Methane/Graphics/Vulkan/CommandListSet.h b/Modules/Graphics/RHI/Vulkan/Include/Methane/Graphics/Vulkan/CommandListSet.h
@@ -42,7 +42,7 @@ class CommandListSet final
 
     // Base::CommandListSet interface
     void Execute(const Rhi::ICommandList::CompletedCallback& completed_callback) override;
-    void WaitUntilCompleted() override;
+    void WaitUntilCompleted(uint32_t timeout_ms) override;
 
     const std::vector<vk::CommandBuffer>& GetNativeCommandBuffers() const noexcept { return m_vk_command_buffers; }
     const vk::Semaphore& GetNativeExecutionCompletedSemaphore() const noexcept     { return m_vk_unique_execution_completed_semaphore.get(); }

diff --git a/Modules/Graphics/RHI/Vulkan/Sources/Methane/Graphics/Vulkan/CommandListSet.cpp b/Modules/Graphics/RHI/Vulkan/Sources/Methane/Graphics/Vulkan/CommandListSet.cpp
@@ -153,7 +153,7 @@ void CommandListSet::Execute(const Rhi::ICommandList::CompletedCallback& complet
     m_signalled_execution_completed_fence = true;
 }
 
-void CommandListSet::WaitUntilCompleted()
+void CommandListSet::WaitUntilCompleted(uint32_t /*timeout_ms*/)
 {
     META_FUNCTION_TASK();
     std::scoped_lock fence_guard(m_execution_completed_fence_mutex);
-Original file line number
+Diff line change
@@ Expand Up @@
     {
     }
-    void CommandListSet::WaitUntilCompleted()
+    void CommandListSet::WaitUntilCompleted(uint32_t /*timeout_ms*/)
     {
         Complete();
     }
@@ Expand Down @@