From 331386951acbf6a3b7b8ac90b5803038e4f5854d Mon Sep 17 00:00:00 2001 From: assiduous Date: Mon, 30 Dec 2019 18:43:16 -0800 Subject: Vulkan backend: fixed issue with swap chain semaphores being destroyed while still in use --- Graphics/GraphicsEngineVulkan/CMakeLists.txt | 1 + .../include/DeviceContextVkImpl.h | 22 ++++-- .../GraphicsEngineVulkan/include/SemaphoreObject.h | 80 ++++++++++++++++++++++ .../GraphicsEngineVulkan/include/SwapChainVkImpl.h | 7 +- .../src/DeviceContextVkImpl.cpp | 9 ++- .../GraphicsEngineVulkan/src/SwapChainVkImpl.cpp | 24 ++++--- 6 files changed, 121 insertions(+), 22 deletions(-) create mode 100644 Graphics/GraphicsEngineVulkan/include/SemaphoreObject.h (limited to 'Graphics/GraphicsEngineVulkan') 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 m_WaitSemaphores; - std::vector m_WaitDstStageMasks; - std::vector m_SignalSemaphores; + std::vector> m_WaitSemaphores; + std::vector m_WaitDstStageMasks; + std::vector> m_SignalSemaphores; + + std::vector m_VkWaitSemaphores; + std::vector m_VkSignalSemaphores; // List of fences to signal next time the command context is flushed std::vector>> 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 +{ +public: + using TDeviceObjectBase = DeviceObjectBase; + + 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(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 m_ImageAcquiredSemaphores; - std::vector m_DrawCompleteSemaphores; - std::vector m_ImageAcquiredFences; + std::vector> m_ImageAcquiredSemaphores; + std::vector> m_DrawCompleteSemaphores; + std::vector m_ImageAcquiredFences; std::vector, STDAllocatorRawMem>> 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(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(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; -- cgit v1.2.3