Assorted drive-by code cleanup

Author: hrydgard

GPU/Common/FramebufferManagerCommon.cpp

@@ -471,10 +471,10 @@ void FramebufferManagerCommon::BlitFramebufferDepth(VirtualFramebuffer *src, Vir
 	// Note: We prefer Blit ahead of Copy here, since at least on GL, Copy will always also copy stencil which we don't want. See #9740.
 	if (gstate_c.Supports(GPU_SUPPORTS_FRAMEBUFFER_BLIT_TO_DEPTH)) {
 		draw_->BlitFramebuffer(src->fbo, 0, 0, w, h, dst->fbo, 0, 0, w, h, Draw::FB_DEPTH_BIT, Draw::FB_BLIT_NEAREST, "BlitFramebufferDepth");
-		RebindFramebuffer("BlitFramebufferDepth");
+		RebindFramebuffer("After BlitFramebufferDepth");
 	} else if (gstate_c.Supports(GPU_SUPPORTS_COPY_IMAGE)) {
 		draw_->CopyFramebufferImage(src->fbo, 0, 0, 0, 0, dst->fbo, 0, 0, 0, 0, w, h, 1, Draw::FB_DEPTH_BIT, "BlitFramebufferDepth");
-		RebindFramebuffer("BlitFramebufferDepth");
+		RebindFramebuffer("After BlitFramebufferDepth");
 	}
 	dst->last_frame_depth_updated = gpuStats.numFlips;
 }
@@ -503,7 +503,7 @@ void FramebufferManagerCommon::NotifyRenderFramebufferUpdated(VirtualFramebuffer
 	if (vfbFormatChanged) {
 		textureCache_->NotifyFramebuffer(vfb, NOTIFY_FB_UPDATED);
 		if (vfb->drawnFormat != vfb->format) {
-			ReinterpretFramebufferFrom(vfb, vfb->drawnFormat);
+			ReinterpretFramebuffer(vfb, vfb->drawnFormat, vfb->format);
 		}
 	}
 
@@ -517,11 +517,15 @@ void FramebufferManagerCommon::NotifyRenderFramebufferUpdated(VirtualFramebuffer
 	}
 }
 
