Merge pull request #9542 from InusualZ/toggle-bp

BreakpointWidget: Allow breakpoints to be toggled between enable/disable

Source/Core/Core/PowerPC/BreakPoints.cpp

@@ -23,6 +23,12 @@ bool BreakPoints::IsAddressBreakPoint(u32 address) const
                      [address](const auto& bp) { return bp.address == address; });
 }
 
+bool BreakPoints::IsBreakPointEnable(u32 address) const
+{
+  return std::any_of(m_breakpoints.begin(), m_breakpoints.end(),
+                     [address](const auto& bp) { return bp.is_enabled && bp.address == address; });
+}
+
 bool BreakPoints::IsTempBreakPoint(u32 address) const
 {
   return std::any_of(m_breakpoints.begin(), m_breakpoints.end(), [address](const auto& bp) {
@@ -51,6 +57,8 @@ BreakPoints::TBreakPointsStr BreakPoints::GetStrings() const
     if (!bp.is_temporary)
     {
       std::ostringstream ss;
+      ss.imbue(std::locale::classic());
+
       ss << std::hex << bp.address << " " << (bp.is_enabled ? "n" : "")
          << (bp.log_on_hit ? "l" : "") << (bp.break_on_hit ? "b" : "");
       bp_strings.push_back(ss.str());
@@ -67,6 +75,8 @@ void BreakPoints::AddFromStrings(const TBreakPointsStr& bp_strings)
     TBreakPoint bp;
     std::string flags;
     std::istringstream iss(bp_string);
+    iss.imbue(std::locale::classic());
+
     iss >> std::hex >> bp.address;
     iss >> flags;
     bp.is_enabled = flags.find('n') != flags.npos;
@@ -110,6 +120,18 @@ void BreakPoints::Add(u32 address, bool temp, bool break_on_hit, bool log_on_hit
   JitInterface::InvalidateICache(address, 4, true);
 }
 
+bool BreakPoints::ToggleBreakPoint(u32 address)
+{
+  auto iter = std::find_if(m_breakpoints.begin(), m_breakpoints.end(),
+                           [address](const auto& bp) { return bp.address == address; });
+
+  if (iter == m_breakpoints.end())
+    return false;
+
+  iter->is_enabled = !iter->is_enabled;
+  return true;
+}
+
 void BreakPoints::Remove(u32 address)
 {
   const auto iter = std::find_if(m_breakpoints.begin(), m_breakpoints.end(),
@@ -155,11 +177,11 @@ MemChecks::TMemChecksStr MemChecks::GetStrings() const
   for (const TMemCheck& mc : m_mem_checks)
   {
     std::ostringstream ss;
-    ss << std::hex << mc.start_address;
-    ss << " " << (mc.is_ranged ? mc.end_address : mc.start_address) << " "
-       << (mc.is_ranged ? "n" : "") << (mc.is_break_on_read ? "r" : "")
-       << (mc.is_break_on_write ? "w" : "") << (mc.log_on_hit ? "l" : "")
-       << (mc.break_on_hit ? "b" : "");
+    ss.imbue(std::locale::classic());
+
+    ss << std::hex << mc.start_address << " " << mc.end_address << " " << (mc.is_enabled ? "n" : "")
+       << (mc.is_break_on_read ? "r" : "") << (mc.is_break_on_write ? "w" : "")
+       << (mc.log_on_hit ? "l" : "") << (mc.break_on_hit ? "b" : "");
     mc_strings.push_back(ss.str());
   }
 
@@ -171,18 +193,19 @@ void MemChecks::AddFromStrings(const TMemChecksStr& mc_strings)
   for (const std::string& mc_string : mc_strings)
   {
     TMemCheck mc;
-    std::stringstream ss;
-    ss << std::hex << mc_string;
-    ss >> mc.start_address;
-    mc.is_ranged = mc_string.find('n') != mc_string.npos;
-    mc.is_break_on_read = mc_string.find('r') != mc_string.npos;
-    mc.is_break_on_write = mc_string.find('w') != mc_string.npos;
-    mc.log_on_hit = mc_string.find('l') != mc_string.npos;
-    mc.break_on_hit = mc_string.find('b') != mc_string.npos;
-    if (mc.is_ranged)
-      ss >> mc.end_address;
-    else
-      mc.end_address = mc.start_address;
+    std::istringstream iss(mc_string);
+    iss.imbue(std::locale::classic());
+
+    std::string flags;
+    iss >> std::hex >> mc.start_address >> mc.end_address >> flags;
+
+    mc.is_ranged = mc.start_address != mc.end_address;
+    mc.is_enabled = flags.find('n') != flags.npos;
+    mc.is_break_on_read = flags.find('r') != flags.npos;
+    mc.is_break_on_write = flags.find('w') != flags.npos;
+    mc.log_on_hit = flags.find('l') != flags.npos;
+    mc.break_on_hit = flags.find('b') != flags.npos;
+
     Add(mc);
   }
 }
@@ -203,6 +226,18 @@ void MemChecks::Add(const TMemCheck& memory_check)
   });
 }
 
+bool MemChecks::ToggleBreakPoint(u32 address)
+{
+  auto iter = std::find_if(m_mem_checks.begin(), m_mem_checks.end(),
+                           [address](const auto& bp) { return bp.start_address == address; });
+
+  if (iter == m_mem_checks.end())
+    return false;
+
+  iter->is_enabled = !iter->is_enabled;
+  return true;
+}
+
 void MemChecks::Remove(u32 address)
 {
   const auto iter =
@@ -262,6 +297,9 @@ bool MemChecks::OverlapsMemcheck(u32 address, u32 length) const
 bool TMemCheck::Action(Common::DebugInterface* debug_interface, u32 value, u32 addr, bool write,
                        size_t size, u32 pc)
 {
+  if (!is_enabled)
+    return false;
+
   if ((write && is_break_on_write) || (!write && is_break_on_read))
   {
     if (log_on_hit)

Source/Core/Core/PowerPC/BreakPoints.h

@@ -29,6 +29,7 @@ struct TMemCheck
   u32 start_address = 0;
   u32 end_address = 0;
 
+  bool is_enabled = true;
   bool is_ranged = false;
 
   bool is_break_on_read = false;
@@ -57,6 +58,7 @@ public:
 
   // is address breakpoint
   bool IsAddressBreakPoint(u32 address) const;
+  bool IsBreakPointEnable(u32 adresss) const;
   bool IsTempBreakPoint(u32 address) const;
   bool IsBreakPointBreakOnHit(u32 address) const;
   bool IsBreakPointLogOnHit(u32 address) const;
@@ -66,6 +68,9 @@ public:
   void Add(u32 address, bool temp = false);
   void Add(const TBreakPoint& bp);
 
+  // Modify Breakpoint
+  bool ToggleBreakPoint(u32 address);
+
   // Remove Breakpoint
   void Remove(u32 address);
   void Clear();
@@ -88,6 +93,8 @@ public:
 
   void Add(const TMemCheck& memory_check);
 
+  bool ToggleBreakPoint(u32 address);
+
   // memory breakpoint
   TMemCheck* GetMemCheck(u32 address, size_t size = 1);
   bool OverlapsMemcheck(u32 address, u32 length) const;

Source/Core/Core/PowerPC/PowerPC.cpp

@@ -602,8 +602,9 @@ void CheckExternalExceptions()
 
 void CheckBreakPoints()
 {
-  if (PowerPC::breakpoints.IsAddressBreakPoint(PC))
-  {
+  if (!PowerPC::breakpoints.IsBreakPointEnable(PC))
+    return;
+
   if (PowerPC::breakpoints.IsBreakPointBreakOnHit(PC))
     CPU::Break();
   if (PowerPC::breakpoints.IsBreakPointLogOnHit(PC))
@@ -617,7 +618,6 @@ void CheckBreakPoints()
   if (PowerPC::breakpoints.IsTempBreakPoint(PC))
     PowerPC::breakpoints.Remove(PC);
 }
-}
 
 void PowerPCState::SetSR(u32 index, u32 value)
 {

Source/Core/DolphinQt/Debugger/BreakpointWidget.cpp

@@ -5,6 +5,7 @@
 #include "DolphinQt/Debugger/BreakpointWidget.h"
 
 #include <QHeaderView>
+#include <QMenu>
 #include <QTableWidget>
 #include <QToolBar>
 #include <QVBoxLayout>
@@ -80,14 +81,10 @@ void BreakpointWidget::CreateWidgets()
   m_table->setEditTriggers(QAbstractItemView::NoEditTriggers);
   m_table->verticalHeader()->hide();
 
-  connect(m_table, &QTableWidget::itemClicked, [this](QTableWidgetItem* item) {
-    if (m_table->selectedItems()[0]->row() == item->row() &&
-        Core::GetState() == Core::State::Paused)
-    {
-      auto address = m_table->selectedItems()[0]->data(Qt::UserRole).toUInt();
-      emit SelectedBreakpoint(address);
-    }
-  });
+  connect(m_table, &QTableWidget::customContextMenuRequested, this,
+          &BreakpointWidget::OnContextMenu);
+
+  m_table->setContextMenuPolicy(Qt::ContextMenuPolicy::CustomContextMenu);
 
   auto* layout = new QVBoxLayout;
 
@@ -168,7 +165,7 @@ void BreakpointWidget::Update()
   {
     m_table->setRowCount(i + 1);
 
-    auto* active = create_item(bp.is_enabled ? tr("on") : QString());
+    auto* active = create_item(bp.is_enabled ? tr("on") : tr("off"));
 
     active->setData(Qt::UserRole, bp.address);
 
@@ -201,7 +198,8 @@ void BreakpointWidget::Update()
   for (const auto& mbp : PowerPC::memchecks.GetMemChecks())
   {
     m_table->setRowCount(i + 1);
-    auto* active = create_item(mbp.break_on_hit || mbp.log_on_hit ? tr("on") : QString());
+    auto* active =
+        create_item(mbp.is_enabled && (mbp.break_on_hit || mbp.log_on_hit) ? tr("on") : tr("off"));
     active->setData(Qt::UserRole, mbp.start_address);
 
     m_table->setItem(i, 0, active);
@@ -310,6 +308,60 @@ void BreakpointWidget::OnSave()
   ini.Save(File::GetUserPath(D_GAMESETTINGS_IDX) + SConfig::GetInstance().GetGameID() + ".ini");
 }
 
+void BreakpointWidget::OnContextMenu()
+{
+  const auto& selected_items = m_table->selectedItems();
+  if (selected_items.isEmpty())
+  {
+    return;
+  }
+
+  const auto& selected_item = selected_items.constFirst();
+  const auto bp_address = static_cast<u32>(selected_item->data(Qt::UserRole).toUInt());
+
+  auto is_memory_breakpoint = false;
+
+  auto* menu = new QMenu(this);
+
+  const auto& inst_breakpoints = PowerPC::breakpoints.GetBreakPoints();
+  const auto bp_iter =
+      std::find_if(inst_breakpoints.begin(), inst_breakpoints.end(),
+                   [bp_address](const auto& bp) { return bp.address == bp_address; });
+  if (bp_iter != inst_breakpoints.end())
+  {
+    menu->addAction(bp_iter->is_enabled ? tr("Disable") : tr("Enable"), [this, &bp_address]() {
+      PowerPC::breakpoints.ToggleBreakPoint(bp_address);
+      Update();
+    });
+  }
+  else
+  {
+    // It should be a memory breakpoint
+    const auto& memory_breakpoints = PowerPC::memchecks.GetMemChecks();
+    const auto mb_iter =
+        std::find_if(memory_breakpoints.begin(), memory_breakpoints.end(),
+                     [bp_address](const auto& bp) { return bp.start_address == bp_address; });
+
+    if (mb_iter == memory_breakpoints.end())
+    {
+      return;
+    }
+
+    is_memory_breakpoint = true;
+
+    menu->addAction(mb_iter->is_enabled ? tr("Disable") : tr("Enable"), [this, &bp_address]() {
+      PowerPC::memchecks.ToggleBreakPoint(bp_address);
+      Update();
+    });
+  }
+
+  if (!is_memory_breakpoint)
+  {
+    menu->addAction(tr("Go to"), [this, bp_address]() { emit SelectedBreakpoint(bp_address); });
+  }
+  menu->exec(QCursor::pos());
+}
+
 void BreakpointWidget::AddBP(u32 addr)
 {
   AddBP(addr, false, true, true);

Source/Core/DolphinQt/Debugger/BreakpointWidget.h

@@ -46,6 +46,7 @@ private:
   void OnNewBreakpoint();
   void OnLoad();
   void OnSave();
+  void OnContextMenu();
 
   void UpdateIcons();