Merge pull request #15894 from unknownbrackets/debugger

GE Debugger: Record only one flip if display framebuf not changed, step on vsync
hrydgard · Aug 24, 2022 · 5d50d02 · 5d50d02
2 parents fc81b76 + 27d0019
commit 5d50d02
Show file tree

Hide file tree

Showing 18 changed files with 41 additions and 39 deletions.
diff --git a/GPU/Common/FramebufferManagerCommon.cpp b/GPU/Common/FramebufferManagerCommon.cpp
@@ -41,6 +41,7 @@
 #include "GPU/Common/PresentationCommon.h"
 #include "GPU/Common/TextureCacheCommon.h"
 #include "GPU/Common/ReinterpretFramebuffer.h"
+#include "GPU/Debugger/Debugger.h"
 #include "GPU/Debugger/Record.h"
 #include "GPU/Debugger/Stepping.h"
 #include "GPU/GPUInterface.h"
@@ -105,6 +106,7 @@ void FramebufferManagerCommon::SetDisplayFramebuffer(u32 framebuf, u32 stride, G
 	displayFramebufPtr_ = framebuf;
 	displayStride_ = stride;
 	displayFormat_ = format;
+	GPUDebug::NotifyDisplay(framebuf, stride, format);
 	GPURecord::NotifyDisplay(framebuf, stride, format);
 }
 

diff --git a/GPU/D3D11/GPU_D3D11.cpp b/GPU/D3D11/GPU_D3D11.cpp
@@ -35,7 +35,6 @@
 #include "GPU/GeDisasm.h"
 
 #include "GPU/Common/FramebufferManagerCommon.h"
-#include "GPU/Debugger/Debugger.h"
 #include "GPU/D3D11/ShaderManagerD3D11.h"
 #include "GPU/D3D11/GPU_D3D11.h"
 #include "GPU/D3D11/FramebufferManagerD3D11.h"
@@ -239,13 +238,6 @@ void GPU_D3D11::BeginFrame() {
 	gstate_c.Dirty(DIRTY_PROJTHROUGHMATRIX);
 }
 
