Fix vulkan fence synchronization (#17969)

* Add fence synchronization back to async upload queue fences * Fix user awaited fence synchronization * Move AttachmentReadback wait to Execute Signed-off-by: Martin Sattlecker <martin.sattlecker@huawei.com> Signed-off-by: Martin Winter <martin.winter@huawei.com> Co-authored-by: Martin Winter <martin.winter@huawei.com>
o3de · May 28, 2024 · ce7058c · ce7058c
1 parent 78c746a
commit ce7058c
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 26 deletions.
diff --git a/Gems/Atom/RHI/Vulkan/Code/Source/RHI/AsyncUploadQueue.cpp b/Gems/Atom/RHI/Vulkan/Code/Source/RHI/AsyncUploadQueue.cpp
@@ -5,13 +5,14 @@
  * SPDX-License-Identifier: Apache-2.0 OR MIT
  *
  */
-#include <Atom/RHI/RHISystemInterface.h>
+#include <Atom/RHI.Reflect/ImageSubresource.h>
 #include <Atom/RHI.Reflect/PlatformLimitsDescriptor.h>
 #include <Atom/RHI/BufferPool.h>
+#include <Atom/RHI/RHISystemInterface.h>
 #include <Atom/RHI/StreamingImagePool.h>
-#include <Atom/RHI.Reflect/ImageSubresource.h>
 #include <AzCore/Component/TickBus.h>
 #include <AzCore/std/containers/vector.h>
+#include <AzCore/std/smart_ptr/make_shared.h>
 #include <RHI/AsyncUploadQueue.h>
 #include <RHI/Buffer.h>
 #include <RHI/BufferPool.h>
@@ -24,6 +25,7 @@
 #include <RHI/Image.h>
 #include <RHI/MemoryView.h>
 #include <RHI/Queue.h>
+#include <RHI/SignalEvent.h>
 
 namespace AZ
 {
@@ -93,6 +95,19 @@ namespace AZ
 
             RHI::Ptr<Fence> uploadFence = Fence::Create();
             uploadFence->Init(device, RHI::FenceState::Reset);
+
+            uploadFence->SetSignalEvent(AZStd::make_shared<SignalEvent>());
+            uploadFence->SetSignalEventBitToSignal(0);
+            uploadFence->SetSignalEventDependencies(1);
+
+            if (request.m_fenceToSignal)
+            {
+                auto fenceToSignal = static_cast<Fence*>(request.m_fenceToSignal);
+                fenceToSignal->SetSignalEvent(AZStd::make_shared<SignalEvent>());
+                fenceToSignal->SetSignalEventBitToSignal(0);
+                fenceToSignal->SetSignalEventDependencies(1);
+            }
+
             CommandQueue::Command command = [=, &device](void* queue)
             {
                 AZ_PROFILE_SCOPE(RHI, "Upload Buffer");
@@ -183,6 +198,10 @@ namespace AZ
             RHI::Ptr<Fence> uploadFence = Fence::Create();
             uploadFence->Init(device, RHI::FenceState::Reset);
 
+            uploadFence->SetSignalEvent(AZStd::make_shared<SignalEvent>());
+            uploadFence->SetSignalEventBitToSignal(0);
+            uploadFence->SetSignalEventDependencies(1);
+
             CommandQueue::Command command = [=, &device](void* queue)
             {
                 AZ_PROFILE_SCOPE(RHI, "Upload Image");
@@ -463,6 +482,9 @@ namespace AZ
                 AZ_Assert(framePacket.m_stagingBuffer, "Failed to acquire staging buffer");
                 framePacket.m_fence = Fence::Create();
                 result = framePacket.m_fence->Init(device, RHI::FenceState::Signaled);
+                framePacket.m_fence->SetSignalEvent(AZStd::make_shared<SignalEvent>());
+                framePacket.m_fence->SetSignalEventBitToSignal(0);
+                framePacket.m_fence->SetSignalEventDependencies(1);
                 RETURN_RESULT_IF_UNSUCCESSFUL(result);
             }
 

diff --git a/Gems/Atom/RHI/Vulkan/Code/Source/RHI/FrameGraphCompiler.cpp b/Gems/Atom/RHI/Vulkan/Code/Source/RHI/FrameGraphCompiler.cpp
@@ -463,7 +463,9 @@ namespace AZ
                     // The fence wait might not be on the framegraph (wait on CPU)
                     // In this case we want wait for the current scopes dependencies (i.e. for signalling of the fence itself)
                     // If the Fence is waited-for on the framegraph at a later scope, we overwrite the dependencies later
-                    fence->SetSignalEventDependencies(currentDependencies);
+                    SignalEvent::BitSet fenceDependencies = currentDependencies;
+                    fenceDependencies.set(currentBitToSignal);
+                    fence->SetSignalEventDependencies(fenceDependencies);
                     hasSemaphoreSignal = true;
                 }
                 for (auto& semaphore : scope->GetSignalSemaphores())

diff --git a/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/AttachmentReadback.cpp b/Gems/Atom/RPI/Code/Source/RPI.Public/Pass/AttachmentReadback.cpp
@@ -338,29 +338,6 @@ namespace AZ
             }
             // Loop the triple buffer index and cache the current index to the callback.
             m_readbackBufferCurrentIndex = (m_readbackBufferCurrentIndex + 1) % RHI::Limits::Device::FrameCountMax;
-
-            uint32_t readbackBufferCurrentIndex = m_readbackBufferCurrentIndex;
-            m_fence->WaitOnCpuAsync([this, readbackBufferCurrentIndex]()
-                {
-                    if (m_state == ReadbackState::Reading)
-                    {
-                        if (CopyBufferData(readbackBufferCurrentIndex))
-                        {
-                            m_state = ReadbackState::Success;
-                        }
-                        else
-                        {
-                            m_state = ReadbackState::Failed;
-                        }
-                    }
-                    if (m_callback)
-                    {
-                        m_callback(GetReadbackResult());
-                    }
-
-                    Reset();
-                }
-            );
         }
 
         void AttachmentReadback::CopyCompile(const RHI::FrameGraphCompileContext& context)
@@ -457,6 +434,29 @@ namespace AZ
                     context.GetCommandList()->Submit(readbackItem.m_copyItem);
                 }
             }
+
+            uint32_t readbackBufferCurrentIndex = m_readbackBufferCurrentIndex;
+            m_fence->WaitOnCpuAsync(
+                [this, readbackBufferCurrentIndex]()
+                {
+                    if (m_state == ReadbackState::Reading)
+                    {
+                        if (CopyBufferData(readbackBufferCurrentIndex))
+                        {
+                            m_state = ReadbackState::Success;
+                        }
+                        else
+                        {
+                            m_state = ReadbackState::Failed;
+                        }
+                    }
+                    if (m_callback)
+                    {
+                        m_callback(GetReadbackResult());
+                    }
+
+                    Reset();
+                });
         }
 
         void AttachmentReadback::Reset()