-void FramebufferManagerCommon::ReinterpretFramebufferFrom(VirtualFramebuffer *vfb, GEBufferFormat oldFormat) {
+void FramebufferManagerCommon::ReinterpretFramebuffer(VirtualFramebuffer *vfb, GEBufferFormat oldFormat, GEBufferFormat newFormat) {
 	if (!useBufferedRendering_ || !vfb->fbo) {
 		return;
 	}
 
+	_assert_(newFormat != oldFormat);
+	// The caller is responsible for updating the format.
+	_assert_(newFormat == vfb->format);
+
 	ShaderLanguage lang = draw_->GetShaderLanguageDesc().shaderLanguage;
 
 	bool doReinterpret = PSP_CoreParameter().compat.flags().ReinterpretFramebuffers &&
@@ -549,10 +553,6 @@ void FramebufferManagerCommon::ReinterpretFramebufferFrom(VirtualFramebuffer *vf
 		return;
 	}
 
-	GEBufferFormat newFormat = vfb->format;
-
-	_assert_(newFormat != oldFormat);
-
 	// We only reinterpret between 16 - bit formats, for now.
 	if (!IsGeBufferFormat16BitColor(oldFormat) || !IsGeBufferFormat16BitColor(newFormat)) {
 		// 16->32 and 32->16 will require some more specialized shaders.
@@ -671,8 +671,9 @@ void FramebufferManagerCommon::NotifyRenderFramebufferSwitched(VirtualFramebuffe
 			}
 		}
 	}
+
 	if (vfb->drawnFormat != vfb->format) {
-		ReinterpretFramebufferFrom(vfb, vfb->drawnFormat);
+		ReinterpretFramebuffer(vfb, vfb->drawnFormat, vfb->format);
 	}
 
 	if (useBufferedRendering_) {
@@ -839,10 +840,8 @@ bool FramebufferManagerCommon::BindFramebufferAsColorTexture(int stage, VirtualF
 
 	// currentRenderVfb_ will always be set when this is called, except from the GE debugger.
 	// Let's just not bother with the copy in that case.
-	bool skipCopy = (flags & BINDFBCOLOR_MAY_COPY) == 0;
-	if (GPUStepping::IsStepping()) {
-		skipCopy = true;
-	}
+	bool skipCopy = !(flags & BINDFBCOLOR_MAY_COPY) || GPUStepping::IsStepping();
+
 	// Currently rendering to this framebuffer. Need to make a copy.
 	if (!skipCopy && framebuffer == currentRenderVfb_) {
 		// TODO: Maybe merge with bvfbs_?  Not sure if those could be packing, and they're created at a different size.
@@ -870,7 +869,6 @@ bool FramebufferManagerCommon::BindFramebufferAsColorTexture(int stage, VirtualF
 		gstate_c.skipDrawReason |= SKIPDRAW_BAD_FB_TEXTURE;
 		return false;
 	}
-
 }
 
 void FramebufferManagerCommon::CopyFramebufferForColorTexture(VirtualFramebuffer *dst, VirtualFramebuffer *src, int flags) {
@@ -996,6 +994,8 @@ void FramebufferManagerCommon::DrawFramebufferToOutput(const u8 *srcPixels, GEBu
 
 	// PresentationCommon sets all kinds of state, we can't rely on anything.
 	gstate_c.Dirty(DIRTY_ALL);
+
+	currentRenderVfb_ = nullptr;
 }
 
 void FramebufferManagerCommon::DownloadFramebufferOnSwitch(VirtualFramebuffer *vfb) {
@@ -1747,10 +1747,10 @@ bool FramebufferManagerCommon::NotifyBlockTransferBefore(u32 dstBasePtr, int dst
 				BlitFramebuffer(dstBuffer, dstX, dstY, srcBuffer, srcX, srcY, dstWidth, dstHeight, bpp, "Blit_IntraBufferBlockTransfer");
 				RebindFramebuffer("rebind after intra block transfer");
 				SetColorUpdated(dstBuffer, skipDrawReason);
-				return true;
+				return true;  // Skip the memory copy.
 			} else {
 				// Ignore, nothing to do.  Tales of Phantasia X does this by accident.
-				return true;
+				return true;  // Skip the memory copy.
 			}
 		} else {
 			WARN_LOG_N_TIMES(dstnotsrc, 100, G3D, "Inter-buffer block transfer  %08x (x:%d y:%d stride:%d) -> %08x (x:%d y:%d stride:%d) (%dx%d %dbpp)",

GPU/Common/FramebufferManagerCommon.h

@@ -15,6 +15,14 @@
 // Official git repository and contact information can be found at
 // https://github.com/hrydgard/ppsspp and http://www.ppsspp.org/.
 
+// TODO: We now have the tools in thin3d to nearly eliminate the backend-specific framebuffer managers.
+// Here's a list of functionality to unify into FramebufferManagerCommon:
+// * DrawActiveTexture
+// * BlitFramebuffer
+// * StencilBuffer*.cpp
+//
+// Also, in TextureCache we should be able to unify texture-based depal.
+
 #pragma once
 
 #include <set>
@@ -131,7 +139,7 @@ void GetFramebufferHeuristicInputs(FramebufferHeuristicParams *params, const GPU
 enum BindFramebufferColorFlags {
 	BINDFBCOLOR_SKIP_COPY = 0,
 	BINDFBCOLOR_MAY_COPY = 1,
-	BINDFBCOLOR_MAY_COPY_WITH_UV = 3,
+	BINDFBCOLOR_MAY_COPY_WITH_UV = 3,  // includes BINDFBCOLOR_MAY_COPY
 	BINDFBCOLOR_APPLY_TEX_OFFSET = 4,
 	// Used when rendering to a temporary surface (e.g. not the current render target.)
 	BINDFBCOLOR_FORCE_SELF = 8,
@@ -322,7 +330,7 @@ class FramebufferManagerCommon {
 	const std::vector<VirtualFramebuffer *> &Framebuffers() {
 		return vfbs_;
 	}
-	void ReinterpretFramebufferFrom(VirtualFramebuffer *vfb, GEBufferFormat old);
+	void ReinterpretFramebuffer(VirtualFramebuffer *vfb, GEBufferFormat oldFormat, GEBufferFormat newFormat);
 
 protected:
 	virtual void PackFramebufferSync_(VirtualFramebuffer *vfb, int x, int y, int w, int h);
@@ -356,7 +364,7 @@ class FramebufferManagerCommon {
 	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 void UpdateDownloadTempBuffer(VirtualFramebuffer *nvfb) = 0;
+	virtual void UpdateDownloadTempBuffer(VirtualFramebuffer *nvfb) {}
 
 	VirtualFramebuffer *CreateRAMFramebuffer(uint32_t fbAddress, int width, int height, int stride, GEBufferFormat format);
 

GPU/Common/TextureCacheCommon.cpp

@@ -958,10 +958,9 @@ void TextureCacheCommon::SetTextureFramebuffer(const AttachCandidate &candidate)
 	FramebufferMatchInfo fbInfo = candidate.match;
 
 	if (candidate.match.reinterpret) {
-		// TODO: Kinda ugly, maybe switch direction of the call?
 		GEBufferFormat oldFormat = candidate.fb->format;
 		candidate.fb->format = candidate.match.reinterpretTo;
-		framebufferManager_->ReinterpretFramebufferFrom(candidate.fb, oldFormat);
+		framebufferManager_->ReinterpretFramebuffer(candidate.fb, oldFormat, candidate.match.reinterpretTo);
 	}
 
 	_dbg_assert_msg_(framebuffer != nullptr, "Framebuffer must not be null.");

GPU/D3D11/FramebufferManagerD3D11.cpp

@@ -277,10 +277,6 @@ static void CopyPixelDepthOnly(u32 *dstp, const u32 *srcp, size_t c) {
 	}
 }
 
-void FramebufferManagerD3D11::UpdateDownloadTempBuffer(VirtualFramebuffer *nvfb) {
-	// Nothing to do here.
-}
-
 void FramebufferManagerD3D11::SimpleBlit(
 	Draw::Framebuffer *dest, float destX1, float destY1, float destX2, float destY2,
 	Draw::Framebuffer *src, float srcX1, float srcY1, float srcX2, float srcY2, bool linearFilter) {

GPU/D3D11/FramebufferManagerD3D11.h

@@ -54,8 +54,6 @@ class FramebufferManagerD3D11 : public FramebufferManagerCommon {
 	// 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, const char *tag) override;
 
-	void UpdateDownloadTempBuffer(VirtualFramebuffer *nvfb) override;
-
 private:
 	void Bind2DShader() override;
 	void PackDepthbuffer(VirtualFramebuffer *vfb, int x, int y, int w, int h);

GPU/D3D11/TextureCacheD3D11.cpp

@@ -412,7 +412,7 @@ void TextureCacheD3D11::ApplyTextureFramebuffer(VirtualFramebuffer *framebuffer,
 		shaderApply.Shade();
 
 		context_->PSSetShaderResources(0, 1, &nullTexture);  // Make D3D11 validation happy. Really of no consequence since we rebind anyway.
-		framebufferManagerD3D11_->RebindFramebuffer("RebindFramebuffer - ApplyTextureFramebuffer");
+		framebufferManager_->RebindFramebuffer("RebindFramebuffer - ApplyTextureFramebuffer");
 		draw_->BindFramebufferAsTexture(depalFBO, 0, Draw::FB_COLOR_BIT, 0);
 
 		const u32 bytesPerColor = clutFormat == GE_CMODE_32BIT_ABGR8888 ? sizeof(u32) : sizeof(u16);
@@ -422,8 +422,8 @@ void TextureCacheD3D11::ApplyTextureFramebuffer(VirtualFramebuffer *framebuffer,
 		gstate_c.SetTextureFullAlpha(alphaStatus == TexCacheEntry::STATUS_ALPHA_FULL);
 	} else {
 		gstate_c.SetTextureFullAlpha(gstate.getTextureFormat() == GE_TFMT_5650);
-		framebufferManagerD3D11_->RebindFramebuffer("RebindFramebuffer - ApplyTextureFramebuffer");
-		framebufferManagerD3D11_->BindFramebufferAsColorTexture(0, framebuffer, BINDFBCOLOR_MAY_COPY_WITH_UV | BINDFBCOLOR_APPLY_TEX_OFFSET);
+		framebufferManager_->RebindFramebuffer("RebindFramebuffer - ApplyTextureFramebuffer");
+		framebufferManager_->BindFramebufferAsColorTexture(0, framebuffer, BINDFBCOLOR_MAY_COPY_WITH_UV | BINDFBCOLOR_APPLY_TEX_OFFSET);
 	}
 
 	SamplerCacheKey samplerKey = GetFramebufferSamplingParams(framebuffer->bufferWidth, framebuffer->bufferHeight);

GPU/Directx9/FramebufferManagerDX9.cpp

@@ -259,10 +259,6 @@ static const D3DVERTEXELEMENT9 g_FramebufferVertexElements[] = {
 		return offscreen;
 	}
 
-	void FramebufferManagerDX9::UpdateDownloadTempBuffer(VirtualFramebuffer *nvfb) {
-		// Nothing to do here.
-	}
-
 	void FramebufferManagerDX9::BlitFramebuffer(VirtualFramebuffer *dst, int dstX, int dstY, VirtualFramebuffer *src, int srcX, int srcY, int w, int h, int bpp, const char *tag) {
 		if (!dst->fbo || !src->fbo || !useBufferedRendering_) {
 			// This can happen if we recently switched from non-buffered.

GPU/Directx9/FramebufferManagerDX9.h

@@ -65,8 +65,6 @@ class FramebufferManagerDX9 : public FramebufferManagerCommon {
 	// 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, const char *tag) override;
 
-	void UpdateDownloadTempBuffer(VirtualFramebuffer *nvfb) override;
-
 private:
 	void PackFramebufferSync_(VirtualFramebuffer *vfb, int x, int y, int w, int h) override;
 	void PackDepthbuffer(VirtualFramebuffer *vfb, int x, int y, int w, int h);

GPU/GPUCommon.cpp

@@ -2721,6 +2721,8 @@ bool GPUCommon::PerformMemoryCopy(u32 dest, u32 src, int size) {
 	// Track stray copies of a framebuffer in RAM. MotoGP does this.
 	if (framebufferManager_->MayIntersectFramebuffer(src) || framebufferManager_->MayIntersectFramebuffer(dest)) {
 		if (!framebufferManager_->NotifyFramebufferCopy(src, dest, size, false, gstate_c.skipDrawReason)) {
+			// TODO: What? Why would a game copy between the mirrors? This check seems entirely
+			// superfluous.
 			// We use a little hack for Download/Upload using a VRAM mirror.
 			// Since they're identical we don't need to copy.
 			if (!Memory::IsVRAMAddress(dest) || (dest ^ 0x00400000) != src) {

GPU/Vulkan/DrawEngineVulkan.cpp

@@ -803,6 +803,7 @@ void DrawEngineVulkan::DoFlush() {
 		}
 
 		PROFILE_THIS_SCOPE("updatestate");
+
 		if (textureNeedsApply) {
 			textureCache_->ApplyTexture();
 			textureCache_->GetVulkanHandles(imageView, sampler);
@@ -860,8 +861,6 @@ void DrawEngineVulkan::DoFlush() {
 		UpdateUBOs(frame);
 
 		VkDescriptorSet ds = GetOrCreateDescriptorSet(imageView, sampler, baseBuf, lightBuf, boneBuf, tess);
-		{
-		PROFILE_THIS_SCOPE("renderman_q");
 
 		const uint32_t dynamicUBOOffsets[3] = {
 			baseUBOOffset, lightUBOOffset, boneUBOOffset,
@@ -870,13 +869,13 @@ void DrawEngineVulkan::DoFlush() {
 		int stride = dec_->GetDecVtxFmt().stride;
 
 		if (useElements) {
-			if (!ibuf)
+			if (!ibuf) {
 				ibOffset = (uint32_t)frame->pushIndex->Push(decIndex, sizeof(uint16_t) * indexGen.VertexCount(), &ibuf);
+			}
 			renderManager->DrawIndexed(pipelineLayout_, ds, ARRAY_SIZE(dynamicUBOOffsets), dynamicUBOOffsets, vbuf, vbOffset, ibuf, ibOffset, vertexCount, 1, VK_INDEX_TYPE_UINT16);
 		} else {
 			renderManager->Draw(pipelineLayout_, ds, ARRAY_SIZE(dynamicUBOOffsets), dynamicUBOOffsets, vbuf, vbOffset, vertexCount);
 		}
-		}
 	} else {
 		PROFILE_THIS_SCOPE("soft");
 		// Decode to "decoded"