-void GPU_D3D11::SetDisplayFramebuffer(u32 framebuf, u32 stride, GEBufferFormat format) {
-	// TODO: Some games like Spongebob - Yellow Avenger, never change framebuffer, they blit to it.
-	// So breaking on frames doesn't work. Might want to move this to sceDisplay vsync.
-	GPUDebug::NotifyDisplay(framebuf, stride, format);
-	framebufferManagerD3D11_->SetDisplayFramebuffer(framebuf, stride, format);
-}
-
 void GPU_D3D11::CopyDisplayToOutput(bool reallyDirty) {
 	// Flush anything left over.
 	drawEngine_.Flush();

diff --git a/GPU/D3D11/GPU_D3D11.h b/GPU/D3D11/GPU_D3D11.h
@@ -41,7 +41,6 @@ class GPU_D3D11 : public GPUCommon {
 	void ExecuteOp(u32 op, u32 diff) override;
 
 	void ReapplyGfxState() override;
-	void SetDisplayFramebuffer(u32 framebuf, u32 stride, GEBufferFormat format) override;
 	void GetStats(char *buffer, size_t bufsize) override;
 	void ClearCacheNextFrame() override;
 	void DeviceLost() override;  // Only happens on Android. Drop all textures and shaders.

diff --git a/GPU/Debugger/Debugger.cpp b/GPU/Debugger/Debugger.cpp
@@ -154,6 +154,15 @@ void NotifyDisplay(u32 framebuf, u32 stride, int format) {
 	}
 }
 
+void NotifyBeginFrame() {
+	if (!active)
+		return;
+	if (breakNext == BreakNext::VSYNC) {
+		// Just start stepping as soon as we can once the vblank finishes.
+		breakNext = BreakNext::OP;
+	}
+}
+
 int PrimsThisFrame() {
 	return primsThisFrame;
 }

diff --git a/GPU/Debugger/Debugger.h b/GPU/Debugger/Debugger.h
@@ -28,6 +28,7 @@ enum class BreakNext {
 	TEX,
 	NONTEX,
 	FRAME,
+	VSYNC,
 	PRIM,
 	CURVE,
 	COUNT,
@@ -43,6 +44,7 @@ void SetBreakCount(int c, bool relative = false);
 bool NotifyCommand(u32 pc);
 void NotifyDraw();
 void NotifyDisplay(u32 framebuf, u32 stride, int format);
+void NotifyBeginFrame();
 
 int PrimsThisFrame();
 int PrimsLastFrame();

diff --git a/GPU/Debugger/Record.cpp b/GPU/Debugger/Record.cpp
@@ -50,6 +50,7 @@ namespace GPURecord {
 static bool active = false;
 static bool nextFrame = false;
 static int flipLastAction = -1;
+static int flipFinishAt = -1;
 static std::function<void(const Path &)> writeCallback;
 
 static std::vector<u8> pushbuf;
@@ -145,6 +146,7 @@ static void BeginRecording() {
 	lastTextures.clear();
 	lastRenderTargets.clear();
 	flipLastAction = gpuStats.numFlips;
+	flipFinishAt = -1;
 
 	u32 ptr = (u32)pushbuf.size();
 	u32 sz = 512 * 4;
@@ -494,6 +496,7 @@ bool Activate() {
 	if (!nextFrame) {
 		nextFrame = true;
 		flipLastAction = gpuStats.numFlips;
+		flipFinishAt = -1;
 		return true;
 	}
 	return false;
@@ -512,6 +515,7 @@ static void FinishRecording() {
 	NOTICE_LOG(SYSTEM, "Recording finished");
 	active = false;
 	flipLastAction = gpuStats.numFlips;
+	flipFinishAt = -1;
 
 	if (writeCallback)
 		writeCallback(filename);
@@ -673,10 +677,10 @@ void NotifyDisplay(u32 framebuf, int stride, int fmt) {
 	}
 }
 
-void NotifyFrame() {
+void NotifyBeginFrame() {
 	const bool noDisplayAction = flipLastAction + 4 < gpuStats.numFlips;
-	// We do this only to catch things that don't call NotifyFrame.
-	if (active && HasDrawCommands() && noDisplayAction) {
+	// We do this only to catch things that don't call NotifyDisplay.
+	if (active && HasDrawCommands() && (noDisplayAction || gpuStats.numFlips == flipFinishAt)) {
 		NOTICE_LOG(SYSTEM, "Recording complete on frame");
 
 		struct DisplayBufData {
@@ -700,6 +704,8 @@ void NotifyFrame() {
 	if (nextFrame && (gstate_c.skipDrawReason & SKIPDRAW_SKIPFRAME) == 0 && noDisplayAction) {
 		NOTICE_LOG(SYSTEM, "Recording starting on frame...");
 		BeginRecording();
+		// If we began on a BeginFrame, end on a BeginFrame.
+		flipFinishAt = gpuStats.numFlips + 1;
 	}
 }
 

diff --git a/GPU/Debugger/Record.h b/GPU/Debugger/Record.h
@@ -36,7 +36,7 @@ void NotifyMemcpy(u32 dest, u32 src, u32 sz);
 void NotifyMemset(u32 dest, int v, u32 sz);
 void NotifyUpload(u32 dest, u32 sz);
 void NotifyDisplay(u32 addr, int stride, int fmt);
-void NotifyFrame();
+void NotifyBeginFrame();
 void NotifyCPU();
 
 };
diff --git a/GPU/Directx9/GPU_DX9.cpp b/GPU/Directx9/GPU_DX9.cpp
@@ -38,7 +38,6 @@
 #include "GPU/GeDisasm.h"
 
 #include "GPU/Common/FramebufferManagerCommon.h"
-#include "GPU/Debugger/Debugger.h"
 #include "GPU/Directx9/ShaderManagerDX9.h"
 #include "GPU/Directx9/GPU_DX9.h"
 #include "GPU/Directx9/FramebufferManagerDX9.h"
@@ -286,11 +285,6 @@ void GPU_DX9::BeginFrame() {
 	framebufferManager_->BeginFrame();
 }
 
-void GPU_DX9::SetDisplayFramebuffer(u32 framebuf, u32 stride, GEBufferFormat format) {
-	GPUDebug::NotifyDisplay(framebuf, stride, format);
-	framebufferManagerDX9_->SetDisplayFramebuffer(framebuf, stride, format);
-}
-
 void GPU_DX9::CopyDisplayToOutput(bool reallyDirty) {
 	dxstate.depthWrite.set(true);
 	dxstate.colorMask.set(0xF);

diff --git a/GPU/Directx9/GPU_DX9.h b/GPU/Directx9/GPU_DX9.h
@@ -40,7 +40,6 @@ class GPU_DX9 : public GPUCommon {
 	void ExecuteOp(u32 op, u32 diff) override;
 
 	void ReapplyGfxState() override;
-	void SetDisplayFramebuffer(u32 framebuf, u32 stride, GEBufferFormat format) override;
 	void GetStats(char *buffer, size_t bufsize) override;
 	void ClearCacheNextFrame() override;
 	void DeviceLost() override;  // Only happens on Android. Drop all textures and shaders.

diff --git a/GPU/GLES/GPU_GLES.cpp b/GPU/GLES/GPU_GLES.cpp
@@ -36,7 +36,6 @@
 #include "GPU/ge_constants.h"
 #include "GPU/GeDisasm.h"
 #include "GPU/Common/FramebufferManagerCommon.h"
-#include "GPU/Debugger/Debugger.h"
 #include "GPU/GLES/ShaderManagerGLES.h"
 #include "GPU/GLES/GPU_GLES.h"
 #include "GPU/GLES/FramebufferManagerGLES.h"
@@ -360,11 +359,6 @@ void GPU_GLES::BeginFrame() {
 	framebufferManagerGL_->BeginFrame();
 }
 
-void GPU_GLES::SetDisplayFramebuffer(u32 framebuf, u32 stride, GEBufferFormat format) {
-	GPUDebug::NotifyDisplay(framebuf, stride, format);
-	framebufferManagerGL_->SetDisplayFramebuffer(framebuf, stride, format);
-}
-
 void GPU_GLES::CopyDisplayToOutput(bool reallyDirty) {
 	// Flush anything left over.
 	framebufferManagerGL_->RebindFramebuffer("RebindFramebuffer - CopyDisplayToOutput");

diff --git a/GPU/GLES/GPU_GLES.h b/GPU/GLES/GPU_GLES.h
@@ -47,7 +47,6 @@ class GPU_GLES : public GPUCommon {
 	void ExecuteOp(u32 op, u32 diff) override;
 
 	void ReapplyGfxState() override;
-	void SetDisplayFramebuffer(u32 framebuf, u32 stride, GEBufferFormat format) override;
 	void GetStats(char *buffer, size_t bufsize) override;
 
 	void ClearCacheNextFrame() override;

diff --git a/GPU/GPUCommon.cpp b/GPU/GPUCommon.cpp
@@ -1109,7 +1109,8 @@ void GPUCommon::BeginFrame() {
 	} else if (dumpThisFrame_) {
 		dumpThisFrame_ = false;
 	}
-	GPURecord::NotifyFrame();
+	GPUDebug::NotifyBeginFrame();
+	GPURecord::NotifyBeginFrame();
 }
 
 void GPUCommon::SlowRunLoop(DisplayList &list)
@@ -2707,7 +2708,8 @@ void GPUCommon::ResetListState(int listID, DisplayListState state) {
 
 GPUDebugOp GPUCommon::DissassembleOp(u32 pc, u32 op) {
 	char buffer[1024];
-	GeDisassembleOp(pc, op, Memory::Read_U32(pc - 4), buffer, sizeof(buffer));
+	u32 prev = Memory::IsValidAddress(pc - 4) ? Memory::ReadUnchecked_U32(pc - 4) : 0;
+	GeDisassembleOp(pc, op, prev, buffer, sizeof(buffer));
 
 	GPUDebugOp info;
 	info.pc = pc;
@@ -2765,6 +2767,10 @@ void GPUCommon::SetCmdValue(u32 op) {
 	downcount = 0;
 }
 
+void GPUCommon::SetDisplayFramebuffer(u32 framebuf, u32 stride, GEBufferFormat format) {
+	framebufferManager_->SetDisplayFramebuffer(framebuf, stride, format);
+}
+
 void GPUCommon::DoBlockTransfer(u32 skipDrawReason) {
 	// TODO: This is used a lot to copy data around between render targets and textures,
 	// and also to quickly load textures from RAM to VRAM. So we should do checks like the following:

diff --git a/GPU/GPUCommon.h b/GPU/GPUCommon.h
@@ -117,6 +117,7 @@ class GPUCommon : public GPUInterface, public GPUDebugInterface {
 	u32  Break(int mode) override;
 	void ReapplyGfxState() override;
 
+	void SetDisplayFramebuffer(u32 framebuf, u32 stride, GEBufferFormat format) override;
 	void CopyDisplayToOutput(bool reallyDirty) override = 0;
 	void InitClear() override = 0;
 	bool PerformMemoryCopy(u32 dest, u32 src, int size) override;

diff --git a/GPU/Vulkan/GPU_Vulkan.cpp b/GPU/Vulkan/GPU_Vulkan.cpp
@@ -37,7 +37,6 @@
 #include "GPU/ge_constants.h"
 #include "GPU/GeDisasm.h"
 #include "GPU/Common/FramebufferManagerCommon.h"
-#include "GPU/Debugger/Debugger.h"
 #include "GPU/Vulkan/ShaderManagerVulkan.h"
 #include "GPU/Vulkan/GPU_Vulkan.h"
 #include "GPU/Vulkan/FramebufferManagerVulkan.h"
@@ -431,11 +430,6 @@ void GPU_Vulkan::InitClear() {
 	}
 }
 
-void GPU_Vulkan::SetDisplayFramebuffer(u32 framebuf, u32 stride, GEBufferFormat format) {
-	GPUDebug::NotifyDisplay(framebuf, stride, format);
-	framebufferManager_->SetDisplayFramebuffer(framebuf, stride, format);
-}
-
 void GPU_Vulkan::CopyDisplayToOutput(bool reallyDirty) {
 	// Flush anything left over.
 	drawEngine_.Flush();

diff --git a/GPU/Vulkan/GPU_Vulkan.h b/GPU/Vulkan/GPU_Vulkan.h
@@ -50,7 +50,6 @@ class GPU_Vulkan : public GPUCommon {
 	void PreExecuteOp(u32 op, u32 diff) override;
 	void ExecuteOp(u32 op, u32 diff) override;
 
-	void SetDisplayFramebuffer(u32 framebuf, u32 stride, GEBufferFormat format) override;
 	void GetStats(char *buffer, size_t bufsize) override;
 	void ClearCacheNextFrame() override;
 	void DeviceLost() override;  // Only happens on Android. Drop all textures and shaders.

diff --git a/Windows/GEDebugger/GEDebugger.cpp b/Windows/GEDebugger/GEDebugger.cpp
@@ -979,6 +979,10 @@ BOOL CGEDebugger::DlgProc(UINT message, WPARAM wParam, LPARAM lParam) {
 			SetBreakNext(BreakNext::FRAME);
 			break;
 
+		case IDC_GEDBG_STEPVSYNC:
+			SetBreakNext(BreakNext::VSYNC);
+			break;
+
 		case IDC_GEDBG_STEPPRIM:
 			SetBreakNext(BreakNext::PRIM);
 			break;

diff --git a/Windows/ppsspp.rc b/Windows/ppsspp.rc
@@ -202,7 +202,7 @@ EXSTYLE WS_EX_ACCEPTFILES | WS_EX_TOOLWINDOW
 CAPTION "GE"
 FONT 8, "MS Shell Dlg", 0, 0, 0x1
 BEGIN
-    PUSHBUTTON      "Step &Frame",IDC_GEDBG_STEPFRAME,10,2,44,14
+    PUSHBUTTON      "Step &Frame",IDC_GEDBG_STEPVSYNC,10,2,44,14
     PUSHBUTTON      "Step &Tex",IDC_GEDBG_STEPTEX,60,2,44,14
     PUSHBUTTON      "Step &Draw",IDC_GEDBG_STEPDRAW,105,2,44,14
     PUSHBUTTON      "Step &Prim",IDC_GEDBG_STEPPRIM,150,2,44,14
@@ -679,7 +679,8 @@ BEGIN
         MENUITEM "Next &Curve",                            IDC_GEDBG_STEPCURVE
         MENUITEM "Next &Texture",                          IDC_GEDBG_STEPTEX
         MENUITEM "Next &Draw Flush",                       IDC_GEDBG_STEPDRAW
-        MENUITEM "Next &Frame",                            IDC_GEDBG_STEPFRAME
+        MENUITEM "Next Display &Framebuf",                 IDC_GEDBG_STEPFRAME
+        MENUITEM "Next &Vsync Frame",                      IDC_GEDBG_STEPVSYNC
         MENUITEM "", 0, MFT_SEPARATOR
         MENUITEM "&Auto Flush Pending",                    IDC_GEDBG_FLUSHAUTO
     END

diff --git a/Windows/resource.h b/Windows/resource.h
@@ -331,6 +331,7 @@
 #define ID_GEDBG_SHOWONLEFT              40219
 #define ID_GEDBG_SHOWONRIGHT             40220
 #define ID_GEDBG_SHOWONTOPRIGHT          40221
+#define IDC_GEDBG_STEPVSYNC              40222
 
 
 // Dummy option to let the buffered rendering hotkey cycle through all the options.
@@ -344,7 +345,7 @@
 #ifdef APSTUDIO_INVOKED
 #ifndef APSTUDIO_READONLY_SYMBOLS
 #define _APS_NEXT_RESOURCE_VALUE        256
-#define _APS_NEXT_COMMAND_VALUE         40222
+#define _APS_NEXT_COMMAND_VALUE         40223
 #define _APS_NEXT_CONTROL_VALUE         1202
 #define _APS_NEXT_SYMED_VALUE           101
 #endif