Merge pull request #19730 from hrydgard/stepping-improvements

ImGeDebugger stepping improvements

Common/Render/Text/draw_text.cpp

@@ -167,6 +167,9 @@ void TextDrawer::MeasureStringRect(std::string_view str, const Bounds &bounds, f
 }
 
 void TextDrawer::DrawStringRect(DrawBuffer &target, std::string_view str, const Bounds &bounds, uint32_t color, int align) {
+	if (bounds.w < 0.0f || bounds.h < 0.0f) {
+		return;
+	}
 	float x = bounds.x;
 	float y = bounds.y;
 	if (align & ALIGN_HCENTER) {

GPU/Debugger/Debugger.cpp

@@ -36,7 +36,7 @@ static int primsLastFrame = 0;
 static int primsThisFrame = 0;
 static int thisFlipNum = 0;
 
-bool g_drawNotified = false;
+bool g_primAfterDraw = false;
 
 static double lastStepTime = -1.0;
 static uint32_t g_skipPcOnce = 0;
@@ -79,6 +79,15 @@ void SetActive(bool flag) {
 		GPUStepping::ResumeFromStepping();
 		lastStepTime = -1.0;
 	}
+	/*
+	active = false;
+	breakAtCount = -1;
+	hasBreakpoints = false;
+	thisFlipNum = 0;
+	g_primAfterDraw = false;
+	lastStepTime = -1.0f;
+	g_skipPcOnce = false;
+	*/
 }
 
 bool IsActive() {
@@ -103,7 +112,12 @@ void SetBreakNext(BreakNext next) {
 	} else if (next == BreakNext::CURVE) {
 		GPUBreakpoints::AddCmdBreakpoint(GE_CMD_BEZIER, true);
 		GPUBreakpoints::AddCmdBreakpoint(GE_CMD_SPLINE, true);
+	} else if (next == BreakNext::DRAW) {
+		// This is now handled by switching to BreakNext::PRIM when we encounter a flush.
+		// This will take us to the following actual draw.
+		g_primAfterDraw = true;
 	}
+
 	if (GPUStepping::IsStepping()) {
 		GPUStepping::ResumeFromStepping();
 	}
@@ -124,12 +138,6 @@ NotifyResult NotifyCommand(u32 pc) {
 		return NotifyResult::Skip;  // return false
 	}
 
-	// Hack to handle draw notifications, that don't come in via NotifyCommand.
-	if (g_drawNotified) {
-		g_drawNotified = false;
-		return NotifyResult::Break;
-	}
-
 	u32 op = Memory::ReadUnchecked_U32(pc);
 	u32 cmd = op >> 24;
 	if (thisFlipNum != gpuStats.numFlips) {
@@ -198,13 +206,13 @@ void NotifyDraw() {
 	if (!active)
 		return;
 	if (breakNext == BreakNext::DRAW && !GPUStepping::IsStepping()) {
-		if (lastStepTime >= 0.0) {
-			NOTICE_LOG(Log::GeDebugger, "Waiting at a draw (%fms)", (time_now_d() - lastStepTime) * 1000.0);
-			lastStepTime = -1.0;
-		} else {
-			NOTICE_LOG(Log::GeDebugger, "Waiting at a draw");
+		// Hack to handle draw notifications, that don't come in via NotifyCommand.
+		if (g_primAfterDraw) {
+			NOTICE_LOG(Log::GeDebugger, "Flush detected, breaking at next PRIM");
+			g_primAfterDraw = false;
+			// Switch to PRIM mode.
+			SetBreakNext(BreakNext::PRIM);
 		}
-		g_drawNotified = true;
 	}
 }
 

GPU/Debugger/State.cpp

@@ -12,6 +12,9 @@
 #include "Core/System.h"
 
 void FormatStateRow(GPUDebugInterface *gpudebug, char *dest, size_t destSize, CmdFormatType fmt, u32 value, bool enabled, u32 otherValue, u32 otherValue2) {
+	value &= 0xFFFFFF;
+	otherValue &= 0xFFFFFF;
+	otherValue2 &= 0xFFFFFF;
 	switch (fmt) {
 	case CMD_FMT_HEX:
 		snprintf(dest, destSize, "%06x", value);

GPU/Debugger/Stepping.cpp

@@ -71,6 +71,7 @@ static int bufferLevel;
 static bool lastWasFramebuffer;
 static u32 pauseSetCmdValue;
 
+// This is used only to highlight differences. Should really be owned by the debugger.
 static GPUgstate lastGState;
 
 const char *PauseActionToString(PauseAction action) {
@@ -161,21 +162,6 @@ void WaitForPauseAction() {
 	actionWait.wait(guard);
 }
 
-static void StartStepping() {
-	if (lastGState.cmdmem[1] == 0) {
-		lastGState = gstate;
-		// Play it safe so we don't keep resetting.
-		lastGState.cmdmem[1] |= 0x01000000;
-	}
-	isStepping = true;
-	stepCounter++;
-}
-
-static void StopStepping() {
-	lastGState = gstate;
-	isStepping = false;
-}
-
 bool ProcessStepping() {
 	_dbg_assert_(gpuDebug);
 
@@ -215,7 +201,15 @@ bool EnterStepping() {
 		return false;
 	}
 
-	StartStepping();
+	// StartStepping
+	if (lastGState.cmdmem[1] == 0) {
+		lastGState = gstate;
+		// Play it safe so we don't keep resetting.
+		lastGState.cmdmem[1] |= 0x01000000;
+	}
+
+	isStepping = true;
+	stepCounter++;
 
 	// Just to be sure.
 	if (pauseAction == PAUSE_CONTINUE) {
@@ -227,7 +221,8 @@ bool EnterStepping() {
 }
 
 void ResumeFromStepping() {
-	StopStepping();
+	lastGState = gstate;
+	isStepping = false;
 	SetPauseAction(PAUSE_CONTINUE, false);
 }
 
@@ -300,7 +295,7 @@ bool GPU_FlushDrawing() {
 	return true;
 }
 
-GPUgstate LastState() {
+const GPUgstate &LastState() {
 	return lastGState;
 }
 

GPU/Debugger/Stepping.h

@@ -46,5 +46,6 @@ namespace GPUStepping {
 	bool GPU_SetCmdValue(u32 op);
 	bool GPU_FlushDrawing();
 
-	GPUgstate LastState();
+	// Can be used to highlight differences in a debugger.
+	const GPUgstate &LastState();
 };

UI/EmuScreen.cpp

@@ -1539,6 +1539,11 @@ ScreenRenderFlags EmuScreen::render(ScreenRenderMode mode) {
 		}
 	}
 
+	// Running it early allows things like direct readbacks of buffers, things we can't do
+	// when we have started the final render pass. Well, technically we probably could with some manipulation
+	// of pass order in the render managers..
+	runImDebugger();
+
 	bool blockedExecution = Achievements::IsBlockingExecution();
 	uint32_t clearColor = 0;
 	if (!blockedExecution) {
@@ -1655,7 +1660,7 @@ ScreenRenderFlags EmuScreen::render(ScreenRenderMode mode) {
 	return flags;
 }
 
-void EmuScreen::renderImDebugger() {
+void EmuScreen::runImDebugger() {
 	if (g_Config.bShowImDebugger) {
 		Draw::DrawContext *draw = screenManager()->getDrawContext();
 		if (!imguiInited_) {
@@ -1691,6 +1696,14 @@ void EmuScreen::renderImDebugger() {
 			imDebugger_->Frame(currentDebugMIPS, gpuDebug);
 
 			ImGui::Render();
+		}
+	}
+}
+
+void EmuScreen::renderImDebugger() {
+	if (g_Config.bShowImDebugger) {
+		Draw::DrawContext *draw = screenManager()->getDrawContext();
+		if (PSP_IsInited()) {
 			ImGui_ImplThin3d_RenderDrawData(ImGui::GetDrawData(), draw);
 		}
 	}

UI/EmuScreen.h

@@ -79,6 +79,7 @@ class EmuScreen : public UIScreen {
 	void bootComplete();
 	bool hasVisibleUI();
 	void renderUI();
+	void runImDebugger();
 	void renderImDebugger();
 
 	void onVKey(int virtualKeyCode, bool down);