From 99fa1d12fc24de053e034d15f351ac5108ee61d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 24 Feb 2025 11:16:15 -0600 Subject: [PATCH 1/3] ImDebugger: Fix various problems with dupe menu items and coloring of popups --- UI/ImDebugger/ImDebugger.cpp | 20 ++++++++++++-------- UI/ImDebugger/ImDebugger.h | 2 +- UI/ImDebugger/ImGe.cpp | 8 ++++---- 3 files changed, 17 insertions(+), 13 deletions(-) diff --git a/UI/ImDebugger/ImDebugger.cpp b/UI/ImDebugger/ImDebugger.cpp index d366ae66fc27..ee1eddf93593 100644 --- a/UI/ImDebugger/ImDebugger.cpp +++ b/UI/ImDebugger/ImDebugger.cpp @@ -54,6 +54,7 @@ #include "UI/ImDebugger/ImGe.h" extern bool g_TakeScreenshot; +static ImVec4 g_normalTextColor; void ShowInMemoryViewerMenuItem(uint32_t addr, ImControl &control) { if (ImGui::BeginMenu("Show in memory viewer")) { @@ -96,14 +97,16 @@ void StatusBar(std::string_view status) { // TODO: Style it. // Left click performs the preferred action, if any. Right click opens a menu for more. -void ImClickableAddress(uint32_t addr, ImControl &control, ImCmd cmd) { +void ImClickableValue(const char *id, uint32_t addr, ImControl &control, ImCmd cmd) { + ImGui::PushID(id); char temp[32]; snprintf(temp, sizeof(temp), "%08x", addr); if (ImGui::SmallButton(temp)) { control.command = { cmd, addr }; } - // Create a right-click popup menu + // Create a right-click popup menu. Restore the color while it's up. NOTE: can't query the theme, pushcolor modifies it! + ImGui::PushStyleColor(ImGuiCol_Text, g_normalTextColor); if (ImGui::BeginPopupContextItem(temp)) { if (ImGui::MenuItem("Copy address to clipboard")) { System_CopyStringToClipboard(temp); @@ -112,6 +115,8 @@ void ImClickableAddress(uint32_t addr, ImControl &control, ImCmd cmd) { ShowInWindowMenuItems(addr, control); ImGui::EndPopup(); } + ImGui::PopStyleColor(); + ImGui::PopID(); } void DrawSchedulerView(ImConfig &cfg) { @@ -171,9 +176,7 @@ static void DrawGPRs(ImConfig &config, ImControl &control, const MIPSDebugInterf ImGui::PushStyleColor(ImGuiCol_Text, IM_COL32(255, 255, 255, 128)); } if (Memory::IsValid4AlignedAddress(value)) { - ImGui::PushID(index); - ImClickableAddress(value, control, index == MIPS_REG_RA ? ImCmd::SHOW_IN_CPU_DISASM : ImCmd::SHOW_IN_MEMORY_VIEWER); - ImGui::PopID(); + ImClickableValue(regname, value, control, index == MIPS_REG_RA ? ImCmd::SHOW_IN_CPU_DISASM : ImCmd::SHOW_IN_MEMORY_VIEWER); } else { ImGui::Text("%08x", value); } @@ -349,10 +352,10 @@ void DrawThreadView(ImConfig &cfg, ImControl &control) { for (int i = 0; i < (int)info.size(); i++) { const DebugThreadInfo &thread = info[i]; ImGui::TableNextRow(); + ImGui::PushID(i); ImGui::TableNextColumn(); ImGui::Text("%d", thread.id); ImGui::TableNextColumn(); - ImGui::PushID(i); if (ImGui::Selectable(thread.name, cfg.selectedThread == i, ImGuiSelectableFlags_AllowDoubleClick | ImGuiSelectableFlags_SpanAllColumns)) { cfg.selectedThread = i; } @@ -361,9 +364,9 @@ void DrawThreadView(ImConfig &cfg, ImControl &control) { ImGui::OpenPopup("threadPopup"); } ImGui::TableNextColumn(); - ImClickableAddress(thread.curPC, control, ImCmd::SHOW_IN_CPU_DISASM); + ImClickableValue("curpc", thread.curPC, control, ImCmd::SHOW_IN_CPU_DISASM); ImGui::TableNextColumn(); - ImClickableAddress(thread.entrypoint, control, ImCmd::SHOW_IN_CPU_DISASM); + ImClickableValue("entry", thread.entrypoint, control, ImCmd::SHOW_IN_CPU_DISASM); ImGui::TableNextColumn(); ImGui::Text("%d", thread.priority); ImGui::TableNextColumn(); @@ -1287,6 +1290,7 @@ void DrawHLEModules(ImConfig &config) { ImDebugger::ImDebugger() { reqToken_ = g_requestManager.GenerateRequesterToken(); cfg_.LoadConfig(ConfigPath()); + g_normalTextColor = ImGui::GetStyleColorVec4(ImGuiCol_Text); } ImDebugger::~ImDebugger() { diff --git a/UI/ImDebugger/ImDebugger.h b/UI/ImDebugger/ImDebugger.h index 685c99373eba..d63a6cdcc240 100644 --- a/UI/ImDebugger/ImDebugger.h +++ b/UI/ImDebugger/ImDebugger.h @@ -233,7 +233,7 @@ class ImDebugger { }; // Simple custom controls and utilities. -void ImClickableAddress(uint32_t addr, ImControl &control, ImCmd cmd); +void ImClickableValue(const char *id, uint32_t addr, ImControl &control, ImCmd cmd); void ShowInWindowMenuItems(uint32_t addr, ImControl &control); void ShowInMemoryViewerMenuItem(uint32_t addr, ImControl &control); void ShowInMemoryDumperMenuItem(uint32_t addr, uint32_t size, MemDumpMode mode, ImControl &control); diff --git a/UI/ImDebugger/ImGe.cpp b/UI/ImDebugger/ImGe.cpp index 6216741fa174..63b10dea867c 100644 --- a/UI/ImDebugger/ImGe.cpp +++ b/UI/ImDebugger/ImGe.cpp @@ -926,17 +926,17 @@ void ImGeDebuggerWindow::Draw(ImConfig &cfg, ImControl &control, GPUDebugInterfa ImGui::Text("State: %s", DLStateToString(list.state)); ImGui::TextUnformatted("PC:"); ImGui::SameLine(); - ImClickableAddress(list.pc, control, ImCmd::SHOW_IN_GE_DISASM); + ImClickableValue("pc", list.pc, control, ImCmd::SHOW_IN_GE_DISASM); ImGui::Text("StartPC:"); ImGui::SameLine(); - ImClickableAddress(list.startpc, control, ImCmd::SHOW_IN_GE_DISASM); + ImClickableValue("startpc", list.startpc, control, ImCmd::SHOW_IN_GE_DISASM); if (list.pendingInterrupt) { ImGui::TextUnformatted("(Pending interrupt)"); } if (list.stall) { ImGui::TextUnformatted("Stall addr:"); ImGui::SameLine(); - ImClickableAddress(list.pc, control, ImCmd::SHOW_IN_GE_DISASM); + ImClickableValue("stall", list.pc, control, ImCmd::SHOW_IN_GE_DISASM); } ImGui::Text("Stack depth: %d", (int)list.stackptr); ImGui::Text("BBOX result: %d", (int)list.bboxResult); @@ -1363,7 +1363,7 @@ void ImGeStateWindow::Draw(ImConfig &cfg, ImControl &control, GPUDebugInterface // Special handling for pointer and pointer/width entries - create an address control if (info.fmt == CMD_FMT_PTRWIDTH) { const u32 val = (value & 0xFFFFFF) | (otherValue & 0x00FF0000) << 8; - ImClickableAddress(val, control, ImCmd::NONE); + ImClickableValue(info.uiName, val, control, ImCmd::NONE); ImGui::SameLine(); ImGui::Text("w=%d", otherValue & 0xFFFF); } else { From 08155748ea898d007431f9ccc9ea280bcebf546c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 24 Feb 2025 11:21:13 -0600 Subject: [PATCH 2/3] Use the clickablevalue thing in more places --- UI/ImDebugger/ImDebugger.cpp | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/UI/ImDebugger/ImDebugger.cpp b/UI/ImDebugger/ImDebugger.cpp index ee1eddf93593..1fbb978a8f37 100644 --- a/UI/ImDebugger/ImDebugger.cpp +++ b/UI/ImDebugger/ImDebugger.cpp @@ -900,7 +900,7 @@ static void DrawBreakpointsView(MIPSDebugInterface *mipsDebug, ImConfig &cfg) { ImGui::End(); } -void DrawAudioDecodersView(ImConfig &cfg) { +void DrawAudioDecodersView(ImConfig &cfg, ImControl &control) { if (!ImGui::Begin("Audio decoding contexts", &cfg.audioDecodersOpen)) { ImGui::End(); return; @@ -1159,8 +1159,8 @@ void DrawSasAudio(ImConfig &cfg) { ImGui::End(); } -static void DrawCallStacks(const MIPSDebugInterface *debug, bool *open) { - if (!ImGui::Begin("Callstacks", open)) { +static void DrawCallStacks(const MIPSDebugInterface *debug, ImConfig &config, ImControl &control) { + if (!ImGui::Begin("Callstacks", &config.callstackOpen)) { ImGui::End(); return; } @@ -1197,25 +1197,24 @@ static void DrawCallStacks(const MIPSDebugInterface *debug, bool *open) { ImGui::TableSetColumnIndex(0); ImGui::TextUnformatted(entrySym.c_str()); ImGui::TableSetColumnIndex(1); - ImGui::Text("%08x", frame.entry); + ImClickableValue("frameentry", frame.entry, control, ImCmd::SHOW_IN_CPU_DISASM); ImGui::TableSetColumnIndex(2); - ImGui::Text("%08x", frame.pc); + ImClickableValue("framepc", frame.pc, control, ImCmd::SHOW_IN_CPU_DISASM); ImGui::TableSetColumnIndex(3); ImGui::TextUnformatted("N/A"); // opcode, see the old debugger ImGui::TableSetColumnIndex(4); - ImGui::Text("%08x", frame.sp); + ImClickableValue("framepc", frame.sp, control, ImCmd::SHOW_IN_MEMORY_VIEWER); ImGui::TableSetColumnIndex(5); ImGui::Text("%d", frame.stackSize); ImGui::TableNextRow(); // TODO: More fields? } - ImGui::EndTable(); } ImGui::End(); } -static void DrawModules(const MIPSDebugInterface *debug, ImConfig &cfg) { +static void DrawModules(const MIPSDebugInterface *debug, ImConfig &cfg, ImControl &control) { if (!ImGui::Begin("Modules", &cfg.modulesOpen) || !g_symbolMap) { ImGui::End(); return; @@ -1239,7 +1238,7 @@ static void DrawModules(const MIPSDebugInterface *debug, ImConfig &cfg) { cfg.selectedModule = i; } ImGui::TableNextColumn(); - ImGui::Text("%08x", module.address); + ImClickableValue("addr", module.address, control, ImCmd::SHOW_IN_MEMORY_VIEWER); ImGui::TableNextColumn(); ImGui::Text("%08x", module.size); ImGui::TableNextColumn(); @@ -1512,15 +1511,15 @@ void ImDebugger::Frame(MIPSDebugInterface *mipsDebug, GPUDebugInterface *gpuDebu } if (cfg_.callstackOpen) { - DrawCallStacks(mipsDebug, &cfg_.callstackOpen); + DrawCallStacks(mipsDebug, cfg_, control); } if (cfg_.modulesOpen) { - DrawModules(mipsDebug, cfg_); + DrawModules(mipsDebug, cfg_, control); } if (cfg_.audioDecodersOpen) { - DrawAudioDecodersView(cfg_); + DrawAudioDecodersView(cfg_, control); } if (cfg_.hleModulesOpen) { From d996e66b261b1cb45d97637f4a6035981eca4af4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 24 Feb 2025 11:43:07 -0600 Subject: [PATCH 3/3] ImClickableValue: Switch to selectable, add polish. Use for all GPRs. --- UI/ImDebugger/ImDebugger.cpp | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/UI/ImDebugger/ImDebugger.cpp b/UI/ImDebugger/ImDebugger.cpp index 1fbb978a8f37..0e7e40dd4fbe 100644 --- a/UI/ImDebugger/ImDebugger.cpp +++ b/UI/ImDebugger/ImDebugger.cpp @@ -99,20 +99,25 @@ void StatusBar(std::string_view status) { // Left click performs the preferred action, if any. Right click opens a menu for more. void ImClickableValue(const char *id, uint32_t addr, ImControl &control, ImCmd cmd) { ImGui::PushID(id); + + bool validAddr = Memory::IsValidAddress(addr); + char temp[32]; snprintf(temp, sizeof(temp), "%08x", addr); - if (ImGui::SmallButton(temp)) { + if (ImGui::Selectable(temp) && validAddr) { control.command = { cmd, addr }; } // Create a right-click popup menu. Restore the color while it's up. NOTE: can't query the theme, pushcolor modifies it! ImGui::PushStyleColor(ImGuiCol_Text, g_normalTextColor); if (ImGui::BeginPopupContextItem(temp)) { - if (ImGui::MenuItem("Copy address to clipboard")) { + if (ImGui::MenuItem(validAddr ? "Copy address to clipboard" : "Copy value to clipboard")) { System_CopyStringToClipboard(temp); } - ImGui::Separator(); - ShowInWindowMenuItems(addr, control); + if (validAddr) { + ImGui::Separator(); + ShowInWindowMenuItems(addr, control); + } ImGui::EndPopup(); } ImGui::PopStyleColor(); @@ -175,11 +180,8 @@ static void DrawGPRs(ImConfig &config, ImControl &control, const MIPSDebugInterf } else if (disabled) { ImGui::PushStyleColor(ImGuiCol_Text, IM_COL32(255, 255, 255, 128)); } - if (Memory::IsValid4AlignedAddress(value)) { - ImClickableValue(regname, value, control, index == MIPS_REG_RA ? ImCmd::SHOW_IN_CPU_DISASM : ImCmd::SHOW_IN_MEMORY_VIEWER); - } else { - ImGui::Text("%08x", value); - } + // TODO: Check if the address is in the code segment to decide default action. + ImClickableValue(regname, value, control, index == MIPS_REG_RA ? ImCmd::SHOW_IN_CPU_DISASM : ImCmd::SHOW_IN_MEMORY_VIEWER); ImGui::TableNextColumn(); if (value >= -1000000 && value <= 1000000) { ImGui::Text("%d", value);