Merge pull request #10049 from unknownbrackets/vulkan-minor

Vulkan threading tweaks and minor

Common/Vulkan/VulkanContext.cpp

@@ -137,6 +137,7 @@ VkResult VulkanContext::CreateInstance(const char *app_name, int app_ver, uint32
 	if (res != VK_SUCCESS) {
 		init_error_ = "Failed to enumerate physical devices";
 		vkDestroyInstance(instance_, nullptr);
+		instance_ = nullptr;
 		return res;
 	}
 

Common/Vulkan/VulkanContext.h

@@ -378,7 +378,7 @@ private:
 
 	std::vector<VkDebugReportCallbackEXT> msg_callbacks;
 
-	VkSwapchainKHR swapchain_;
+	VkSwapchainKHR swapchain_ = VK_NULL_HANDLE;
 	VkFormat swapchainFormat_;
 
 	uint32_t queue_count = 0;

GPU/Common/FramebufferCommon.cpp

@@ -838,7 +838,7 @@ void FramebufferManagerCommon::SetViewport2D(int x, int y, int w, int h) {
 }
 
 void FramebufferManagerCommon::CopyDisplayToOutput() {
-	// DownloadFramebufferOnSwitch(currentRenderVfb_);
+	DownloadFramebufferOnSwitch(currentRenderVfb_);
 
 	currentRenderVfb_ = 0;
 
@@ -1073,7 +1073,7 @@ void FramebufferManagerCommon::DecimateFBOs() {
 	currentRenderVfb_ = 0;
 
 	for (auto iter : fbosToDelete_) {
-		delete iter;
+		iter->Release();
 	}
 	fbosToDelete_.clear();
 

UI/EmuScreen.cpp

@@ -1038,6 +1038,9 @@ void EmuScreen::render() {
 	if (!osm.IsEmpty() || g_Config.bShowDebugStats || g_Config.iShowFPSCounter || g_Config.bShowTouchControls || g_Config.bShowDeveloperMenu || g_Config.bShowAudioDebug || saveStatePreview_->GetVisibility() != UI::V_GONE || g_Config.bShowFrameProfiler) {
 		DrawContext *thin3d = screenManager()->getDrawContext();
 
+		// It's possible we never ended up outputted anything - make sure we have the backbuffer.
+		thin3d->BindFramebufferAsRenderTarget(nullptr, { RPAction::KEEP, RPAction::KEEP });
+
 		// This sets up some important states but not the viewport.
 		screenManager()->getUIContext()->Begin();
 

Windows/GPU/WindowsVulkanContext.cpp

@@ -208,7 +208,8 @@ bool WindowsVulkanContext::Init(HINSTANCE hInst, HWND hWnd, std::string *error_m
 }
 
 void WindowsVulkanContext::Shutdown() {
-	draw_->HandleEvent(Draw::Event::LOST_BACKBUFFER, g_Vulkan->GetBackbufferWidth(), g_Vulkan->GetBackbufferHeight());
+	if (draw_)
+		draw_->HandleEvent(Draw::Event::LOST_BACKBUFFER, g_Vulkan->GetBackbufferWidth(), g_Vulkan->GetBackbufferHeight());
 
 	delete draw_;
 	draw_ = nullptr;
@@ -227,7 +228,6 @@ void WindowsVulkanContext::SwapBuffers() {
 }
 
 void WindowsVulkanContext::Resize() {
-	g_Vulkan->WaitUntilQueueIdle();
 	draw_->HandleEvent(Draw::Event::LOST_BACKBUFFER, g_Vulkan->GetBackbufferWidth(), g_Vulkan->GetBackbufferHeight());
 	g_Vulkan->DestroyObjects();
 

android/jni/app-android.cpp

@@ -341,7 +341,6 @@ void AndroidVulkanContext::SwapBuffers() {
 }
 
 void AndroidVulkanContext::Resize() {
-	g_Vulkan->WaitUntilQueueIdle();
 	draw_->HandleEvent(Draw::Event::LOST_BACKBUFFER, g_Vulkan->GetBackbufferWidth(), g_Vulkan->GetBackbufferHeight());
 	g_Vulkan->DestroyObjects();
 

ext/native/thin3d/VulkanQueueRunner.cpp

@@ -326,6 +326,7 @@ void VulkanQueueRunner::PerformRenderPass(const VKRStep &step, VkCommandBuffer c
 
 			vkCmdPipelineBarrier(cmd, srcStage, dstStage, 0, 0, nullptr, 0, nullptr, 1, &barrier);
 			iter.fb->color.layout = barrier.newLayout;
+			iter.fb->Release();
 		}
 	}
 
@@ -573,6 +574,10 @@ void VulkanQueueRunner::PerformBindFramebufferAsRenderTarget(const VKRStep &step
 	rp_begin.clearValueCount = numClearVals;
 	rp_begin.pClearValues = numClearVals ? clearVal : nullptr;
 	vkCmdBeginRenderPass(cmd, &rp_begin, VK_SUBPASS_CONTENTS_INLINE);
+
+	if (step.render.framebuffer) {
+		step.render.framebuffer->Release();
+	}
 }
 
 void VulkanQueueRunner::PerformCopy(const VKRStep &step, VkCommandBuffer cmd) {
@@ -646,6 +651,9 @@ void VulkanQueueRunner::PerformCopy(const VKRStep &step, VkCommandBuffer cmd) {
 		}
 		vkCmdCopyImage(cmd, src->depth.image, src->depth.layout, dst->depth.image, dst->depth.layout, 1, &copy);
 	}
+
+	src->Release();
+	dst->Release();
 }
 
 void VulkanQueueRunner::PerformBlit(const VKRStep &step, VkCommandBuffer cmd) {
@@ -729,6 +737,9 @@ void VulkanQueueRunner::PerformBlit(const VKRStep &step, VkCommandBuffer cmd) {
 		}
 		vkCmdBlitImage(cmd, src->depth.image, src->depth.layout, dst->depth.image, dst->depth.layout, 1, &blit, step.blit.filter);
 	}
+
+	src->Release();
+	dst->Release();
 }
 
 void VulkanQueueRunner::SetupTransitionToTransferSrc(VKRImage &img, VkImageMemoryBarrier &barrier, VkPipelineStageFlags &stage, VkImageAspectFlags aspect) {
@@ -831,6 +842,8 @@ void VulkanQueueRunner::PerformReadback(const VKRStep &step, VkCommandBuffer cmd
 	vkCmdCopyImageToBuffer(cmd, srcImage->image, srcImage->layout, readbackBuffer_, 1, &region);
 
 	// NOTE: Can't read the buffer using the CPU here - need to sync first.
+
+	step.readback.src->Release();
 }
 
 void VulkanQueueRunner::CopyReadbackBuffer(int width, int height, Draw::DataFormat destFormat, int pixelStride, uint8_t *pixels) {

ext/native/thin3d/VulkanRenderManager.cpp

@@ -96,6 +96,16 @@ void CreateImage(VulkanContext *vulkan, VkCommandBuffer cmd, VKRImage &img, int
 	img.format = format;
 }
 
+bool VKRFramebuffer::Release() {
+	if (--refcount_ == 0) {
+		delete this;
+		return true;
+	} else if (refcount_ >= 10000 || refcount_ < 0) {
+		ELOG("Refcount (%d) invalid for object %p - corrupt?", refcount_.load(), this);
+	}
+	return false;
+}
+
 VulkanRenderManager::VulkanRenderManager(VulkanContext *vulkan) : vulkan_(vulkan), queueRunner_(vulkan) {
 	VkSemaphoreCreateInfo semaphoreCreateInfo = { VK_STRUCTURE_TYPE_SEMAPHORE_CREATE_INFO };
 	semaphoreCreateInfo.flags = 0;
@@ -180,26 +190,45 @@ void VulkanRenderManager::CreateBackbuffers() {
 	// Start the thread.
 	if (useThread) {
 		run_ = true;
+		// Won't necessarily be 0.
+		threadInitFrame_ = vulkan_->GetCurFrame();
 		thread_ = std::thread(&VulkanRenderManager::ThreadFunc, this);
 	}
 }
 
-void VulkanRenderManager::DestroyBackbuffers() {
-	if (useThread) {
+void VulkanRenderManager::StopThread(bool shutdown) {
+	if (useThread && run_) {
 		run_ = false;
 		// Stop the thread.
 		for (int i = 0; i < vulkan_->GetInflightFrames(); i++) {
+			auto &frameData = frameData_[i];
 			{
-				std::unique_lock<std::mutex> lock(frameData_[i].push_mutex);
-				frameData_[i].push_condVar.notify_all();
+				std::unique_lock<std::mutex> lock(frameData.push_mutex);
+				frameData.push_condVar.notify_all();
 			}
 			{
-				std::unique_lock<std::mutex> lock(frameData_[i].pull_mutex);
-				frameData_[i].pull_condVar.notify_all();
+				std::unique_lock<std::mutex> lock(frameData.pull_mutex);
+				frameData.pull_condVar.notify_all();
 			}
 		}
 		thread_.join();
+
+		// Resignal fences for next time around - must be done after join.
+		for (int i = 0; i < vulkan_->GetInflightFrames(); i++) {
+			auto &frameData = frameData_[i];
+			frameData.readyForRun = false;
+			if (!shutdown && !frameData.readyForFence) {
+				vkDestroyFence(vulkan_->GetDevice(), frameData.fence, nullptr);
+				frameData.fence = vulkan_->CreateFence(true);
+			}
+		}
 	}
+}
+
+void VulkanRenderManager::DestroyBackbuffers() {
+	StopThread(false);
+	vulkan_->WaitUntilQueueIdle();
+
 	VkDevice device = vulkan_->GetDevice();
 	for (uint32_t i = 0; i < swapchainImageCount_; i++) {
 		vulkan_->Delete().QueueDeleteImageView(swapchainImages_[i].view);
@@ -217,9 +246,10 @@ void VulkanRenderManager::DestroyBackbuffers() {
 }
 
 VulkanRenderManager::~VulkanRenderManager() {
-	run_ = false;
-	VkDevice device = vulkan_->GetDevice();
+	StopThread(true);
 	vulkan_->WaitUntilQueueIdle();
+
+	VkDevice device = vulkan_->GetDevice();
 	vkDestroySemaphore(device, acquireSemaphore_, nullptr);
 	vkDestroySemaphore(device, renderingCompleteSemaphore_, nullptr);
 	for (int i = 0; i < vulkan_->GetInflightFrames(); i++) {
@@ -236,12 +266,15 @@ VulkanRenderManager::~VulkanRenderManager() {
 // TODO: Activate this code.
 void VulkanRenderManager::ThreadFunc() {
 	setCurrentThreadName("RenderMan");
-	int threadFrame = -1;  // Increment first, start at 0.
+	int threadFrame = threadInitFrame_;
+	bool nextFrame = false;
 	while (run_) {
 		{
-			threadFrame++;
-			if (threadFrame >= vulkan_->GetInflightFrames())
-				threadFrame = 0;
+			if (nextFrame) {
+				threadFrame++;
+				if (threadFrame >= vulkan_->GetInflightFrames())
+					threadFrame = 0;
+			}
 			FrameData &frameData = frameData_[threadFrame];
 			std::unique_lock<std::mutex> lock(frameData.pull_mutex);
 			while (!frameData.readyForRun && run_) {
@@ -252,10 +285,16 @@ void VulkanRenderManager::ThreadFunc() {
 			frameData.readyForRun = false;
 			if (!run_)  // quick exit if bailing.
 				return;
+
+			// Only increment next time if we're done.
+			nextFrame = frameData.type == VKRRunType::END;
+			assert(frameData.type == VKRRunType::END || frameData.type == VKRRunType::SYNC);
 		}
 		VLOG("PULL: Running frame %d", threadFrame);
 		Run(threadFrame);
+		VLOG("PULL: Finished frame %d", threadFrame);
 	}
+	VLOG("PULL: Quitting");
 }
 
 void VulkanRenderManager::BeginFrame() {
@@ -334,6 +373,9 @@ void VulkanRenderManager::BindFramebufferAsRenderTarget(VKRFramebuffer *fb, VKRR
 	step->render.numDraws = 0;
 	step->render.finalColorLayout = !fb ? VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL : VK_IMAGE_LAYOUT_UNDEFINED;
 	steps_.push_back(step);
+	if (fb) {
+		fb->AddRef();
+	}
 
 	curRenderStep_ = step;
 	curWidth_ = fb ? fb->width : vulkan_->GetBackbufferWidth();
@@ -347,6 +389,7 @@ void VulkanRenderManager::CopyFramebufferToMemorySync(VKRFramebuffer *src, int a
 	step->readback.srcRect.offset = { x, y };
 	step->readback.srcRect.extent = { (uint32_t)w, (uint32_t)h };
 	steps_.push_back(step);
+	src->AddRef();
 
 	curRenderStep_ = nullptr;
 
@@ -488,6 +531,8 @@ void VulkanRenderManager::CopyFramebuffer(VKRFramebuffer *src, VkRect2D srcRect,
 	step->copy.srcRect = srcRect;
 	step->copy.dst = dst;
 	step->copy.dstPos = dstPos;
+	src->AddRef();
+	dst->AddRef();
 
 	std::unique_lock<std::mutex> lock(mutex_);
 	steps_.push_back(step);
@@ -513,6 +558,8 @@ void VulkanRenderManager::BlitFramebuffer(VKRFramebuffer *src, VkRect2D srcRect,
 	step->blit.dst = dst;
 	step->blit.dstRect = dstRect;
 	step->blit.filter = filter;
+	src->AddRef();
+	dst->AddRef();
 
 	std::unique_lock<std::mutex> lock(mutex_);
 	steps_.push_back(step);
@@ -536,6 +583,7 @@ VkImageView VulkanRenderManager::BindFramebufferAsTexture(VKRFramebuffer *fb, in
 	}
 
 	curRenderStep_->preTransitions.push_back({ fb, VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL });
+	fb->AddRef();
 	return fb->color.imageView;
 }
 
@@ -545,15 +593,19 @@ void VulkanRenderManager::Finish() {
 	FrameData &frameData = frameData_[curFrame];
 	if (!useThread) {
 		frameData.steps = std::move(steps_);
+		frameData.type = VKRRunType::END;
 		Run(curFrame);
 	} else {
 		std::unique_lock<std::mutex> lock(frameData.pull_mutex);
 		VLOG("PUSH: Frame[%d].readyForRun = true", curFrame);
 		frameData.steps = std::move(steps_);
 		frameData.readyForRun = true;
+		frameData.type = VKRRunType::END;
 		frameData.pull_condVar.notify_all();
 	}
 	vulkan_->EndFrame();
+
+	insideFrame_ = false;
 }
 
 void VulkanRenderManager::Wipe() {
@@ -640,7 +692,8 @@ void VulkanRenderManager::Submit(int frame, bool triggerFence) {
 		assert(res == VK_SUCCESS);
 	}
 
-	if (useThread) {
+	// When !triggerFence, we notify after syncing with Vulkan.
+	if (useThread && triggerFence) {
 		VLOG("PULL: Frame %d.readyForFence = true", frame);
 		std::unique_lock<std::mutex> lock(frameData.push_mutex);
 		frameData.readyForFence = true;
@@ -651,7 +704,6 @@ void VulkanRenderManager::Submit(int frame, bool triggerFence) {
 void VulkanRenderManager::EndSubmitFrame(int frame) {
 	FrameData &frameData = frameData_[frame];
 	frameData.hasBegun = false;
-	insideFrame_ = false;
 
 	Submit(frame, true);
 
@@ -674,8 +726,6 @@ void VulkanRenderManager::EndSubmitFrame(int frame) {
 }
 
 void VulkanRenderManager::Run(int frame) {
-	VkDevice device = vulkan_->GetDevice();
-
 	BeginSubmitFrame(frame);
 
 	FrameData &frameData = frameData_[frame];
@@ -685,23 +735,24 @@ void VulkanRenderManager::Run(int frame) {
 	queueRunner_.RunSteps(cmd, stepsOnThread);
 	stepsOnThread.clear();
 
-	EndSubmitFrame(frame);
+	switch (frameData.type) {
+	case VKRRunType::END:
+		EndSubmitFrame(frame);
+		break;
+
+	case VKRRunType::SYNC:
+		EndSyncFrame(frame);
+		break;
+
+	default:
+		assert(false);
+	}
 
 	VLOG("PULL: Finished running frame %d", frame);
 }
 
-void VulkanRenderManager::FlushSync() {
-	int frame = vulkan_->GetCurFrame();
-	BeginSubmitFrame(frame);
-
+void VulkanRenderManager::EndSyncFrame(int frame) {
 	FrameData &frameData = frameData_[frame];
-	frameData.steps = std::move(steps_);
-
-	auto &stepsOnThread = frameData_[frame].steps;
-	VkCommandBuffer cmd = frameData.mainCmd;
-	queueRunner_.RunSteps(cmd, stepsOnThread);
-	stepsOnThread.clear();
-
 	Submit(frame, false);
 
 	// This is brutal! Should probably wait for a fence instead, not that it'll matter much since we'll
@@ -718,4 +769,39 @@ void VulkanRenderManager::FlushSync() {
 	};
 	VkResult res = vkBeginCommandBuffer(frameData.mainCmd, &begin);
 	assert(res == VK_SUCCESS);
+
+	if (useThread) {
+		std::unique_lock<std::mutex> lock(frameData.push_mutex);
+		frameData.readyForFence = true;
+		frameData.push_condVar.notify_all();
+	}
+}
+
+void VulkanRenderManager::FlushSync() {
+	// TODO: Reset curRenderStep_?
+	int curFrame = vulkan_->GetCurFrame();
+	FrameData &frameData = frameData_[curFrame];
+	if (!useThread) {
+		frameData.steps = std::move(steps_);
+		frameData.type = VKRRunType::SYNC;
+		Run(curFrame);
+	} else {
+		std::unique_lock<std::mutex> lock(frameData.pull_mutex);
+		VLOG("PUSH: Frame[%d].readyForRun = true (sync)", curFrame);
+		frameData.steps = std::move(steps_);
+		frameData.readyForRun = true;
+		assert(frameData.readyForFence == false);
+		frameData.type = VKRRunType::SYNC;
+		frameData.pull_condVar.notify_all();
+	}
+
+	if (useThread) {
+		std::unique_lock<std::mutex> lock(frameData.push_mutex);
+		// Wait for the flush to be hit, since we're syncing.
+		while (!frameData.readyForFence) {
+			VLOG("PUSH: Waiting for frame[%d].readyForFence = 1 (sync)", curFrame);
+			frameData.push_condVar.wait(lock);
+		}
+		frameData.readyForFence = false;
+	}
 }

ext/native/thin3d/VulkanRenderManager.h

@@ -4,10 +4,11 @@
 // Only draws and binds are handled here, resource creation and allocations are handled as normal -
 // that's the nice thing with Vulkan.
 
-#include <cstdint>
-#include <thread>
-#include <mutex>
+#include <atomic>
 #include <condition_variable>
+#include <cstdint>
+#include <mutex>
+#include <thread>
 
 #include "Common/Vulkan/VulkanContext.h"
 #include "math/dataconv.h"
@@ -28,6 +29,7 @@ void CreateImage(VulkanContext *vulkan, VkCommandBuffer cmd, VKRImage &img, int
 class VKRFramebuffer {
 public:
 	VKRFramebuffer(VulkanContext *vk, VkCommandBuffer initCmd, VkRenderPass renderPass, int _width, int _height) : vulkan_(vk) {
+		refcount_ = 1;
 		width = _width;
 		height = _height;
 
@@ -58,6 +60,11 @@ public:
 		vulkan_->Delete().QueueDeleteFramebuffer(framebuf);
 	}
 
+	void AddRef() {
+		refcount_++;
+	}
+	bool Release();
+
 	int numShadows = 1;  // TODO: Support this.
 
 	VkFramebuffer framebuf = VK_NULL_HANDLE;
@@ -68,6 +75,12 @@ public:
 
 private:
 	VulkanContext *vulkan_;
+	std::atomic<int> refcount_;
+};
+
+enum class VKRRunType {
+	END,
+	SYNC,
 };
 
 class VulkanRenderManager {
@@ -206,6 +219,9 @@ private:
 
 	// Bad for performance but sometimes necessary for synchronous CPU readbacks (screenshots and whatnot).
 	void FlushSync();
+	void EndSyncFrame(int frame);
+
+	void StopThread(bool shutdown);
 
 	// Permanent objects
 	VkSemaphore acquireSemaphore_;
@@ -220,8 +236,9 @@ private:
 		std::condition_variable pull_condVar;
 
 		bool readyForFence = true;
-
 		bool readyForRun = false;
+		VKRRunType type = VKRRunType::END;
+
 		VkFence fence;
 		// These are on different threads so need separate pools.
 		VkCommandPool cmdPoolInit;
@@ -249,6 +266,7 @@ private:
 	VulkanContext *vulkan_;
 	std::thread thread_;
 	std::mutex mutex_;
+	int threadInitFrame_ = 0;
 	VulkanQueueRunner queueRunner_;
 
 	// Swap chain management

ext/native/thin3d/thin3d_vulkan.cpp

@@ -1248,9 +1248,10 @@ uint32_t VKContext::GetDataFormatSupport(DataFormat fmt) const {
 // use this frame's init command buffer.
 class VKFramebuffer : public Framebuffer {
 public:
+	// Inherits ownership so no AddRef.
 	VKFramebuffer(VKRFramebuffer *fb) : buf_(fb) {}
 	~VKFramebuffer() {
-		delete buf_;
+		buf_->Release();
 	}
 	VKRFramebuffer *GetFB() const { return buf_; }
 private: