From 0ad2827e14a1015918fe2ec865d2f6ab630ab352 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 11 Oct 2023 09:04:28 +0200 Subject: [PATCH 1/2] Vulkan: Fix synchronization when shutting the GPU down in-game. --- Common/GPU/Vulkan/VulkanQueueRunner.cpp | 1 + Common/GPU/Vulkan/VulkanRenderManager.cpp | 12 ++++++++++++ Common/GPU/Vulkan/VulkanRenderManager.h | 6 +----- GPU/Vulkan/DrawEngineVulkan.cpp | 24 ++++++++++------------- GPU/Vulkan/GPU_Vulkan.cpp | 2 ++ UI/EmuScreen.cpp | 15 ++++++++------ UI/EmuScreen.h | 2 +- Windows/EmuThread.cpp | 2 ++ ext/rcheevos | 2 +- 9 files changed, 39 insertions(+), 27 deletions(-) diff --git a/Common/GPU/Vulkan/VulkanQueueRunner.cpp b/Common/GPU/Vulkan/VulkanQueueRunner.cpp index 99df5d242fab..023a588936b0 100644 --- a/Common/GPU/Vulkan/VulkanQueueRunner.cpp +++ b/Common/GPU/Vulkan/VulkanQueueRunner.cpp @@ -1244,6 +1244,7 @@ void VulkanQueueRunner::PerformRenderPass(const VKRStep &step, VkCommandBuffer c vkCmdBindPipeline(cmd, VK_PIPELINE_BIND_POINT_GRAPHICS, pipeline); descSets = &c.graphics_pipeline.pipelineLayout->frameData[curFrame].descSets_; pipelineLayout = c.graphics_pipeline.pipelineLayout->pipelineLayout; + _dbg_assert_(pipelineLayout != VK_NULL_HANDLE); lastGraphicsPipeline = graphicsPipeline; pipelineOK = true; } else { diff --git a/Common/GPU/Vulkan/VulkanRenderManager.cpp b/Common/GPU/Vulkan/VulkanRenderManager.cpp index e9a75b56d59e..df2e656c8962 100644 --- a/Common/GPU/Vulkan/VulkanRenderManager.cpp +++ b/Common/GPU/Vulkan/VulkanRenderManager.cpp @@ -413,6 +413,8 @@ VulkanRenderManager::~VulkanRenderManager() { vulkan_->WaitUntilQueueIdle(); + _dbg_assert_(pipelineLayouts_.empty()); + VkDevice device = vulkan_->GetDevice(); frameDataShared_.Destroy(vulkan_); for (int i = 0; i < inflightFramesAtStart_; i++) { @@ -519,12 +521,14 @@ void VulkanRenderManager::CompileThreadFunc() { } void VulkanRenderManager::DrainAndBlockCompileQueue() { + EndCurRenderStep(); std::unique_lock lock(compileMutex_); compileBlocked_ = true; compileCond_.notify_all(); while (!compileQueue_.empty()) { queueRunner_.WaitForCompileNotification(); } + FlushSync(); } void VulkanRenderManager::ReleaseCompileQueue() { @@ -1538,6 +1542,8 @@ void VulkanRenderManager::Run(VKRRenderThreadTask &task) { // Called from main thread. void VulkanRenderManager::FlushSync() { + _dbg_assert_(!curRenderStep_); + if (invalidationCallback_) { invalidationCallback_(InvalidationCallbackFlags::COMMAND_BUFFER_STATE); } @@ -1669,6 +1675,7 @@ void VulkanRenderManager::DestroyPipelineLayout(VKRPipelineLayout *layout) { break; } } + delete layout; } void VulkanRenderManager::FlushDescriptors(int frame) { @@ -1687,6 +1694,11 @@ void VulkanRenderManager::ResetDescriptorLists(int frame) { } } +VKRPipelineLayout::~VKRPipelineLayout() { + _assert_(!pipelineLayout && !descriptorSetLayout); + _assert_(frameData[0].pool.IsDestroyed()); +} + void VKRPipelineLayout::FlushDescSets(VulkanContext *vulkan, int frame, QueueProfileContext *profile) { _dbg_assert_(frame < VulkanContext::MAX_INFLIGHT_FRAMES); diff --git a/Common/GPU/Vulkan/VulkanRenderManager.h b/Common/GPU/Vulkan/VulkanRenderManager.h index f63aba59b4ae..c38de4956d2d 100644 --- a/Common/GPU/Vulkan/VulkanRenderManager.h +++ b/Common/GPU/Vulkan/VulkanRenderManager.h @@ -210,11 +210,7 @@ struct PackedDescriptor { // Note that we only support a single descriptor set due to compatibility with some ancient devices. // We should probably eventually give that up. struct VKRPipelineLayout { - VKRPipelineLayout() {} - ~VKRPipelineLayout() { - _assert_(!pipelineLayout && !descriptorSetLayout); - _assert_(frameData[0].pool.IsDestroyed()); - } + ~VKRPipelineLayout(); enum { MAX_DESC_SET_BINDINGS = 10 }; BindingType bindingTypes[MAX_DESC_SET_BINDINGS]; diff --git a/GPU/Vulkan/DrawEngineVulkan.cpp b/GPU/Vulkan/DrawEngineVulkan.cpp index e2b456727733..a31d26340460 100644 --- a/GPU/Vulkan/DrawEngineVulkan.cpp +++ b/GPU/Vulkan/DrawEngineVulkan.cpp @@ -254,11 +254,8 @@ void DrawEngineVulkan::DoFlush() { const bool forceIndexed = draw_->GetDeviceCaps().verySlowShaderCompiler; if (useHWTransform) { - int vertexCount = 0; - bool useElements = true; VkBuffer vbuf = VK_NULL_HANDLE; VkBuffer ibuf = VK_NULL_HANDLE; - bool useIndexGen = true; if (decOptions_.applySkinInDecode && (lastVType_ & GE_VTYPE_WEIGHT_MASK)) { // If software skinning, we're predecoding into "decoded". So make sure we're done, then push that content. DecodeVerts(decoded_); @@ -272,18 +269,17 @@ void DrawEngineVulkan::DoFlush() { DecodeInds(); gpuStats.numUncachedVertsDrawn += indexGen.VertexCount(); - if (useIndexGen) { - vertexCount = indexGen.VertexCount(); - if (forceIndexed) { - useElements = true; - prim = indexGen.GeneralPrim(); - } else { - useElements = !indexGen.SeenOnlyPurePrims(); - if (!useElements && indexGen.PureCount()) { - vertexCount = indexGen.PureCount(); - } - prim = indexGen.Prim(); + bool useElements; + int vertexCount = indexGen.VertexCount(); + if (forceIndexed) { + useElements = true; + prim = indexGen.GeneralPrim(); + } else { + useElements = !indexGen.SeenOnlyPurePrims(); + if (!useElements && indexGen.PureCount()) { + vertexCount = indexGen.PureCount(); } + prim = indexGen.Prim(); } bool hasColor = (lastVType_ & GE_VTYPE_COL_MASK) != GE_VTYPE_COL_NONE; diff --git a/GPU/Vulkan/GPU_Vulkan.cpp b/GPU/Vulkan/GPU_Vulkan.cpp index 2eefea2bdbf3..2d977f66d7cf 100644 --- a/GPU/Vulkan/GPU_Vulkan.cpp +++ b/GPU/Vulkan/GPU_Vulkan.cpp @@ -167,6 +167,7 @@ void GPU_Vulkan::SaveCache(const Path &filename) { GPU_Vulkan::~GPU_Vulkan() { if (draw_) { VulkanRenderManager *rm = (VulkanRenderManager *)draw_->GetNativeObject(Draw::NativeObject::RENDER_MANAGER); + // This now also does a hard sync with the render thread, so that we can safely delete our pipeline layout below. rm->DrainAndBlockCompileQueue(); } @@ -423,6 +424,7 @@ void GPU_Vulkan::CheckRenderResized() { void GPU_Vulkan::DeviceLost() { // draw_ is normally actually still valid here in Vulkan. But we null it out in GPUCommonHW::DeviceLost so we don't try to use it again. + // So, we have to save it here to be able to call ReleaseCompileQueue(). Draw::DrawContext *draw = draw_; if (draw) { VulkanRenderManager *rm = (VulkanRenderManager *)draw->GetNativeObject(Draw::NativeObject::RENDER_MANAGER); diff --git a/UI/EmuScreen.cpp b/UI/EmuScreen.cpp index 4841be7622bc..f1c7371af590 100644 --- a/UI/EmuScreen.cpp +++ b/UI/EmuScreen.cpp @@ -1177,7 +1177,7 @@ void EmuScreen::update() { } } -void EmuScreen::checkPowerDown() { +bool EmuScreen::checkPowerDown() { if (PSP_IsRebooting()) { bootPending_ = true; invalid_ = true; @@ -1191,7 +1191,9 @@ void EmuScreen::checkPowerDown() { screenManager()->switchScreen(new MainScreen()); bootPending_ = false; invalid_ = true; + return true; } + return false; } static const char *CPUCoreAsString(int core) { @@ -1454,6 +1456,7 @@ void EmuScreen::render() { Core_UpdateDebugStats((DebugOverlay)g_Config.iDebugOverlay == DebugOverlay::DEBUG_STATS || g_Config.bLogFrameDrops); bool blockedExecution = Achievements::IsBlockingExecution(); + bool rebind = false; if (!blockedExecution) { PSP_BeginHostFrame(); PSP_RunLoopWhileState(); @@ -1490,7 +1493,7 @@ void EmuScreen::render() { // Didn't actually reach the end of the frame, ran out of the blockTicks cycles. // In this case we need to bind and wipe the backbuffer, at least. // It's possible we never ended up outputted anything - make sure we have the backbuffer cleared - thin3d->BindFramebufferAsRenderTarget(nullptr, { RPAction::CLEAR, RPAction::CLEAR, RPAction::CLEAR }, "EmuScreen_NoFrame"); + rebind = true; break; } @@ -1498,10 +1501,10 @@ void EmuScreen::render() { } // This must happen after PSP_EndHostFrame so that things like push buffers are end-frame'd before we start destroying stuff. - checkPowerDown(); - - if (invalid_) - return; + if (checkPowerDown() || rebind) { + // Shutting down can end up ending the current render pass + thin3d->BindFramebufferAsRenderTarget(nullptr, { RPAction::CLEAR, RPAction::CLEAR, RPAction::CLEAR }, "EmuScreen_NoFrame"); + } if (hasVisibleUI()) { // In most cases, this should already be bound and a no-op. diff --git a/UI/EmuScreen.h b/UI/EmuScreen.h index a2f47630d12e..cfc7370b9ba3 100644 --- a/UI/EmuScreen.h +++ b/UI/EmuScreen.h @@ -76,7 +76,7 @@ class EmuScreen : public UIScreen { void onVKeyAnalog(int virtualKeyCode, float value); void autoLoad(); - void checkPowerDown(); + bool checkPowerDown(); UI::Event OnDevMenu; UI::Event OnChatMenu; diff --git a/Windows/EmuThread.cpp b/Windows/EmuThread.cpp index 71af97e683f7..e0ae538222a0 100644 --- a/Windows/EmuThread.cpp +++ b/Windows/EmuThread.cpp @@ -335,6 +335,8 @@ void MainThreadFunc() { g_graphicsContext->Shutdown(); + delete g_graphicsContext; + UpdateConsolePosition(); NativeShutdown(); diff --git a/ext/rcheevos b/ext/rcheevos index 6f02e791aa20..24dc84ca2dfb 160000 --- a/ext/rcheevos +++ b/ext/rcheevos @@ -1 +1 @@ -Subproject commit 6f02e791aa202de5eaf3e86ed269dda6d456b779 +Subproject commit 24dc84ca2dfbdbddebc9a26f4df225888e56a4f4 From f769b2c8a35502e6a38604991425ee9fd0d44259 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 11 Oct 2023 12:16:51 +0200 Subject: [PATCH 2/2] Remove unused functionality from descpool --- Common/GPU/Vulkan/VulkanDescSet.cpp | 3 --- Common/GPU/Vulkan/VulkanDescSet.h | 13 ++----------- Common/GPU/Vulkan/VulkanRenderManager.cpp | 1 - Common/GPU/Vulkan/VulkanRenderManager.h | 1 + GPU/Vulkan/VulkanUtil.h | 4 +--- 5 files changed, 4 insertions(+), 18 deletions(-) diff --git a/Common/GPU/Vulkan/VulkanDescSet.cpp b/Common/GPU/Vulkan/VulkanDescSet.cpp index cb85dd8425ec..906fc2209918 100644 --- a/Common/GPU/Vulkan/VulkanDescSet.cpp +++ b/Common/GPU/Vulkan/VulkanDescSet.cpp @@ -81,7 +81,6 @@ void VulkanDescSetPool::Reset() { _assert_msg_(descPool_ != VK_NULL_HANDLE, "VulkanDescSetPool::Reset without valid pool"); vkResetDescriptorPool(vulkan_->GetDevice(), descPool_, 0); - clear_(); usage_ = 0; } @@ -90,7 +89,6 @@ void VulkanDescSetPool::Destroy() { if (descPool_ != VK_NULL_HANDLE) { vulkan_->Delete().QueueDeleteDescriptorPool(descPool_); - clear_(); usage_ = 0; } sizes_.clear(); @@ -110,7 +108,6 @@ VkResult VulkanDescSetPool::Recreate(bool grow) { if (descPool_ != VK_NULL_HANDLE) { INFO_LOG(G3D, "Reallocating %s desc pool from %d to %d", tag_, prevSize, info_.maxSets); vulkan_->Delete().QueueDeleteDescriptorPool(descPool_); - clear_(); usage_ = 0; } diff --git a/Common/GPU/Vulkan/VulkanDescSet.h b/Common/GPU/Vulkan/VulkanDescSet.h index 3ed4673240a7..014d3aeaaa9a 100644 --- a/Common/GPU/Vulkan/VulkanDescSet.h +++ b/Common/GPU/Vulkan/VulkanDescSet.h @@ -3,7 +3,6 @@ #include "Common/Data/Collections/FastVec.h" #include "Common/GPU/Vulkan/VulkanContext.h" -#include #include enum class BindingType { @@ -18,23 +17,16 @@ enum class BindingType { // Only appropriate for use in a per-frame pool. class VulkanDescSetPool { public: - VulkanDescSetPool(const char *tag = "", bool grow = true) : tag_(tag), grow_(grow) {} + VulkanDescSetPool(const char *tag, bool grow = true) : tag_(tag), grow_(grow) {} ~VulkanDescSetPool(); - // Must call this before use: defines how to clear cache of ANY returned values from Allocate(). - void Setup(const std::function &clear) { - clear_ = clear; - } void Create(VulkanContext *vulkan, const BindingType *bindingTypes, uint32_t bindingTypesCount, uint32_t descriptorCount); // Allocate a new set, which may resize and empty the current sets. - // Use only for the current frame, unless in a cache cleared by clear_. + // Use only for the current frame. bool Allocate(VkDescriptorSet *descriptorSets, int count, const VkDescriptorSetLayout *layouts); void Reset(); void Destroy(); - void SetTag(const char *tag) { - tag_ = tag; - } bool IsDestroyed() const { return !descPool_; } @@ -47,7 +39,6 @@ class VulkanDescSetPool { VkDescriptorPool descPool_ = VK_NULL_HANDLE; VkDescriptorPoolCreateInfo info_{}; std::vector sizes_; - std::function clear_; uint32_t usage_ = 0; bool grow_; }; diff --git a/Common/GPU/Vulkan/VulkanRenderManager.cpp b/Common/GPU/Vulkan/VulkanRenderManager.cpp index df2e656c8962..ae8f523a81bf 100644 --- a/Common/GPU/Vulkan/VulkanRenderManager.cpp +++ b/Common/GPU/Vulkan/VulkanRenderManager.cpp @@ -1655,7 +1655,6 @@ VKRPipelineLayout *VulkanRenderManager::CreatePipelineLayout(BindingType *bindin for (int i = 0; i < VulkanContext::MAX_INFLIGHT_FRAMES; i++) { // Some games go beyond 1024 and end up having to resize like GTA, but most stay below so we start there. layout->frameData[i].pool.Create(vulkan_, bindingTypes, (uint32_t)bindingTypesCount, 1024); - layout->frameData[i].pool.Setup([]() {}); } pipelineLayouts_.push_back(layout); diff --git a/Common/GPU/Vulkan/VulkanRenderManager.h b/Common/GPU/Vulkan/VulkanRenderManager.h index c38de4956d2d..9a3372d3e2d8 100644 --- a/Common/GPU/Vulkan/VulkanRenderManager.h +++ b/Common/GPU/Vulkan/VulkanRenderManager.h @@ -221,6 +221,7 @@ struct VKRPipelineLayout { const char *tag = nullptr; struct FrameData { + FrameData() : pool("GameDescPool", true) {} VulkanDescSetPool pool; FastVec descData_; FastVec descSets_; diff --git a/GPU/Vulkan/VulkanUtil.h b/GPU/Vulkan/VulkanUtil.h index f45c816b128c..7cadc957f5ee 100644 --- a/GPU/Vulkan/VulkanUtil.h +++ b/GPU/Vulkan/VulkanUtil.h @@ -72,9 +72,7 @@ class VulkanComputeShaderManager { VkPipelineCache pipelineCache_ = VK_NULL_HANDLE; struct FrameData { - FrameData() : descPool("VulkanComputeShaderManager", true) { - descPool.Setup([] { }); - } + FrameData() : descPool("VulkanComputeShaderManager", true) {} VulkanDescSetPool descPool; bool descPoolUsed = false;