From cf6a392979cab265a28f39810e3a1c545e58b5bf Mon Sep 17 00:00:00 2001 From: mitaclaw <140017135+mitaclaw@users.noreply.github.com> Date: Fri, 24 May 2024 17:26:14 -0700 Subject: [PATCH] Branch Watch Tool: Smarter Context Menu Also, right-clicking the table's scroll area when all columns are hidden will show the column visibility menu. --- .../DolphinQt/Debugger/BranchWatchDialog.cpp | 217 ++++++++++-------- .../DolphinQt/Debugger/BranchWatchDialog.h | 18 +- .../Debugger/BranchWatchTableModel.cpp | 12 - .../Debugger/BranchWatchTableModel.h | 1 - 4 files changed, 130 insertions(+), 118 deletions(-) diff --git a/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp b/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp index c20b2e3c2d..810c13cc9c 100644 --- a/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp +++ b/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp @@ -5,6 +5,7 @@ #include #include +#include #include #include @@ -99,7 +100,6 @@ public: this->*member = std::nullopt; invalidateRowsFilter(); } - void OnDelete(QModelIndexList index_list); bool IsBranchTypeAllowed(UGeckoInstruction inst) const; void SetInspected(const QModelIndex& index); @@ -155,13 +155,6 @@ bool BranchWatchProxyModel::filterAcceptsRow(int source_row, const QModelIndex&) return true; } -void BranchWatchProxyModel::OnDelete(QModelIndexList index_list) -{ - std::transform(index_list.begin(), index_list.end(), index_list.begin(), - [this](const QModelIndex& index) { return mapToSource(index); }); - sourceModel()->OnDelete(std::move(index_list)); -} - static constexpr bool BranchSavesLR(UGeckoInstruction inst) { DEBUG_ASSERT(inst.OPCD == 18 || inst.OPCD == 16 || @@ -801,68 +794,18 @@ void BranchWatchDialog::OnTableClicked(const QModelIndex& index) void BranchWatchDialog::OnTableContextMenu(const QPoint& pos) { + if (m_table_view->horizontalHeader()->hiddenSectionCount() == Column::NumberOfColumns) + { + m_mnu_column_visibility->exec(m_table_view->viewport()->mapToGlobal(pos)); + return; + } const QModelIndex index = m_table_view->indexAt(pos); if (!index.isValid()) return; - QModelIndexList index_list = m_table_view->selectionModel()->selectedRows(index.column()); - - QMenu* const menu = new QMenu; - menu->setAttribute(Qt::WA_DeleteOnClose, true); - - menu->addAction(tr("&Delete"), [this, index_list]() { OnTableDelete(std::move(index_list)); }); - switch (index.column()) - { - case Column::Origin: - { - QAction* const action = menu->addAction(tr("Insert &NOP")); - if (Core::GetState(m_system) != Core::State::Uninitialized) - connect(action, &QAction::triggered, - [this, index_list]() { OnTableSetNOP(std::move(index_list)); }); - else - action->setEnabled(false); - menu->addAction(tr("&Copy Address"), [this, index_list = std::move(index_list)]() { - OnTableCopyAddress(std::move(index_list)); - }); - break; - } - case Column::Destination: - { - QAction* const action = menu->addAction(tr("Insert &BLR")); - const bool enable_action = - Core::GetState(m_system) != Core::State::Uninitialized && - std::all_of(index_list.begin(), index_list.end(), [this](const QModelIndex& idx) { - const QModelIndex sibling = idx.siblingAtColumn(Column::Instruction); - return BranchSavesLR(m_table_proxy->data(sibling, UserRole::ClickRole).value()); - }); - if (enable_action) - connect(action, &QAction::triggered, - [this, index_list]() { OnTableSetBLR(std::move(index_list)); }); - else - action->setEnabled(false); - menu->addAction(tr("&Copy Address"), [this, index_list = std::move(index_list)]() { - OnTableCopyAddress(std::move(index_list)); - }); - break; - } - case Column::OriginSymbol: - case Column::DestinSymbol: - { - QAction* const action = menu->addAction(tr("Insert &BLR at start")); - const bool enable_action = - Core::GetState(m_system) != Core::State::Uninitialized && - std::all_of(index_list.begin(), index_list.end(), [this](const QModelIndex& idx) { - return m_table_proxy->data(idx, UserRole::ClickRole).isValid(); - }); - if (enable_action) - connect(action, &QAction::triggered, [this, index_list = std::move(index_list)]() { - OnTableSetBLR(std::move(index_list)); - }); - else - action->setEnabled(false); - break; - } - } - menu->exec(m_table_view->viewport()->mapToGlobal(pos)); + m_index_list_temp = m_table_view->selectionModel()->selectedRows(index.column()); + GetTableContextMenu(index)->exec(m_table_view->viewport()->mapToGlobal(pos)); + m_index_list_temp.clear(); + m_index_list_temp.shrink_to_fit(); } void BranchWatchDialog::OnTableHeaderContextMenu(const QPoint& pos) @@ -870,61 +813,51 @@ void BranchWatchDialog::OnTableHeaderContextMenu(const QPoint& pos) m_mnu_column_visibility->exec(m_table_view->horizontalHeader()->mapToGlobal(pos)); } -void BranchWatchDialog::OnTableDelete(QModelIndexList index_list) +void BranchWatchDialog::OnTableDelete() { - m_table_proxy->OnDelete(std::move(index_list)); + std::ranges::transform( + m_index_list_temp, m_index_list_temp.begin(), + [this](const QModelIndex& index) { return m_table_proxy->mapToSource(index); }); + std::ranges::sort(m_index_list_temp, std::less{}); + for (const auto& index : std::ranges::reverse_view{m_index_list_temp}) + { + if (!index.isValid()) + continue; + m_table_model->removeRow(index.row()); + } UpdateStatus(); } void BranchWatchDialog::OnTableDeleteKeypress() { - OnTableDelete(m_table_view->selectionModel()->selectedRows()); + m_index_list_temp = m_table_view->selectionModel()->selectedRows(); + OnTableDelete(); + m_index_list_temp.clear(); + m_index_list_temp.shrink_to_fit(); } -void BranchWatchDialog::OnTableSetBLR(QModelIndexList index_list) +void BranchWatchDialog::OnTableSetBLR() { - for (const QModelIndex& index : index_list) - { - m_system.GetPowerPC().GetDebugInterface().SetPatch( - Core::CPUThreadGuard{m_system}, - m_table_proxy->data(index, UserRole::ClickRole).value(), 0x4e800020); - m_table_proxy->SetInspected(index); - } - // TODO: This is not ideal. What I need is a signal for when memory has been changed by the GUI, - // but I cannot find one. UpdateDisasmDialog comes close, but does too much in one signal. For - // example, CodeViewWidget will scroll to the current PC when UpdateDisasmDialog is signaled. This - // seems like a pervasive issue. For example, modifying an instruction in the CodeViewWidget will - // not reflect in the MemoryViewWidget, and vice versa. Neither of these widgets changing memory - // will reflect in the JITWidget, either. At the very least, we can make sure the CodeWidget - // is updated in an acceptable way. - m_code_widget->Update(); + SetStubPatches(0x4e800020); } -void BranchWatchDialog::OnTableSetNOP(QModelIndexList index_list) +void BranchWatchDialog::OnTableSetNOP() { - for (const QModelIndex& index : index_list) - { - m_system.GetPowerPC().GetDebugInterface().SetPatch( - Core::CPUThreadGuard{m_system}, - m_table_proxy->data(index, UserRole::ClickRole).value(), 0x60000000); - m_table_proxy->SetInspected(index); - } - // Same issue as OnSetBLR. - m_code_widget->Update(); + SetStubPatches(0x60000000); } -void BranchWatchDialog::OnTableCopyAddress(QModelIndexList index_list) +void BranchWatchDialog::OnTableCopyAddress() { - auto iter = index_list.begin(); - if (iter == index_list.end()) + auto iter = m_index_list_temp.begin(); + if (iter == m_index_list_temp.end()) return; QString text; - text.reserve(index_list.size() * 9 - 1); + text.reserve(m_index_list_temp.size() * 9 - 1); while (true) { text.append(QString::number(m_table_proxy->data(*iter, UserRole::ClickRole).value(), 16)); - if (++iter == index_list.end()) + if (++iter == m_index_list_temp.end()) break; text.append(QChar::fromLatin1('\n')); } @@ -1019,3 +952,85 @@ void BranchWatchDialog::AutoSave(const Core::CPUThreadGuard& guard) return; Save(guard, m_autosave_filepath ? m_autosave_filepath.value() : GetSnapshotDefaultFilepath()); } + +void BranchWatchDialog::SetStubPatches(u32 value) const +{ + auto& debug_interface = m_system.GetPowerPC().GetDebugInterface(); + for (const Core::CPUThreadGuard guard(m_system); const QModelIndex& index : m_index_list_temp) + { + debug_interface.SetPatch(guard, m_table_proxy->data(index, UserRole::ClickRole).value(), + value); + m_table_proxy->SetInspected(index); + } + // TODO: This is not ideal. What I need is a signal for when memory has been changed by the GUI, + // but I cannot find one. UpdateDisasmDialog comes close, but does too much in one signal. For + // example, CodeViewWidget will scroll to the current PC when UpdateDisasmDialog is signaled. This + // seems like a pervasive issue. For example, modifying an instruction in the CodeViewWidget will + // not reflect in the MemoryViewWidget, and vice versa. Neither of these widgets changing memory + // will reflect in the JITWidget, either. At the very least, we can make sure the CodeWidget + // is updated in an acceptable way. + m_code_widget->Update(); +} + +QMenu* BranchWatchDialog::GetTableContextMenu(const QModelIndex& index) +{ + if (m_mnu_table_context == nullptr) + { + m_mnu_table_context = new QMenu(this); + m_mnu_table_context->addAction(tr("&Delete"), this, &BranchWatchDialog::OnTableDelete); + m_act_insert_nop = + m_mnu_table_context->addAction(tr("Insert &NOP"), this, &BranchWatchDialog::OnTableSetNOP); + m_act_insert_blr = + m_mnu_table_context->addAction(tr("Insert &BLR"), this, &BranchWatchDialog::OnTableSetBLR); + m_act_copy_address = m_mnu_table_context->addAction(tr("&Copy Address"), this, + &BranchWatchDialog::OnTableCopyAddress); + } + + bool supported_column = true; + switch (index.column()) + { + case Column::Origin: + m_act_insert_blr->setVisible(false); + m_act_insert_nop->setVisible(true); + m_act_insert_nop->setEnabled(Core::GetState(m_system) != Core::State::Uninitialized); + m_act_copy_address->setEnabled(true); + break; + case Column::Destination: + { + m_act_insert_nop->setVisible(false); + m_act_insert_blr->setVisible(true); + const bool all_branches_save_lr = + Core::GetState(m_system) != Core::State::Uninitialized && + std::all_of( + m_index_list_temp.begin(), m_index_list_temp.end(), [this](const QModelIndex& idx) { + const QModelIndex sibling = idx.siblingAtColumn(Column::Instruction); + return BranchSavesLR(m_table_proxy->data(sibling, UserRole::ClickRole).value()); + }); + m_act_insert_blr->setEnabled(all_branches_save_lr); + m_act_copy_address->setEnabled(true); + break; + } + case Column::OriginSymbol: + case Column::DestinSymbol: + { + m_act_insert_nop->setVisible(false); + m_act_insert_blr->setVisible(true); + const bool all_symbols_valid = + Core::GetState(m_system) != Core::State::Uninitialized && + std::all_of(m_index_list_temp.begin(), m_index_list_temp.end(), + [this](const QModelIndex& idx) { + return m_table_proxy->data(idx, UserRole::ClickRole).isValid(); + }); + m_act_insert_blr->setEnabled(all_symbols_valid); + m_act_copy_address->setEnabled(all_symbols_valid); + break; + } + default: + m_act_insert_nop->setVisible(false); + m_act_insert_blr->setVisible(false); + supported_column = false; + break; + } + m_act_copy_address->setVisible(supported_column); + return m_mnu_table_context; +} diff --git a/Source/Core/DolphinQt/Debugger/BranchWatchDialog.h b/Source/Core/DolphinQt/Debugger/BranchWatchDialog.h index b45f4c8563..b4dcc8812d 100644 --- a/Source/Core/DolphinQt/Debugger/BranchWatchDialog.h +++ b/Source/Core/DolphinQt/Debugger/BranchWatchDialog.h @@ -9,6 +9,8 @@ #include #include +#include "Common/CommonTypes.h" + namespace Core { class BranchWatch; @@ -85,11 +87,11 @@ private: void OnTableClicked(const QModelIndex& index); void OnTableContextMenu(const QPoint& pos); void OnTableHeaderContextMenu(const QPoint& pos); - void OnTableDelete(QModelIndexList index_list); + void OnTableDelete(); void OnTableDeleteKeypress(); - void OnTableSetBLR(QModelIndexList index_list); - void OnTableSetNOP(QModelIndexList index_list); - void OnTableCopyAddress(QModelIndexList index_list); + void OnTableSetBLR(); + void OnTableSetNOP(); + void OnTableCopyAddress(); void SaveSettings(); @@ -102,6 +104,9 @@ private: void Save(const Core::CPUThreadGuard& guard, const std::string& filepath); void Load(const Core::CPUThreadGuard& guard, const std::string& filepath); void AutoSave(const Core::CPUThreadGuard& guard); + void SetStubPatches(u32 value) const; + + [[nodiscard]] QMenu* GetTableContextMenu(const QModelIndex& index); Core::System& m_system; Core::BranchWatch& m_branch_watch; @@ -110,6 +115,10 @@ private: QPushButton *m_btn_start_pause, *m_btn_clear_watch, *m_btn_path_was_taken, *m_btn_path_not_taken, *m_btn_was_overwritten, *m_btn_not_overwritten, *m_btn_wipe_recent_hits; QAction* m_act_autosave; + QAction* m_act_insert_nop; + QAction* m_act_insert_blr; + QAction* m_act_copy_address; + QMenu* m_mnu_table_context = nullptr; QMenu* m_mnu_column_visibility; QToolBar* m_control_toolbar; @@ -119,5 +128,6 @@ private: QStatusBar* m_status_bar; QTimer* m_timer; + QModelIndexList m_index_list_temp; std::optional m_autosave_filepath; }; diff --git a/Source/Core/DolphinQt/Debugger/BranchWatchTableModel.cpp b/Source/Core/DolphinQt/Debugger/BranchWatchTableModel.cpp index 08d7ae021d..488530b34b 100644 --- a/Source/Core/DolphinQt/Debugger/BranchWatchTableModel.cpp +++ b/Source/Core/DolphinQt/Debugger/BranchWatchTableModel.cpp @@ -144,18 +144,6 @@ void BranchWatchTableModel::OnWipeInspection() roles); } -void BranchWatchTableModel::OnDelete(QModelIndexList index_list) -{ - std::sort(index_list.begin(), index_list.end()); - // TODO C++20: std::ranges::reverse_view - for (auto iter = index_list.rbegin(); iter != index_list.rend(); ++iter) - { - if (!iter->isValid()) - continue; - removeRow(iter->row()); - } -} - void BranchWatchTableModel::Save(const Core::CPUThreadGuard& guard, std::FILE* file) const { m_branch_watch.Save(guard, file); diff --git a/Source/Core/DolphinQt/Debugger/BranchWatchTableModel.h b/Source/Core/DolphinQt/Debugger/BranchWatchTableModel.h index aad2fd783a..500a8ca53c 100644 --- a/Source/Core/DolphinQt/Debugger/BranchWatchTableModel.h +++ b/Source/Core/DolphinQt/Debugger/BranchWatchTableModel.h @@ -97,7 +97,6 @@ public: void OnBranchNotOverwritten(const Core::CPUThreadGuard& guard); void OnWipeRecentHits(); void OnWipeInspection(); - void OnDelete(QModelIndexList index_list); void Save(const Core::CPUThreadGuard& guard, std::FILE* file) const; void Load(const Core::CPUThreadGuard& guard, std::FILE* file);