Merge pull request #16090 from hrydgard/more-vulkan-cleanup-work

Simplify synchronization in VulkanRenderManager

Common/GPU/Vulkan/VulkanFrameData.cpp

@@ -1,3 +1,5 @@
+#include <mutex>
+
 #include "VulkanFrameData.h"
 #include "Common/Log.h"
 
@@ -26,6 +28,7 @@ void FrameData::Init(VulkanContext *vulkan, int index) {
 
 	// Creating the frame fence with true so they can be instantly waited on the first frame
 	fence = vulkan->CreateFence(true);
+	readyForFence = true;
 
 	// This fence one is used for synchronizing readbacks. Does not need preinitialization.
 	readbackFence = vulkan->CreateFence(false);
@@ -169,7 +172,18 @@ void FrameData::SubmitPending(VulkanContext *vulkan, FrameSubmitType type, Frame
 		submit_info.signalSemaphoreCount = 1;
 		submit_info.pSignalSemaphores = &sharedData.renderingCompleteSemaphore;
 	}
-	VkResult res = vkQueueSubmit(vulkan->GetGraphicsQueue(), 1, &submit_info, fenceToTrigger);
+
+	VkResult res;
+	if (fenceToTrigger == fence) {
+		// The fence is waited on by the main thread, they are not allowed to access it simultaneously.
+		res = vkQueueSubmit(vulkan->GetGraphicsQueue(), 1, &submit_info, fenceToTrigger);
+		std::lock_guard<std::mutex> lock(fenceMutex);
+		readyForFence = true;
+		fenceCondVar.notify_one();
+	} else {
+		res = vkQueueSubmit(vulkan->GetGraphicsQueue(), 1, &submit_info, fenceToTrigger);
+	}
+
 	if (res == VK_ERROR_DEVICE_LOST) {
 		_assert_msg_(false, "Lost the Vulkan device in vkQueueSubmit! If this happens again, switch Graphics Backend away from Vulkan");
 	} else {
@@ -180,14 +194,7 @@ void FrameData::SubmitPending(VulkanContext *vulkan, FrameSubmitType type, Frame
 		// Hard stall of the GPU, not ideal, but necessary so the CPU has the contents of the readback.
 		vkWaitForFences(vulkan->GetDevice(), 1, &readbackFence, true, UINT64_MAX);
 		vkResetFences(vulkan->GetDevice(), 1, &readbackFence);
-	}
-
-	// When !triggerFence, we notify after syncing with Vulkan.
-	if (type == FrameSubmitType::Present || type == FrameSubmitType::Sync) {
-		VERBOSE_LOG(G3D, "PULL: Frame %d.readyForFence = true", index);
-		std::unique_lock<std::mutex> lock(push_mutex);
-		readyForFence = true;  // misnomer in sync mode!
-		push_condVar.notify_all();
+		syncDone = true;
 	}
 }
 

Common/GPU/Vulkan/VulkanFrameData.h

@@ -7,15 +7,14 @@
 
 #include "Common/GPU/Vulkan/VulkanContext.h"
 
-struct VKRStep;
-
 enum {
 	MAX_TIMESTAMP_QUERIES = 128,
 };
 
 enum class VKRRunType {
-	END,
+	PRESENT,
 	SYNC,
+	EXIT,
 };
 
 struct QueueProfileContext {
@@ -43,16 +42,12 @@ enum class FrameSubmitType {
 
 // Per-frame data, round-robin so we can overlap submission with execution of the previous frame.
 struct FrameData {
-	std::mutex push_mutex;
-	std::condition_variable push_condVar;
-
-	std::mutex pull_mutex;
-	std::condition_variable pull_condVar;
-
-	bool readyForFence = true;
-	bool readyForRun = false;  // protected by pull_mutex
 	bool skipSwap = false;
 
+	std::mutex fenceMutex;
+	std::condition_variable fenceCondVar;
+	bool readyForFence = true;
+
 	VkFence fence;
 	VkFence readbackFence;  // Strictly speaking we might only need one global of these.
 
@@ -68,9 +63,10 @@ struct FrameData {
 	bool hasMainCommands = false;
 	bool hasPresentCommands = false;
 
+	bool hasFencePending = false;
 	bool hasAcquired = false;
 
-	std::vector<VKRStep *> steps;
+	bool syncDone = false;
 
 	// Swapchain.
 	uint32_t curSwapchainImage = -1;
@@ -89,12 +85,6 @@ struct FrameData {
 	// This will only submit if we are actually recording init commands.
 	void SubmitPending(VulkanContext *vulkan, FrameSubmitType type, FrameDataShared &shared);
 
-	VKRRunType RunType() const {
-		return runType_;
-	}
-
-	VKRRunType runType_ = VKRRunType::END;
-
 private:
 	// Metadata for logging etc
 	int index;

Common/GPU/Vulkan/VulkanQueueRunner.cpp

@@ -554,7 +554,7 @@ void VulkanQueueRunner::PreprocessSteps(std::vector<VKRStep *> &steps) {
 	}
 }
 
-void VulkanQueueRunner::RunSteps(FrameData &frameData, FrameDataShared &frameDataShared) {
+void VulkanQueueRunner::RunSteps(std::vector<VKRStep *> &steps, FrameData &frameData, FrameDataShared &frameDataShared) {
 	QueueProfileContext *profile = frameData.profilingEnabled_ ? &frameData.profile : nullptr;
 
 	if (profile)
@@ -564,8 +564,8 @@ void VulkanQueueRunner::RunSteps(FrameData &frameData, FrameDataShared &frameDat
 
 	VkCommandBuffer cmd = frameData.hasPresentCommands ? frameData.presentCmd : frameData.mainCmd;
 
-	for (size_t i = 0; i < frameData.steps.size(); i++) {
-		const VKRStep &step = *frameData.steps[i];
+	for (size_t i = 0; i < steps.size(); i++) {
+		const VKRStep &step = *steps[i];
 
 		if (emitLabels) {
 			VkDebugUtilsLabelEXT labelInfo{ VK_STRUCTURE_TYPE_DEBUG_UTILS_LABEL_EXT };
@@ -625,11 +625,11 @@ void VulkanQueueRunner::RunSteps(FrameData &frameData, FrameDataShared &frameDat
 
 	// Deleting all in one go should be easier on the instruction cache than deleting
 	// them as we go - and easier to debug because we can look backwards in the frame.
-	for (auto step : frameData.steps) {
+	for (auto step : steps) {
 		delete step;
 	}
 
-	frameData.steps.clear();
+	steps.clear();
 
 	if (profile)
 		profile->cpuEndTime = time_now_d();

Common/GPU/Vulkan/VulkanQueueRunner.h

@@ -247,6 +247,14 @@ private:
 	RPKey key_;
 };
 
+// These are enqueued from the main thread,
+// and the render thread pops them off
+struct VKRRenderThreadTask {
+	std::vector<VKRStep *> steps;
+	int frame;
+	VKRRunType runType;
+};
+
 class VulkanQueueRunner {
 public:
 	VulkanQueueRunner(VulkanContext *vulkan) : vulkan_(vulkan), renderPasses_(16) {}
@@ -257,7 +265,7 @@ public:
 	}
 
 	void PreprocessSteps(std::vector<VKRStep *> &steps);
-	void RunSteps(FrameData &frameData, FrameDataShared &frameDataShared);
+	void RunSteps(std::vector<VKRStep *> &steps, FrameData &frameData, FrameDataShared &frameDataShared);
 	void LogSteps(const std::vector<VKRStep *> &steps, bool verbose);
 
 	std::string StepToString(const VKRStep &step) const;

Common/GPU/Vulkan/VulkanRenderManager.cpp

@@ -14,7 +14,7 @@
 #include "Common/Thread/ThreadUtil.h"
 
 #if 0 // def _DEBUG
-#define VLOG(...) INFO_LOG(G3D, __VA_ARGS__)
+#define VLOG(...) NOTICE_LOG(G3D, __VA_ARGS__)
 #else
 #define VLOG(...)
 #endif
@@ -333,10 +333,9 @@ bool VulkanRenderManager::CreateBackbuffers() {
 
 	// Start the thread.
 	if (HasBackbuffers()) {
-		run_ = true;
-		// Won't necessarily be 0.
-		threadInitFrame_ = vulkan_->GetCurFrame();
-		INFO_LOG(G3D, "Starting Vulkan submission thread (threadInitFrame_ = %d)", vulkan_->GetCurFrame());
+		run_ = true;  // For controlling the compiler thread's exit
+
+		INFO_LOG(G3D, "Starting Vulkan submission thread");
 		thread_ = std::thread(&VulkanRenderManager::ThreadFunc, this);
 		INFO_LOG(G3D, "Starting Vulkan compiler thread");
 		compileThread_ = std::thread(&VulkanRenderManager::CompileThreadFunc, this);
@@ -344,29 +343,32 @@ bool VulkanRenderManager::CreateBackbuffers() {
 	return true;
 }
 
+// Called from main thread.
 void VulkanRenderManager::StopThread() {
-	if (!run_) {
-		INFO_LOG(G3D, "Vulkan submission thread was already stopped.");
-		return;
+	{
+		// Tell the render thread to quit when it's done.
+		VKRRenderThreadTask task;
+		task.frame = vulkan_->GetCurFrame();
+		task.runType = VKRRunType::EXIT;
+		std::unique_lock<std::mutex> lock(pushMutex_);
+		renderThreadQueue_.push(task);
+		pushCondVar_.notify_one();
 	}
 
+	// Compiler thread still relies on this.
 	run_ = false;
+
 	// Stop the thread.
+	thread_.join();
+
 	for (int i = 0; i < vulkan_->GetInflightFrames(); i++) {
 		auto &frameData = frameData_[i];
-		{
-			std::unique_lock<std::mutex> lock(frameData.push_mutex);
-			frameData.push_condVar.notify_all();
-		}
-		{
-			std::unique_lock<std::mutex> lock(frameData.pull_mutex);
-			frameData.pull_condVar.notify_all();
-		}
 		// Zero the queries so we don't try to pull them later.
 		frameData.profile.timestampDescriptions.clear();
 	}
-	thread_.join();
+
 	INFO_LOG(G3D, "Vulkan submission thread joined. Frame=%d", vulkan_->GetCurFrame());
+
 	compileCond_.notify_all();
 	compileThread_.join();
 	INFO_LOG(G3D, "Vulkan compiler thread joined.");
@@ -374,28 +376,24 @@ void VulkanRenderManager::StopThread() {
 	// Eat whatever has been queued up for this frame if anything.
 	Wipe();
 
-	// Wait for any fences to finish and be resignaled, so we don't have sync issues.
-	// Also clean out any queued data, which might refer to things that might not be valid
-	// when we restart...
+	// Clean out any remaining queued data, which might refer to things that might not be valid
+	// when we restart the thread...
+
+	// Not sure if this is still needed
 	for (int i = 0; i < vulkan_->GetInflightFrames(); i++) {
 		auto &frameData = frameData_[i];
-		_assert_(!frameData.readyForRun);
-		_assert_(frameData.steps.empty());
 		if (frameData.hasInitCommands) {
 			// Clear 'em out.  This can happen on restart sometimes.
 			vkEndCommandBuffer(frameData.initCmd);
 			frameData.hasInitCommands = false;
 		}
-		frameData.readyForRun = false;
-		for (size_t i = 0; i < frameData.steps.size(); i++) {
-			delete frameData.steps[i];
+		if (frameData.hasMainCommands) {
+			vkEndCommandBuffer(frameData.mainCmd);
+			frameData.hasMainCommands = false;
 		}
-		frameData.steps.clear();
-
-		std::unique_lock<std::mutex> lock(frameData.push_mutex);
-		while (!frameData.readyForFence) {
-			VLOG("PUSH: Waiting for frame[%d].readyForFence = 1 (stop)", i);
-			frameData.push_condVar.wait(lock);
+		if (frameData.hasPresentCommands) {
+			vkEndCommandBuffer(frameData.presentCmd);
+			frameData.hasPresentCommands = false;
 		}
 	}
 }
@@ -409,7 +407,9 @@ void VulkanRenderManager::DestroyBackbuffers() {
 
 VulkanRenderManager::~VulkanRenderManager() {
 	INFO_LOG(G3D, "VulkanRenderManager destructor");
-	StopThread();
+
+	_dbg_assert_(!run_);  // StopThread should already have been called from DestroyBackbuffers.
+
 	vulkan_->WaitUntilQueueIdle();
 
 	DrainCompileQueue();
@@ -468,45 +468,30 @@ void VulkanRenderManager::DrainCompileQueue() {
 
 void VulkanRenderManager::ThreadFunc() {
 	SetCurrentThreadName("RenderMan");
-	int threadFrame = threadInitFrame_;
-	bool nextFrame = false;
-	bool firstFrame = true;
 	while (true) {
+		// Pop a task of the queue and execute it.
+		VKRRenderThreadTask task;
 		{
-			if (nextFrame) {
-				threadFrame++;
-				if (threadFrame >= vulkan_->GetInflightFrames())
-					threadFrame = 0;
+			std::unique_lock<std::mutex> lock(pushMutex_);
+			while (renderThreadQueue_.empty()) {
+				pushCondVar_.wait(lock);
 			}
-			FrameData &frameData = frameData_[threadFrame];
-
-			std::unique_lock<std::mutex> lock(frameData.pull_mutex);
-			while (!frameData.readyForRun && run_) {
-				VLOG("PULL: Waiting for frame[%d].readyForRun", threadFrame);
-				frameData.pull_condVar.wait(lock);
-			}
-			if (!frameData.readyForRun && !run_) {
-				// This means we're out of frames to render and run_ is false, so bail.
-				break;
-			}
-			VLOG("PULL: frame[%d].readyForRun = false", threadFrame);
-			frameData.readyForRun = false;
-			// Previously we had a quick exit here that avoided calling Run() if run_ was suddenly false,
-			// but that created a race condition where frames could end up not finished properly on resize etc.
-
-			// Only increment next time if we're done.
-			nextFrame = frameData.RunType() == VKRRunType::END;
+			task = renderThreadQueue_.front();
+			renderThreadQueue_.pop();
 		}
-		VLOG("PULL: Running frame %d", threadFrame);
-		if (firstFrame) {
-			INFO_LOG(G3D, "Running first frame (%d)", threadFrame);
-			firstFrame = false;
+
+		// Oh, we got a task! We can now have pushMutex_ unlocked, allowing the host to
+		// push more work when it feels like it, and just start working.
+		if (task.runType == VKRRunType::EXIT) {
+			// Oh, host wanted out. Let's leave.
+			break;
 		}
-		Run(threadFrame);
-		VLOG("PULL: Finished frame %d", threadFrame);
+
+		Run(task);
 	}
 
 	// Wait for the device to be done with everything, before tearing stuff down.
+	// TODO: Do we need this?
 	vkDeviceWaitIdle(vulkan_->GetDevice());
 
 	VLOG("PULL: Quitting");
@@ -519,18 +504,20 @@ void VulkanRenderManager::BeginFrame(bool enableProfiling, bool enableLogProfile
 	int curFrame = vulkan_->GetCurFrame();
 	FrameData &frameData = frameData_[curFrame];
 
-	// Make sure the very last command buffer from the frame before the previous has been fully executed.
+	VLOG("PUSH: Fencing %d", curFrame);
+
+	// Makes sure the submission from the previous time around has happened. Otherwise
+	// we are not allowed to wait from another thread here..
 	{
-		std::unique_lock<std::mutex> lock(frameData.push_mutex);
+		std::unique_lock<std::mutex> lock(frameData.fenceMutex);
 		while (!frameData.readyForFence) {
-			VLOG("PUSH: Waiting for frame[%d].readyForFence = 1", curFrame);
-			frameData.push_condVar.wait(lock);
+			frameData.fenceCondVar.wait(lock);
 		}
 		frameData.readyForFence = false;
 	}
 
-	VLOG("PUSH: Fencing %d", curFrame);
-
+	// This must be the very first Vulkan call we do in a new frame.
+	// Makes sure the very last command buffer from the frame before the previous has been fully executed.
 	if (vkWaitForFences(device, 1, &frameData.fence, true, UINT64_MAX) == VK_ERROR_DEVICE_LOST) {
 		_assert_msg_(false, "Device lost in vkWaitForFences");
 	}
@@ -577,9 +564,6 @@ void VulkanRenderManager::BeginFrame(bool enableProfiling, bool enableLogProfile
 
 	// Must be after the fence - this performs deletes.
 	VLOG("PUSH: BeginFrame %d", curFrame);
-	if (!run_) {
-		WARN_LOG(G3D, "BeginFrame while !run_!");
-	}
 
 	vulkan_->BeginFrame(enableLogProfiler ? GetInitCmd() : VK_NULL_HANDLE);
 
@@ -1084,7 +1068,6 @@ void VulkanRenderManager::CopyFramebuffer(VKRFramebuffer *src, VkRect2D srcRect,
 	if (dstPos.x != 0 || dstPos.y != 0 || !fillsDst)
 		step->dependencies.insert(dst);
 
-	std::unique_lock<std::mutex> lock(mutex_);
 	steps_.push_back(step);
 }
 
@@ -1130,7 +1113,6 @@ void VulkanRenderManager::BlitFramebuffer(VKRFramebuffer *src, VkRect2D srcRect,
 	if (!fillsDst)
 		step->dependencies.insert(dst);
 
-	std::unique_lock<std::mutex> lock(mutex_);
 	steps_.push_back(step);
 }
 
@@ -1190,16 +1172,18 @@ void VulkanRenderManager::Finish() {
 	FrameData &frameData = frameData_[curFrame];
 
 	{
-		std::unique_lock<std::mutex> lock(frameData.pull_mutex);
-		VLOG("PUSH: Frame[%d].readyForRun = true", curFrame);
-		frameData.steps = std::move(steps_);
-		steps_.clear();
-		frameData.readyForRun = true;
-		frameData.runType_ = VKRRunType::END;
-		frameData.pull_condVar.notify_all();
+		VLOG("PUSH: Frame[%d]", curFrame);
+		VKRRenderThreadTask task;
+		task.frame = curFrame;
+		task.runType = VKRRunType::PRESENT;
+		std::unique_lock<std::mutex> lock(pushMutex_);
+		renderThreadQueue_.push(task);
+		renderThreadQueue_.back().steps = std::move(steps_);
+		pushCondVar_.notify_one();
 	}
-	vulkan_->EndFrame();
 
+	steps_.clear();
+	vulkan_->EndFrame();
 	insideFrame_ = false;
 }
 
@@ -1212,13 +1196,10 @@ void VulkanRenderManager::Wipe() {
 
 // Called on the render thread.
 //
-// Can be called multiple times with no bad side effects. This is so that we can either begin a frame the normal way,
-// or stop it in the middle for a synchronous readback, then start over again mostly normally but without repeating
-// the backbuffer image acquisition.
-void VulkanRenderManager::BeginSubmitFrame(int frame) {
-	FrameData &frameData = frameData_[frame];
+// Can be called again after a VKRRunType::SYNC on the same frame.
+void VulkanRenderManager::Run(VKRRenderThreadTask &task) {
+	FrameData &frameData = frameData_[task.frame];
 
-	// Should only have at most the init command buffer pending here (that one came from the other thread).
 	_dbg_assert_(!frameData.hasPresentCommands);
 	frameData.SubmitPending(vulkan_, FrameSubmitType::Pending, frameDataShared_);
 
@@ -1233,76 +1214,61 @@ void VulkanRenderManager::BeginSubmitFrame(int frame) {
 		frameData.hasMainCommands = true;
 		_assert_msg_(res == VK_SUCCESS, "vkBeginCommandBuffer failed! result=%s", VulkanResultToString(res));
 	}
-}
 
-// Called on the render thread.
-void VulkanRenderManager::EndSubmitFrame(int frame) {
-	FrameData &frameData = frameData_[frame];
-
-	frameData.SubmitPending(vulkan_, FrameSubmitType::Present, frameDataShared_);
-
-	if (!frameData.skipSwap) {
-		VkResult res = frameData.QueuePresent(vulkan_, frameDataShared_);
-		if (res == VK_ERROR_OUT_OF_DATE_KHR) {
-			// We clearly didn't get this in vkAcquireNextImageKHR because of the skipSwap check above.
-			// Do the increment.
-			outOfDateFrames_++;
-		} else if (res == VK_SUBOPTIMAL_KHR) {
-			outOfDateFrames_++;
-		} else if (res != VK_SUCCESS) {
-			_assert_msg_(false, "vkQueuePresentKHR failed! result=%s", VulkanResultToString(res));
-		} else {
-			// Success
-			outOfDateFrames_ = 0;
-		}
-	} else {
-		// We only get here if vkAcquireNextImage returned VK_ERROR_OUT_OF_DATE.
-		outOfDateFrames_++;
-		frameData.skipSwap = false;
-	}
-}
-
-void VulkanRenderManager::EndSyncFrame(int frame) {
-	FrameData &frameData = frameData_[frame];
-
-	// The submit will trigger the readbackFence, and also do the wait for it.
-	frameData.SubmitPending(vulkan_, FrameSubmitType::Sync, frameDataShared_);
-
-	// At this point we can resume filling the command buffers for the current frame since
-	// we know the device is idle - and thus all previously enqueued command buffers have been processed.
-	// No need to switch to the next frame number, would just be confusing.
-	std::unique_lock<std::mutex> lock(frameData.push_mutex);
-	frameData.readyForFence = true;
-	frameData.push_condVar.notify_all();
-}
-
-void VulkanRenderManager::Run(int frame) {
-	BeginSubmitFrame(frame);
-
-	FrameData &frameData = frameData_[frame];
-	queueRunner_.PreprocessSteps(frameData.steps);
+	queueRunner_.PreprocessSteps(task.steps);
 	// Likely during shutdown, happens in headless.
-	if (frameData.steps.empty() && !frameData.hasAcquired)
+	if (task.steps.empty() && !frameData.hasAcquired)
 		frameData.skipSwap = true;
 	//queueRunner_.LogSteps(stepsOnThread, false);
-	queueRunner_.RunSteps(frameData, frameDataShared_);
+	queueRunner_.RunSteps(task.steps, frameData, frameDataShared_);
 
-	switch (frameData.runType_) {
-	case VKRRunType::END:
-		EndSubmitFrame(frame);
+	switch (task.runType) {
+	case VKRRunType::PRESENT:
+		frameData.SubmitPending(vulkan_, FrameSubmitType::Present, frameDataShared_);
+
+		if (!frameData.skipSwap) {
+			VkResult res = frameData.QueuePresent(vulkan_, frameDataShared_);
+			if (res == VK_ERROR_OUT_OF_DATE_KHR) {
+				// We clearly didn't get this in vkAcquireNextImageKHR because of the skipSwap check above.
+				// Do the increment.
+				outOfDateFrames_++;
+			} else if (res == VK_SUBOPTIMAL_KHR) {
+				outOfDateFrames_++;
+			} else if (res != VK_SUCCESS) {
+				_assert_msg_(false, "vkQueuePresentKHR failed! result=%s", VulkanResultToString(res));
+			} else {
+				// Success
+				outOfDateFrames_ = 0;
+			}
+		} else {
+			// We only get here if vkAcquireNextImage returned VK_ERROR_OUT_OF_DATE.
+			outOfDateFrames_++;
+			frameData.skipSwap = false;
+		}
 		break;
 
 	case VKRRunType::SYNC:
-		EndSyncFrame(frame);
+		// The submit will trigger the readbackFence, and also do the wait for it.
+		frameData.SubmitPending(vulkan_, FrameSubmitType::Sync, frameDataShared_);
+
+		{
+			std::unique_lock<std::mutex> lock(syncMutex_);
+			syncCondVar_.notify_one();
+		}
+
+		// At this point the GPU is idle, and we can resume filling the command buffers for the
+		// current frame since and thus all previously enqueued command buffers have been
+		// processed. No need to switch to the next frame number, would just be confusing.
 		break;
 
 	default:
 		_dbg_assert_(false);
 	}
 
-	VLOG("PULL: Finished running frame %d", frame);
+	VLOG("PULL: Finished running frame %d", task.frame);
 }
 
+// Called from main thread.
 void VulkanRenderManager::FlushSync() {
 	renderStepOffset_ += (int)steps_.size();
 
@@ -1310,23 +1276,23 @@ void VulkanRenderManager::FlushSync() {
 	FrameData &frameData = frameData_[curFrame];
 	
 	{
-		std::unique_lock<std::mutex> lock(frameData.pull_mutex);
-		VLOG("PUSH: Frame[%d].readyForRun = true (sync)", curFrame);
-		frameData.steps = std::move(steps_);
-		steps_.clear();
-		frameData.readyForRun = true;
-		_dbg_assert_(!frameData.readyForFence);
-		frameData.runType_ = VKRRunType::SYNC;
-		frameData.pull_condVar.notify_all();
+		VLOG("PUSH: Frame[%d]", curFrame);
+		VKRRenderThreadTask task;
+		task.frame = curFrame;
+		task.runType = VKRRunType::SYNC;
+		std::unique_lock<std::mutex> lock(pushMutex_);
+		renderThreadQueue_.push(task);
+		renderThreadQueue_.back().steps = std::move(steps_);
+		pushCondVar_.notify_one();
 	}
 
 	{
-		std::unique_lock<std::mutex> lock(frameData.push_mutex);
+		std::unique_lock<std::mutex> lock(syncMutex_);
 		// Wait for the flush to be hit, since we're syncing.
-		while (!frameData.readyForFence) {
+		while (!frameData.syncDone) {
 			VLOG("PUSH: Waiting for frame[%d].readyForFence = 1 (sync)", curFrame);
-			frameData.push_condVar.wait(lock);
+			syncCondVar_.wait(lock);
 		}
-		frameData.readyForFence = false;
+		frameData.syncDone = false;
 	}
 }

Common/GPU/Vulkan/VulkanRenderManager.h

@@ -200,16 +200,10 @@ public:
 	VulkanRenderManager(VulkanContext *vulkan);
 	~VulkanRenderManager();
 
-	void ThreadFunc();
-	void CompileThreadFunc();
-	void DrainCompileQueue();
-
 	// Makes sure that the GPU has caught up enough that we can start writing buffers of this frame again.
 	void BeginFrame(bool enableProfiling, bool enableLogProfiler);
 	// Can run on a different thread!
 	void Finish();
-	void Run(int frame);
-
 	// Zaps queued up commands. Use if you know there's a risk you've queued up stuff that has already been deleted. Can happen during in-game shutdown.
 	void Wipe();
 
@@ -468,13 +462,15 @@ public:
 private:
 	void EndCurRenderStep();
 
+	void ThreadFunc();
+	void CompileThreadFunc();
+	void DrainCompileQueue();
+
+	void Run(VKRRenderThreadTask &task);
 	void BeginSubmitFrame(int frame);
-	void EndSubmitFrame(int frame);
 
 	// Bad for performance but sometimes necessary for synchronous CPU readbacks (screenshots and whatnot).
 	void FlushSync();
-	void EndSyncFrame(int frame);
-
 	void StopThread();
 
 	FrameDataShared frameDataShared_;
@@ -496,6 +492,8 @@ private:
 	int curHeight_ = -1;
 
 	bool insideFrame_ = false;
+	bool run_ = false;
+
 	// This is the offset within this frame, in case of a mid-frame sync.
 	int renderStepOffset_ = 0;
 	VKRStep *curRenderStep_ = nullptr;
@@ -507,13 +505,20 @@ private:
 	std::vector<VKRStep *> steps_;
 
 	// Execution time state
-	bool run_ = true;
 	VulkanContext *vulkan_;
 	std::thread thread_;
-	std::mutex mutex_;
-	int threadInitFrame_ = 0;
 	VulkanQueueRunner queueRunner_;
 
+	// For pushing data on the queue.
+	std::mutex pushMutex_;
+	std::condition_variable pushCondVar_;
+
+	std::queue<VKRRenderThreadTask> renderThreadQueue_;
+
+	// For readbacks and other reasons we need to sync with the render thread.
+	std::mutex syncMutex_;
+	std::condition_variable syncCondVar_;
+
 	// Shader compilation thread to compile while emulating the rest of the frame.
 	// Only one right now but we could use more.
 	std::thread compileThread_;

Common/Thread/Promise.h

@@ -79,7 +79,6 @@ public:
 
 	// Returns T if the data is ready, nullptr if it's not.
 	T Poll() {
-		_dbg_assert_(this != nullptr);
 		std::lock_guard<std::mutex> guard(readyMutex_);
 		if (ready_) {
 			return data_;
@@ -96,7 +95,6 @@ public:
 	}
 
 	T BlockUntilReady() {
-		_dbg_assert_(this != nullptr);
 		std::lock_guard<std::mutex> guard(readyMutex_);
 		if (ready_) {
 			return data_;

GPU/Vulkan/StateMappingVulkan.cpp

@@ -152,7 +152,7 @@ void DrawEngineVulkan::ConvertStateToVulkanKey(FramebufferManagerVulkan &fbManag
 			GenericBlendState &blendState = pipelineState_.blendState;
 			GenericLogicState &logicState = pipelineState_.logicState;
 
-			if (pipelineState_.FramebufferRead()) {
+			if (pipelineState_.FramebufferRead() && useBufferedRendering) {
 				ApplyFramebufferRead(&fboTexBindState_);
 				// The shader takes over the responsibility for blending, so recompute.
 				// We might still end up using blend to write something to alpha.