From b87aed937cc854fd36a1d0d23b8617118ba448a1 Mon Sep 17 00:00:00 2001 From: Sasha Szpakowski Date: Fri, 20 Dec 2024 23:37:06 -0400 Subject: [PATCH] vulkan: clean up framebuffers when textures they reference are deleted Fixes corruption/crashing in some situations if a new texture's image view has the same pointer as a previously deleted texture's image view. --- src/modules/graphics/vulkan/Graphics.cpp | 93 ++++++++++++++---------- src/modules/graphics/vulkan/Graphics.h | 4 +- src/modules/graphics/vulkan/Texture.cpp | 9 ++- 3 files changed, 63 insertions(+), 43 deletions(-) diff --git a/src/modules/graphics/vulkan/Graphics.cpp b/src/modules/graphics/vulkan/Graphics.cpp index b9b185242..e696c7773 100644 --- a/src/modules/graphics/vulkan/Graphics.cpp +++ b/src/modules/graphics/vulkan/Graphics.cpp @@ -59,8 +59,6 @@ static const std::vector deviceExtensions = { VK_KHR_SWAPCHAIN_EXTENSION_NAME, }; -constexpr uint32_t USAGES_POLL_INTERVAL = 5000; - constexpr int DEFAULT_VERTEX_BUFFER_BINDING = 0; constexpr int VERTEX_BUFFER_BINDING_START = 1; @@ -577,7 +575,6 @@ void Graphics::present(void *screenshotCallbackdata) updatePendingReadbacks(); updateTemporaryResources(); - frameCounter++; currentFrame = (currentFrame + 1) % MAX_FRAMES_IN_FLIGHT; beginFrame(); @@ -703,7 +700,6 @@ bool Graphics::setMode(void *context, int width, int height, int pixelwidth, int createQuadIndexBuffer(); createFanIndexBuffer(); - frameCounter = 0; currentFrame = 0; } @@ -1314,12 +1310,6 @@ void Graphics::beginFrame() { vkWaitForFences(device, 1, &inFlightFences[currentFrame], VK_TRUE, UINT64_MAX); - if (frameCounter >= USAGES_POLL_INTERVAL) - { - cleanupUnusedObjects(); - frameCounter = 0; - } - if (swapChain != VK_NULL_HANDLE) { while (true) @@ -2152,6 +2142,54 @@ VkFramebuffer Graphics::getFramebuffer(FramebufferConfiguration &configuration) return framebuffer; } +void Graphics::cleanupFramebuffers(VkImageView imageView, PixelFormat format) +{ + bool depthstencil = isPixelFormatDepthStencil(format); + + for (auto it = framebuffers.begin(); it != framebuffers.end();) + { + bool foundView = false; + + if (depthstencil) + { + if (it->first.staticData.depthView == imageView) + foundView = true; + } + else + { + for (VkImageView view : it->first.colorViews) + { + if (view == imageView) + { + foundView = true; + break; + } + } + if (!foundView) + { + for (VkImageView view : it->first.colorResolveViews) + { + if (view == imageView) + { + foundView = true; + break; + } + } + } + } + + if (foundView) + { + vkDestroyFramebuffer(device, it->second, nullptr); + it = framebuffers.erase(it); + } + else + { + ++it; + } + } +} + void Graphics::createDefaultShaders() { for (int i = 0; i < Shader::STANDARD_MAX_ENUM; i++) @@ -2737,36 +2775,6 @@ VkSampler Graphics::createSampler(const SamplerState &samplerState) return sampler; } -template -static void eraseUnusedObjects( - std::unordered_map &objects, - std::unordered_map &usages, - Deleter deleter, - VkDevice device) -{ - std::vector deletionKeys; - - for (const auto &entry : objects) - { - if (!usages[entry.second]) - { - deletionKeys.push_back(entry.first); - usages.erase(entry.second); - deleter(device, entry.second, nullptr); - } - else - usages[entry.second] = false; - } - - for (const auto &key : deletionKeys) - objects.erase(key); -} - -void Graphics::cleanupUnusedObjects() -{ - eraseUnusedObjects(framebuffers, framebufferUsages, vkDestroyFramebuffer, device); -} - void Graphics::requestSwapchainRecreation() { if (swapChain != VK_NULL_HANDLE) @@ -3242,16 +3250,21 @@ void Graphics::cleanupSwapChain() { if (colorImage) { + cleanupFramebuffers(colorImageView, swapChainPixelFormat); vkDestroyImageView(device, colorImageView, nullptr); vmaDestroyImage(vmaAllocator, colorImage, colorImageAllocation); } if (depthImage) { + cleanupFramebuffers(depthImageView, swapChainPixelFormat); vkDestroyImageView(device, depthImageView, nullptr); vmaDestroyImage(vmaAllocator, depthImage, depthImageAllocation); } for (const auto &swapChainImageView : swapChainImageViews) + { + cleanupFramebuffers(swapChainImageView, swapChainPixelFormat); vkDestroyImageView(device, swapChainImageView, nullptr); + } swapChainImageViews.clear(); vkDestroySwapchainKHR(device, swapChain, nullptr); swapChainImages.clear(); diff --git a/src/modules/graphics/vulkan/Graphics.h b/src/modules/graphics/vulkan/Graphics.h index 47fbaebed..c4c1ec143 100644 --- a/src/modules/graphics/vulkan/Graphics.h +++ b/src/modules/graphics/vulkan/Graphics.h @@ -281,6 +281,8 @@ public: int getVsync() const; void mapLocalUniformData(void *data, size_t size, VkDescriptorBufferInfo &bufferInfo); + void cleanupFramebuffers(VkImageView imageView, PixelFormat format); + VkPipeline createGraphicsPipeline(Shader *shader, const GraphicsPipelineConfigurationCore &configuration, const GraphicsPipelineConfigurationNoDynamicState *noDynamicStateConfiguration); uint32 getDeviceApiVersion() const { return deviceApiVersion; } @@ -346,7 +348,6 @@ private: void endRenderPass(); void applyScissor(); VkSampler createSampler(const SamplerState &sampler); - void cleanupUnusedObjects(); void requestSwapchainRecreation(); VkInstance instance = VK_NULL_HANDLE; @@ -389,7 +390,6 @@ private: int vsync = 1; VkDeviceSize minUniformBufferOffsetAlignment = 0; bool imageRequested = false; - uint32_t frameCounter = 0; size_t currentFrame = 0; uint32_t imageIndex = 0; bool swapChainRecreationRequested = false; diff --git a/src/modules/graphics/vulkan/Texture.cpp b/src/modules/graphics/vulkan/Texture.cpp index ed2821094..b2c1f219a 100644 --- a/src/modules/graphics/vulkan/Texture.cpp +++ b/src/modules/graphics/vulkan/Texture.cpp @@ -301,9 +301,16 @@ void Texture::unloadVolatile() vgfx->queueCleanUp([ device = device, allocator = allocator, - imageData = std::move(*data)] () { + imageData = *data, + vgfx = vgfx, + renderTarget = renderTarget, + format = format] () { if (imageData.imageView != VK_NULL_HANDLE) + { + if (renderTarget) + vgfx->cleanupFramebuffers(imageData.imageView, format); vkDestroyImageView(device, imageData.imageView, nullptr); + } if (imageData.allocation != VK_NULL_HANDLE) vmaDestroyImage(allocator, imageData.image, imageData.allocation); for (const auto &views : imageData.renderTargetImageViews)