From 86085335ca3841aa64c5c459a99749ef6a6cfcab Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Tue, 23 Aug 2022 19:20:14 -0700 Subject: [PATCH 1/4] GE Debugger: Record 1 flip if no display calls. Before we were waiting 4 flips before ending recording. --- GPU/Debugger/Record.cpp | 12 +++++++++--- GPU/Debugger/Record.h | 2 +- GPU/GPUCommon.cpp | 2 +- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/GPU/Debugger/Record.cpp b/GPU/Debugger/Record.cpp index 907c7b82c4..cc63dccf72 100644 --- 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 writeCallback; static std::vector 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 index b66a2ea17f..5c5d2f6dbc 100644 --- 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/GPUCommon.cpp b/GPU/GPUCommon.cpp index 7db2fd4b42..841547302a 100644 --- a/GPU/GPUCommon.cpp +++ b/GPU/GPUCommon.cpp @@ -1109,7 +1109,7 @@ void GPUCommon::BeginFrame() { } else if (dumpThisFrame_) { dumpThisFrame_ = false; } - GPURecord::NotifyFrame(); + GPURecord::NotifyBeginFrame(); } void GPUCommon::SlowRunLoop(DisplayList &list) From c581a8389677a5a32314eb76ef80429f8bc00a27 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Tue, 23 Aug 2022 19:29:06 -0700 Subject: [PATCH 2/4] GPU: Centralize SetDisplayFramebuffer(). --- GPU/Common/FramebufferManagerCommon.cpp | 4 ++++ GPU/D3D11/GPU_D3D11.cpp | 8 -------- GPU/D3D11/GPU_D3D11.h | 1 - GPU/Directx9/GPU_DX9.cpp | 6 ------ GPU/Directx9/GPU_DX9.h | 1 - GPU/GLES/GPU_GLES.cpp | 6 ------ GPU/GLES/GPU_GLES.h | 1 - GPU/GPUCommon.cpp | 4 ++++ GPU/GPUCommon.h | 1 + GPU/Vulkan/GPU_Vulkan.cpp | 2 -- GPU/Vulkan/GPU_Vulkan.h | 1 - 11 files changed, 9 insertions(+), 26 deletions(-) diff --git a/GPU/Common/FramebufferManagerCommon.cpp b/GPU/Common/FramebufferManagerCommon.cpp index bfdc23815f..92da28b882 100644 --- 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,9 @@ void FramebufferManagerCommon::SetDisplayFramebuffer(u32 framebuf, u32 stride, G displayFramebufPtr_ = framebuf; displayStride_ = stride; displayFormat_ = 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); GPURecord::NotifyDisplay(framebuf, stride, format); } diff --git a/GPU/D3D11/GPU_D3D11.cpp b/GPU/D3D11/GPU_D3D11.cpp index 956d17fc85..4d1e299a6a 100644 --- 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 index 33790c7f99..d1e5287782 100644 --- a/GPU/D3D11/GPU_D3D11.h +++ b/GPU/D3D11/GPU_D3D11.h @@ -41,7 +41,6 @@ public: 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/Directx9/GPU_DX9.cpp b/GPU/Directx9/GPU_DX9.cpp index deaeeafb68..61be1a5d39 100644 --- 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 index 35ac9817e1..0e313da397 100644 --- a/GPU/Directx9/GPU_DX9.h +++ b/GPU/Directx9/GPU_DX9.h @@ -40,7 +40,6 @@ public: 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 index 1dd004aab8..6b135e273e 100644 --- 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 index f308d15445..cbe6bc00b0 100644 --- a/GPU/GLES/GPU_GLES.h +++ b/GPU/GLES/GPU_GLES.h @@ -47,7 +47,6 @@ public: 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 index 841547302a..ea6bab864f 100644 --- a/GPU/GPUCommon.cpp +++ b/GPU/GPUCommon.cpp @@ -2765,6 +2765,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 index e61d5dada9..f389b788e2 100644 --- a/GPU/GPUCommon.h +++ b/GPU/GPUCommon.h @@ -117,6 +117,7 @@ public: 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 index adc0a9c127..ee902e92f6 100644 --- 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" @@ -432,7 +431,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); } diff --git a/GPU/Vulkan/GPU_Vulkan.h b/GPU/Vulkan/GPU_Vulkan.h index 18da82e6b0..3c13d57621 100644 --- a/GPU/Vulkan/GPU_Vulkan.h +++ b/GPU/Vulkan/GPU_Vulkan.h @@ -50,7 +50,6 @@ public: 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. From a901fa431520588f4207ce232e6087296bb7df6d Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Tue, 23 Aug 2022 19:48:34 -0700 Subject: [PATCH 3/4] GE Debugger: Add separate step based on vsync. I think there were some games where this would step in the middle of a frame, but not seeing it commonly now. So make it the default, but allow both methods in the menu. Fixes #15893. --- GPU/Common/FramebufferManagerCommon.cpp | 2 -- GPU/Debugger/Debugger.cpp | 9 +++++++++ GPU/Debugger/Debugger.h | 2 ++ GPU/GPUCommon.cpp | 1 + GPU/Vulkan/GPU_Vulkan.cpp | 4 ---- Windows/GEDebugger/GEDebugger.cpp | 4 ++++ Windows/ppsspp.rc | 5 +++-- Windows/resource.h | 3 ++- 8 files changed, 21 insertions(+), 9 deletions(-) diff --git a/GPU/Common/FramebufferManagerCommon.cpp b/GPU/Common/FramebufferManagerCommon.cpp index 92da28b882..95be5742fa 100644 --- a/GPU/Common/FramebufferManagerCommon.cpp +++ b/GPU/Common/FramebufferManagerCommon.cpp @@ -106,8 +106,6 @@ void FramebufferManagerCommon::SetDisplayFramebuffer(u32 framebuf, u32 stride, G displayFramebufPtr_ = framebuf; displayStride_ = stride; displayFormat_ = 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); GPURecord::NotifyDisplay(framebuf, stride, format); } diff --git a/GPU/Debugger/Debugger.cpp b/GPU/Debugger/Debugger.cpp index 04fb647f1d..439c763e8e 100644 --- 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 index 43ec3174b9..b17c6b4ad0 100644 --- 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/GPUCommon.cpp b/GPU/GPUCommon.cpp index ea6bab864f..9358a99ed9 100644 --- a/GPU/GPUCommon.cpp +++ b/GPU/GPUCommon.cpp @@ -1109,6 +1109,7 @@ void GPUCommon::BeginFrame() { } else if (dumpThisFrame_) { dumpThisFrame_ = false; } + GPUDebug::NotifyBeginFrame(); GPURecord::NotifyBeginFrame(); } diff --git a/GPU/Vulkan/GPU_Vulkan.cpp b/GPU/Vulkan/GPU_Vulkan.cpp index ee902e92f6..fa1024670b 100644 --- a/GPU/Vulkan/GPU_Vulkan.cpp +++ b/GPU/Vulkan/GPU_Vulkan.cpp @@ -430,10 +430,6 @@ void GPU_Vulkan::InitClear() { } } -void GPU_Vulkan::SetDisplayFramebuffer(u32 framebuf, u32 stride, GEBufferFormat format) { - framebufferManager_->SetDisplayFramebuffer(framebuf, stride, format); -} - void GPU_Vulkan::CopyDisplayToOutput(bool reallyDirty) { // Flush anything left over. drawEngine_.Flush(); diff --git a/Windows/GEDebugger/GEDebugger.cpp b/Windows/GEDebugger/GEDebugger.cpp index 49ecc6af4e..7c78dcf80e 100644 --- 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 index e2f38fd8e6..3bc7dfed34 100644 --- 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 index eacb42760b..13104152e9 100644 --- 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 From 27d00199c8b5a2d9420a3cd2e0591f4f55743dc3 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Tue, 23 Aug 2022 19:50:01 -0700 Subject: [PATCH 4/4] GE Debugger: Fix bad read on step at start of VRAM. --- GPU/GPUCommon.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/GPU/GPUCommon.cpp b/GPU/GPUCommon.cpp index 9358a99ed9..167f7ddbf8 100644 --- a/GPU/GPUCommon.cpp +++ b/GPU/GPUCommon.cpp @@ -2708,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;