Merge pull request #18344 from hrydgard/shutdown-fixes

Vulkan: Fix synchronization when shutting the app down in-game.

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;
 	}
 

Common/GPU/Vulkan/VulkanDescSet.h

@@ -3,7 +3,6 @@
 #include "Common/Data/Collections/FastVec.h"
 #include "Common/GPU/Vulkan/VulkanContext.h"
 
-#include <functional>
 #include <vector>
 
 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<void()> &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<VkDescriptorPoolSize> sizes_;
-	std::function<void()> clear_;
 	uint32_t usage_ = 0;
 	bool grow_;
 };

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 {

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<std::mutex> 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);
 	}
@@ -1649,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);
@@ -1669,6 +1674,7 @@ void VulkanRenderManager::DestroyPipelineLayout(VKRPipelineLayout *layout) {
 			break;
 		}
 	}
+	delete layout;
 }
 
 void VulkanRenderManager::FlushDescriptors(int frame) {
@@ -1687,6 +1693,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);
 

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];
 
@@ -225,6 +221,7 @@ struct VKRPipelineLayout {
 	const char *tag = nullptr;
 
 	struct FrameData {
+		FrameData() : pool("GameDescPool", true) {}
 		VulkanDescSetPool pool;
 		FastVec<PackedDescriptor> descData_;
 		FastVec<PendingDescSet> descSets_;

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;

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);

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;

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,18 +1493,18 @@ 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;
 		}
 
 		PSP_EndHostFrame();
 	}
 
 	// 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.

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;

Windows/EmuThread.cpp

@@ -335,6 +335,8 @@ void MainThreadFunc() {
 
 	g_graphicsContext->Shutdown();
 
+	delete g_graphicsContext;
+
 	UpdateConsolePosition();
 	NativeShutdown();