diff --git a/Common/GPU/Vulkan/VulkanQueueRunner.cpp b/Common/GPU/Vulkan/VulkanQueueRunner.cpp index 0e9620d474..0b4ca51983 100644 --- a/Common/GPU/Vulkan/VulkanQueueRunner.cpp +++ b/Common/GPU/Vulkan/VulkanQueueRunner.cpp @@ -1281,21 +1281,29 @@ void VulkanQueueRunner::PerformRenderPass(const VKRStep &step, VkCommandBuffer c "expected %d sample count, got %d", fbSampleCount, graphicsPipeline->SampleCount()); } - VkPipeline pipeline; - + Promise *promise; + bool compileHere = false; { std::lock_guard lock(graphicsPipeline->mutex_); - if (!graphicsPipeline->pipeline[(size_t)rpType]) { + promise = graphicsPipeline->pipeline[(size_t)rpType]; + if (!promise) { // NOTE: If render steps got merged, it can happen that, as they ended during recording, // they didn't know their final render pass type so they created the wrong pipelines in EndCurRenderStep(). // Unfortunately I don't know if we can fix it in any more sensible place than here. // Maybe a middle pass. But let's try to just block and compile here for now, this doesn't // happen all that much. - graphicsPipeline->pipeline[(size_t)rpType] = Promise::CreateEmpty(); - graphicsPipeline->Create(vulkan_, renderPass->Get(vulkan_, rpType, fbSampleCount), rpType, fbSampleCount, time_now_d(), -1); + promise = Promise::CreateEmpty(); + graphicsPipeline->pipeline[(size_t)rpType] = promise; + compileHere = true; } - pipeline = graphicsPipeline->pipeline[(size_t)rpType]->BlockUntilReady(); } + // Not under the lock: the main thread takes it for every pipeline a render step used + // (EndCurRenderStep), and waiting for the shaders and the compile can take hundreds of ms. + // DestroyVariants deletes Promises later, so this one stays valid. + if (compileHere) { + graphicsPipeline->Create(vulkan_, renderPass->Get(vulkan_, rpType, fbSampleCount), rpType, fbSampleCount, time_now_d(), -1); + } + VkPipeline pipeline = promise->BlockUntilReady(); if (pipeline != VK_NULL_HANDLE) { vkCmdBindPipeline(cmd, VK_PIPELINE_BIND_POINT_GRAPHICS, pipeline); diff --git a/Common/GPU/Vulkan/VulkanRenderManager.cpp b/Common/GPU/Vulkan/VulkanRenderManager.cpp index 81b5860801..033db37843 100644 --- a/Common/GPU/Vulkan/VulkanRenderManager.cpp +++ b/Common/GPU/Vulkan/VulkanRenderManager.cpp @@ -171,8 +171,8 @@ bool VKRGraphicsPipeline::Create(VulkanContext *vulkan, VkRenderPass compatibleR void VKRGraphicsPipeline::DestroyVariants(VulkanContext *vulkan, bool msaaOnly) { // Called from InvalidateMSAAPipelines on the main thread, mid-frame, while the render thread may be // reading and replacing these same slots in PerformRenderPass - so take the lock that's documented - // as protecting the array. It also has to be held across the delete below, or the render thread can - // be left holding a freed Promise. + // as protecting the array. The render thread waits for a Promise without the lock, so the Promise + // is deleted later, like the VkPipeline. std::lock_guard lock(mutex_); for (size_t i = 0; i < (size_t)RenderPassType::TYPE_COUNT; i++) { @@ -188,7 +188,9 @@ void VKRGraphicsPipeline::DestroyVariants(VulkanContext *vulkan, bool msaaOnly) } // The array owns the Promise - DestroyVariantsInstant deletes it too. Forgetting it here leaked // one per destroyed variant on every MSAA or resolution change. - delete this->pipeline[i]; + vulkan->Delete().QueueCallback([promise = this->pipeline[i]](VulkanContext *vulkan) { + delete promise; + }); this->pipeline[i] = nullptr; } sampleCount_ = VK_SAMPLE_COUNT_FLAG_BITS_MAX_ENUM;