Hopefully robustify some logic in EmuScreen enough

Common/GPU/Vulkan/VulkanRenderManager.cpp

@@ -365,6 +365,9 @@ void VulkanRenderManager::StartThreads() {
 
 // Called from main thread.
 void VulkanRenderManager::StopThreads() {
+	// Not sure this is a sensible check - should be ok even if not.
+	// _dbg_assert_(steps_.empty());
+
 	if (useRenderThread_) {
 		_dbg_assert_(renderThread_.joinable());
 		// Tell the render thread to quit when it's done.
@@ -400,7 +403,6 @@ void VulkanRenderManager::StopThreads() {
 
 	INFO_LOG(G3D, "Vulkan compiler thread joined. Now wait for any straggling compile tasks.");
 	CreateMultiPipelinesTask::WaitForAll();
-	_dbg_assert_(steps_.empty());
 }
 
 void VulkanRenderManager::DestroyBackbuffers() {
@@ -522,9 +524,8 @@ void VulkanRenderManager::RenderThreadFunc() {
 	}
 
 	// Wait for the device to be done with everything, before tearing stuff down.
-	// TODO: Do we need this?
+	// TODO: Do we really need this? It's probably a good idea, though.
 	vkDeviceWaitIdle(vulkan_->GetDevice());
-
 	VLOG("PULL: Quitting");
 }
 
@@ -956,6 +957,7 @@ void VulkanRenderManager::BindFramebufferAsRenderTarget(VKRFramebuffer *fb, VKRR
 	step->render.numReads = 0;
 	step->render.finalColorLayout = !fb ? VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL : VK_IMAGE_LAYOUT_UNDEFINED;
 	step->render.finalDepthStencilLayout = !fb ? VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL : VK_IMAGE_LAYOUT_UNDEFINED;
+	// pipelineFlags, renderArea and renderPassType get filled in when we finalize the step. Do not read from them before that.
 	step->tag = tag;
 	steps_.push_back(step);
 

Common/Thread/Promise.h

@@ -82,11 +82,13 @@ public:
 		// A promise should have been fulfilled before it's destroyed.
 		_assert_(ready_);
 		_assert_(!rx_);
+		sentinel_ = 0xeeeeeeee;
 	}
 
 	// Returns T if the data is ready, nullptr if it's not.
 	// Obviously, can only be used if T is nullable, otherwise it won't compile.
 	T Poll() {
+		_assert_(sentinel_ == 0xffc0ffee);
 		std::lock_guard<std::mutex> guard(readyMutex_);
 		if (ready_) {
 			return data_;
@@ -103,6 +105,7 @@ public:
 	}
 
 	T BlockUntilReady() {
+		_assert_(sentinel_ == 0xffc0ffee);
 		std::lock_guard<std::mutex> guard(readyMutex_);
 		if (ready_) {
 			return data_;
@@ -128,4 +131,5 @@ private:
 	bool ready_ = false;
 	std::mutex readyMutex_;
 	Mailbox<T> *rx_ = nullptr;
+	uint32_t sentinel_ = 0xffc0ffee;
 };

UI/DebugOverlay.cpp

@@ -406,7 +406,7 @@ Invalid / Unknown (%d)
 	}
 	if (checkingISO) {
 		tips += "* (waiting for CRC...)\n";
-	} else if (!isoOK) {
+	} else if (!isoOK) {  // TODO: Should check that it actually is an ISO and not a homebrew
 		tips += "* Verify and possibly re-dump your ISO\n  (CRC not recognized)\n";
 	}
 	if (!tips.empty()) {

UI/EmuScreen.cpp

@@ -1306,6 +1306,8 @@ ScreenRenderFlags EmuScreen::render(ScreenRenderMode mode) {
 
 	bool skipBufferEffects = g_Config.bSkipBufferEffects;
 
+	bool framebufferBound = false;
+
 	if (mode & ScreenRenderMode::FIRST) {
 		// Actually, always gonna be first when it exists (?)
 
@@ -1327,6 +1329,7 @@ ScreenRenderFlags EmuScreen::render(ScreenRenderMode mode) {
 			draw->SetViewport(viewport);
 			draw->SetScissorRect(0, 0, g_display.pixel_xres, g_display.pixel_yres);
 			skipBufferEffects = true;
+			framebufferBound = true;
 		}
 		draw->SetTargetSize(g_display.pixel_xres, g_display.pixel_yres);
 	}
@@ -1387,7 +1390,6 @@ ScreenRenderFlags EmuScreen::render(ScreenRenderMode mode) {
 	Core_UpdateDebugStats((DebugOverlay)g_Config.iDebugOverlay == DebugOverlay::DEBUG_STATS || g_Config.bLogFrameDrops);
 
 	bool blockedExecution = Achievements::IsBlockingExecution();
-	bool rebind = false;
 	uint32_t clearColor = 0;
 	if (!blockedExecution) {
 		PSP_BeginHostFrame();
@@ -1411,11 +1413,13 @@ ScreenRenderFlags EmuScreen::render(ScreenRenderMode mode) {
 				bool dangerousSettings = !Reporting::IsSupported();
 				clearColor = dangerousSettings ? 0xFF900050 : 0xFF900000;
 				draw->BindFramebufferAsRenderTarget(nullptr, { RPAction::CLEAR, RPAction::CLEAR, RPAction::CLEAR, clearColor }, "EmuScreen_RuntimeError");
+				framebufferBound = true;
 				// The info is drawn later in renderUI
 			} else {
 				// If we're stepping, it's convenient not to clear the screen entirely, so we copy display to output.
 				// This won't work in non-buffered, but that's fine.
 				draw->BindFramebufferAsRenderTarget(nullptr, { RPAction::CLEAR, RPAction::CLEAR, RPAction::CLEAR, clearColor }, "EmuScreen_Stepping");
+				framebufferBound = true;
 				// Just to make sure.
 				if (PSP_IsInited()) {
 					gpu->CopyDisplayToOutput(true);
@@ -1427,7 +1431,7 @@ ScreenRenderFlags EmuScreen::render(ScreenRenderMode mode) {
 			// 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
-			rebind = true;
+			// So, we don't set framebufferBound here.
 			break;
 		}
 
@@ -1437,8 +1441,11 @@ ScreenRenderFlags EmuScreen::render(ScreenRenderMode mode) {
 		Achievements::FrameUpdate();
 	}
 
+	if (gpu->PresentedThisFrame()) {
+		framebufferBound = true;
+	}
 
-	if (gpu && !gpu->PresentedThisFrame() && !skipBufferEffects) {
+	if (!framebufferBound) {
 		draw->BindFramebufferAsRenderTarget(nullptr, { RPAction::CLEAR, RPAction::CLEAR, RPAction::CLEAR, clearColor }, "EmuScreen_NoFrame");
 		draw->SetViewport(viewport);
 		draw->SetScissorRect(0, 0, g_display.pixel_xres, g_display.pixel_yres);
@@ -1460,8 +1467,6 @@ ScreenRenderFlags EmuScreen::render(ScreenRenderMode mode) {
 	checkPowerDown();
 
 	if (hasVisibleUI()) {
-		// In most cases, this should already be bound and a no-op.
-		draw->BindFramebufferAsRenderTarget(nullptr, { RPAction::KEEP, RPAction::CLEAR, RPAction::CLEAR }, "EmuScreen_UI");
 		draw->SetViewport(viewport);
 		cardboardDisableButton_->SetVisibility(g_Config.bEnableCardboardVR ? UI::V_VISIBLE : UI::V_GONE);
 		screenManager()->getUIContext()->BeginFrame();