From d6d1b5bbdca1461392e6eb8f57dabf9d88038250 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 3 Mar 2025 12:57:27 +0100 Subject: [PATCH 1/7] Add mechanism to run any closure on the main thread --- Common/System/System.h | 3 +++ UI/NativeApp.cpp | 42 ++++++++++++++++++++++++++------------ Windows/MainWindowMenu.cpp | 4 +--- headless/Headless.cpp | 1 + libretro/libretro.cpp | 1 + unittest/UnitTest.cpp | 5 ++--- 6 files changed, 37 insertions(+), 19 deletions(-) diff --git a/Common/System/System.h b/Common/System/System.h index 6c945be000..700ea141a4 100644 --- a/Common/System/System.h +++ b/Common/System/System.h @@ -100,6 +100,9 @@ enum class SystemRequestType { RUN_CALLBACK_IN_WNDPROC, }; +// Run a closure on the main thread. Used to safely implement UI that runs on another thread. +void System_RunOnMainThread(std::function func); + // Implementations are supposed to process the request, and post the response to the g_RequestManager (see Message.h). // This is not to be used directly by applications, instead use the g_RequestManager to make the requests. // This can return false if it's known that the platform doesn't support the request, the app is supposed to handle diff --git a/UI/NativeApp.cpp b/UI/NativeApp.cpp index 0be097cd21..e3058fbb7c 100644 --- a/UI/NativeApp.cpp +++ b/UI/NativeApp.cpp @@ -177,7 +177,7 @@ struct PendingMessage { std::string value; }; -static std::mutex pendingMutex; +static std::mutex g_pendingMutex; static std::vector pendingMessages; static Draw::DrawContext *g_draw; static Draw::Pipeline *colorPipeline; @@ -185,6 +185,7 @@ static Draw::Pipeline *texColorPipeline; static UIContext *uiContext; static int g_restartGraphics; static bool g_windowHidden = false; +std::vector> g_pendingClosures; #ifdef _WIN32 WindowsAudioBackend *winAudioBackend; @@ -345,6 +346,7 @@ void NativeInit(int argc, const char *argv[], const char *savegame_dir, const ch setlocale( LC_ALL, "C" ); std::string user_data_path = savegame_dir; pendingMessages.clear(); + g_pendingClosures.clear(); g_requestManager.Clear(); // external_dir has all kinds of meanings depending on platform. @@ -1047,19 +1049,28 @@ void NativeFrame(GraphicsContext *graphicsContext) { g_screenManager->update(); // Do this after g_screenManager.update() so we can receive setting changes before rendering. - std::vector toProcess; { - std::lock_guard lock(pendingMutex); - toProcess = std::move(pendingMessages); - pendingMessages.clear(); - } - - for (const auto &item : toProcess) { - if (HandleGlobalMessage(item.message, item.value)) { - // TODO: Add a to-string thingy. - INFO_LOG(Log::System, "Handled global message: %d / %s", (int)item.message, item.value.c_str()); + std::vector toProcess; + std::vector> toRun; + { + std::lock_guard lock(g_pendingMutex); + toProcess = std::move(pendingMessages); + toRun = std::move(g_pendingClosures); + pendingMessages.clear(); + g_pendingClosures.clear(); + } + + for (auto &item : toRun) { + item(); + } + + for (const auto &item : toProcess) { + if (HandleGlobalMessage(item.message, item.value)) { + // TODO: Add a to-string thingy. + INFO_LOG(Log::System, "Handled global message: %d / %s", (int)item.message, item.value.c_str()); + } + g_screenManager->sendMessage(item.message, item.value.c_str()); } - g_screenManager->sendMessage(item.message, item.value.c_str()); } g_requestManager.ProcessRequests(); @@ -1404,13 +1415,18 @@ void NativeAccelerometer(float tiltX, float tiltY, float tiltZ) { } void System_PostUIMessage(UIMessage message, const std::string &value) { - std::lock_guard lock(pendingMutex); + std::lock_guard lock(g_pendingMutex); PendingMessage pendingMessage; pendingMessage.message = message; pendingMessage.value = value; pendingMessages.push_back(pendingMessage); } +void System_RunOnMainThread(std::function func) { + std::lock_guard lock(g_pendingMutex); + g_pendingClosures.push_back(std::move(func)); +} + void NativeResized() { // NativeResized can come from any thread so we just set a flag, then process it later. VERBOSE_LOG(Log::G3D, "NativeResized - setting flag"); diff --git a/Windows/MainWindowMenu.cpp b/Windows/MainWindowMenu.cpp index 7d931a6093..ab57497406 100644 --- a/Windows/MainWindowMenu.cpp +++ b/Windows/MainWindowMenu.cpp @@ -497,9 +497,7 @@ namespace MainWindow { break; case ID_EMULATION_PAUSE: - if (!NetworkWarnUserIfOnlineAndCantSpeed()) { - System_PostUIMessage(UIMessage::REQUEST_GAME_PAUSE); - } + System_PostUIMessage(UIMessage::REQUEST_GAME_PAUSE); break; case ID_EMULATION_STOP: diff --git a/headless/Headless.cpp b/headless/Headless.cpp index 53f41c6516..b818eebd86 100644 --- a/headless/Headless.cpp +++ b/headless/Headless.cpp @@ -93,6 +93,7 @@ bool System_GetPropertyBool(SystemProperty prop) { } void System_Notify(SystemNotification notification) {} void System_PostUIMessage(UIMessage message, const std::string ¶m) {} +void System_RunOnMainThread(std::function) {} bool System_MakeRequest(SystemRequestType type, int requestId, const std::string ¶m1, const std::string ¶m2, int64_t param3, int64_t param4) { switch (type) { case SystemRequestType::SEND_DEBUG_OUTPUT: diff --git a/libretro/libretro.cpp b/libretro/libretro.cpp index f31af7beeb..523c5f4d3b 100644 --- a/libretro/libretro.cpp +++ b/libretro/libretro.cpp @@ -1913,6 +1913,7 @@ void System_Notify(SystemNotification notification) { } bool System_MakeRequest(SystemRequestType type, int requestId, const std::string ¶m1, const std::string ¶m2, int64_t param3, int64_t param4) { return false; } void System_PostUIMessage(UIMessage message, const std::string ¶m) {} +void System_RunOnMainThread(std::function) {} void NativeFrame(GraphicsContext *graphicsContext) {} void NativeResized() {} diff --git a/unittest/UnitTest.cpp b/unittest/UnitTest.cpp index 733bb10884..4c45c365a1 100644 --- a/unittest/UnitTest.cpp +++ b/unittest/UnitTest.cpp @@ -112,6 +112,7 @@ bool System_GetPropertyBool(SystemProperty prop) { } void System_Notify(SystemNotification notification) {} void System_PostUIMessage(UIMessage message, const std::string ¶m) {} +void System_RunOnMainThread(std::function) {} void System_AudioGetDebugStats(char *buf, size_t bufSize) { if (buf) buf[0] = '\0'; } void System_AudioClear() {} void System_AudioPushSamples(const s32 *audio, int numSamples, float volume) {} @@ -119,9 +120,7 @@ void System_AudioPushSamples(const s32 *audio, int numSamples, float volume) {} // TODO: To avoid having to define these here, these should probably be turned into system "requests". // To clear the secret entirely, just save an empty string. bool NativeSaveSecret(std::string_view nameOfSecret, std::string_view data) { return false; } -std::string NativeLoadSecret(std::string_view nameOfSecret) { - return ""; -} +std::string NativeLoadSecret(std::string_view nameOfSecret) { return ""; } #if PPSSPP_PLATFORM(ANDROID) JNIEnv *getEnv() { From 4989ec61d9ed5541ed34b11a02f291500add4e8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 3 Mar 2025 13:24:42 +0100 Subject: [PATCH 2/7] BreakpointManager: Safer and simpler updates. --- Core/Core.cpp | 1 - Core/Core.h | 1 - Core/Debugger/Breakpoints.cpp | 91 +++++++++++------------------------ Core/Debugger/Breakpoints.h | 12 ++++- UI/MainScreen.cpp | 2 +- UI/NativeApp.cpp | 3 ++ Windows/MainWindowMenu.cpp | 1 + 7 files changed, 44 insertions(+), 67 deletions(-) diff --git a/Core/Core.cpp b/Core/Core.cpp index a37c590907..8c4d6f5062 100644 --- a/Core/Core.cpp +++ b/Core/Core.cpp @@ -108,7 +108,6 @@ const char *BreakReasonToString(BreakReason reason) { case BreakReason::SavestateCrash: return "savestate.crash"; case BreakReason::MemoryBreakpoint: return "memory.breakpoint"; case BreakReason::CpuBreakpoint: return "cpu.breakpoint"; - case BreakReason::BreakpointUpdate: return "cpu.breakpoint.update"; case BreakReason::MemoryAccess: return "memory.access"; // ??? case BreakReason::JitBranchDebug: return "jit.branchdebug"; case BreakReason::RABreak: return "ra.break"; diff --git a/Core/Core.h b/Core/Core.h index 0839def403..8aff05161c 100644 --- a/Core/Core.h +++ b/Core/Core.h @@ -61,7 +61,6 @@ enum class BreakReason { SavestateCrash, MemoryBreakpoint, CpuBreakpoint, - BreakpointUpdate, MemoryAccess, // ??? JitBranchDebug, BreakOnBoot, diff --git a/Core/Debugger/Breakpoints.cpp b/Core/Debugger/Breakpoints.cpp index ed1c6e7ff9..176a172e7c 100644 --- a/Core/Debugger/Breakpoints.cpp +++ b/Core/Debugger/Breakpoints.cpp @@ -32,7 +32,7 @@ BreakpointManager g_breakpoints; -void MemCheck::Log(u32 addr, bool write, int size, u32 pc, const char *reason) { +void MemCheck::Log(u32 addr, bool write, int size, u32 pc, const char *reason) const { if (result & BREAK_ACTION_LOG) { const char *type = write ? "Write" : "Read"; if (logFormat.empty()) { @@ -71,11 +71,9 @@ BreakAction MemCheck::Action(u32 addr, bool write, int size, u32 pc, const char } // Note: must lock while calling this. -size_t BreakpointManager::FindBreakpoint(u32 addr, bool matchTemp, bool temp) -{ +size_t BreakpointManager::FindBreakpoint(u32 addr, bool matchTemp, bool temp) { size_t found = INVALID_BREAKPOINT; - for (size_t i = 0; i < breakPoints_.size(); ++i) - { + for (size_t i = 0; i < breakPoints_.size(); ++i) { const auto &bp = breakPoints_[i]; if (bp.addr == addr && (!matchTemp || bp.temporary == temp)) { @@ -90,10 +88,8 @@ size_t BreakpointManager::FindBreakpoint(u32 addr, bool matchTemp, bool temp) return found; } -size_t BreakpointManager::FindMemCheck(u32 start, u32 end) -{ - for (size_t i = 0; i < memChecks_.size(); ++i) - { +size_t BreakpointManager::FindMemCheck(u32 start, u32 end) { + for (size_t i = 0; i < memChecks_.size(); ++i) { if (memChecks_[i].start == start && memChecks_[i].end == end) return i; } @@ -155,14 +151,11 @@ int BreakpointManager::AddBreakPoint(u32 addr, bool temp) { breakPoints_.push_back(pt); anyBreakPoints_ = true; - guard.unlock(); Update(addr); - return (int)breakPoints_.size() - 1; } else if (!breakPoints_[bp].IsEnabled()) { breakPoints_[bp].result |= BREAK_ACTION_PAUSE; breakPoints_[bp].hasCond = false; - guard.unlock(); Update(addr); return (int)bp; } else { @@ -183,7 +176,6 @@ void BreakpointManager::RemoveBreakPoint(u32 addr) { breakPoints_.erase(breakPoints_.begin() + bp); anyBreakPoints_ = !breakPoints_.empty(); - guard.unlock(); Update(addr); } } @@ -196,8 +188,6 @@ void BreakpointManager::ChangeBreakPoint(u32 addr, bool status) { breakPoints_[bp].result |= BREAK_ACTION_PAUSE; else breakPoints_[bp].result = BreakAction(breakPoints_[bp].result & ~BREAK_ACTION_PAUSE); - - guard.unlock(); Update(addr); } } @@ -207,7 +197,6 @@ void BreakpointManager::ChangeBreakPoint(u32 addr, BreakAction result) { size_t bp = FindBreakpoint(addr); if (bp != INVALID_BREAKPOINT) { breakPoints_[bp].result = result; - guard.unlock(); Update(addr); } } @@ -220,7 +209,6 @@ void BreakpointManager::ClearAllBreakPoints() if (!breakPoints_.empty()) { breakPoints_.clear(); - guard.unlock(); Update(); } } @@ -230,20 +218,14 @@ void BreakpointManager::ClearTemporaryBreakPoints() if (!anyBreakPoints_) return; std::unique_lock guard(breakPointsMutex_); - - bool update = false; for (int i = (int)breakPoints_.size()-1; i >= 0; --i) { if (breakPoints_[i].temporary) { breakPoints_.erase(breakPoints_.begin() + i); - update = true; + Update(); } } - - guard.unlock(); - if (update) - Update(); } void BreakpointManager::ChangeBreakPointAddCond(u32 addr, const BreakPointCond &cond) @@ -254,30 +236,25 @@ void BreakpointManager::ChangeBreakPointAddCond(u32 addr, const BreakPointCond & { breakPoints_[bp].hasCond = true; breakPoints_[bp].cond = cond; - guard.unlock(); Update(addr); } } -void BreakpointManager::ChangeBreakPointRemoveCond(u32 addr) -{ +void BreakpointManager::ChangeBreakPointRemoveCond(u32 addr) { std::unique_lock guard(breakPointsMutex_); size_t bp = FindBreakpoint(addr); - if (bp != INVALID_BREAKPOINT) - { + if (bp != INVALID_BREAKPOINT) { breakPoints_[bp].hasCond = false; - guard.unlock(); Update(addr); } } -BreakPointCond *BreakpointManager::GetBreakPointCondition(u32 addr) -{ +BreakPointCond *BreakpointManager::GetBreakPointCondition(u32 addr) { std::lock_guard guard(breakPointsMutex_); size_t bp = FindBreakpoint(addr); if (bp != INVALID_BREAKPOINT && breakPoints_[bp].hasCond) return &breakPoints_[bp].cond; - return NULL; + return nullptr; } void BreakpointManager::ChangeBreakPointLogFormat(u32 addr, const std::string &fmt) { @@ -285,7 +262,6 @@ void BreakpointManager::ChangeBreakPointLogFormat(u32 addr, const std::string &f size_t bp = FindBreakpoint(addr, true, false); if (bp != INVALID_BREAKPOINT) { breakPoints_[bp].logFormat = fmt; - guard.unlock(); Update(addr); } } @@ -338,18 +314,18 @@ int BreakpointManager::AddMemCheck(u32 start, u32 end, MemCheckCondition cond, B memChecks_.push_back(check); bool hadAny = anyMemChecks_.exchange(true); - if (!hadAny) + if (!hadAny) { MemBlockOverrideDetailed(); - guard.unlock(); + } Update(); return (int)memChecks_.size() - 1; } else { memChecks_[mc].cond = (MemCheckCondition)(memChecks_[mc].cond | cond); memChecks_[mc].result = (BreakAction)(memChecks_[mc].result | result); bool hadAny = anyMemChecks_.exchange(true); - if (!hadAny) + if (!hadAny) { MemBlockOverrideDetailed(); - guard.unlock(); + } Update(); return (int)mc; } @@ -366,7 +342,6 @@ void BreakpointManager::RemoveMemCheck(u32 start, u32 end) bool hadAny = anyMemChecks_.exchange(!memChecks_.empty()); if (hadAny) MemBlockReleaseDetailed(); - guard.unlock(); Update(); } } @@ -379,7 +354,6 @@ void BreakpointManager::ChangeMemCheck(u32 start, u32 end, MemCheckCondition con { memChecks_[mc].cond = cond; memChecks_[mc].result = result; - guard.unlock(); Update(); } } @@ -394,7 +368,6 @@ void BreakpointManager::ClearAllMemChecks() bool hadAny = anyMemChecks_.exchange(false); if (hadAny) MemBlockReleaseDetailed(); - guard.unlock(); Update(); } } @@ -406,7 +379,6 @@ void BreakpointManager::ChangeMemCheckAddCond(u32 start, u32 end, const BreakPoi if (mc != INVALID_MEMCHECK) { memChecks_[mc].hasCondition = true; memChecks_[mc].condition = cond; - guard.unlock(); // No need to update jit for a condition add/remove, they're not baked in. Update(-1); } @@ -417,7 +389,6 @@ void BreakpointManager::ChangeMemCheckRemoveCond(u32 start, u32 end) { size_t mc = FindMemCheck(start, end); if (mc != INVALID_MEMCHECK) { memChecks_[mc].hasCondition = false; - guard.unlock(); // No need to update jit for a condition add/remove, they're not baked in. Update(-1); } @@ -436,7 +407,6 @@ void BreakpointManager::ChangeMemCheckLogFormat(u32 start, u32 end, const std::s size_t mc = FindMemCheck(start, end); if (mc != INVALID_MEMCHECK) { memChecks_[mc].logFormat = fmt; - guard.unlock(); Update(); } } @@ -620,30 +590,27 @@ std::vector BreakpointManager::GetBreakpoints() { return breakPoints_; } -void BreakpointManager::Update(u32 addr) { - if (MIPSComp::jit && addr != -1) { - bool resume = false; - if (Core_IsStepping() == false) { - Core_Break(BreakReason::BreakpointUpdate, addr); - Core_WaitInactive(); - resume = true; - } - - // In case this is a delay slot, clear the previous instruction too. - if (addr != 0) - mipsr4k.InvalidateICache(addr - 4, 8); - else - mipsr4k.ClearJitCache(); - - if (resume) - Core_Resume(); +void BreakpointManager::Frame() { + // outside the lock here, should be ok. + if (!needsUpdate_) { + return; } - if (anyMemChecks_ && addr != -1) + std::lock_guard guard(breakPointsMutex_); + if (MIPSComp::jit && updateAddr_ != -1) { + // In case this is a delay slot, clear the previous instruction too. + if (updateAddr_ != 0) + mipsr4k.InvalidateICache(updateAddr_ - 4, 8); + else + mipsr4k.ClearJitCache(); + } + + if (anyMemChecks_ && updateAddr_ != -1) UpdateCachedMemCheckRanges(); // Redraw in order to show the breakpoint. System_Notify(SystemNotification::DISASSEMBLY); + needsUpdate_ = false; } bool BreakpointManager::ValidateLogFormat(MIPSDebugInterface *cpu, const std::string &fmt) { diff --git a/Core/Debugger/Breakpoints.h b/Core/Debugger/Breakpoints.h index e8ed006a43..08a30a5757 100644 --- a/Core/Debugger/Breakpoints.h +++ b/Core/Debugger/Breakpoints.h @@ -103,7 +103,7 @@ struct MemCheck { // Called on a copy. BreakAction Action(u32 addr, bool write, int size, u32 pc, const char *reason); - void Log(u32 addr, bool write, int size, u32 pc, const char *reason); + void Log(u32 addr, bool write, int size, u32 pc, const char *reason) const; bool IsEnabled() const { return (result & BREAK_ACTION_PAUSE) != 0; @@ -183,12 +183,17 @@ public: return anyMemChecks_; } - void Update(u32 addr = 0); + void Frame(); bool ValidateLogFormat(MIPSDebugInterface *cpu, const std::string &fmt); bool EvaluateLogFormat(MIPSDebugInterface *cpu, const std::string &fmt, std::string &result); private: + // Should be called under lock. + void Update(u32 addr = 0) { + needsUpdate_ = true; + updateAddr_ = addr; + } size_t FindBreakpoint(u32 addr, bool matchTemp = false, bool temp = false); // Finds exactly, not using a range check. size_t FindMemCheck(u32 start, u32 end); @@ -208,6 +213,9 @@ private: std::vector memChecks_; std::vector memCheckRangesRead_; std::vector memCheckRangesWrite_; + + bool needsUpdate_ = true; + u32 updateAddr_ = 0; }; extern BreakpointManager g_breakpoints; diff --git a/UI/MainScreen.cpp b/UI/MainScreen.cpp index 2decc35572..1dc7f1c61b 100644 --- a/UI/MainScreen.cpp +++ b/UI/MainScreen.cpp @@ -1691,7 +1691,7 @@ void UmdReplaceScreen::CreateViews() { if (System_GetPropertyBool(SYSPROP_HAS_FILE_BROWSER)) { rightColumnItems->Add(new Choice(mm->T("Load", "Load...")))->OnClick.Add([&](UI::EventParams &e) { auto mm = GetI18NCategory(I18NCat::MAINMENU); - System_BrowseForFile(GetRequesterToken(), mm->T("Load"), BrowseFileType::BOOTABLE, [&](const std::string &value, int) { + System_BrowseForFile(GetRequesterToken(), mm->T("Load"), BrowseFileType::BOOTABLE, [this](const std::string &value, int) { __UmdReplace(Path(value)); TriggerFinish(DR_OK); }); diff --git a/UI/NativeApp.cpp b/UI/NativeApp.cpp index e3058fbb7c..eaf06f64de 100644 --- a/UI/NativeApp.cpp +++ b/UI/NativeApp.cpp @@ -89,6 +89,7 @@ #include "Core/Config.h" #include "Core/ConfigValues.h" #include "Core/Core.h" +#include "Core/Debugger/Breakpoints.h" #include "Core/FileLoaders/DiskCachingFileLoader.h" #include "Core/FrameTiming.h" #include "Core/KeyMap.h" @@ -1075,6 +1076,8 @@ void NativeFrame(GraphicsContext *graphicsContext) { g_requestManager.ProcessRequests(); + g_breakpoints.Frame(); + // Apply the UIContext bounds as a 2D transformation matrix. Matrix4x4 ortho = ComputeOrthoMatrix(g_display.dp_xres, g_display.dp_yres, graphicsContext->GetDrawContext()->GetDeviceCaps().coordConvention); diff --git a/Windows/MainWindowMenu.cpp b/Windows/MainWindowMenu.cpp index ab57497406..3c02a354e2 100644 --- a/Windows/MainWindowMenu.cpp +++ b/Windows/MainWindowMenu.cpp @@ -370,6 +370,7 @@ namespace MainWindow { static void UmdSwitchAction(RequesterToken token) { auto mm = GetI18NCategory(I18NCat::MAINMENU); System_BrowseForFile(token, mm->T("Switch UMD"), BrowseFileType::BOOTABLE, [](const std::string &value, int) { + // This is safe because the callback runs on the emu thread. __UmdReplace(Path(value)); }); } From 95cf9c2e28aed913ab2490f274ffbe634c53593b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 3 Mar 2025 13:53:02 +0100 Subject: [PATCH 3/7] Just some constification --- Core/Config.cpp | 2 +- Core/Config.h | 2 +- GPU/Common/GPUDebugInterface.h | 2 +- GPU/Debugger/Breakpoints.cpp | 10 +++++----- GPU/Debugger/Breakpoints.h | 15 +++++---------- GPU/GPUCommon.h | 2 +- GPU/Software/SoftGpu.h | 2 +- 7 files changed, 15 insertions(+), 20 deletions(-) diff --git a/Core/Config.cpp b/Core/Config.cpp index 4080f1bd98..2d7c53fafb 100644 --- a/Core/Config.cpp +++ b/Core/Config.cpp @@ -2015,7 +2015,7 @@ void Config::ResetControlLayout() { g_Config.fRightStickHeadScale = 1.0f; } -void Config::GetReportingInfo(UrlEncoder &data) { +void Config::GetReportingInfo(UrlEncoder &data) const { for (size_t i = 0; i < numSections; ++i) { const std::string prefix = std::string("config.") + sections[i].section; for (size_t j = 0; j < sections[i].settingsCount; j++) { diff --git a/Core/Config.h b/Core/Config.h index 7e60fa50f8..844c124d20 100644 --- a/Core/Config.h +++ b/Core/Config.h @@ -632,7 +632,7 @@ public: void ResetControlLayout(); - void GetReportingInfo(UrlEncoder &data); + void GetReportingInfo(UrlEncoder &data) const; bool IsPortrait() const; int NextValidBackend(); diff --git a/GPU/Common/GPUDebugInterface.h b/GPU/Common/GPUDebugInterface.h index a4b7c1c83a..378ac361f4 100644 --- a/GPU/Common/GPUDebugInterface.h +++ b/GPU/Common/GPUDebugInterface.h @@ -205,7 +205,7 @@ struct GPUDebugVertex { class GPUDebugInterface { public: - virtual ~GPUDebugInterface() {} + virtual ~GPUDebugInterface() = default; virtual bool GetCurrentDisplayList(DisplayList &list) = 0; virtual int GetCurrentPrimCount() = 0; virtual std::vector ActiveDisplayLists() = 0; diff --git a/GPU/Debugger/Breakpoints.cpp b/GPU/Debugger/Breakpoints.cpp index 5dc20f3f36..d22c6e11ee 100644 --- a/GPU/Debugger/Breakpoints.cpp +++ b/GPU/Debugger/Breakpoints.cpp @@ -42,7 +42,7 @@ const static u8 textureRelatedCmds[] = { GE_CMD_TEXFLUSH, GE_CMD_TEXSYNC, }; -void GPUBreakpoints::Init() { +GPUBreakpoints::GPUBreakpoints() { ClearAllBreakpoints(); nonTextureCmds.clear(); @@ -268,20 +268,20 @@ bool GPUBreakpoints::IsRenderTargetBreakpoint(u32 addr) { return breakRenderTargets.find(addr) != breakRenderTargets.end(); } -bool GPUBreakpoints::IsOpBreakpoint(u32 op, bool &temp) { +bool GPUBreakpoints::IsOpBreakpoint(u32 op, bool &temp) const { return IsCmdBreakpoint(op >> 24, temp); } -bool GPUBreakpoints::IsOpBreakpoint(u32 op) { +bool GPUBreakpoints::IsOpBreakpoint(u32 op) const { return IsCmdBreakpoint(op >> 24); } -bool GPUBreakpoints::IsCmdBreakpoint(u8 cmd, bool &temp) { +bool GPUBreakpoints::IsCmdBreakpoint(u8 cmd, bool &temp) const { temp = breakCmdsTemp[cmd]; return breakCmds[cmd]; } -bool GPUBreakpoints::IsCmdBreakpoint(u8 cmd) { +bool GPUBreakpoints::IsCmdBreakpoint(u8 cmd) const { return breakCmds[cmd]; } diff --git a/GPU/Debugger/Breakpoints.h b/GPU/Debugger/Breakpoints.h index 10de2f005d..f2ddb24be8 100644 --- a/GPU/Debugger/Breakpoints.h +++ b/GPU/Debugger/Breakpoints.h @@ -29,17 +29,16 @@ struct GECmdInfo; class GPUBreakpoints { public: - GPUBreakpoints() { - Init(); - } - void Init(); + GPUBreakpoints(); bool IsBreakpoint(u32 pc, u32 op); bool IsAddressBreakpoint(u32 addr, bool &temp); bool IsAddressBreakpoint(u32 addr); - bool IsCmdBreakpoint(u8 cmd, bool &temp); - bool IsCmdBreakpoint(u8 cmd); + bool IsCmdBreakpoint(u8 cmd, bool &temp) const; + bool IsCmdBreakpoint(u8 cmd) const; + bool IsOpBreakpoint(u32 op, bool &temp) const; + bool IsOpBreakpoint(u32 op) const; bool IsTextureBreakpoint(u32 addr, bool &temp); bool IsTextureBreakpoint(u32 addr); bool IsRenderTargetBreakpoint(u32 addr, bool &temp); @@ -69,10 +68,6 @@ public: void ClearAllBreakpoints(); void ClearTempBreakpoints(); - bool IsOpBreakpoint(u32 op, bool &temp); - - bool IsOpBreakpoint(u32 op); - bool HasBreakpoints() const { return hasBreakpoints_; } diff --git a/GPU/GPUCommon.h b/GPU/GPUCommon.h index 68bfcfe9ef..fa5d5672b2 100644 --- a/GPU/GPUCommon.h +++ b/GPU/GPUCommon.h @@ -370,7 +370,7 @@ public: return -1; } - virtual void GetReportingInfo(std::string &primaryInfo, std::string &fullInfo) { + virtual void GetReportingInfo(std::string &primaryInfo, std::string &fullInfo) const { primaryInfo = reportingPrimaryInfo_; fullInfo = reportingFullInfo_; } diff --git a/GPU/Software/SoftGpu.h b/GPU/Software/SoftGpu.h index 058a5b3428..5faaee3568 100644 --- a/GPU/Software/SoftGpu.h +++ b/GPU/Software/SoftGpu.h @@ -156,7 +156,7 @@ public: void CheckDisplayResized() override; void CheckConfigChanged() override; - void GetReportingInfo(std::string &primaryInfo, std::string &fullInfo) override { + void GetReportingInfo(std::string &primaryInfo, std::string &fullInfo) const override { primaryInfo = "Software"; fullInfo = "Software"; } From 65ade393c993635d60a5954282f3c324b35f1abb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 3 Mar 2025 14:01:56 +0100 Subject: [PATCH 4/7] Win32 menu: Fix threading issues with stop/reset commands --- Windows/MainWindowMenu.cpp | 6 ------ 1 file changed, 6 deletions(-) diff --git a/Windows/MainWindowMenu.cpp b/Windows/MainWindowMenu.cpp index 3c02a354e2..228331072f 100644 --- a/Windows/MainWindowMenu.cpp +++ b/Windows/MainWindowMenu.cpp @@ -502,17 +502,11 @@ namespace MainWindow { break; case ID_EMULATION_STOP: - if (Core_IsStepping()) - Core_Resume(); - - Core_Stop(); System_PostUIMessage(UIMessage::REQUEST_GAME_STOP); - Core_WaitInactive(); break; case ID_EMULATION_RESET: System_PostUIMessage(UIMessage::REQUEST_GAME_RESET); - Core_Resume(); break; case ID_EMULATION_SWITCH_UMD: From 0c299bc21d78d76dce03c14d23006618c7340b0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 3 Mar 2025 14:24:07 +0100 Subject: [PATCH 5/7] Windows graphics settings: Change critical settings on the main thread to avoid in-frame "value tearing" --- Windows/MainWindowMenu.cpp | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/Windows/MainWindowMenu.cpp b/Windows/MainWindowMenu.cpp index 228331072f..1697ee288c 100644 --- a/Windows/MainWindowMenu.cpp +++ b/Windows/MainWindowMenu.cpp @@ -384,17 +384,24 @@ namespace MainWindow { // not static void setTexScalingMultiplier(int level) { - g_Config.iTexScalingLevel = level; + System_RunOnMainThread([level]() { + g_Config.iTexScalingLevel = level; + }); System_PostUIMessage(UIMessage::GPU_CONFIG_CHANGED); } static void setTexScalingType(int type) { - g_Config.iTexScalingType = type; + System_RunOnMainThread([type]() { + g_Config.iTexScalingType = type; + }); System_PostUIMessage(UIMessage::GPU_CONFIG_CHANGED); } static void setSkipBufferEffects(bool skip) { - g_Config.bSkipBufferEffects = skip; + System_RunOnMainThread([skip]() { + g_Config.bSkipBufferEffects = skip; + }); + System_PostUIMessage(UIMessage::GPU_RENDER_RESIZED); System_PostUIMessage(UIMessage::GPU_CONFIG_CHANGED); } @@ -655,14 +662,13 @@ namespace MainWindow { case ID_OPTIONS_VSYNC: g_Config.bVSync = !g_Config.bVSync; - NativeResized(); + System_PostUIMessage(UIMessage::GPU_CONFIG_CHANGED); break; case ID_OPTIONS_FRAMESKIP_AUTO: g_Config.bAutoFrameSkip = !g_Config.bAutoFrameSkip; if (g_Config.bAutoFrameSkip && g_Config.bSkipBufferEffects) { - g_Config.bSkipBufferEffects = false; - System_PostUIMessage(UIMessage::GPU_CONFIG_CHANGED); + setSkipBufferEffects(false); } break; @@ -707,8 +713,7 @@ namespace MainWindow { break; case ID_OPTIONS_SKIP_BUFFER_EFFECTS: - g_Config.bSkipBufferEffects = !g_Config.bSkipBufferEffects; - System_PostUIMessage(UIMessage::GPU_RENDER_RESIZED); + setSkipBufferEffects(!g_Config.bSkipBufferEffects); g_OSD.ShowOnOff(gr->T("Skip Buffer Effects"), g_Config.bSkipBufferEffects); break; @@ -724,9 +729,12 @@ namespace MainWindow { break; case ID_OPTIONS_HARDWARETRANSFORM: - g_Config.bHardwareTransform = !g_Config.bHardwareTransform; - System_PostUIMessage(UIMessage::GPU_CONFIG_CHANGED); - g_OSD.ShowOnOff(gr->T("Hardware Transform"), g_Config.bHardwareTransform); + System_RunOnMainThread([]() { + auto gr = GetI18NCategory(I18NCat::GRAPHICS); + g_Config.bHardwareTransform = !g_Config.bHardwareTransform; + System_PostUIMessage(UIMessage::GPU_CONFIG_CHANGED); + g_OSD.ShowOnOff(gr->T("Hardware Transform"), g_Config.bHardwareTransform); + }); break; case ID_OPTIONS_DISPLAY_LAYOUT: From 3ae469a4f85fe62b06f488236c4a244a31134226 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 3 Mar 2025 14:47:00 +0100 Subject: [PATCH 6/7] Remove an indentation level for clearer code --- GPU/GPUCommon.cpp | 45 ++++++++++++++++++++++----------------------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/GPU/GPUCommon.cpp b/GPU/GPUCommon.cpp index e0ab9df04e..f1f49faccf 100644 --- a/GPU/GPUCommon.cpp +++ b/GPU/GPUCommon.cpp @@ -623,32 +623,31 @@ bool GPUCommon::SlowRunLoop(DisplayList &list) { const bool dumpThisFrame = dumpThisFrame_; while (downcount > 0) { GPUDebug::NotifyResult result = NotifyCommand(list.pc, &breakpoints_); - - if (result == GPUDebug::NotifyResult::Execute) { - recorder_.NotifyCommand(list.pc); - u32 op = Memory::ReadUnchecked_U32(list.pc); - u32 cmd = op >> 24; - - u32 diff = op ^ gstate.cmdmem[cmd]; - PreExecuteOp(op, diff); - if (dumpThisFrame) { - char temp[256]; - u32 prev; - if (Memory::IsValidAddress(list.pc - 4)) { - prev = Memory::ReadUnchecked_U32(list.pc - 4); - } else { - prev = 0; - } - GeDisassembleOp(list.pc, op, prev, temp, 256); - NOTICE_LOG(Log::G3D, "%08x: %s", op, temp); - } - gstate.cmdmem[cmd] = op; - - ExecuteOp(op, diff); - } else if (result == GPUDebug::NotifyResult::Break) { + if (result == GPUDebug::NotifyResult::Break) { return false; } + recorder_.NotifyCommand(list.pc); + u32 op = Memory::ReadUnchecked_U32(list.pc); + u32 cmd = op >> 24; + + u32 diff = op ^ gstate.cmdmem[cmd]; + PreExecuteOp(op, diff); + if (dumpThisFrame) { + char temp[256]; + u32 prev; + if (Memory::IsValidAddress(list.pc - 4)) { + prev = Memory::ReadUnchecked_U32(list.pc - 4); + } else { + prev = 0; + } + GeDisassembleOp(list.pc, op, prev, temp, 256); + NOTICE_LOG(Log::G3D, "%08x: %s", op, temp); + } + gstate.cmdmem[cmd] = op; + + ExecuteOp(op, diff); + list.pc += 4; --downcount; } From 2b2d2396f13db672ed895cfdc9c5d45dc0e42d9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 3 Mar 2025 14:47:14 +0100 Subject: [PATCH 7/7] Fix miscounting of prim calls when stepping by draw --- GPU/GPUCommon.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/GPU/GPUCommon.cpp b/GPU/GPUCommon.cpp index f1f49faccf..972c36f528 100644 --- a/GPU/GPUCommon.cpp +++ b/GPU/GPUCommon.cpp @@ -2140,6 +2140,10 @@ void GPUCommon::NotifyFlush() { if (primAfterDraw_) { NOTICE_LOG(Log::GeDebugger, "Flush detected, breaking at next PRIM"); primAfterDraw_ = false; + + // We've got one to rewind. + primsThisFrame_--; + // Switch to PRIM mode. SetBreakNext(BreakNext::PRIM); }