Vulkan backend: fixed issue with swap chain semaphores being destroyed while still in use

6 files changed, 121 insertions, 22 deletions

diff --git a/Graphics/GraphicsEngineVulkan/CMakeLists.txt b/Graphics/GraphicsEngineVulkan/CMakeLists.txt
index afa73a10..a4ac2b92 100644
--- a/Graphics/GraphicsEngineVulkan/CMakeLists.txt
+++ b/Graphics/GraphicsEngineVulkan/CMakeLists.txt
@@ -21,6 +21,7 @@ set(INCLUDE
     include/RenderPassCache.h
     include/SamplerVkImpl.h
     include/ShaderVkImpl.h
+    include/Semaphore.h
     include/ShaderResourceBindingVkImpl.h
     include/ShaderResourceCacheVk.h
     include/ShaderResourceLayoutVk.h
diff --git a/Graphics/GraphicsEngineVulkan/include/DeviceContextVkImpl.h b/Graphics/GraphicsEngineVulkan/include/DeviceContextVkImpl.h
index d8b12a19..9b9531ac 100644
--- a/Graphics/GraphicsEngineVulkan/include/DeviceContextVkImpl.h
+++ b/Graphics/GraphicsEngineVulkan/include/DeviceContextVkImpl.h
@@ -41,6 +41,7 @@
 #include "TextureVkImpl.h"
 #include "PipelineStateVkImpl.h"
 #include "HashUtils.h"
+#include "SemaphoreObject.h"
 
 namespace Diligent
 {
@@ -234,14 +235,18 @@ public:
     virtual void BufferMemoryBarrier(IBuffer* pBuffer, VkAccessFlags NewAccessFlags) override final;
 
 
-    void AddWaitSemaphore(VkSemaphore Semaphore, VkPipelineStageFlags WaitDstStageMask)
+    void AddWaitSemaphore(SemaphoreObject* pWaitSemaphore, VkPipelineStageFlags WaitDstStageMask)
     {
-        m_WaitSemaphores.push_back(Semaphore);
+        VERIFY_EXPR(pWaitSemaphore != nullptr);
+        m_WaitSemaphores.emplace_back(pWaitSemaphore);
+        m_VkWaitSemaphores.push_back(pWaitSemaphore->GetVkSemaphore());
         m_WaitDstStageMasks.push_back(WaitDstStageMask);
     }
-    void AddSignalSemaphore(VkSemaphore Semaphore)
+    void AddSignalSemaphore(SemaphoreObject* pSignalSemaphore)
     {
-        m_SignalSemaphores.push_back(Semaphore);
+        VERIFY_EXPR(pSignalSemaphore != nullptr);
+        m_SignalSemaphores.emplace_back(pSignalSemaphore);
+        m_VkSignalSemaphores.push_back(pSignalSemaphore->GetVkSemaphore());
     }
 
     void UpdateBufferRegion(BufferVkImpl*                  pBuffVk,
@@ -401,9 +406,12 @@ private:
     FixedBlockMemoryAllocator m_CmdListAllocator;
 
     // Semaphores are not owned by the command context
-    std::vector<VkSemaphore>          m_WaitSemaphores;
-    std::vector<VkPipelineStageFlags> m_WaitDstStageMasks;
-    std::vector<VkSemaphore>          m_SignalSemaphores;
+    std::vector<RefCntAutoPtr<SemaphoreObject>> m_WaitSemaphores;
+    std::vector<VkPipelineStageFlags>           m_WaitDstStageMasks;
+    std::vector<RefCntAutoPtr<SemaphoreObject>> m_SignalSemaphores;
+
+    std::vector<VkSemaphore> m_VkWaitSemaphores;
+    std::vector<VkSemaphore> m_VkSignalSemaphores;
 
     // List of fences to signal next time the command context is flushed
     std::vector<std::pair<Uint64, RefCntAutoPtr<IFence>>> m_PendingFences;
diff --git a/Graphics/GraphicsEngineVulkan/include/SemaphoreObject.h b/Graphics/GraphicsEngineVulkan/include/SemaphoreObject.h
new file mode 100644
index 00000000..c0dc8b94
--- /dev/null
+++ b/Graphics/GraphicsEngineVulkan/include/SemaphoreObject.h
@@ -0,0 +1,80 @@
+/*     Copyright 2019 Diligent Graphics LLC
+ *  
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ * 
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ *  THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ *  IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ *  FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT OF ANY PROPRIETARY RIGHTS.
+ *
+ *  In no event and under no legal theory, whether in tort (including negligence), 
+ *  contract, or otherwise, unless required by applicable law (such as deliberate 
+ *  and grossly negligent acts) or agreed to in writing, shall any Contributor be
+ *  liable for any damages, including any direct, indirect, special, incidental, 
+ *  or consequential damages of any character arising as a result of this License or 
+ *  out of the use or inability to use the software (including but not limited to damages 
+ *  for loss of goodwill, work stoppage, computer failure or malfunction, or any and 
+ *  all other commercial damages or losses), even if such Contributor has been advised 
+ *  of the possibility of such damages.
+ */
+
+#pragma once
+
+#include "DeviceObjectBase.h"
+#include "RenderDeviceVkImpl.h"
+#include "VulkanUtilities/VulkanObjectWrappers.h"
+
+namespace Diligent
+{
+
+struct SemaphoreObjectDesc : DeviceObjectAttribs
+{};
+
+class SemaphoreObject : public DeviceObjectBase<IDeviceObject, RenderDeviceVkImpl, SemaphoreObjectDesc>
+{
+public:
+    using TDeviceObjectBase = DeviceObjectBase<IDeviceObject, RenderDeviceVkImpl, SemaphoreObjectDesc>;
+
+    SemaphoreObject(IReferenceCounters*        pRefCounters,
+                    RenderDeviceVkImpl*        pDevice,
+                    const SemaphoreObjectDesc& ObjDesc,
+                    bool                       bIsDeviceInternal = false) :
+        TDeviceObjectBase{pRefCounters, pDevice, ObjDesc, bIsDeviceInternal}
+    {
+        const auto& LogicalDevice = m_pDevice->GetLogicalDevice();
+
+        VkSemaphoreCreateInfo SemaphoreCI = {};
+
+        SemaphoreCI.sType = VK_STRUCTURE_TYPE_SEMAPHORE_CREATE_INFO;
+        SemaphoreCI.pNext = nullptr;
+        SemaphoreCI.flags = 0; // reserved for future use
+
+        m_VkSemaphore = LogicalDevice.CreateSemaphore(SemaphoreCI, m_Desc.Name);
+    }
+
+    ~SemaphoreObject()
+    {
+        m_pDevice->SafeReleaseDeviceObject(std::move(m_VkSemaphore), ~Uint64{0});
+    }
+
+    static void Create(RenderDeviceVkImpl* pDevice, const char* Name, SemaphoreObject** ppSemaphore)
+    {
+        SemaphoreObjectDesc Desc;
+        Desc.Name = Name;
+        auto* pSemaphoreObj(NEW_RC_OBJ(GetRawAllocator(), "SemaphoreObject instance", SemaphoreObject)(pDevice, Desc));
+        pSemaphoreObj->QueryInterface(IID_DeviceObject, reinterpret_cast<IObject**>(ppSemaphore));
+    }
+
+    VkSemaphore GetVkSemaphore() const
+    {
+        return m_VkSemaphore;
+    }
+
+private:
+    VulkanUtilities::SemaphoreWrapper m_VkSemaphore;
+};
+
+} // namespace Diligent
diff --git a/Graphics/GraphicsEngineVulkan/include/SwapChainVkImpl.h b/Graphics/GraphicsEngineVulkan/include/SwapChainVkImpl.h
index 88c3e392..ca3bc981 100644
--- a/Graphics/GraphicsEngineVulkan/include/SwapChainVkImpl.h
+++ b/Graphics/GraphicsEngineVulkan/include/SwapChainVkImpl.h
@@ -30,6 +30,7 @@
 #include "SwapChainBase.h"
 #include "VulkanUtilities/VulkanInstance.h"
 #include "VulkanUtilities/VulkanObjectWrappers.h"
+#include "SemaphoreObject.h"
 
 namespace Diligent
 {
@@ -90,9 +91,9 @@ private:
     VkSwapchainKHR m_VkSwapChain   = VK_NULL_HANDLE;
     VkFormat       m_VkColorFormat = VK_FORMAT_UNDEFINED;
 
-    std::vector<VulkanUtilities::SemaphoreWrapper> m_ImageAcquiredSemaphores;
-    std::vector<VulkanUtilities::SemaphoreWrapper> m_DrawCompleteSemaphores;
-    std::vector<VulkanUtilities::FenceWrapper>     m_ImageAcquiredFences;
+    std::vector<RefCntAutoPtr<SemaphoreObject>> m_ImageAcquiredSemaphores;
+    std::vector<RefCntAutoPtr<SemaphoreObject>> m_DrawCompleteSemaphores;
+    std::vector<VulkanUtilities::FenceWrapper>  m_ImageAcquiredFences;
 
     std::vector<RefCntAutoPtr<ITextureViewVk>, STDAllocatorRawMem<RefCntAutoPtr<ITextureViewVk>>> m_pBackBufferRTV;
 
diff --git a/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp
index 330fdcb9..07443698 100644
--- a/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp
+++ b/Graphics/GraphicsEngineVulkan/src/DeviceContextVkImpl.cpp
@@ -876,12 +876,15 @@ void DeviceContextVkImpl::Flush()
         }
     }
 
+    VERIFY_EXPR(m_VkWaitSemaphores.size() == m_WaitSemaphores.size());
+    VERIFY_EXPR(m_VkSignalSemaphores.size() == m_SignalSemaphores.size());
+
     SubmitInfo.waitSemaphoreCount = static_cast<uint32_t>(m_WaitSemaphores.size());
     VERIFY_EXPR(m_WaitSemaphores.size() == m_WaitDstStageMasks.size());
-    SubmitInfo.pWaitSemaphores      = SubmitInfo.waitSemaphoreCount != 0 ? m_WaitSemaphores.data() : nullptr;
+    SubmitInfo.pWaitSemaphores      = SubmitInfo.waitSemaphoreCount != 0 ? m_VkWaitSemaphores.data() : nullptr;
     SubmitInfo.pWaitDstStageMask    = SubmitInfo.waitSemaphoreCount != 0 ? m_WaitDstStageMasks.data() : nullptr;
     SubmitInfo.signalSemaphoreCount = static_cast<uint32_t>(m_SignalSemaphores.size());
-    SubmitInfo.pSignalSemaphores    = SubmitInfo.signalSemaphoreCount != 0 ? m_SignalSemaphores.data() : nullptr;
+    SubmitInfo.pSignalSemaphores    = SubmitInfo.signalSemaphoreCount != 0 ? m_VkSignalSemaphores.data() : nullptr;
 
     // Submit command buffer even if there are no commands to release stale resources.
     //if (SubmitInfo.commandBufferCount != 0 || SubmitInfo.waitSemaphoreCount !=0 || SubmitInfo.signalSemaphoreCount != 0)
@@ -890,6 +893,8 @@ void DeviceContextVkImpl::Flush()
     m_WaitSemaphores.clear();
     m_WaitDstStageMasks.clear();
     m_SignalSemaphores.clear();
+    m_VkWaitSemaphores.clear();
+    m_VkSignalSemaphores.clear();
     m_PendingFences.clear();
 
     if (vkCmdBuff != VK_NULL_HANDLE)
diff --git a/Graphics/GraphicsEngineVulkan/src/SwapChainVkImpl.cpp b/Graphics/GraphicsEngineVulkan/src/SwapChainVkImpl.cpp
index cf4536b5..4fa9a523 100644
--- a/Graphics/GraphicsEngineVulkan/src/SwapChainVkImpl.cpp
+++ b/Graphics/GraphicsEngineVulkan/src/SwapChainVkImpl.cpp
@@ -350,15 +350,19 @@ void SwapChainVkImpl::CreateVulkanSwapChain()
     m_ImageAcquiredFences.resize(swapchainImageCount);
     for (uint32_t i = 0; i < swapchainImageCount; ++i)
     {
-        VkSemaphoreCreateInfo SemaphoreCI = {};
-
-        SemaphoreCI.sType = VK_STRUCTURE_TYPE_SEMAPHORE_CREATE_INFO;
-        SemaphoreCI.pNext = nullptr;
-        SemaphoreCI.flags = 0; // reserved for future use
-
-        m_ImageAcquiredSemaphores[i] = LogicalDevice.CreateSemaphore(SemaphoreCI);
-        m_DrawCompleteSemaphores[i]  = LogicalDevice.CreateSemaphore(SemaphoreCI);
+        {
+            std::stringstream ss;
+            ss << "Swap chain image acquired semaphore " << i;
+            auto Name = ss.str();
+            SemaphoreObject::Create(pRenderDeviceVk, Name.c_str(), &m_ImageAcquiredSemaphores[i]);
+        }
 
+        {
+            std::stringstream ss;
+            ss << "Swap chain draw complete semaphore " << i;
+            auto Name = ss.str();
+            SemaphoreObject::Create(pRenderDeviceVk, Name.c_str(), &m_DrawCompleteSemaphores[i]);
+        }
 
         VkFenceCreateInfo FenceCI = {};
 
@@ -500,7 +504,7 @@ VkResult SwapChainVkImpl::AcquireNextImage(DeviceContextVkImpl* pDeviceCtxVk)
     }
 
     VkFence     ImageAcquiredFence     = m_ImageAcquiredFences[m_SemaphoreIndex];
-    VkSemaphore ImageAcquiredSemaphore = m_ImageAcquiredSemaphores[m_SemaphoreIndex];
+    VkSemaphore ImageAcquiredSemaphore = m_ImageAcquiredSemaphores[m_SemaphoreIndex]->GetVkSemaphore();
 
     auto res = vkAcquireNextImageKHR(LogicalDevice.GetVkDevice(), m_VkSwapChain, UINT64_MAX, ImageAcquiredSemaphore, ImageAcquiredFence, &m_BackBufferIndex);
 
@@ -564,7 +568,7 @@ void SwapChainVkImpl::Present(Uint32 SyncInterval)
         PresentInfo.pNext              = nullptr;
         PresentInfo.waitSemaphoreCount = 1;
         // Unlike fences or events, the act of waiting for a semaphore also unsignals that semaphore (6.4.2)
-        VkSemaphore WaitSemaphore[] = {m_DrawCompleteSemaphores[m_SemaphoreIndex]};
+        VkSemaphore WaitSemaphore[] = {m_DrawCompleteSemaphores[m_SemaphoreIndex]->GetVkSemaphore()};
         PresentInfo.pWaitSemaphores = WaitSemaphore;
         PresentInfo.swapchainCount  = 1;
         PresentInfo.pSwapchains     = &m_VkSwapChain;