Merge pull request #12922 from unknownbrackets/gpu-centralize

Centralize a bit, track render pass dependencies better

CMakeLists.txt

@@ -1070,6 +1070,7 @@ add_library(native STATIC
 	ext/native/util/text/wrap_text.h
 	ext/native/util/text/wrap_text.cpp
 	ext/native/util/const_map.h
+	ext/native/util/tiny_set.h
 	ext/native/ext/jpge/jpgd.cpp
 	ext/native/ext/jpge/jpgd.h
 	ext/native/ext/jpge/jpge.cpp

GPU/Common/FramebufferCommon.cpp

@@ -1351,11 +1351,10 @@ VirtualFramebuffer *FramebufferManagerCommon::CreateRAMFramebuffer(uint32_t fbAd
 VirtualFramebuffer *FramebufferManagerCommon::FindDownloadTempBuffer(VirtualFramebuffer *vfb) {
 	// For now we'll keep these on the same struct as the ones that can get displayed
 	// (and blatantly copy work already done above while at it).
-	VirtualFramebuffer *nvfb = 0;
+	VirtualFramebuffer *nvfb = nullptr;
 
 	// We maintain a separate vector of framebuffer objects for blitting.
-	for (size_t i = 0; i < bvfbs_.size(); ++i) {
-		VirtualFramebuffer *v = bvfbs_[i];
+	for (VirtualFramebuffer *v : bvfbs_) {
 		if (v->fb_address == vfb->fb_address && v->format == vfb->format) {
 			if (v->bufferWidth == vfb->bufferWidth && v->bufferHeight == vfb->bufferHeight) {
 				nvfb = v;
@@ -1405,6 +1404,34 @@ VirtualFramebuffer *FramebufferManagerCommon::FindDownloadTempBuffer(VirtualFram
 	return nvfb;
 }
 
+bool FramebufferManagerCommon::CreateDownloadTempBuffer(VirtualFramebuffer *nvfb) {
+	// When updating VRAM, it need to be exact format.
+	if (!gstate_c.Supports(GPU_PREFER_CPU_DOWNLOAD)) {
+		switch (nvfb->format) {
+		case GE_FORMAT_4444:
+			nvfb->colorDepth = Draw::FBO_4444;
+			break;
+		case GE_FORMAT_5551:
+			nvfb->colorDepth = Draw::FBO_5551;
+			break;
+		case GE_FORMAT_565:
+			nvfb->colorDepth = Draw::FBO_565;
+			break;
+		case GE_FORMAT_8888:
+		default:
+			nvfb->colorDepth = Draw::FBO_8888;
+			break;
+		}
+	}
+
+	nvfb->fbo = draw_->CreateFramebuffer({ nvfb->bufferWidth, nvfb->bufferHeight, 1, 1, false, (Draw::FBColorDepth)nvfb->colorDepth });
+	if (!nvfb->fbo) {
+		ERROR_LOG(FRAMEBUF, "Error creating GL FBO! %i x %i", nvfb->renderWidth, nvfb->renderHeight);
+		return false;
+	}
+	return true;
+}
+
 void FramebufferManagerCommon::ApplyClearToMemory(int x1, int y1, int x2, int y2, u32 clearColor) {
 	if (currentRenderVfb_) {
 		if ((currentRenderVfb_->usageFlags & FB_USAGE_DOWNLOAD_CLEAR) != 0) {
@@ -1969,8 +1996,10 @@ void FramebufferManagerCommon::ReadFramebufferToMemory(VirtualFramebuffer *vfb,
 			PackFramebufferSync_(vfb, x, y, w, h);
 		} else {
 			VirtualFramebuffer *nvfb = FindDownloadTempBuffer(vfb);
-			BlitFramebuffer(nvfb, x, y, vfb, x, y, w, h, 0);
-			PackFramebufferSync_(nvfb, x, y, w, h);
+			if (nvfb) {
+				BlitFramebuffer(nvfb, x, y, vfb, x, y, w, h, 0);
+				PackFramebufferSync_(nvfb, x, y, w, h);
+			}
 		}
 
 		textureCache_->ForgetLastTexture();
@@ -2014,9 +2043,10 @@ void FramebufferManagerCommon::DownloadFramebufferForClut(u32 fb_address, u32 lo
 
 			// We'll pseudo-blit framebuffers here to get a resized version of vfb.
 			VirtualFramebuffer *nvfb = FindDownloadTempBuffer(vfb);
-			BlitFramebuffer(nvfb, x, y, vfb, x, y, w, h, 0);
-
-			PackFramebufferSync_(nvfb, x, y, w, h);
+			if (nvfb) {
+				BlitFramebuffer(nvfb, x, y, vfb, x, y, w, h, 0);
+				PackFramebufferSync_(nvfb, x, y, w, h);
+			}
 
 			textureCache_->ForgetLastTexture();
 			RebindFramebuffer();

GPU/Common/FramebufferCommon.h

@@ -339,7 +339,7 @@ protected:
 	void DownloadFramebufferOnSwitch(VirtualFramebuffer *vfb);
 	void FindTransferFramebuffers(VirtualFramebuffer *&dstBuffer, VirtualFramebuffer *&srcBuffer, u32 dstBasePtr, int dstStride, int &dstX, int &dstY, u32 srcBasePtr, int srcStride, int &srcX, int &srcY, int &srcWidth, int &srcHeight, int &dstWidth, int &dstHeight, int bpp);
 	VirtualFramebuffer *FindDownloadTempBuffer(VirtualFramebuffer *vfb);
-	virtual bool CreateDownloadTempBuffer(VirtualFramebuffer *nvfb) = 0;
+	virtual bool CreateDownloadTempBuffer(VirtualFramebuffer *nvfb);
 	virtual void UpdateDownloadTempBuffer(VirtualFramebuffer *nvfb) = 0;
 
 	VirtualFramebuffer *CreateRAMFramebuffer(uint32_t fbAddress, int width, int height, int stride, GEBufferFormat format);

GPU/D3D11/FramebufferManagerD3D11.cpp

@@ -366,19 +366,6 @@ void FramebufferManagerD3D11::BindFramebufferAsColorTexture(int stage, VirtualFr
 	}
 }
 
-bool FramebufferManagerD3D11::CreateDownloadTempBuffer(VirtualFramebuffer *nvfb) {
-	nvfb->colorDepth = Draw::FBO_8888;
-
-	nvfb->fbo = draw_->CreateFramebuffer({ nvfb->bufferWidth, nvfb->bufferHeight, 1, 1, true, (Draw::FBColorDepth)nvfb->colorDepth });
-	if (!(nvfb->fbo)) {
-		ERROR_LOG(FRAMEBUF, "Error creating FBO! %i x %i", nvfb->renderWidth, nvfb->renderHeight);
-		return false;
-	}
-
-	draw_->BindFramebufferAsRenderTarget(nvfb->fbo, { Draw::RPAction::CLEAR, Draw::RPAction::CLEAR, Draw::RPAction::CLEAR });
-	return true;
-}
-
 void FramebufferManagerD3D11::UpdateDownloadTempBuffer(VirtualFramebuffer *nvfb) {
 	// Nothing to do here.
 }

GPU/D3D11/FramebufferManagerD3D11.h

@@ -60,7 +60,6 @@ protected:
 	// Used by ReadFramebufferToMemory and later framebuffer block copies
 	void BlitFramebuffer(VirtualFramebuffer *dst, int dstX, int dstY, VirtualFramebuffer *src, int srcX, int srcY, int w, int h, int bpp) override;
 
-	bool CreateDownloadTempBuffer(VirtualFramebuffer *nvfb) override;
 	void UpdateDownloadTempBuffer(VirtualFramebuffer *nvfb) override;
 
 private:

GPU/Directx9/FramebufferDX9.cpp

@@ -379,19 +379,6 @@ static const D3DVERTEXELEMENT9 g_FramebufferVertexElements[] = {
 		}
 	}
 
-	bool FramebufferManagerDX9::CreateDownloadTempBuffer(VirtualFramebuffer *nvfb) {
-		nvfb->colorDepth = Draw::FBO_8888;
-
-		nvfb->fbo = draw_->CreateFramebuffer({ nvfb->bufferWidth, nvfb->bufferHeight, 1, 1, true, (Draw::FBColorDepth)nvfb->colorDepth });
-		if (!(nvfb->fbo)) {
-			ERROR_LOG(FRAMEBUF, "Error creating FBO! %i x %i", nvfb->renderWidth, nvfb->renderHeight);
-			return false;
-		}
-
-		draw_->BindFramebufferAsRenderTarget(nvfb->fbo, { Draw::RPAction::CLEAR, Draw::RPAction::CLEAR, Draw::RPAction::CLEAR });
-		return true;
-	}
-
 	void FramebufferManagerDX9::UpdateDownloadTempBuffer(VirtualFramebuffer *nvfb) {
 		// Nothing to do here.
 	}

GPU/Directx9/FramebufferDX9.h

@@ -71,7 +71,6 @@ protected:
 	// Used by ReadFramebufferToMemory and later framebuffer block copies
 	void BlitFramebuffer(VirtualFramebuffer *dst, int dstX, int dstY, VirtualFramebuffer *src, int srcX, int srcY, int w, int h, int bpp) override;
 
-	bool CreateDownloadTempBuffer(VirtualFramebuffer *nvfb) override;
 	void UpdateDownloadTempBuffer(VirtualFramebuffer *nvfb) override;
 
 private:

GPU/GLES/FramebufferManagerGLES.cpp

@@ -313,39 +313,11 @@ void FramebufferManagerGLES::BindFramebufferAsColorTexture(int stage, VirtualFra
 	}
 }
 
-bool FramebufferManagerGLES::CreateDownloadTempBuffer(VirtualFramebuffer *nvfb) {
-	// When updating VRAM, it need to be exact format.
-	if (!gstate_c.Supports(GPU_PREFER_CPU_DOWNLOAD)) {
-		switch (nvfb->format) {
-		case GE_FORMAT_4444:
-			nvfb->colorDepth = Draw::FBO_4444;
-			break;
-		case GE_FORMAT_5551:
-			nvfb->colorDepth = Draw::FBO_5551;
-			break;
-		case GE_FORMAT_565:
-			nvfb->colorDepth = Draw::FBO_565;
-			break;
-		case GE_FORMAT_8888:
-		default:
-			nvfb->colorDepth = Draw::FBO_8888;
-			break;
-		}
-	}
-
-	nvfb->fbo = draw_->CreateFramebuffer({ nvfb->bufferWidth, nvfb->bufferHeight, 1, 1, false, (Draw::FBColorDepth)nvfb->colorDepth });
-	if (!nvfb->fbo) {
-		ERROR_LOG(FRAMEBUF, "Error creating GL FBO! %i x %i", nvfb->renderWidth, nvfb->renderHeight);
-		return false;
-	}
-	return true;
-}
-
 void FramebufferManagerGLES::UpdateDownloadTempBuffer(VirtualFramebuffer *nvfb) {
 	_assert_msg_(G3D, nvfb->fbo, "Expecting a valid nvfb in UpdateDownloadTempBuffer");
 
 	// Discard the previous contents of this buffer where possible.
-	if (gl_extensions.GLES3 && glInvalidateFramebuffer != nullptr) {
+	if (gl_extensions.GLES3) {
 		draw_->BindFramebufferAsRenderTarget(nvfb->fbo, { Draw::RPAction::DONT_CARE, Draw::RPAction::DONT_CARE, Draw::RPAction::DONT_CARE });
 	} else if (gl_extensions.IsGLES) {
 		draw_->BindFramebufferAsRenderTarget(nvfb->fbo, { Draw::RPAction::CLEAR, Draw::RPAction::CLEAR, Draw::RPAction::CLEAR });

GPU/GLES/FramebufferManagerGLES.h

@@ -64,7 +64,6 @@ protected:
 	// Used by ReadFramebufferToMemory and later framebuffer block copies
 	void BlitFramebuffer(VirtualFramebuffer *dst, int dstX, int dstY, VirtualFramebuffer *src, int srcX, int srcY, int w, int h, int bpp) override;
 
-	bool CreateDownloadTempBuffer(VirtualFramebuffer *nvfb) override;
 	void UpdateDownloadTempBuffer(VirtualFramebuffer *nvfb) override;
 
 private:

GPU/Vulkan/FramebufferVulkan.cpp

@@ -325,19 +325,6 @@ VkImageView FramebufferManagerVulkan::BindFramebufferAsColorTexture(int stage, V
 	}
 }
 
-bool FramebufferManagerVulkan::CreateDownloadTempBuffer(VirtualFramebuffer *nvfb) {
-	nvfb->colorDepth = Draw::FBO_8888;
-
-	nvfb->fbo = draw_->CreateFramebuffer({ nvfb->bufferWidth, nvfb->bufferHeight, 1, 1, true, (Draw::FBColorDepth)nvfb->colorDepth });
-	if (!(nvfb->fbo)) {
-		ERROR_LOG(FRAMEBUF, "Error creating FBO! %i x %i", nvfb->renderWidth, nvfb->renderHeight);
-		return false;
-	}
-
-	draw_->BindFramebufferAsRenderTarget(nvfb->fbo, { Draw::RPAction::CLEAR, Draw::RPAction::CLEAR, Draw::RPAction::CLEAR });
-	return true;
-}
-
 void FramebufferManagerVulkan::UpdateDownloadTempBuffer(VirtualFramebuffer *nvfb) {
 	// Nothing to do here.
 }

GPU/Vulkan/FramebufferVulkan.h

@@ -71,7 +71,6 @@ protected:
 
 	// Used by ReadFramebufferToMemory and later framebuffer block copies
 	void BlitFramebuffer(VirtualFramebuffer *dst, int dstX, int dstY, VirtualFramebuffer *src, int srcX, int srcY, int w, int h, int bpp) override;
-	bool CreateDownloadTempBuffer(VirtualFramebuffer *nvfb) override;
 	void UpdateDownloadTempBuffer(VirtualFramebuffer *nvfb) override;
 
 private:

GPU/Vulkan/GPU_Vulkan.cpp

@@ -231,6 +231,7 @@ void GPU_Vulkan::CheckGPUFeatures() {
 	features |= GPU_SUPPORTS_INSTANCE_RENDERING;
 	features |= GPU_SUPPORTS_VERTEX_TEXTURE_FETCH;
 	features |= GPU_SUPPORTS_TEXTURE_FLOAT;
+	features |= GPU_PREFER_CPU_DOWNLOAD;
 
 	if (vulkan_->GetDeviceFeatures().enabled.wideLines) {
 		features |= GPU_SUPPORTS_WIDE_LINES;

ext/native/native.vcxproj

@@ -489,6 +489,7 @@
     <ClInclude Include="util\text\shiftjis.h" />
     <ClInclude Include="util\text\utf16.h" />
     <ClInclude Include="util\text\utf8.h" />
+    <ClInclude Include="util\tiny_set.h" />
   </ItemGroup>
   <ItemGroup>
     <ClCompile Include="base\backtrace.cpp" />

ext/native/native.vcxproj.filters

@@ -350,6 +350,9 @@
     <ClInclude Include="ui\root.h">
       <Filter>ui</Filter>
     </ClInclude>
+    <ClInclude Include="util\tiny_set.h">
+      <Filter>util</Filter>
+    </ClInclude>
   </ItemGroup>
   <ItemGroup>
     <ClCompile Include="gfx\gl_debug_log.cpp">

ext/native/thin3d/GLQueueRunner.cpp

@@ -827,13 +827,16 @@ void GLQueueRunner::PerformRenderPass(const GLRStep &step) {
 		{
 			GLenum attachments[3];
 			int count = 0;
+			bool isFBO = step.render.framebuffer != nullptr;
+			bool hasDepth = isFBO ? step.render.framebuffer->z_stencil_ : false;
 			if (c.clear.clearMask & GL_COLOR_BUFFER_BIT)
-				attachments[count++] = GL_COLOR_ATTACHMENT0;
-			if (c.clear.clearMask & GL_DEPTH_BUFFER_BIT)
-				attachments[count++] = GL_DEPTH_ATTACHMENT;
-			if (c.clear.clearMask & GL_STENCIL_BUFFER_BIT)
-				attachments[count++] = GL_STENCIL_BUFFER_BIT;
-			glInvalidateFramebuffer(GL_FRAMEBUFFER, count, attachments);
+				attachments[count++] = isFBO ? GL_COLOR_ATTACHMENT0 : GL_COLOR;
+			if (hasDepth && (c.clear.clearMask & GL_DEPTH_BUFFER_BIT))
+				attachments[count++] = isFBO ? GL_DEPTH_ATTACHMENT : GL_DEPTH;
+			if (hasDepth && (c.clear.clearMask & GL_STENCIL_BUFFER_BIT))
+				attachments[count++] = isFBO ? GL_STENCIL_ATTACHMENT : GL_STENCIL;
+			if (glInvalidateFramebuffer != nullptr && count != 0)
+				glInvalidateFramebuffer(GL_FRAMEBUFFER, count, attachments);
 			CHECK_GL_ERROR_IF_DEBUG();
 			break;
 		}

ext/native/thin3d/GLQueueRunner.h

@@ -6,6 +6,7 @@
 
 #include "gfx/gl_common.h"
 #include "thin3d/DataFormat.h"
+#include "util/tiny_set.h"
 
 struct GLRViewport {
 	float x, y, w, h, minZ, maxZ;
@@ -295,9 +296,14 @@ struct GLRStep {
 	GLRStep(GLRStepType _type) : stepType(_type) {}
 	GLRStepType stepType;
 	std::vector<GLRRenderData> commands;
+	TinySet<const GLRFramebuffer *, 8> dependencies;
 	union {
 		struct {
 			GLRFramebuffer *framebuffer;
+			GLRRenderPassAction color;
+			GLRRenderPassAction depth;
+			GLRRenderPassAction stencil;
+			// Note: not accurate.
 			int numDraws;
 		} render;
 		struct {

ext/native/thin3d/GLRenderManager.cpp

@@ -279,6 +279,9 @@ void GLRenderManager::BindFramebufferAsRenderTarget(GLRFramebuffer *fb, GLRRende
 	GLRStep *step = new GLRStep{ GLRStepType::RENDER };
 	// This is what queues up new passes, and can end previous ones.
 	step->render.framebuffer = fb;
+	step->render.color = color;
+	step->render.depth = depth;
+	step->render.stencil = stencil;
 	step->render.numDraws = 0;
 	steps_.push_back(step);
 
@@ -308,6 +311,12 @@ void GLRenderManager::BindFramebufferAsRenderTarget(GLRFramebuffer *fb, GLRRende
 	}
 	curRenderStep_ = step;
 
+	if (fb) {
+		if (color == GLRRenderPassAction::KEEP || depth == GLRRenderPassAction::KEEP || stencil == GLRRenderPassAction::KEEP) {
+			step->dependencies.insert(fb);
+		}
+	}
+
 	// Every step clears this state.
 	gstate_c.Dirty(DIRTY_BLEND_STATE | DIRTY_DEPTHSTENCIL_STATE | DIRTY_RASTER_STATE);
 }
@@ -319,6 +328,7 @@ void GLRenderManager::BindFramebufferAsTexture(GLRFramebuffer *fb, int binding,
 	data.bind_fb_texture.framebuffer = fb;
 	data.bind_fb_texture.aspect = aspectBit;
 	curRenderStep_->commands.push_back(data);
+	curRenderStep_->dependencies.insert(fb);
 }
 
 void GLRenderManager::CopyFramebuffer(GLRFramebuffer *src, GLRect2D srcRect, GLRFramebuffer *dst, GLOffset2D dstPos, int aspectMask) {
@@ -328,6 +338,10 @@ void GLRenderManager::CopyFramebuffer(GLRFramebuffer *src, GLRect2D srcRect, GLR
 	step->copy.src = src;
 	step->copy.dst = dst;
 	step->copy.aspectMask = aspectMask;
+	step->dependencies.insert(src);
+	bool fillsDst = dst && srcRect.x == 0 && srcRect.y == 0 && srcRect.w == dst->width && srcRect.h == dst->height;
+	if (dstPos.x != 0 || dstPos.y != 0 || !fillsDst)
+		step->dependencies.insert(dst);
 	steps_.push_back(step);
 
 	// Every step clears this state.
@@ -342,6 +356,10 @@ void GLRenderManager::BlitFramebuffer(GLRFramebuffer *src, GLRect2D srcRect, GLR
 	step->blit.dst = dst;
 	step->blit.aspectMask = aspectMask;
 	step->blit.filter = filter;
+	step->dependencies.insert(src);
+	bool fillsDst = dst && dstRect.x == 0 && dstRect.y == 0 && dstRect.w == dst->width && dstRect.h == dst->height;
+	if (!fillsDst)
+		step->dependencies.insert(dst);
 	steps_.push_back(step);
 
 	// Every step clears this state.
@@ -356,6 +374,7 @@ bool GLRenderManager::CopyFramebufferToMemorySync(GLRFramebuffer *src, int aspec
 	step->readback.srcRect = { x, y, w, h };
 	step->readback.aspectMask = aspectBits;
 	step->readback.dstFormat = destFormat;
+	step->dependencies.insert(src);
 	steps_.push_back(step);
 
 	// Every step clears this state.

ext/native/thin3d/VulkanQueueRunner.cpp

@@ -1,4 +1,4 @@
-#include <map>
+#include <unordered_map>
 
 #include "base/timeutil.h"
 #include "DataFormat.h"
@@ -752,13 +752,27 @@ std::string VulkanQueueRunner::StepToString(const VKRStep &step) const {
 void VulkanQueueRunner::ApplyRenderPassMerge(std::vector<VKRStep *> &steps) {
 	// First let's count how many times each framebuffer is rendered to.
 	// If it's more than one, let's do our best to merge them. This can help God of War quite a bit.
-	std::map<VKRFramebuffer *, int> counts;
+	std::unordered_map<VKRFramebuffer *, int> counts;
 	for (int i = 0; i < (int)steps.size(); i++) {
 		if (steps[i]->stepType == VKRStepType::RENDER) {
 			counts[steps[i]->render.framebuffer]++;
 		}
 	}
 
+	auto mergeRenderSteps = [](VKRStep *dst, VKRStep *src) {
+		// OK. Now, if it's a render, slurp up all the commands and kill the step.
+		// Also slurp up any pretransitions.
+		dst->preTransitions.insert(dst->preTransitions.end(), src->preTransitions.begin(), src->preTransitions.end());
+		dst->commands.insert(dst->commands.end(), src->commands.begin(), src->commands.end());
+		// So we don't consider it for other things, maybe doesn't matter.
+		src->dependencies.clear();
+		src->stepType = VKRStepType::RENDER_SKIP;
+	};
+	auto renderHasClear = [](const VKRStep *step) {
+		const auto &r = step->render;
+		return r.color == VKRRenderPassAction::CLEAR || r.depth == VKRRenderPassAction::CLEAR || r.stencil == VKRRenderPassAction::CLEAR;
+	};
+
 	// Now, let's go through the steps. If we find one that is rendered to more than once,
 	// we'll scan forward and slurp up any rendering that can be merged across.
 	for (int i = 0; i < (int)steps.size(); i++) {
@@ -767,39 +781,36 @@ void VulkanQueueRunner::ApplyRenderPassMerge(std::vector<VKRStep *> &steps) {
 			TinySet<VKRFramebuffer *, 8> touchedFramebuffers;  // must be the same fast-size as the dependencies TinySet for annoying reasons.
 			for (int j = i + 1; j < (int)steps.size(); j++) {
 				// If any other passes are reading from this framebuffer as-is, we cancel the scan.
+				if (steps[j]->dependencies.contains(fb)) {
+					// Reading from itself means a KEEP, which is okay.
+					if (steps[j]->stepType != VKRStepType::RENDER || steps[j]->render.framebuffer != fb)
+						break;
+				}
 				switch (steps[j]->stepType) {
 				case VKRStepType::RENDER:
-					if (steps[j]->dependencies.contains(fb)) {
-						goto done_fb;
-					}
-					// Prevent Unknown's example case from https://github.com/hrydgard/ppsspp/pull/12242
-					if (steps[j]->dependencies.contains(touchedFramebuffers)) {
-						goto done_fb;
-					}
-					if (steps[j]->render.framebuffer == fb &&
-						steps[j]->render.color != VKRRenderPassAction::CLEAR &&
-						steps[j]->render.depth != VKRRenderPassAction::CLEAR &&
-						steps[j]->render.stencil != VKRRenderPassAction::CLEAR) {
-						// ok. Now, if it's a render, slurp up all the commands
-						// and kill the step.
-						// Also slurp up any pretransitions.
-						steps[i]->preTransitions.insert(steps[i]->preTransitions.end(), steps[j]->preTransitions.begin(), steps[j]->preTransitions.end());
-						steps[i]->commands.insert(steps[i]->commands.end(), steps[j]->commands.begin(), steps[j]->commands.end());
-						steps[j]->stepType = VKRStepType::RENDER_SKIP;
-					}
-					// Remember the framebuffer this wrote to. We can't merge with later passes that depend on these.
-					if (steps[j]->render.framebuffer != fb) {
+					if (steps[j]->render.framebuffer == fb) {
+						// Prevent Unknown's example case from https://github.com/hrydgard/ppsspp/pull/12242
+						if (renderHasClear(steps[j]) || steps[j]->dependencies.contains(touchedFramebuffers)) {
+							goto done_fb;
+						} else {
+							// Safe to merge, great.
+							mergeRenderSteps(steps[i], steps[j]);
+						}
+					} else {
+						// Remember the framebuffer this wrote to. We can't merge with later passes that depend on these.
 						touchedFramebuffers.insert(steps[j]->render.framebuffer);
 					}
 					break;
 				case VKRStepType::COPY:
-					if (steps[j]->copy.src == fb || steps[j]->copy.dst == fb) {
+					if (steps[j]->copy.dst == fb) {
+						// Without framebuffer "renaming", we can't merge past a clobbered fb.
 						goto done_fb;
 					}
 					touchedFramebuffers.insert(steps[j]->copy.dst);
 					break;
 				case VKRStepType::BLIT:
-					if (steps[j]->blit.src == fb || steps[j]->blit.dst == fb) {
+					if (steps[j]->blit.dst == fb) {
+						// Without framebuffer "renaming", we can't merge past a clobbered fb.
 						goto done_fb;
 					}
 					touchedFramebuffers.insert(steps[j]->blit.dst);
@@ -807,12 +818,13 @@ void VulkanQueueRunner::ApplyRenderPassMerge(std::vector<VKRStep *> &steps) {
 				case VKRStepType::READBACK:
 					// Not sure this has much effect, when executed READBACK is always the last step
 					// since we stall the GPU and wait immediately after.
-					if (steps[j]->readback.src == fb) {
-						goto done_fb;
-					}
+					break;
+				case VKRStepType::RENDER_SKIP:
+				case VKRStepType::READBACK_IMAGE:
 					break;
 				default:
-					break;
+					// We added a new step?  Might be unsafe.
+					goto done_fb;
 				}
 			}
 			done_fb:

ext/native/thin3d/VulkanQueueRunner.h

@@ -6,6 +6,7 @@
 #include "Common/Vulkan/VulkanContext.h"
 #include "math/dataconv.h"
 #include "thin3d/DataFormat.h"
+#include "util/tiny_set.h"
 
 class VKRFramebuffer;
 struct VKRImage;
@@ -17,70 +18,6 @@ enum {
 	QUEUE_HACK_RENDERPASS_MERGE = 8,
 };
 
-// Insert-only small-set implementation. Performs no allocation unless MaxFastSize is exceeded.
-template <class T, int MaxFastSize>
-struct TinySet {
-	~TinySet() { delete slowLookup_; }
-	inline void insert(T t) {
-		// Fast linear scan.
-		for (int i = 0; i < fastCount; i++) {
-			if (fastLookup_[i] == t)
-				return;  // We already have it.
-		}
-		// Fast insertion
-		if (fastCount < MaxFastSize) {
-			fastLookup_[fastCount++] = t;
-			return;
-		}
-		// Fall back to slow path.
-		insertSlow(t);
-	}
-	bool contains(T t) const {
-		for (int i = 0; i < fastCount; i++) {
-			if (fastLookup_[i] == t)
-				return true;
-		}
-		if (slowLookup_) {
-			for (auto x : *slowLookup_) {
-				if (x == t)
-					return true;
-			}
-		}
-		return false;
-	}
-	bool contains(const TinySet<T, MaxFastSize> &otherSet) {
-		// Awkward, kind of ruins the fun.
-		for (int i = 0; i < fastCount; i++) {
-			if (otherSet.contains(fastLookup_[i]))
-				return true;
-		}
-		if (slowLookup_) {
-			for (auto x : *slowLookup_) {
-				if (otherSet.contains(x))
-					return true;
-			}
-		}
-		return false;
-	}
-
-private:
-	void insertSlow(T t) {
-		if (!slowLookup_) {
-			slowLookup_ = new std::vector<T>();
-		} else {
-			for (size_t i = 0; i < slowLookup_->size(); i++) {
-				if ((*slowLookup_)[i] == t)
-					return;
-			}
-		}
-		slowLookup_->push_back(t);
-	}
-	T fastLookup_[MaxFastSize];
-	int fastCount = 0;
-	int slowCount = 0;
-	std::vector<T> *slowLookup_ = nullptr;
-};
-
 enum class VKRRenderCommand : uint8_t {
 	REMOVED,
 	BIND_PIPELINE,

ext/native/thin3d/VulkanRenderManager.cpp

@@ -490,6 +490,13 @@ void VulkanRenderManager::BindFramebufferAsRenderTarget(VKRFramebuffer *fb, VKRR
 	step->render.finalColorLayout = !fb ? VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL : VK_IMAGE_LAYOUT_UNDEFINED;
 	steps_.push_back(step);
 
+	if (fb) {
+		// If there's a KEEP, we naturally read from the framebuffer.
+		if (color == VKRRenderPassAction::KEEP || depth == VKRRenderPassAction::KEEP || stencil == VKRRenderPassAction::KEEP) {
+			step->dependencies.insert(fb);
+		}
+	}
+
 	curRenderStep_ = step;
 	if (fb) {
 		curWidth_ = fb->width;
@@ -513,6 +520,7 @@ bool VulkanRenderManager::CopyFramebufferToMemorySync(VKRFramebuffer *src, int a
 	step->readback.src = src;
 	step->readback.srcRect.offset = { x, y };
 	step->readback.srcRect.extent = { (uint32_t)w, (uint32_t)h };
+	step->dependencies.insert(src);
 	steps_.push_back(step);
 
 	curRenderStep_ = nullptr;
@@ -821,6 +829,10 @@ void VulkanRenderManager::CopyFramebuffer(VKRFramebuffer *src, VkRect2D srcRect,
 	step->copy.srcRect = srcRect;
 	step->copy.dst = dst;
 	step->copy.dstPos = dstPos;
+	step->dependencies.insert(src);
+	bool fillsDst = dst && srcRect.offset.x == 0 && srcRect.offset.y == 0 && srcRect.extent.width == dst->width && srcRect.extent.height == dst->height;
+	if (dstPos.x != 0 || dstPos.y != 0 || !fillsDst)
+		step->dependencies.insert(dst);
 
 	std::unique_lock<std::mutex> lock(mutex_);
 	steps_.push_back(step);
@@ -859,6 +871,10 @@ void VulkanRenderManager::BlitFramebuffer(VKRFramebuffer *src, VkRect2D srcRect,
 	step->blit.dst = dst;
 	step->blit.dstRect = dstRect;
 	step->blit.filter = filter;
+	step->dependencies.insert(src);
+	bool fillsDst = dst && dstRect.offset.x == 0 && dstRect.offset.y == 0 && dstRect.extent.width == dst->width && dstRect.extent.height == dst->height;
+	if (!fillsDst)
+		step->dependencies.insert(dst);
 
 	std::unique_lock<std::mutex> lock(mutex_);
 	steps_.push_back(step);

ext/native/util/tiny_set.h

@@ -0,0 +1,71 @@
+#pragma once
+
+#include <vector>
+
+// Insert-only small-set implementation. Performs no allocation unless MaxFastSize is exceeded.
+template <class T, int MaxFastSize>
+struct TinySet {
+	~TinySet() { delete slowLookup_; }
+	inline void insert(T t) {
+		// Fast linear scan.
+		for (int i = 0; i < fastCount; i++) {
+			if (fastLookup_[i] == t)
+				return;  // We already have it.
+		}
+		// Fast insertion
+		if (fastCount < MaxFastSize) {
+			fastLookup_[fastCount++] = t;
+			return;
+		}
+		// Fall back to slow path.
+		insertSlow(t);
+	}
+	bool contains(T t) const {
+		for (int i = 0; i < fastCount; i++) {
+			if (fastLookup_[i] == t)
+				return true;
+		}
+		if (slowLookup_) {
+			for (auto x : *slowLookup_) {
+				if (x == t)
+					return true;
+			}
+		}
+		return false;
+	}
+	bool contains(const TinySet<T, MaxFastSize> &otherSet) {
+		// Awkward, kind of ruins the fun.
+		for (int i = 0; i < fastCount; i++) {
+			if (otherSet.contains(fastLookup_[i]))
+				return true;
+		}
+		if (slowLookup_) {
+			for (auto x : *slowLookup_) {
+				if (otherSet.contains(x))
+					return true;
+			}
+		}
+		return false;
+	}
+	void clear() {
+		delete slowLookup_;
+		slowLookup_ = nullptr;
+		fastCount = 0;
+	}
+
+private:
+	void insertSlow(T t) {
+		if (!slowLookup_) {
+			slowLookup_ = new std::vector<T>();
+		} else {
+			for (size_t i = 0; i < slowLookup_->size(); i++) {
+				if ((*slowLookup_)[i] == t)
+					return;
+			}
+		}
+		slowLookup_->push_back(t);
+	}
+	T fastLookup_[MaxFastSize];
+	int fastCount = 0;
+	std::vector<T> *slowLookup_ = nullptr;
+};