From 2655627a4bac5c0adf6d5ecd99a175e0e4c4bc18 Mon Sep 17 00:00:00 2001 From: Jupeyy Date: Fri, 10 May 2024 17:39:57 +0200 Subject: [PATCH 1/3] Rename `m_CurFrames` to make clear it's about the sync objects. Use one more sync object than there are inflight frames. Remove unused memory sync objects. --- .../client/backend/vulkan/backend_vulkan.cpp | 40 ++++++++----------- 1 file changed, 17 insertions(+), 23 deletions(-) diff --git a/src/engine/client/backend/vulkan/backend_vulkan.cpp b/src/engine/client/backend/vulkan/backend_vulkan.cpp index bd98079d6..131ab3850 100644 --- a/src/engine/client/backend/vulkan/backend_vulkan.cpp +++ b/src/engine/client/backend/vulkan/backend_vulkan.cpp @@ -986,8 +986,6 @@ private: std::vector m_vWaitSemaphores; std::vector m_vSigSemaphores; - std::vector m_vMemorySemaphores; - std::vector m_vFrameFences; std::vector m_vImagesFences; @@ -1053,7 +1051,7 @@ private: std::vector> m_vStreamedVertexBuffers; std::vector> m_vStreamedUniformBuffers; - uint32_t m_CurFrames = 0; + uint32_t m_CurFrameSyncObject = 0; uint32_t m_CurImageIndex = 0; uint32_t m_CanvasWidth; @@ -2275,7 +2273,7 @@ protected: return false; } - VkSemaphore WaitSemaphore = m_vWaitSemaphores[m_CurFrames]; + VkSemaphore WaitSemaphore = m_vWaitSemaphores[m_CurFrameSyncObject]; VkSubmitInfo SubmitInfo{}; SubmitInfo.sType = VK_STRUCTURE_TYPE_SUBMIT_INFO; @@ -2304,13 +2302,13 @@ protected: SubmitInfo.pWaitSemaphores = aWaitSemaphores.data(); SubmitInfo.pWaitDstStageMask = aWaitStages.data(); - std::array aSignalSemaphores = {m_vSigSemaphores[m_CurFrames]}; + std::array aSignalSemaphores = {m_vSigSemaphores[m_CurFrameSyncObject]}; SubmitInfo.signalSemaphoreCount = aSignalSemaphores.size(); SubmitInfo.pSignalSemaphores = aSignalSemaphores.data(); - vkResetFences(m_VKDevice, 1, &m_vFrameFences[m_CurFrames]); + vkResetFences(m_VKDevice, 1, &m_vFrameFences[m_CurFrameSyncObject]); - VkResult QueueSubmitRes = vkQueueSubmit(m_VKGraphicsQueue, 1, &SubmitInfo, m_vFrameFences[m_CurFrames]); + VkResult QueueSubmitRes = vkQueueSubmit(m_VKGraphicsQueue, 1, &SubmitInfo, m_vFrameFences[m_CurFrameSyncObject]); if(QueueSubmitRes != VK_SUCCESS) { const char *pCritErrorMsg = CheckVulkanCriticalError(QueueSubmitRes); @@ -2321,7 +2319,7 @@ protected: } } - std::swap(m_vWaitSemaphores[m_CurFrames], m_vSigSemaphores[m_CurFrames]); + std::swap(m_vWaitSemaphores[m_CurFrameSyncObject], m_vSigSemaphores[m_CurFrameSyncObject]); VkPresentInfoKHR PresentInfo{}; PresentInfo.sType = VK_STRUCTURE_TYPE_PRESENT_INFO_KHR; @@ -2348,7 +2346,7 @@ protected: } } - m_CurFrames = (m_CurFrames + 1) % m_SwapChainImageCount; + m_CurFrameSyncObject = (m_CurFrameSyncObject + 1) % m_vWaitSemaphores.size(); return true; } @@ -2364,7 +2362,7 @@ protected: RecreateSwapChain(); } - auto AcqResult = vkAcquireNextImageKHR(m_VKDevice, m_VKSwapChain, std::numeric_limits::max(), m_vSigSemaphores[m_CurFrames], VK_NULL_HANDLE, &m_CurImageIndex); + auto AcqResult = vkAcquireNextImageKHR(m_VKDevice, m_VKSwapChain, std::numeric_limits::max(), m_vSigSemaphores[m_CurFrameSyncObject], VK_NULL_HANDLE, &m_CurImageIndex); if(AcqResult != VK_SUCCESS) { if(AcqResult == VK_ERROR_OUT_OF_DATE_KHR || m_RecreateSwapChain) @@ -2395,13 +2393,13 @@ protected: } } } - std::swap(m_vWaitSemaphores[m_CurFrames], m_vSigSemaphores[m_CurFrames]); + std::swap(m_vWaitSemaphores[m_CurFrameSyncObject], m_vSigSemaphores[m_CurFrameSyncObject]); if(m_vImagesFences[m_CurImageIndex] != VK_NULL_HANDLE) { vkWaitForFences(m_VKDevice, 1, &m_vImagesFences[m_CurImageIndex], VK_TRUE, std::numeric_limits::max()); } - m_vImagesFences[m_CurImageIndex] = m_vFrameFences[m_CurFrames]; + m_vImagesFences[m_CurImageIndex] = m_vFrameFences[m_CurFrameSyncObject]; // next frame m_CurFrame++; @@ -5333,12 +5331,12 @@ public: [[nodiscard]] bool CreateSyncObjects() { - m_vWaitSemaphores.resize(m_SwapChainImageCount); - m_vSigSemaphores.resize(m_SwapChainImageCount); + // Create one more sync object than there are frames in flight + auto SyncObjectCount = m_SwapChainImageCount + 1; + m_vWaitSemaphores.resize(SyncObjectCount); + m_vSigSemaphores.resize(SyncObjectCount); - m_vMemorySemaphores.resize(m_SwapChainImageCount); - - m_vFrameFences.resize(m_SwapChainImageCount); + m_vFrameFences.resize(SyncObjectCount); m_vImagesFences.resize(m_SwapChainImageCount, VK_NULL_HANDLE); VkSemaphoreCreateInfo CreateSemaphoreInfo{}; @@ -5348,11 +5346,10 @@ public: FenceInfo.sType = VK_STRUCTURE_TYPE_FENCE_CREATE_INFO; FenceInfo.flags = VK_FENCE_CREATE_SIGNALED_BIT; - for(size_t i = 0; i < m_SwapChainImageCount; i++) + for(size_t i = 0; i < SyncObjectCount; i++) { if(vkCreateSemaphore(m_VKDevice, &CreateSemaphoreInfo, nullptr, &m_vWaitSemaphores[i]) != VK_SUCCESS || vkCreateSemaphore(m_VKDevice, &CreateSemaphoreInfo, nullptr, &m_vSigSemaphores[i]) != VK_SUCCESS || - vkCreateSemaphore(m_VKDevice, &CreateSemaphoreInfo, nullptr, &m_vMemorySemaphores[i]) != VK_SUCCESS || vkCreateFence(m_VKDevice, &FenceInfo, nullptr, &m_vFrameFences[i]) != VK_SUCCESS) { SetError(EGfxErrorType::GFX_ERROR_TYPE_INIT, "Creating swap chain sync objects(fences, semaphores) failed."); @@ -5365,19 +5362,16 @@ public: void DestroySyncObjects() { - for(size_t i = 0; i < m_SwapChainImageCount; i++) + for(size_t i = 0; i < m_vWaitSemaphores.size(); i++) { vkDestroySemaphore(m_VKDevice, m_vWaitSemaphores[i], nullptr); vkDestroySemaphore(m_VKDevice, m_vSigSemaphores[i], nullptr); - vkDestroySemaphore(m_VKDevice, m_vMemorySemaphores[i], nullptr); vkDestroyFence(m_VKDevice, m_vFrameFences[i], nullptr); } m_vWaitSemaphores.clear(); m_vSigSemaphores.clear(); - m_vMemorySemaphores.clear(); - m_vFrameFences.clear(); m_vImagesFences.clear(); } From 128302d7268750084865d574483bd4ba90c7f25e Mon Sep 17 00:00:00 2001 From: Jupeyy Date: Fri, 10 May 2024 18:00:50 +0200 Subject: [PATCH 2/3] Cleanup Vulkan now takes `SwapchainCount` as explicit parameter. This fixes a crash if the swapchain count changed. fixes #8328 --- .../client/backend/vulkan/backend_vulkan.cpp | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/engine/client/backend/vulkan/backend_vulkan.cpp b/src/engine/client/backend/vulkan/backend_vulkan.cpp index 131ab3850..1ad571b6a 100644 --- a/src/engine/client/backend/vulkan/backend_vulkan.cpp +++ b/src/engine/client/backend/vulkan/backend_vulkan.cpp @@ -5374,6 +5374,8 @@ public: m_vFrameFences.clear(); m_vImagesFences.clear(); + + m_CurFrameSyncObject = 0; } void DestroyBufferOfFrame(size_t ImageIndex, SFrameBuffers &Buffer) @@ -5427,7 +5429,7 @@ public: } template - void CleanupVulkan() + void CleanupVulkan(size_t SwapchainCount) { if(IsLastCleanup) { @@ -5465,7 +5467,7 @@ public: m_vStreamedVertexBuffers.clear(); m_vStreamedUniformBuffers.clear(); - for(size_t i = 0; i < m_SwapChainImageCount; ++i) + for(size_t i = 0; i < SwapchainCount; ++i) { ClearFrameData(i); } @@ -5474,11 +5476,11 @@ public: m_vvFrameDelayedTextureCleanup.clear(); m_vvFrameDelayedTextTexturesCleanup.clear(); - m_StagingBufferCache.DestroyFrameData(m_SwapChainImageCount); - m_StagingBufferCacheImage.DestroyFrameData(m_SwapChainImageCount); - m_VertexBufferCache.DestroyFrameData(m_SwapChainImageCount); + m_StagingBufferCache.DestroyFrameData(SwapchainCount); + m_StagingBufferCacheImage.DestroyFrameData(SwapchainCount); + m_VertexBufferCache.DestroyFrameData(SwapchainCount); for(auto &ImageBufferCache : m_ImageBufferCaches) - ImageBufferCache.second.DestroyFrameData(m_SwapChainImageCount); + ImageBufferCache.second.DestroyFrameData(SwapchainCount); if(IsLastCleanup) { @@ -5556,7 +5558,7 @@ public: if(OldSwapChainImageCount != m_SwapChainImageCount) { - CleanupVulkan(); + CleanupVulkan(OldSwapChainImageCount); InitVulkan(); } @@ -6620,7 +6622,7 @@ public: DestroyIndexBuffer(m_IndexBuffer, m_IndexBufferMemory); DestroyIndexBuffer(m_RenderIndexBuffer, m_RenderIndexBufferMemory); - CleanupVulkan(); + CleanupVulkan(m_SwapChainImageCount); return true; } From 15afc50865a948db7b2b829844105f58e0472801 Mon Sep 17 00:00:00 2001 From: Jupeyy Date: Fri, 10 May 2024 18:22:26 +0200 Subject: [PATCH 3/3] Don't recreate DescriptorSetLayouts if swapchain image count changes --- .../client/backend/vulkan/backend_vulkan.cpp | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/engine/client/backend/vulkan/backend_vulkan.cpp b/src/engine/client/backend/vulkan/backend_vulkan.cpp index 1ad571b6a..3fdcd9b95 100644 --- a/src/engine/client/backend/vulkan/backend_vulkan.cpp +++ b/src/engine/client/backend/vulkan/backend_vulkan.cpp @@ -6112,20 +6112,20 @@ public: template int InitVulkan() { - if(!CreateDescriptorSetLayouts()) - return -1; - - if(!CreateTextDescriptorSetLayout()) - return -1; - - if(!CreateSpriteMultiUniformDescriptorSetLayout()) - return -1; - - if(!CreateQuadUniformDescriptorSetLayout()) - return -1; - if(IsFirstInitialization) { + if(!CreateDescriptorSetLayouts()) + return -1; + + if(!CreateTextDescriptorSetLayout()) + return -1; + + if(!CreateSpriteMultiUniformDescriptorSetLayout()) + return -1; + + if(!CreateQuadUniformDescriptorSetLayout()) + return -1; + VkSwapchainKHR OldSwapChain = VK_NULL_HANDLE; if(InitVulkanSwapChain(OldSwapChain) != 0) return -1;