From d7e2edeaa20a38e3d6e741b11e36ac2a66afda2d Mon Sep 17 00:00:00 2001 From: Scott Mansell Date: Mon, 23 Jan 2023 19:51:12 +1300 Subject: [PATCH] PresentWait: Correctly handle swapchain destruction Also, reimplmented as WorkQueueThread. --- .../Core/VideoBackends/Vulkan/PresentWait.cpp | 69 +++++++++---------- .../Core/VideoBackends/Vulkan/PresentWait.h | 3 + Source/Core/VideoBackends/Vulkan/VKMain.cpp | 5 +- .../Core/VideoBackends/Vulkan/VKSwapChain.cpp | 9 +++ 4 files changed, 47 insertions(+), 39 deletions(-) diff --git a/Source/Core/VideoBackends/Vulkan/PresentWait.cpp b/Source/Core/VideoBackends/Vulkan/PresentWait.cpp index 33ad7b096f..1e7d63d06b 100644 --- a/Source/Core/VideoBackends/Vulkan/PresentWait.cpp +++ b/Source/Core/VideoBackends/Vulkan/PresentWait.cpp @@ -7,75 +7,70 @@ #include #include -#include "Common/BlockingLoop.h" +#include "Common/WorkQueueThread.h" #include "VideoBackends/Vulkan/VulkanContext.h" #include "VideoBackends/Vulkan/VulkanLoader.h" #include "VideoCommon/PerformanceMetrics.h" -#include - namespace Vulkan { +static VkDevice s_device; -static std::thread s_present_wait_thread; -static Common::BlockingLoop s_present_wait_loop; - -static std::deque> s_present_wait_queue; - -static void PresentWaitThreadFunc() +struct Wait { - VkDevice device = g_vulkan_context->GetDevice(); + u64 present_id; + VkSwapchainKHR swapchain; +}; - s_present_wait_loop.Run([device]() { - if (s_present_wait_queue.empty()) - { - s_present_wait_loop.AllowSleep(); - return; - } - u64 present_id; - VkSwapchainKHR swapchain; - std::tie(present_id, swapchain) = s_present_wait_queue.back(); +static Common::WorkQueueThread s_present_wait_thread; - auto start = std::chrono::high_resolution_clock::now(); - - VkResult res = vkWaitForPresentKHR(device, swapchain, present_id, 100'000'000); // 100ms +void WaitFunction(Wait wait) +{ + do + { + // We choose a timeout of 20ms so can poll for IsFlushing + VkResult res = vkWaitForPresentKHR(s_device, wait.swapchain, wait.present_id, 20'000'000); if (res == VK_TIMEOUT) { - WARN_LOG_FMT(VIDEO, "vkWaitForPresentKHR timed out, retrying {}", present_id); - return; + WARN_LOG_FMT(VIDEO, "vkWaitForPresentKHR timed out, retrying {}", wait.present_id); + continue; } - s_present_wait_queue.pop_back(); - if (res != VK_SUCCESS) { LOG_VULKAN_ERROR(res, "vkWaitForPresentKHR failed: "); } if (res == VK_SUCCESS) - { - auto end = std::chrono::high_resolution_clock::now(); - auto duration = std::chrono::duration_cast(end - start); - fmt::print("vkWaitForPresentKHR took {}us\n", duration.count()); - g_perf_metrics.CountPresent(); - } - }); + + return; + } while (!s_present_wait_thread.IsCancelling()); } void StartPresentWaitThread() { - fmt::print("Starting PresentWaitThread"); - s_present_wait_thread = std::thread(PresentWaitThreadFunc); + s_device = g_vulkan_context->GetDevice(); + s_present_wait_thread.Reset("PresentWait", WaitFunction); +} + +void StopPresentWaitThread() +{ + s_present_wait_thread.Shutdown(); } void PresentQueued(u64 present_id, VkSwapchainKHR swapchain) { - s_present_wait_queue.emplace_front(std::make_tuple(present_id, swapchain)); - s_present_wait_loop.Wakeup(); + s_present_wait_thread.EmplaceItem(Wait{present_id, swapchain}); +} + +void FlushPresentWaitQueue() +{ + s_present_wait_thread.Cancel(); + s_present_wait_thread.WaitForCompletion(); } } // namespace Vulkan diff --git a/Source/Core/VideoBackends/Vulkan/PresentWait.h b/Source/Core/VideoBackends/Vulkan/PresentWait.h index 399f1cb8b3..8a650f8838 100644 --- a/Source/Core/VideoBackends/Vulkan/PresentWait.h +++ b/Source/Core/VideoBackends/Vulkan/PresentWait.h @@ -10,6 +10,9 @@ namespace Vulkan { void StartPresentWaitThread(); +void StopPresentWaitThread(); + void PresentQueued(u64 present_id, VkSwapchainKHR swapchain); +void FlushPresentWaitQueue(); } // namespace Vulkan diff --git a/Source/Core/VideoBackends/Vulkan/VKMain.cpp b/Source/Core/VideoBackends/Vulkan/VKMain.cpp index f362f28838..77c6a29374 100644 --- a/Source/Core/VideoBackends/Vulkan/VKMain.cpp +++ b/Source/Core/VideoBackends/Vulkan/VKMain.cpp @@ -228,9 +228,7 @@ bool VideoBackend::Initialize(const WindowSystemInfo& wsi) } if (g_vulkan_context->SupportsPresentWait()) - { StartPresentWaitThread(); - } } if (!StateTracker::CreateInstance()) @@ -251,6 +249,9 @@ bool VideoBackend::Initialize(const WindowSystemInfo& wsi) void VideoBackend::Shutdown() { + if (g_vulkan_context->SupportsPresentWait()) + StopPresentWaitThread(); + if (g_vulkan_context) vkDeviceWaitIdle(g_vulkan_context->GetDevice()); diff --git a/Source/Core/VideoBackends/Vulkan/VKSwapChain.cpp b/Source/Core/VideoBackends/Vulkan/VKSwapChain.cpp index 48102df64d..570da779a5 100644 --- a/Source/Core/VideoBackends/Vulkan/VKSwapChain.cpp +++ b/Source/Core/VideoBackends/Vulkan/VKSwapChain.cpp @@ -13,6 +13,7 @@ #include "VideoBackends/Vulkan/CommandBufferManager.h" #include "VideoBackends/Vulkan/ObjectCache.h" +#include "VideoBackends/Vulkan/PresentWait.h" #include "VideoBackends/Vulkan/VKTexture.h" #include "VideoBackends/Vulkan/VulkanContext.h" #include "VideoCommon/Present.h" @@ -332,6 +333,11 @@ bool SwapChain::CreateSwapChain() VkSwapchainKHR old_swap_chain = m_swap_chain; m_swap_chain = VK_NULL_HANDLE; + // vkWaitForPresentKHR uses a copy of swapchain, we need to make sure it's flushed + // before passing it as the old swapchain arg of vkCreateSwapchainKHR + if (old_swap_chain && g_vulkan_context->SupportsPresentWait()) + FlushPresentWaitQueue(); + // Now we can actually create the swap chain VkSwapchainCreateInfoKHR swap_chain_info = {VK_STRUCTURE_TYPE_SWAPCHAIN_CREATE_INFO_KHR, nullptr, @@ -487,6 +493,9 @@ void SwapChain::DestroySwapChain() if (m_current_fullscreen_state) SetFullscreenState(false); + if (g_vulkan_context->SupportsPresentWait()) + FlushPresentWaitQueue(); + vkDestroySwapchainKHR(g_vulkan_context->GetDevice(), m_swap_chain, nullptr); m_swap_chain = VK_NULL_HANDLE; }