From b6c20b715ac2703389ff03c7205565adf975f5b7 Mon Sep 17 00:00:00 2001 From: mitaclaw <140017135+mitaclaw@users.noreply.github.com> Date: Tue, 6 Aug 2024 03:29:50 -0700 Subject: [PATCH 01/13] BranchWatch: Don't Save Irrelevant Hits In Reduction Phase --- Source/Core/Core/Debugger/BranchWatch.cpp | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/Source/Core/Core/Debugger/BranchWatch.cpp b/Source/Core/Core/Debugger/BranchWatch.cpp index b7cb76ff27..73e6892258 100644 --- a/Source/Core/Core/Debugger/BranchWatch.cpp +++ b/Source/Core/Core/Debugger/BranchWatch.cpp @@ -69,18 +69,22 @@ void BranchWatch::Save(const CPUThreadGuard& guard, std::FILE* file) const if (file == nullptr) return; + const bool is_reduction_phase = GetRecordingPhase() == Phase::Reduction; + const auto routine = [&](const Collection& collection, bool is_virtual, bool condition) { for (const Collection::value_type& kv : collection) { - const auto iter = std::find_if( - m_selection.begin(), m_selection.end(), - [&](const Selection::value_type& value) { return value.collection_ptr == &kv; }); + const auto iter = std::ranges::find_if(m_selection, [&](const Selection::value_type& value) { + return value.collection_ptr == &kv; + }); + const bool selected = iter != m_selection.end(); + if (is_reduction_phase && !selected) + continue; // Unselected hits are irrelevant to the reduction phase. + const auto inspection = selected ? iter->inspection : SelectionInspection{}; fmt::println(file, "{:08x} {:08x} {:08x} {} {} {:x}", kv.first.origin_addr, kv.first.destin_addr, kv.first.original_inst.hex, kv.second.total_hits, kv.second.hits_snapshot, - iter == m_selection.end() ? - USnapshotMetadata(is_virtual, condition, false, {}).hex : - USnapshotMetadata(is_virtual, condition, true, iter->inspection).hex); + USnapshotMetadata(is_virtual, condition, selected, inspection).hex); } }; routine(m_collection_vt, true, true); From 9eb79f1d2836b76f0938c7f8a1bd93f14fc491bd Mon Sep 17 00:00:00 2001 From: mitaclaw <140017135+mitaclaw@users.noreply.github.com> Date: Mon, 29 Jul 2024 00:00:17 -0700 Subject: [PATCH 02/13] BranchWatchDialog: De-lambda-ize Constructor --- .../DolphinQt/Debugger/BranchWatchDialog.cpp | 491 +++++++++--------- 1 file changed, 239 insertions(+), 252 deletions(-) diff --git a/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp b/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp index 3a8aaf479a..a3675c50d2 100644 --- a/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp +++ b/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp @@ -201,308 +201,295 @@ BranchWatchDialog::BranchWatchDialog(Core::System& system, Core::BranchWatch& br { setWindowTitle(tr("Branch Watch Tool")); setWindowFlags((windowFlags() | Qt::WindowMinMaxButtonsHint) & ~Qt::WindowContextHelpButtonHint); - setLayout([this, &ppc_symbol_db]() { - auto* main_layout = new QVBoxLayout; - // Controls Toolbar (widgets are added later) - main_layout->addWidget(m_control_toolbar = new QToolBar); + auto* main_layout = new QVBoxLayout; - // Branch Watch Table - main_layout->addWidget(m_table_view = [this, &ppc_symbol_db]() { - const auto& ui_settings = Settings::Instance(); + // Controls Toolbar (widgets are added later) + main_layout->addWidget(m_control_toolbar = new QToolBar); - m_table_proxy = new BranchWatchProxyModel(m_branch_watch); - m_table_proxy->setSourceModel( - m_table_model = new BranchWatchTableModel(m_system, m_branch_watch, ppc_symbol_db)); - m_table_proxy->setSortRole(UserRole::SortRole); - m_table_proxy->setSortCaseSensitivity(Qt::CaseInsensitive); + // Branch Watch Table + const auto& ui_settings = Settings::Instance(); - m_table_model->setFont(ui_settings.GetDebugFont()); - connect(&ui_settings, &Settings::DebugFontChanged, m_table_model, - &BranchWatchTableModel::setFont); - connect(Host::GetInstance(), &Host::PPCSymbolsChanged, m_table_model, - &BranchWatchTableModel::UpdateSymbols); + m_table_proxy = new BranchWatchProxyModel(m_branch_watch); + m_table_proxy->setSourceModel( + m_table_model = new BranchWatchTableModel(m_system, m_branch_watch, ppc_symbol_db)); + m_table_proxy->setSortRole(UserRole::SortRole); + m_table_proxy->setSortCaseSensitivity(Qt::CaseInsensitive); - auto* const table_view = new QTableView; - table_view->setModel(m_table_proxy); - table_view->setSortingEnabled(true); - table_view->sortByColumn(Column::Origin, Qt::AscendingOrder); - table_view->setSelectionMode(QAbstractItemView::ExtendedSelection); - table_view->setSelectionBehavior(QAbstractItemView::SelectRows); - table_view->setSizePolicy(QSizePolicy::Expanding, QSizePolicy::Expanding); - table_view->setContextMenuPolicy(Qt::CustomContextMenu); - table_view->setEditTriggers(QAbstractItemView::NoEditTriggers); - table_view->setCornerButtonEnabled(false); - table_view->verticalHeader()->hide(); - table_view->setColumnWidth(Column::Instruction, 50); - table_view->setColumnWidth(Column::Condition, 50); - table_view->setColumnWidth(Column::OriginSymbol, 250); - table_view->setColumnWidth(Column::DestinSymbol, 250); - // The default column width (100 units) is fine for the rest. + m_table_model->setFont(ui_settings.GetDebugFont()); + connect(&ui_settings, &Settings::DebugFontChanged, m_table_model, + &BranchWatchTableModel::setFont); + connect(Host::GetInstance(), &Host::PPCSymbolsChanged, m_table_model, + &BranchWatchTableModel::UpdateSymbols); - QHeaderView* const horizontal_header = table_view->horizontalHeader(); - horizontal_header->restoreState( // Restore column visibility state. - Settings::GetQSettings() - .value(QStringLiteral("branchwatchdialog/tableheader/state")) - .toByteArray()); - horizontal_header->setContextMenuPolicy(Qt::CustomContextMenu); - horizontal_header->setStretchLastSection(true); - horizontal_header->setSectionsMovable(true); - horizontal_header->setFirstSectionMovable(true); + m_table_view = new QTableView; + m_table_view->setModel(m_table_proxy); + m_table_view->setSortingEnabled(true); + m_table_view->sortByColumn(Column::Origin, Qt::AscendingOrder); + m_table_view->setSelectionMode(QAbstractItemView::ExtendedSelection); + m_table_view->setSelectionBehavior(QAbstractItemView::SelectRows); + m_table_view->setSizePolicy(QSizePolicy::Expanding, QSizePolicy::Expanding); + m_table_view->setContextMenuPolicy(Qt::CustomContextMenu); + m_table_view->setEditTriggers(QAbstractItemView::NoEditTriggers); + m_table_view->setCornerButtonEnabled(false); + m_table_view->verticalHeader()->hide(); + m_table_view->setColumnWidth(Column::Instruction, 50); + m_table_view->setColumnWidth(Column::Condition, 50); + m_table_view->setColumnWidth(Column::OriginSymbol, 250); + m_table_view->setColumnWidth(Column::DestinSymbol, 250); + // The default column width (100 units) is fine for the rest. - connect(table_view, &QTableView::clicked, this, &BranchWatchDialog::OnTableClicked); - connect(table_view, &QTableView::customContextMenuRequested, this, - &BranchWatchDialog::OnTableContextMenu); - connect(horizontal_header, &QHeaderView::customContextMenuRequested, this, - &BranchWatchDialog::OnTableHeaderContextMenu); - connect(new QShortcut(QKeySequence(Qt::Key_Delete), this), &QShortcut::activated, this, - &BranchWatchDialog::OnTableDeleteKeypress); + QHeaderView* const horizontal_header = m_table_view->horizontalHeader(); + horizontal_header->restoreState( // Restore column visibility state. + Settings::GetQSettings() + .value(QStringLiteral("branchwatchdialog/tableheader/state")) + .toByteArray()); + horizontal_header->setContextMenuPolicy(Qt::CustomContextMenu); + horizontal_header->setStretchLastSection(true); + horizontal_header->setSectionsMovable(true); + horizontal_header->setFirstSectionMovable(true); - return table_view; - }()); + connect(m_table_view, &QTableView::clicked, this, &BranchWatchDialog::OnTableClicked); + connect(m_table_view, &QTableView::customContextMenuRequested, this, + &BranchWatchDialog::OnTableContextMenu); + connect(horizontal_header, &QHeaderView::customContextMenuRequested, this, + &BranchWatchDialog::OnTableHeaderContextMenu); + connect(new QShortcut(QKeySequence(Qt::Key_Delete), this), &QShortcut::activated, this, + &BranchWatchDialog::OnTableDeleteKeypress); - m_mnu_column_visibility = [this]() { - static constexpr std::array headers = { - QT_TR_NOOP("Instruction"), QT_TR_NOOP("Condition"), QT_TR_NOOP("Origin"), - QT_TR_NOOP("Destination"), QT_TR_NOOP("Recent Hits"), QT_TR_NOOP("Total Hits"), - QT_TR_NOOP("Origin Symbol"), QT_TR_NOOP("Destination Symbol")}; + main_layout->addWidget(m_table_view); - auto* const menu = new QMenu(); - for (int column = 0; column < Column::NumberOfColumns; ++column) - { - QAction* action = menu->addAction(tr(headers[column]), [this, column](bool enabled) { - m_table_view->setColumnHidden(column, !enabled); - }); - action->setChecked(!m_table_view->isColumnHidden(column)); - action->setCheckable(true); - } - return menu; - }(); + { + // Column Visibility Menu + static constexpr std::array headers = { + QT_TR_NOOP("Instruction"), QT_TR_NOOP("Condition"), QT_TR_NOOP("Origin"), + QT_TR_NOOP("Destination"), QT_TR_NOOP("Recent Hits"), QT_TR_NOOP("Total Hits"), + QT_TR_NOOP("Origin Symbol"), QT_TR_NOOP("Destination Symbol")}; + m_mnu_column_visibility = new QMenu(); + for (int column = 0; column < Column::NumberOfColumns; ++column) + { + QAction* const action = + m_mnu_column_visibility->addAction(tr(headers[column]), [this, column](bool enabled) { + m_table_view->setColumnHidden(column, !enabled); + }); + action->setChecked(!m_table_view->isColumnHidden(column)); + action->setCheckable(true); + } + } + { // Menu Bar - main_layout->setMenuBar([this]() { - QMenuBar* const menu_bar = new QMenuBar; - menu_bar->setNativeMenuBar(false); + QMenuBar* const menu_bar = new QMenuBar; + menu_bar->setNativeMenuBar(false); - QMenu* const menu_file = new QMenu(tr("&File"), menu_bar); - menu_file->addAction(tr("&Save Branch Watch"), this, &BranchWatchDialog::OnSave); - menu_file->addAction(tr("Save Branch Watch &As..."), this, &BranchWatchDialog::OnSaveAs); - menu_file->addAction(tr("&Load Branch Watch"), this, &BranchWatchDialog::OnLoad); - menu_file->addAction(tr("Load Branch Watch &From..."), this, &BranchWatchDialog::OnLoadFrom); - m_act_autosave = menu_file->addAction(tr("A&uto Save")); - m_act_autosave->setCheckable(true); - connect(m_act_autosave, &QAction::toggled, this, &BranchWatchDialog::OnToggleAutoSave); - menu_bar->addMenu(menu_file); + QMenu* const menu_file = new QMenu(tr("&File"), menu_bar); + menu_file->addAction(tr("&Save Branch Watch"), this, &BranchWatchDialog::OnSave); + menu_file->addAction(tr("Save Branch Watch &As..."), this, &BranchWatchDialog::OnSaveAs); + menu_file->addAction(tr("&Load Branch Watch"), this, &BranchWatchDialog::OnLoad); + menu_file->addAction(tr("Load Branch Watch &From..."), this, &BranchWatchDialog::OnLoadFrom); + m_act_autosave = menu_file->addAction(tr("A&uto Save")); + m_act_autosave->setCheckable(true); + connect(m_act_autosave, &QAction::toggled, this, &BranchWatchDialog::OnToggleAutoSave); + menu_bar->addMenu(menu_file); - QMenu* const menu_tool = new QMenu(tr("&Tool"), menu_bar); - menu_tool->setToolTipsVisible(true); - menu_tool->addAction(tr("Hide &Controls"), this, &BranchWatchDialog::OnHideShowControls) - ->setCheckable(true); - QAction* const act_ignore_apploader = - menu_tool->addAction(tr("Ignore &Apploader Branch Hits")); - act_ignore_apploader->setToolTip( - tr("This only applies to the initial boot of the emulated software.")); - act_ignore_apploader->setChecked(m_system.IsBranchWatchIgnoreApploader()); - act_ignore_apploader->setCheckable(true); - connect(act_ignore_apploader, &QAction::toggled, this, - &BranchWatchDialog::OnToggleIgnoreApploader); + QMenu* const menu_tool = new QMenu(tr("&Tool"), menu_bar); + menu_tool->setToolTipsVisible(true); + menu_tool->addAction(tr("Hide &Controls"), this, &BranchWatchDialog::OnHideShowControls) + ->setCheckable(true); + QAction* const act_ignore_apploader = menu_tool->addAction(tr("Ignore &Apploader Branch Hits")); + act_ignore_apploader->setToolTip( + tr("This only applies to the initial boot of the emulated software.")); + act_ignore_apploader->setChecked(m_system.IsBranchWatchIgnoreApploader()); + act_ignore_apploader->setCheckable(true); + connect(act_ignore_apploader, &QAction::toggled, this, + &BranchWatchDialog::OnToggleIgnoreApploader); - menu_tool->addMenu(m_mnu_column_visibility)->setText(tr("Column &Visibility")); - menu_tool->addAction(tr("Wipe &Inspection Data"), this, &BranchWatchDialog::OnWipeInspection); - menu_tool->addAction(tr("&Help"), this, &BranchWatchDialog::OnHelp); + menu_tool->addMenu(m_mnu_column_visibility)->setText(tr("Column &Visibility")); + menu_tool->addAction(tr("Wipe &Inspection Data"), this, &BranchWatchDialog::OnWipeInspection); + menu_tool->addAction(tr("&Help"), this, &BranchWatchDialog::OnHelp); - menu_bar->addMenu(menu_tool); - - return menu_bar; - }()); + menu_bar->addMenu(menu_tool); + main_layout->setMenuBar(menu_bar); + } + { // Status Bar - main_layout->addWidget(m_status_bar = []() { - auto* const status_bar = new QStatusBar; - status_bar->setSizeGripEnabled(false); - return status_bar; - }()); - + m_status_bar = new QStatusBar; + m_status_bar->setSizeGripEnabled(false); + main_layout->addWidget(m_status_bar); + } + { // Tool Controls - m_control_toolbar->addWidget([this]() { - auto* const layout = new QGridLayout; + auto* const layout = new QGridLayout; - layout->addWidget(m_btn_start_pause = new QPushButton(tr("Start Branch Watch")), 0, 0); - connect(m_btn_start_pause, &QPushButton::toggled, this, &BranchWatchDialog::OnStartPause); - m_btn_start_pause->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Expanding); - m_btn_start_pause->setCheckable(true); + layout->addWidget(m_btn_start_pause = new QPushButton(tr("Start Branch Watch")), 0, 0); + connect(m_btn_start_pause, &QPushButton::toggled, this, &BranchWatchDialog::OnStartPause); + m_btn_start_pause->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Expanding); + m_btn_start_pause->setCheckable(true); - layout->addWidget(m_btn_clear_watch = new QPushButton(tr("Clear Branch Watch")), 1, 0); - connect(m_btn_clear_watch, &QPushButton::pressed, this, - &BranchWatchDialog::OnClearBranchWatch); - m_btn_clear_watch->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Expanding); + layout->addWidget(m_btn_clear_watch = new QPushButton(tr("Clear Branch Watch")), 1, 0); + connect(m_btn_clear_watch, &QPushButton::pressed, this, &BranchWatchDialog::OnClearBranchWatch); + m_btn_clear_watch->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Expanding); - layout->addWidget(m_btn_path_was_taken = new QPushButton(tr("Code Path Was Taken")), 0, 1); - connect(m_btn_path_was_taken, &QPushButton::pressed, this, - &BranchWatchDialog::OnCodePathWasTaken); - m_btn_path_was_taken->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Expanding); + layout->addWidget(m_btn_path_was_taken = new QPushButton(tr("Code Path Was Taken")), 0, 1); + connect(m_btn_path_was_taken, &QPushButton::pressed, this, + &BranchWatchDialog::OnCodePathWasTaken); + m_btn_path_was_taken->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Expanding); - layout->addWidget(m_btn_path_not_taken = new QPushButton(tr("Code Path Not Taken")), 1, 1); - connect(m_btn_path_not_taken, &QPushButton::pressed, this, - &BranchWatchDialog::OnCodePathNotTaken); - m_btn_path_not_taken->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Expanding); + layout->addWidget(m_btn_path_not_taken = new QPushButton(tr("Code Path Not Taken")), 1, 1); + connect(m_btn_path_not_taken, &QPushButton::pressed, this, + &BranchWatchDialog::OnCodePathNotTaken); + m_btn_path_not_taken->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Expanding); - auto* const group_box = new QGroupBox(tr("Tool Controls")); - group_box->setLayout(layout); - group_box->setAlignment(Qt::AlignHCenter); - - return group_box; - }()); + auto* const group_box = new QGroupBox(tr("Tool Controls")); + group_box->setLayout(layout); + group_box->setAlignment(Qt::AlignHCenter); + m_control_toolbar->addWidget(group_box); + } + { // Spacer - m_control_toolbar->addWidget([]() { - auto* const widget = new QWidget; - widget->setSizePolicy(QSizePolicy::MinimumExpanding, QSizePolicy::Preferred); - return widget; - }()); - + auto* const widget = new QWidget; + widget->setSizePolicy(QSizePolicy::MinimumExpanding, QSizePolicy::Preferred); + m_control_toolbar->addWidget(widget); + } + { // Branch Type Filter Options - m_control_toolbar->addWidget([this]() { - auto* const layout = new QGridLayout; + auto* const layout = new QGridLayout; - const auto routine = [this, layout](const QString& text, const QString& tooltip, int row, - int column, void (BranchWatchProxyModel::*slot)(bool)) { - QCheckBox* const check_box = new QCheckBox(text); - check_box->setToolTip(tooltip); - layout->addWidget(check_box, row, column); - connect(check_box, &QCheckBox::toggled, [this, slot](bool checked) { - (m_table_proxy->*slot)(checked); - UpdateStatus(); - }); - check_box->setChecked(true); - }; + const auto routine = [this, layout](const QString& text, const QString& tooltip, int row, + int column, void (BranchWatchProxyModel::*slot)(bool)) { + QCheckBox* const check_box = new QCheckBox(text); + check_box->setToolTip(tooltip); + layout->addWidget(check_box, row, column); + connect(check_box, &QCheckBox::toggled, [this, slot](bool checked) { + (m_table_proxy->*slot)(checked); + UpdateStatus(); + }); + check_box->setChecked(true); + }; - // clang-format off - routine(QStringLiteral("b" ), tr("Branch" ), 0, 0, &BranchWatchProxyModel::OnToggled<&BranchWatchProxyModel::m_b >); - routine(QStringLiteral("bl" ), tr("Branch (LR saved)" ), 0, 1, &BranchWatchProxyModel::OnToggled<&BranchWatchProxyModel::m_bl >); - routine(QStringLiteral("bc" ), tr("Branch Conditional" ), 0, 2, &BranchWatchProxyModel::OnToggled<&BranchWatchProxyModel::m_bc >); - routine(QStringLiteral("bcl" ), tr("Branch Conditional (LR saved)" ), 0, 3, &BranchWatchProxyModel::OnToggled<&BranchWatchProxyModel::m_bcl >); - routine(QStringLiteral("blr" ), tr("Branch to Link Register" ), 1, 0, &BranchWatchProxyModel::OnToggled<&BranchWatchProxyModel::m_blr >); - routine(QStringLiteral("blrl" ), tr("Branch to Link Register (LR saved)" ), 1, 1, &BranchWatchProxyModel::OnToggled<&BranchWatchProxyModel::m_blrl >); - routine(QStringLiteral("bclr" ), tr("Branch Conditional to Link Register" ), 1, 2, &BranchWatchProxyModel::OnToggled<&BranchWatchProxyModel::m_bclr >); - routine(QStringLiteral("bclrl" ), tr("Branch Conditional to Link Register (LR saved)" ), 1, 3, &BranchWatchProxyModel::OnToggled<&BranchWatchProxyModel::m_bclrl >); - routine(QStringLiteral("bctr" ), tr("Branch to Count Register" ), 2, 0, &BranchWatchProxyModel::OnToggled<&BranchWatchProxyModel::m_bctr >); - routine(QStringLiteral("bctrl" ), tr("Branch to Count Register (LR saved)" ), 2, 1, &BranchWatchProxyModel::OnToggled<&BranchWatchProxyModel::m_bctrl >); - routine(QStringLiteral("bcctr" ), tr("Branch Conditional to Count Register" ), 2, 2, &BranchWatchProxyModel::OnToggled<&BranchWatchProxyModel::m_bcctr >); - routine(QStringLiteral("bcctrl"), tr("Branch Conditional to Count Register (LR saved)"), 2, 3, &BranchWatchProxyModel::OnToggled<&BranchWatchProxyModel::m_bcctrl>); - // clang-format on + // clang-format off + routine(QStringLiteral("b" ), tr("Branch" ), 0, 0, &BranchWatchProxyModel::OnToggled<&BranchWatchProxyModel::m_b >); + routine(QStringLiteral("bl" ), tr("Branch (LR saved)" ), 0, 1, &BranchWatchProxyModel::OnToggled<&BranchWatchProxyModel::m_bl >); + routine(QStringLiteral("bc" ), tr("Branch Conditional" ), 0, 2, &BranchWatchProxyModel::OnToggled<&BranchWatchProxyModel::m_bc >); + routine(QStringLiteral("bcl" ), tr("Branch Conditional (LR saved)" ), 0, 3, &BranchWatchProxyModel::OnToggled<&BranchWatchProxyModel::m_bcl >); + routine(QStringLiteral("blr" ), tr("Branch to Link Register" ), 1, 0, &BranchWatchProxyModel::OnToggled<&BranchWatchProxyModel::m_blr >); + routine(QStringLiteral("blrl" ), tr("Branch to Link Register (LR saved)" ), 1, 1, &BranchWatchProxyModel::OnToggled<&BranchWatchProxyModel::m_blrl >); + routine(QStringLiteral("bclr" ), tr("Branch Conditional to Link Register" ), 1, 2, &BranchWatchProxyModel::OnToggled<&BranchWatchProxyModel::m_bclr >); + routine(QStringLiteral("bclrl" ), tr("Branch Conditional to Link Register (LR saved)" ), 1, 3, &BranchWatchProxyModel::OnToggled<&BranchWatchProxyModel::m_bclrl >); + routine(QStringLiteral("bctr" ), tr("Branch to Count Register" ), 2, 0, &BranchWatchProxyModel::OnToggled<&BranchWatchProxyModel::m_bctr >); + routine(QStringLiteral("bctrl" ), tr("Branch to Count Register (LR saved)" ), 2, 1, &BranchWatchProxyModel::OnToggled<&BranchWatchProxyModel::m_bctrl >); + routine(QStringLiteral("bcctr" ), tr("Branch Conditional to Count Register" ), 2, 2, &BranchWatchProxyModel::OnToggled<&BranchWatchProxyModel::m_bcctr >); + routine(QStringLiteral("bcctrl"), tr("Branch Conditional to Count Register (LR saved)"), 2, 3, &BranchWatchProxyModel::OnToggled<&BranchWatchProxyModel::m_bcctrl>); + // clang-format on - auto* const group_box = new QGroupBox(tr("Branch Type")); - group_box->setLayout(layout); - group_box->setAlignment(Qt::AlignHCenter); - - return group_box; - }()); + auto* const group_box = new QGroupBox(tr("Branch Type")); + group_box->setLayout(layout); + group_box->setAlignment(Qt::AlignHCenter); + m_control_toolbar->addWidget(group_box); + } + { // Origin and Destination Filter Options - m_control_toolbar->addWidget([this]() { - auto* const layout = new QGridLayout; + auto* const layout = new QGridLayout; - const auto routine = [this, layout](const QString& placeholder_text, int row, int column, - int width, - void (BranchWatchProxyModel::*slot)(const QString&)) { - QLineEdit* const line_edit = new QLineEdit; - layout->addWidget(line_edit, row, column, 1, width); - connect(line_edit, &QLineEdit::textChanged, [this, slot](const QString& text) { - (m_table_proxy->*slot)(text); - UpdateStatus(); - }); - line_edit->setPlaceholderText(placeholder_text); - return line_edit; - }; + const auto routine = [this, layout](const QString& placeholder_text, int row, int column, + int width, + void (BranchWatchProxyModel::*slot)(const QString&)) { + QLineEdit* const line_edit = new QLineEdit; + layout->addWidget(line_edit, row, column, 1, width); + connect(line_edit, &QLineEdit::textChanged, [this, slot](const QString& text) { + (m_table_proxy->*slot)(text); + UpdateStatus(); + }); + line_edit->setPlaceholderText(placeholder_text); + return line_edit; + }; - // clang-format off - routine(tr("Origin Symbol" ), 0, 0, 1, &BranchWatchProxyModel::OnSymbolTextChanged<&BranchWatchProxyModel::m_origin_symbol_name>); - routine(tr("Origin Min" ), 1, 0, 1, &BranchWatchProxyModel::OnAddressTextChanged<&BranchWatchProxyModel::m_origin_min>)->setMaxLength(8); - routine(tr("Origin Max" ), 2, 0, 1, &BranchWatchProxyModel::OnAddressTextChanged<&BranchWatchProxyModel::m_origin_max>)->setMaxLength(8); - routine(tr("Destination Symbol"), 0, 1, 1, &BranchWatchProxyModel::OnSymbolTextChanged<&BranchWatchProxyModel::m_destin_symbol_name>); - routine(tr("Destination Min" ), 1, 1, 1, &BranchWatchProxyModel::OnAddressTextChanged<&BranchWatchProxyModel::m_destin_min>)->setMaxLength(8); - routine(tr("Destination Max" ), 2, 1, 1, &BranchWatchProxyModel::OnAddressTextChanged<&BranchWatchProxyModel::m_destin_max>)->setMaxLength(8); - // clang-format on + // clang-format off + routine(tr("Origin Symbol" ), 0, 0, 1, &BranchWatchProxyModel::OnSymbolTextChanged<&BranchWatchProxyModel::m_origin_symbol_name>); + routine(tr("Origin Min" ), 1, 0, 1, &BranchWatchProxyModel::OnAddressTextChanged<&BranchWatchProxyModel::m_origin_min>)->setMaxLength(8); + routine(tr("Origin Max" ), 2, 0, 1, &BranchWatchProxyModel::OnAddressTextChanged<&BranchWatchProxyModel::m_origin_max>)->setMaxLength(8); + routine(tr("Destination Symbol"), 0, 1, 1, &BranchWatchProxyModel::OnSymbolTextChanged<&BranchWatchProxyModel::m_destin_symbol_name>); + routine(tr("Destination Min" ), 1, 1, 1, &BranchWatchProxyModel::OnAddressTextChanged<&BranchWatchProxyModel::m_destin_min>)->setMaxLength(8); + routine(tr("Destination Max" ), 2, 1, 1, &BranchWatchProxyModel::OnAddressTextChanged<&BranchWatchProxyModel::m_destin_max>)->setMaxLength(8); + // clang-format on - auto* const group_box = new QGroupBox(tr("Origin and Destination")); - group_box->setLayout(layout); - group_box->setAlignment(Qt::AlignHCenter); - - return group_box; - }()); + auto* const group_box = new QGroupBox(tr("Origin and Destination")); + group_box->setLayout(layout); + group_box->setAlignment(Qt::AlignHCenter); + m_control_toolbar->addWidget(group_box); + } + { // Condition Filter Options - m_control_toolbar->addWidget([this]() { - auto* const layout = new QVBoxLayout; - layout->setAlignment(Qt::AlignHCenter); + auto* const layout = new QVBoxLayout; + layout->setAlignment(Qt::AlignHCenter); - const auto routine = [this, layout](const QString& text, - void (BranchWatchProxyModel::*slot)(bool)) { - QCheckBox* const check_box = new QCheckBox(text); - layout->addWidget(check_box); - connect(check_box, &QCheckBox::toggled, [this, slot](bool checked) { - (m_table_proxy->*slot)(checked); - UpdateStatus(); - }); - check_box->setChecked(true); - return check_box; - }; + const auto routine = [this, layout](const QString& text, + void (BranchWatchProxyModel::*slot)(bool)) { + QCheckBox* const check_box = new QCheckBox(text); + layout->addWidget(check_box); + connect(check_box, &QCheckBox::toggled, [this, slot](bool checked) { + (m_table_proxy->*slot)(checked); + UpdateStatus(); + }); + check_box->setChecked(true); + return check_box; + }; - routine(QStringLiteral("true"), - &BranchWatchProxyModel::OnToggled<&BranchWatchProxyModel::m_cond_true>) - ->setToolTip(tr("This will also filter unconditional branches.\n" - "To filter for or against unconditional branches,\n" - "use the Branch Type filter options.")); - routine(QStringLiteral("false"), - &BranchWatchProxyModel::OnToggled<&BranchWatchProxyModel::m_cond_false>); + routine(QStringLiteral("true"), + &BranchWatchProxyModel::OnToggled<&BranchWatchProxyModel::m_cond_true>) + ->setToolTip(tr("This will also filter unconditional branches.\n" + "To filter for or against unconditional branches,\n" + "use the Branch Type filter options.")); + routine(QStringLiteral("false"), + &BranchWatchProxyModel::OnToggled<&BranchWatchProxyModel::m_cond_false>); - auto* const group_box = new QGroupBox(tr("Condition")); - group_box->setLayout(layout); - group_box->setAlignment(Qt::AlignHCenter); - - return group_box; - }()); + auto* const group_box = new QGroupBox(tr("Condition")); + group_box->setLayout(layout); + group_box->setAlignment(Qt::AlignHCenter); + m_control_toolbar->addWidget(group_box); + } + { // Misc. Controls - m_control_toolbar->addWidget([this]() { - auto* const layout = new QVBoxLayout; + auto* const layout = new QVBoxLayout; - layout->addWidget(m_btn_was_overwritten = new QPushButton(tr("Branch Was Overwritten"))); - connect(m_btn_was_overwritten, &QPushButton::pressed, this, - &BranchWatchDialog::OnBranchWasOverwritten); - m_btn_was_overwritten->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Expanding); + layout->addWidget(m_btn_was_overwritten = new QPushButton(tr("Branch Was Overwritten"))); + connect(m_btn_was_overwritten, &QPushButton::pressed, this, + &BranchWatchDialog::OnBranchWasOverwritten); + m_btn_was_overwritten->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Expanding); - layout->addWidget(m_btn_not_overwritten = new QPushButton(tr("Branch Not Overwritten"))); - connect(m_btn_not_overwritten, &QPushButton::pressed, this, - &BranchWatchDialog::OnBranchNotOverwritten); - m_btn_not_overwritten->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Expanding); + layout->addWidget(m_btn_not_overwritten = new QPushButton(tr("Branch Not Overwritten"))); + connect(m_btn_not_overwritten, &QPushButton::pressed, this, + &BranchWatchDialog::OnBranchNotOverwritten); + m_btn_not_overwritten->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Expanding); - layout->addWidget(m_btn_wipe_recent_hits = new QPushButton(tr("Wipe Recent Hits"))); - connect(m_btn_wipe_recent_hits, &QPushButton::pressed, this, - &BranchWatchDialog::OnWipeRecentHits); - m_btn_wipe_recent_hits->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Expanding); - m_btn_wipe_recent_hits->setEnabled(false); + layout->addWidget(m_btn_wipe_recent_hits = new QPushButton(tr("Wipe Recent Hits"))); + connect(m_btn_wipe_recent_hits, &QPushButton::pressed, this, + &BranchWatchDialog::OnWipeRecentHits); + m_btn_wipe_recent_hits->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Expanding); + m_btn_wipe_recent_hits->setEnabled(false); - auto* const group_box = new QGroupBox(tr("Misc. Controls")); - group_box->setLayout(layout); - group_box->setAlignment(Qt::AlignHCenter); + auto* const group_box = new QGroupBox(tr("Misc. Controls")); + group_box->setLayout(layout); + group_box->setAlignment(Qt::AlignHCenter); - return group_box; - }()); + m_control_toolbar->addWidget(group_box); + } - UpdateIcons(); + UpdateIcons(); - connect(m_timer = new QTimer, &QTimer::timeout, this, &BranchWatchDialog::OnTimeout); - connect(&Settings::Instance(), &Settings::EmulationStateChanged, this, - &BranchWatchDialog::OnEmulationStateChanged); - connect(&Settings::Instance(), &Settings::ThemeChanged, this, - &BranchWatchDialog::OnThemeChanged); - connect(m_table_proxy, &BranchWatchProxyModel::layoutChanged, this, - &BranchWatchDialog::UpdateStatus); + connect(m_timer = new QTimer, &QTimer::timeout, this, &BranchWatchDialog::OnTimeout); + connect(&Settings::Instance(), &Settings::EmulationStateChanged, this, + &BranchWatchDialog::OnEmulationStateChanged); + connect(&Settings::Instance(), &Settings::ThemeChanged, this, &BranchWatchDialog::OnThemeChanged); + connect(m_table_proxy, &BranchWatchProxyModel::layoutChanged, this, + &BranchWatchDialog::UpdateStatus); - return main_layout; - }()); + setLayout(main_layout); const auto& settings = Settings::GetQSettings(); restoreGeometry(settings.value(QStringLiteral("branchwatchdialog/geometry")).toByteArray()); From 7b89730daac45c09c85b7a97742860f43ccc5edf Mon Sep 17 00:00:00 2001 From: mitaclaw <140017135+mitaclaw@users.noreply.github.com> Date: Sun, 4 Aug 2024 05:54:28 -0700 Subject: [PATCH 03/13] BranchWatchDialog: Defer Layout Construction When Possible The main layout, tool controls layout, and misc. controls layout all don't need the QLayout constructed so early. --- .../DolphinQt/Debugger/BranchWatchDialog.cpp | 61 ++++++++++--------- 1 file changed, 33 insertions(+), 28 deletions(-) diff --git a/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp b/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp index a3675c50d2..4bd57bb76f 100644 --- a/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp +++ b/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp @@ -202,10 +202,8 @@ BranchWatchDialog::BranchWatchDialog(Core::System& system, Core::BranchWatch& br setWindowTitle(tr("Branch Watch Tool")); setWindowFlags((windowFlags() | Qt::WindowMinMaxButtonsHint) & ~Qt::WindowContextHelpButtonHint); - auto* main_layout = new QVBoxLayout; - // Controls Toolbar (widgets are added later) - main_layout->addWidget(m_control_toolbar = new QToolBar); + m_control_toolbar = new QToolBar; // Branch Watch Table const auto& ui_settings = Settings::Instance(); @@ -257,8 +255,6 @@ BranchWatchDialog::BranchWatchDialog(Core::System& system, Core::BranchWatch& br connect(new QShortcut(QKeySequence(Qt::Key_Delete), this), &QShortcut::activated, this, &BranchWatchDialog::OnTableDeleteKeypress); - main_layout->addWidget(m_table_view); - { // Column Visibility Menu static constexpr std::array headers = { @@ -277,11 +273,11 @@ BranchWatchDialog::BranchWatchDialog(Core::System& system, Core::BranchWatch& br action->setCheckable(true); } } - { - // Menu Bar - QMenuBar* const menu_bar = new QMenuBar; - menu_bar->setNativeMenuBar(false); + // Menu Bar + QMenuBar* const menu_bar = new QMenuBar; + menu_bar->setNativeMenuBar(false); + { QMenu* const menu_file = new QMenu(tr("&File"), menu_bar); menu_file->addAction(tr("&Save Branch Watch"), this, &BranchWatchDialog::OnSave); menu_file->addAction(tr("Save Branch Watch &As..."), this, &BranchWatchDialog::OnSaveAs); @@ -309,38 +305,39 @@ BranchWatchDialog::BranchWatchDialog(Core::System& system, Core::BranchWatch& br menu_tool->addAction(tr("&Help"), this, &BranchWatchDialog::OnHelp); menu_bar->addMenu(menu_tool); + } + + // Status Bar + m_status_bar = new QStatusBar; + m_status_bar->setSizeGripEnabled(false); - main_layout->setMenuBar(menu_bar); - } - { - // Status Bar - m_status_bar = new QStatusBar; - m_status_bar->setSizeGripEnabled(false); - main_layout->addWidget(m_status_bar); - } { // Tool Controls - auto* const layout = new QGridLayout; - - layout->addWidget(m_btn_start_pause = new QPushButton(tr("Start Branch Watch")), 0, 0); + m_btn_start_pause = new QPushButton(tr("Start Branch Watch")); connect(m_btn_start_pause, &QPushButton::toggled, this, &BranchWatchDialog::OnStartPause); m_btn_start_pause->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Expanding); m_btn_start_pause->setCheckable(true); - layout->addWidget(m_btn_clear_watch = new QPushButton(tr("Clear Branch Watch")), 1, 0); + m_btn_clear_watch = new QPushButton(tr("Clear Branch Watch")); connect(m_btn_clear_watch, &QPushButton::pressed, this, &BranchWatchDialog::OnClearBranchWatch); m_btn_clear_watch->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Expanding); - layout->addWidget(m_btn_path_was_taken = new QPushButton(tr("Code Path Was Taken")), 0, 1); + m_btn_path_was_taken = new QPushButton(tr("Code Path Was Taken")); connect(m_btn_path_was_taken, &QPushButton::pressed, this, &BranchWatchDialog::OnCodePathWasTaken); m_btn_path_was_taken->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Expanding); - layout->addWidget(m_btn_path_not_taken = new QPushButton(tr("Code Path Not Taken")), 1, 1); + m_btn_path_not_taken = new QPushButton(tr("Code Path Not Taken")); connect(m_btn_path_not_taken, &QPushButton::pressed, this, &BranchWatchDialog::OnCodePathNotTaken); m_btn_path_not_taken->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Expanding); + auto* const layout = new QGridLayout; + layout->addWidget(m_btn_start_pause, 0, 0); + layout->addWidget(m_btn_clear_watch, 1, 0); + layout->addWidget(m_btn_path_was_taken, 0, 1); + layout->addWidget(m_btn_path_not_taken, 1, 1); + auto* const group_box = new QGroupBox(tr("Tool Controls")); group_box->setLayout(layout); group_box->setAlignment(Qt::AlignHCenter); @@ -455,24 +452,27 @@ BranchWatchDialog::BranchWatchDialog(Core::System& system, Core::BranchWatch& br } { // Misc. Controls - auto* const layout = new QVBoxLayout; - - layout->addWidget(m_btn_was_overwritten = new QPushButton(tr("Branch Was Overwritten"))); + m_btn_was_overwritten = new QPushButton(tr("Branch Was Overwritten")); connect(m_btn_was_overwritten, &QPushButton::pressed, this, &BranchWatchDialog::OnBranchWasOverwritten); m_btn_was_overwritten->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Expanding); - layout->addWidget(m_btn_not_overwritten = new QPushButton(tr("Branch Not Overwritten"))); + m_btn_not_overwritten = new QPushButton(tr("Branch Not Overwritten")); connect(m_btn_not_overwritten, &QPushButton::pressed, this, &BranchWatchDialog::OnBranchNotOverwritten); m_btn_not_overwritten->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Expanding); - layout->addWidget(m_btn_wipe_recent_hits = new QPushButton(tr("Wipe Recent Hits"))); + m_btn_wipe_recent_hits = new QPushButton(tr("Wipe Recent Hits")); connect(m_btn_wipe_recent_hits, &QPushButton::pressed, this, &BranchWatchDialog::OnWipeRecentHits); m_btn_wipe_recent_hits->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Expanding); m_btn_wipe_recent_hits->setEnabled(false); + auto* const layout = new QVBoxLayout; + layout->addWidget(m_btn_was_overwritten); + layout->addWidget(m_btn_not_overwritten); + layout->addWidget(m_btn_wipe_recent_hits); + auto* const group_box = new QGroupBox(tr("Misc. Controls")); group_box->setLayout(layout); group_box->setAlignment(Qt::AlignHCenter); @@ -489,6 +489,11 @@ BranchWatchDialog::BranchWatchDialog(Core::System& system, Core::BranchWatch& br connect(m_table_proxy, &BranchWatchProxyModel::layoutChanged, this, &BranchWatchDialog::UpdateStatus); + auto* const main_layout = new QVBoxLayout; + main_layout->setMenuBar(menu_bar); + main_layout->addWidget(m_control_toolbar); + main_layout->addWidget(m_table_view); + main_layout->addWidget(m_status_bar); setLayout(main_layout); const auto& settings = Settings::GetQSettings(); From ffaba268308dd5238ffede98b2cad8da953fdf4a Mon Sep 17 00:00:00 2001 From: mitaclaw <140017135+mitaclaw@users.noreply.github.com> Date: Sun, 4 Aug 2024 06:04:29 -0700 Subject: [PATCH 04/13] BranchWatchDialog: Refactor For LoadQSettings / SaveQSettings --- .../DolphinQt/Debugger/BranchWatchDialog.cpp | 129 +++++++++--------- .../DolphinQt/Debugger/BranchWatchDialog.h | 3 +- 2 files changed, 67 insertions(+), 65 deletions(-) diff --git a/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp b/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp index 4bd57bb76f..a2b205b12d 100644 --- a/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp +++ b/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp @@ -202,9 +202,6 @@ BranchWatchDialog::BranchWatchDialog(Core::System& system, Core::BranchWatch& br setWindowTitle(tr("Branch Watch Tool")); setWindowFlags((windowFlags() | Qt::WindowMinMaxButtonsHint) & ~Qt::WindowContextHelpButtonHint); - // Controls Toolbar (widgets are added later) - m_control_toolbar = new QToolBar; - // Branch Watch Table const auto& ui_settings = Settings::Instance(); @@ -238,10 +235,6 @@ BranchWatchDialog::BranchWatchDialog(Core::System& system, Core::BranchWatch& br // The default column width (100 units) is fine for the rest. QHeaderView* const horizontal_header = m_table_view->horizontalHeader(); - horizontal_header->restoreState( // Restore column visibility state. - Settings::GetQSettings() - .value(QStringLiteral("branchwatchdialog/tableheader/state")) - .toByteArray()); horizontal_header->setContextMenuPolicy(Qt::CustomContextMenu); horizontal_header->setStretchLastSection(true); horizontal_header->setSectionsMovable(true); @@ -255,62 +248,12 @@ BranchWatchDialog::BranchWatchDialog(Core::System& system, Core::BranchWatch& br connect(new QShortcut(QKeySequence(Qt::Key_Delete), this), &QShortcut::activated, this, &BranchWatchDialog::OnTableDeleteKeypress); - { - // Column Visibility Menu - static constexpr std::array headers = { - QT_TR_NOOP("Instruction"), QT_TR_NOOP("Condition"), QT_TR_NOOP("Origin"), - QT_TR_NOOP("Destination"), QT_TR_NOOP("Recent Hits"), QT_TR_NOOP("Total Hits"), - QT_TR_NOOP("Origin Symbol"), QT_TR_NOOP("Destination Symbol")}; - - m_mnu_column_visibility = new QMenu(); - for (int column = 0; column < Column::NumberOfColumns; ++column) - { - QAction* const action = - m_mnu_column_visibility->addAction(tr(headers[column]), [this, column](bool enabled) { - m_table_view->setColumnHidden(column, !enabled); - }); - action->setChecked(!m_table_view->isColumnHidden(column)); - action->setCheckable(true); - } - } - - // Menu Bar - QMenuBar* const menu_bar = new QMenuBar; - menu_bar->setNativeMenuBar(false); - { - QMenu* const menu_file = new QMenu(tr("&File"), menu_bar); - menu_file->addAction(tr("&Save Branch Watch"), this, &BranchWatchDialog::OnSave); - menu_file->addAction(tr("Save Branch Watch &As..."), this, &BranchWatchDialog::OnSaveAs); - menu_file->addAction(tr("&Load Branch Watch"), this, &BranchWatchDialog::OnLoad); - menu_file->addAction(tr("Load Branch Watch &From..."), this, &BranchWatchDialog::OnLoadFrom); - m_act_autosave = menu_file->addAction(tr("A&uto Save")); - m_act_autosave->setCheckable(true); - connect(m_act_autosave, &QAction::toggled, this, &BranchWatchDialog::OnToggleAutoSave); - menu_bar->addMenu(menu_file); - - QMenu* const menu_tool = new QMenu(tr("&Tool"), menu_bar); - menu_tool->setToolTipsVisible(true); - menu_tool->addAction(tr("Hide &Controls"), this, &BranchWatchDialog::OnHideShowControls) - ->setCheckable(true); - QAction* const act_ignore_apploader = menu_tool->addAction(tr("Ignore &Apploader Branch Hits")); - act_ignore_apploader->setToolTip( - tr("This only applies to the initial boot of the emulated software.")); - act_ignore_apploader->setChecked(m_system.IsBranchWatchIgnoreApploader()); - act_ignore_apploader->setCheckable(true); - connect(act_ignore_apploader, &QAction::toggled, this, - &BranchWatchDialog::OnToggleIgnoreApploader); - - menu_tool->addMenu(m_mnu_column_visibility)->setText(tr("Column &Visibility")); - menu_tool->addAction(tr("Wipe &Inspection Data"), this, &BranchWatchDialog::OnWipeInspection); - menu_tool->addAction(tr("&Help"), this, &BranchWatchDialog::OnHelp); - - menu_bar->addMenu(menu_tool); - } - // Status Bar m_status_bar = new QStatusBar; m_status_bar->setSizeGripEnabled(false); + // Controls Toolbar + m_control_toolbar = new QToolBar; { // Tool Controls m_btn_start_pause = new QPushButton(tr("Start Branch Watch")); @@ -480,6 +423,59 @@ BranchWatchDialog::BranchWatchDialog(Core::System& system, Core::BranchWatch& br m_control_toolbar->addWidget(group_box); } + LoadQSettings(); + + // Column Visibility Menu + m_mnu_column_visibility = new QMenu(); + { + static constexpr std::array headers = { + QT_TR_NOOP("Instruction"), QT_TR_NOOP("Condition"), QT_TR_NOOP("Origin"), + QT_TR_NOOP("Destination"), QT_TR_NOOP("Recent Hits"), QT_TR_NOOP("Total Hits"), + QT_TR_NOOP("Origin Symbol"), QT_TR_NOOP("Destination Symbol")}; + + for (int column = 0; column < Column::NumberOfColumns; ++column) + { + QAction* const action = + m_mnu_column_visibility->addAction(tr(headers[column]), [this, column](bool enabled) { + m_table_view->setColumnHidden(column, !enabled); + }); + action->setChecked(!m_table_view->isColumnHidden(column)); + action->setCheckable(true); + } + } + + // Menu Bar + QMenuBar* const menu_bar = new QMenuBar; + menu_bar->setNativeMenuBar(false); + { + QMenu* const menu = new QMenu(tr("&File"), menu_bar); + menu->addAction(tr("&Save Branch Watch"), this, &BranchWatchDialog::OnSave); + menu->addAction(tr("Save Branch Watch &As..."), this, &BranchWatchDialog::OnSaveAs); + menu->addAction(tr("&Load Branch Watch"), this, &BranchWatchDialog::OnLoad); + menu->addAction(tr("Load Branch Watch &From..."), this, &BranchWatchDialog::OnLoadFrom); + m_act_autosave = menu->addAction(tr("A&uto Save")); + m_act_autosave->setCheckable(true); + connect(m_act_autosave, &QAction::toggled, this, &BranchWatchDialog::OnToggleAutoSave); + menu_bar->addMenu(menu); + } + { + QMenu* const menu = new QMenu(tr("&Tool"), menu_bar); + menu->setToolTipsVisible(true); + menu->addAction(tr("Hide &Controls"), this, &BranchWatchDialog::OnHideShowControls) + ->setCheckable(true); + QAction* const act_ignore_apploader = menu->addAction(tr("Ignore &Apploader Branch Hits")); + act_ignore_apploader->setToolTip( + tr("This only applies to the initial boot of the emulated software.")); + act_ignore_apploader->setChecked(m_system.IsBranchWatchIgnoreApploader()); + act_ignore_apploader->setCheckable(true); + connect(act_ignore_apploader, &QAction::toggled, this, + &BranchWatchDialog::OnToggleIgnoreApploader); + menu->addMenu(m_mnu_column_visibility)->setText(tr("Column &Visibility")); + menu->addAction(tr("Wipe &Inspection Data"), this, &BranchWatchDialog::OnWipeInspection); + menu->addAction(tr("&Help"), this, &BranchWatchDialog::OnHelp); + menu_bar->addMenu(menu); + } + UpdateIcons(); connect(m_timer = new QTimer, &QTimer::timeout, this, &BranchWatchDialog::OnTimeout); @@ -495,14 +491,11 @@ BranchWatchDialog::BranchWatchDialog(Core::System& system, Core::BranchWatch& br main_layout->addWidget(m_table_view); main_layout->addWidget(m_status_bar); setLayout(main_layout); - - const auto& settings = Settings::GetQSettings(); - restoreGeometry(settings.value(QStringLiteral("branchwatchdialog/geometry")).toByteArray()); } BranchWatchDialog::~BranchWatchDialog() { - SaveSettings(); + SaveQSettings(); } static constexpr int BRANCH_WATCH_TOOL_TIMER_DELAY_MS = 100; @@ -893,7 +886,15 @@ void BranchWatchDialog::OnTableSetBreakpointBoth() SetBreakpoints(true, true); } -void BranchWatchDialog::SaveSettings() +void BranchWatchDialog::LoadQSettings() +{ + const auto& settings = Settings::GetQSettings(); + restoreGeometry(settings.value(QStringLiteral("branchwatchdialog/geometry")).toByteArray()); + m_table_view->horizontalHeader()->restoreState( // Restore column visibility state. + settings.value(QStringLiteral("branchwatchdialog/tableheader/state")).toByteArray()); +} + +void BranchWatchDialog::SaveQSettings() const { auto& settings = Settings::GetQSettings(); settings.setValue(QStringLiteral("branchwatchdialog/geometry"), saveGeometry()); diff --git a/Source/Core/DolphinQt/Debugger/BranchWatchDialog.h b/Source/Core/DolphinQt/Debugger/BranchWatchDialog.h index 05a8c82432..52d684ff34 100644 --- a/Source/Core/DolphinQt/Debugger/BranchWatchDialog.h +++ b/Source/Core/DolphinQt/Debugger/BranchWatchDialog.h @@ -98,7 +98,8 @@ private: void OnTableSetBreakpointLog(); void OnTableSetBreakpointBoth(); - void SaveSettings(); + void LoadQSettings(); + void SaveQSettings() const; public: // TODO: Step doesn't cause EmulationStateChanged to be emitted, so it has to call this manually. From f9f0806022b997a9bcee532685f54a0120b04187 Mon Sep 17 00:00:00 2001 From: mitaclaw <140017135+mitaclaw@users.noreply.github.com> Date: Sun, 4 Aug 2024 06:48:37 -0700 Subject: [PATCH 05/13] BranchWatchDialog: Disconnect Slots When Hidden --- .../DolphinQt/Debugger/BranchWatchDialog.cpp | 63 +++++++++++++------ .../DolphinQt/Debugger/BranchWatchDialog.h | 4 ++ .../Debugger/BranchWatchTableModel.cpp | 10 +++ .../Debugger/BranchWatchTableModel.h | 2 + 4 files changed, 59 insertions(+), 20 deletions(-) diff --git a/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp b/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp index a2b205b12d..e2eabd4e0a 100644 --- a/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp +++ b/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp @@ -203,20 +203,12 @@ BranchWatchDialog::BranchWatchDialog(Core::System& system, Core::BranchWatch& br setWindowFlags((windowFlags() | Qt::WindowMinMaxButtonsHint) & ~Qt::WindowContextHelpButtonHint); // Branch Watch Table - const auto& ui_settings = Settings::Instance(); - m_table_proxy = new BranchWatchProxyModel(m_branch_watch); m_table_proxy->setSourceModel( m_table_model = new BranchWatchTableModel(m_system, m_branch_watch, ppc_symbol_db)); m_table_proxy->setSortRole(UserRole::SortRole); m_table_proxy->setSortCaseSensitivity(Qt::CaseInsensitive); - m_table_model->setFont(ui_settings.GetDebugFont()); - connect(&ui_settings, &Settings::DebugFontChanged, m_table_model, - &BranchWatchTableModel::setFont); - connect(Host::GetInstance(), &Host::PPCSymbolsChanged, m_table_model, - &BranchWatchTableModel::UpdateSymbols); - m_table_view = new QTableView; m_table_view->setModel(m_table_proxy); m_table_view->setSortingEnabled(true); @@ -476,12 +468,7 @@ BranchWatchDialog::BranchWatchDialog(Core::System& system, Core::BranchWatch& br menu_bar->addMenu(menu); } - UpdateIcons(); - connect(m_timer = new QTimer, &QTimer::timeout, this, &BranchWatchDialog::OnTimeout); - connect(&Settings::Instance(), &Settings::EmulationStateChanged, this, - &BranchWatchDialog::OnEmulationStateChanged); - connect(&Settings::Instance(), &Settings::ThemeChanged, this, &BranchWatchDialog::OnThemeChanged); connect(m_table_proxy, &BranchWatchProxyModel::layoutChanged, this, &BranchWatchDialog::UpdateStatus); @@ -508,15 +495,13 @@ static bool TimerCondition(const Core::BranchWatch& branch_watch, Core::State st void BranchWatchDialog::hideEvent(QHideEvent* event) { - if (m_timer->isActive()) - m_timer->stop(); + Hide(); QDialog::hideEvent(event); } void BranchWatchDialog::showEvent(QShowEvent* event) { - if (TimerCondition(m_branch_watch, Core::GetState(m_system))) - m_timer->start(BRANCH_WATCH_TOOL_TIMER_DELAY_MS); + Show(); QDialog::showEvent(event); } @@ -674,9 +659,6 @@ void BranchWatchDialog::OnTimeout() void BranchWatchDialog::OnEmulationStateChanged(Core::State new_state) { - if (!isVisible()) - return; - if (TimerCondition(m_branch_watch, new_state)) m_timer->start(BRANCH_WATCH_TOOL_TIMER_DELAY_MS); else if (m_timer->isActive()) @@ -886,6 +868,47 @@ void BranchWatchDialog::OnTableSetBreakpointBoth() SetBreakpoints(true, true); } +void BranchWatchDialog::ConnectSlots() +{ + const auto* const settings = &Settings::Instance(); + connect(settings, &Settings::EmulationStateChanged, this, + &BranchWatchDialog::OnEmulationStateChanged); + connect(settings, &Settings::ThemeChanged, this, &BranchWatchDialog::OnThemeChanged); + connect(settings, &Settings::DebugFontChanged, m_table_model, &BranchWatchTableModel::setFont); + const auto* const host = Host::GetInstance(); + connect(host, &Host::PPCSymbolsChanged, m_table_model, &BranchWatchTableModel::UpdateSymbols); +} + +void BranchWatchDialog::DisconnectSlots() +{ + const auto* const settings = &Settings::Instance(); + disconnect(settings, &Settings::EmulationStateChanged, this, + &BranchWatchDialog::OnEmulationStateChanged); + disconnect(settings, &Settings::ThemeChanged, this, &BranchWatchDialog::OnThemeChanged); + disconnect(settings, &Settings::DebugFontChanged, m_table_model, + &BranchWatchTableModel::OnDebugFontChanged); + const auto* const host = Host::GetInstance(); + disconnect(host, &Host::PPCSymbolsChanged, m_table_model, + &BranchWatchTableModel::OnPPCSymbolsChanged); +} + +void BranchWatchDialog::Show() +{ + ConnectSlots(); + // Hit every slot that may have missed a signal while this widget was hidden. + OnEmulationStateChanged(Core::GetState(m_system)); + OnThemeChanged(); + m_table_model->OnDebugFontChanged(Settings::Instance().GetDebugFont()); + m_table_model->OnPPCSymbolsChanged(); +} + +void BranchWatchDialog::Hide() +{ + DisconnectSlots(); + if (m_timer->isActive()) + m_timer->stop(); +} + void BranchWatchDialog::LoadQSettings() { const auto& settings = Settings::GetQSettings(); diff --git a/Source/Core/DolphinQt/Debugger/BranchWatchDialog.h b/Source/Core/DolphinQt/Debugger/BranchWatchDialog.h index 52d684ff34..35f89c8f87 100644 --- a/Source/Core/DolphinQt/Debugger/BranchWatchDialog.h +++ b/Source/Core/DolphinQt/Debugger/BranchWatchDialog.h @@ -98,6 +98,10 @@ private: void OnTableSetBreakpointLog(); void OnTableSetBreakpointBoth(); + void ConnectSlots(); + void DisconnectSlots(); + void Show(); + void Hide(); void LoadQSettings(); void SaveQSettings() const; diff --git a/Source/Core/DolphinQt/Debugger/BranchWatchTableModel.cpp b/Source/Core/DolphinQt/Debugger/BranchWatchTableModel.cpp index a377f82c4e..be0c095d53 100644 --- a/Source/Core/DolphinQt/Debugger/BranchWatchTableModel.cpp +++ b/Source/Core/DolphinQt/Debugger/BranchWatchTableModel.cpp @@ -145,6 +145,16 @@ void BranchWatchTableModel::OnWipeInspection() roles); } +void BranchWatchTableModel::OnDebugFontChanged(const QFont& font) +{ + setFont(font); +} + +void BranchWatchTableModel::OnPPCSymbolsChanged() +{ + UpdateSymbols(); +} + 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 500a8ca53c..616c2da023 100644 --- a/Source/Core/DolphinQt/Debugger/BranchWatchTableModel.h +++ b/Source/Core/DolphinQt/Debugger/BranchWatchTableModel.h @@ -97,6 +97,8 @@ public: void OnBranchNotOverwritten(const Core::CPUThreadGuard& guard); void OnWipeRecentHits(); void OnWipeInspection(); + void OnDebugFontChanged(const QFont& font); + void OnPPCSymbolsChanged(); void Save(const Core::CPUThreadGuard& guard, std::FILE* file) const; void Load(const Core::CPUThreadGuard& guard, std::FILE* file); From 107c08b77f51a7b7c421c80bebc58f6b8d435b96 Mon Sep 17 00:00:00 2001 From: mitaclaw <140017135+mitaclaw@users.noreply.github.com> Date: Sun, 4 Aug 2024 07:44:39 -0700 Subject: [PATCH 06/13] BranchWatchDialog: Clean Up Object Parenting and Prefer Auto Objects which get parented automatically by later processing now pass a nullptr to the constructor to make the intent clearer. Also fixed "true" and "false" not being translatable strings. --- .../DolphinQt/Debugger/BranchWatchDialog.cpp | 83 +++++++++---------- 1 file changed, 40 insertions(+), 43 deletions(-) diff --git a/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp b/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp index e2eabd4e0a..928a907e64 100644 --- a/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp +++ b/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp @@ -203,13 +203,14 @@ BranchWatchDialog::BranchWatchDialog(Core::System& system, Core::BranchWatch& br setWindowFlags((windowFlags() | Qt::WindowMinMaxButtonsHint) & ~Qt::WindowContextHelpButtonHint); // Branch Watch Table - m_table_proxy = new BranchWatchProxyModel(m_branch_watch); - m_table_proxy->setSourceModel( - m_table_model = new BranchWatchTableModel(m_system, m_branch_watch, ppc_symbol_db)); + m_table_view = new QTableView(nullptr); + m_table_proxy = new BranchWatchProxyModel(m_branch_watch, m_table_view); + m_table_model = new BranchWatchTableModel(m_system, m_branch_watch, ppc_symbol_db, m_table_proxy); + + m_table_proxy->setSourceModel(m_table_model); m_table_proxy->setSortRole(UserRole::SortRole); m_table_proxy->setSortCaseSensitivity(Qt::CaseInsensitive); - m_table_view = new QTableView; m_table_view->setModel(m_table_proxy); m_table_view->setSortingEnabled(true); m_table_view->sortByColumn(Column::Origin, Qt::AscendingOrder); @@ -226,7 +227,7 @@ BranchWatchDialog::BranchWatchDialog(Core::System& system, Core::BranchWatch& br m_table_view->setColumnWidth(Column::DestinSymbol, 250); // The default column width (100 units) is fine for the rest. - QHeaderView* const horizontal_header = m_table_view->horizontalHeader(); + auto* const horizontal_header = m_table_view->horizontalHeader(); horizontal_header->setContextMenuPolicy(Qt::CustomContextMenu); horizontal_header->setStretchLastSection(true); horizontal_header->setSectionsMovable(true); @@ -241,39 +242,39 @@ BranchWatchDialog::BranchWatchDialog(Core::System& system, Core::BranchWatch& br &BranchWatchDialog::OnTableDeleteKeypress); // Status Bar - m_status_bar = new QStatusBar; + m_status_bar = new QStatusBar(nullptr); m_status_bar->setSizeGripEnabled(false); // Controls Toolbar - m_control_toolbar = new QToolBar; + m_control_toolbar = new QToolBar(nullptr); { // Tool Controls - m_btn_start_pause = new QPushButton(tr("Start Branch Watch")); + m_btn_start_pause = new QPushButton(tr("Start Branch Watch"), nullptr); connect(m_btn_start_pause, &QPushButton::toggled, this, &BranchWatchDialog::OnStartPause); m_btn_start_pause->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Expanding); m_btn_start_pause->setCheckable(true); - m_btn_clear_watch = new QPushButton(tr("Clear Branch Watch")); + m_btn_clear_watch = new QPushButton(tr("Clear Branch Watch"), nullptr); connect(m_btn_clear_watch, &QPushButton::pressed, this, &BranchWatchDialog::OnClearBranchWatch); m_btn_clear_watch->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Expanding); - m_btn_path_was_taken = new QPushButton(tr("Code Path Was Taken")); + m_btn_path_was_taken = new QPushButton(tr("Code Path Was Taken"), nullptr); connect(m_btn_path_was_taken, &QPushButton::pressed, this, &BranchWatchDialog::OnCodePathWasTaken); m_btn_path_was_taken->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Expanding); - m_btn_path_not_taken = new QPushButton(tr("Code Path Not Taken")); + m_btn_path_not_taken = new QPushButton(tr("Code Path Not Taken"), nullptr); connect(m_btn_path_not_taken, &QPushButton::pressed, this, &BranchWatchDialog::OnCodePathNotTaken); m_btn_path_not_taken->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Expanding); - auto* const layout = new QGridLayout; + auto* const layout = new QGridLayout(nullptr); layout->addWidget(m_btn_start_pause, 0, 0); layout->addWidget(m_btn_clear_watch, 1, 0); layout->addWidget(m_btn_path_was_taken, 0, 1); layout->addWidget(m_btn_path_not_taken, 1, 1); - auto* const group_box = new QGroupBox(tr("Tool Controls")); + auto* const group_box = new QGroupBox(tr("Tool Controls"), nullptr); group_box->setLayout(layout); group_box->setAlignment(Qt::AlignHCenter); @@ -281,17 +282,17 @@ BranchWatchDialog::BranchWatchDialog(Core::System& system, Core::BranchWatch& br } { // Spacer - auto* const widget = new QWidget; + auto* const widget = new QWidget(nullptr); widget->setSizePolicy(QSizePolicy::MinimumExpanding, QSizePolicy::Preferred); m_control_toolbar->addWidget(widget); } { // Branch Type Filter Options - auto* const layout = new QGridLayout; + auto* const layout = new QGridLayout(nullptr); const auto routine = [this, layout](const QString& text, const QString& tooltip, int row, int column, void (BranchWatchProxyModel::*slot)(bool)) { - QCheckBox* const check_box = new QCheckBox(text); + auto* const check_box = new QCheckBox(text, nullptr); check_box->setToolTip(tooltip); layout->addWidget(check_box, row, column); connect(check_box, &QCheckBox::toggled, [this, slot](bool checked) { @@ -316,7 +317,7 @@ BranchWatchDialog::BranchWatchDialog(Core::System& system, Core::BranchWatch& br routine(QStringLiteral("bcctrl"), tr("Branch Conditional to Count Register (LR saved)"), 2, 3, &BranchWatchProxyModel::OnToggled<&BranchWatchProxyModel::m_bcctrl>); // clang-format on - auto* const group_box = new QGroupBox(tr("Branch Type")); + auto* const group_box = new QGroupBox(tr("Branch Type"), nullptr); group_box->setLayout(layout); group_box->setAlignment(Qt::AlignHCenter); @@ -324,12 +325,12 @@ BranchWatchDialog::BranchWatchDialog(Core::System& system, Core::BranchWatch& br } { // Origin and Destination Filter Options - auto* const layout = new QGridLayout; + auto* const layout = new QGridLayout(nullptr); const auto routine = [this, layout](const QString& placeholder_text, int row, int column, int width, void (BranchWatchProxyModel::*slot)(const QString&)) { - QLineEdit* const line_edit = new QLineEdit; + auto* const line_edit = new QLineEdit(nullptr); layout->addWidget(line_edit, row, column, 1, width); connect(line_edit, &QLineEdit::textChanged, [this, slot](const QString& text) { (m_table_proxy->*slot)(text); @@ -348,7 +349,7 @@ BranchWatchDialog::BranchWatchDialog(Core::System& system, Core::BranchWatch& br routine(tr("Destination Max" ), 2, 1, 1, &BranchWatchProxyModel::OnAddressTextChanged<&BranchWatchProxyModel::m_destin_max>)->setMaxLength(8); // clang-format on - auto* const group_box = new QGroupBox(tr("Origin and Destination")); + auto* const group_box = new QGroupBox(tr("Origin and Destination"), nullptr); group_box->setLayout(layout); group_box->setAlignment(Qt::AlignHCenter); @@ -356,12 +357,12 @@ BranchWatchDialog::BranchWatchDialog(Core::System& system, Core::BranchWatch& br } { // Condition Filter Options - auto* const layout = new QVBoxLayout; + auto* const layout = new QVBoxLayout(nullptr); layout->setAlignment(Qt::AlignHCenter); const auto routine = [this, layout](const QString& text, void (BranchWatchProxyModel::*slot)(bool)) { - QCheckBox* const check_box = new QCheckBox(text); + auto* const check_box = new QCheckBox(text, nullptr); layout->addWidget(check_box); connect(check_box, &QCheckBox::toggled, [this, slot](bool checked) { (m_table_proxy->*slot)(checked); @@ -371,15 +372,13 @@ BranchWatchDialog::BranchWatchDialog(Core::System& system, Core::BranchWatch& br return check_box; }; - routine(QStringLiteral("true"), - &BranchWatchProxyModel::OnToggled<&BranchWatchProxyModel::m_cond_true>) + routine(tr("true"), &BranchWatchProxyModel::OnToggled<&BranchWatchProxyModel::m_cond_true>) ->setToolTip(tr("This will also filter unconditional branches.\n" "To filter for or against unconditional branches,\n" "use the Branch Type filter options.")); - routine(QStringLiteral("false"), - &BranchWatchProxyModel::OnToggled<&BranchWatchProxyModel::m_cond_false>); + routine(tr("false"), &BranchWatchProxyModel::OnToggled<&BranchWatchProxyModel::m_cond_false>); - auto* const group_box = new QGroupBox(tr("Condition")); + auto* const group_box = new QGroupBox(tr("Condition"), nullptr); group_box->setLayout(layout); group_box->setAlignment(Qt::AlignHCenter); @@ -387,28 +386,28 @@ BranchWatchDialog::BranchWatchDialog(Core::System& system, Core::BranchWatch& br } { // Misc. Controls - m_btn_was_overwritten = new QPushButton(tr("Branch Was Overwritten")); + m_btn_was_overwritten = new QPushButton(tr("Branch Was Overwritten"), nullptr); connect(m_btn_was_overwritten, &QPushButton::pressed, this, &BranchWatchDialog::OnBranchWasOverwritten); m_btn_was_overwritten->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Expanding); - m_btn_not_overwritten = new QPushButton(tr("Branch Not Overwritten")); + m_btn_not_overwritten = new QPushButton(tr("Branch Not Overwritten"), nullptr); connect(m_btn_not_overwritten, &QPushButton::pressed, this, &BranchWatchDialog::OnBranchNotOverwritten); m_btn_not_overwritten->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Expanding); - m_btn_wipe_recent_hits = new QPushButton(tr("Wipe Recent Hits")); + m_btn_wipe_recent_hits = new QPushButton(tr("Wipe Recent Hits"), nullptr); connect(m_btn_wipe_recent_hits, &QPushButton::pressed, this, &BranchWatchDialog::OnWipeRecentHits); m_btn_wipe_recent_hits->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Expanding); m_btn_wipe_recent_hits->setEnabled(false); - auto* const layout = new QVBoxLayout; + auto* const layout = new QVBoxLayout(nullptr); layout->addWidget(m_btn_was_overwritten); layout->addWidget(m_btn_not_overwritten); layout->addWidget(m_btn_wipe_recent_hits); - auto* const group_box = new QGroupBox(tr("Misc. Controls")); + auto* const group_box = new QGroupBox(tr("Misc. Controls"), nullptr); group_box->setLayout(layout); group_box->setAlignment(Qt::AlignHCenter); @@ -418,7 +417,7 @@ BranchWatchDialog::BranchWatchDialog(Core::System& system, Core::BranchWatch& br LoadQSettings(); // Column Visibility Menu - m_mnu_column_visibility = new QMenu(); + m_mnu_column_visibility = new QMenu(this); { static constexpr std::array headers = { QT_TR_NOOP("Instruction"), QT_TR_NOOP("Condition"), QT_TR_NOOP("Origin"), @@ -427,7 +426,7 @@ BranchWatchDialog::BranchWatchDialog(Core::System& system, Core::BranchWatch& br for (int column = 0; column < Column::NumberOfColumns; ++column) { - QAction* const action = + auto* const action = m_mnu_column_visibility->addAction(tr(headers[column]), [this, column](bool enabled) { m_table_view->setColumnHidden(column, !enabled); }); @@ -437,10 +436,10 @@ BranchWatchDialog::BranchWatchDialog(Core::System& system, Core::BranchWatch& br } // Menu Bar - QMenuBar* const menu_bar = new QMenuBar; + auto* const menu_bar = new QMenuBar(nullptr); menu_bar->setNativeMenuBar(false); { - QMenu* const menu = new QMenu(tr("&File"), menu_bar); + auto* const menu = menu_bar->addMenu(tr("&File")); menu->addAction(tr("&Save Branch Watch"), this, &BranchWatchDialog::OnSave); menu->addAction(tr("Save Branch Watch &As..."), this, &BranchWatchDialog::OnSaveAs); menu->addAction(tr("&Load Branch Watch"), this, &BranchWatchDialog::OnLoad); @@ -448,14 +447,13 @@ BranchWatchDialog::BranchWatchDialog(Core::System& system, Core::BranchWatch& br m_act_autosave = menu->addAction(tr("A&uto Save")); m_act_autosave->setCheckable(true); connect(m_act_autosave, &QAction::toggled, this, &BranchWatchDialog::OnToggleAutoSave); - menu_bar->addMenu(menu); } { - QMenu* const menu = new QMenu(tr("&Tool"), menu_bar); + auto* const menu = menu_bar->addMenu(tr("&Tool")); menu->setToolTipsVisible(true); menu->addAction(tr("Hide &Controls"), this, &BranchWatchDialog::OnHideShowControls) ->setCheckable(true); - QAction* const act_ignore_apploader = menu->addAction(tr("Ignore &Apploader Branch Hits")); + auto* const act_ignore_apploader = menu->addAction(tr("Ignore &Apploader Branch Hits")); act_ignore_apploader->setToolTip( tr("This only applies to the initial boot of the emulated software.")); act_ignore_apploader->setChecked(m_system.IsBranchWatchIgnoreApploader()); @@ -465,14 +463,13 @@ BranchWatchDialog::BranchWatchDialog(Core::System& system, Core::BranchWatch& br menu->addMenu(m_mnu_column_visibility)->setText(tr("Column &Visibility")); menu->addAction(tr("Wipe &Inspection Data"), this, &BranchWatchDialog::OnWipeInspection); menu->addAction(tr("&Help"), this, &BranchWatchDialog::OnHelp); - menu_bar->addMenu(menu); } - connect(m_timer = new QTimer, &QTimer::timeout, this, &BranchWatchDialog::OnTimeout); + connect(m_timer = new QTimer(this), &QTimer::timeout, this, &BranchWatchDialog::OnTimeout); connect(m_table_proxy, &BranchWatchProxyModel::layoutChanged, this, &BranchWatchDialog::UpdateStatus); - auto* const main_layout = new QVBoxLayout; + auto* const main_layout = new QVBoxLayout(nullptr); main_layout->setMenuBar(menu_bar); main_layout->addWidget(m_control_toolbar); main_layout->addWidget(m_table_view); @@ -1056,7 +1053,7 @@ QMenu* BranchWatchDialog::GetTableContextMenu(const QModelIndex& index) m_act_copy_address = m_mnu_table_context->addAction(tr("&Copy Address"), this, &BranchWatchDialog::OnTableCopyAddress); - m_mnu_set_breakpoint = new QMenu(tr("Set Brea&kpoint")); + m_mnu_set_breakpoint = new QMenu(tr("Set Brea&kpoint"), this); m_act_break_on_hit = m_mnu_set_breakpoint->addAction( tr("&Break on Hit"), this, &BranchWatchDialog::OnTableSetBreakpointBreak); m_act_log_on_hit = m_mnu_set_breakpoint->addAction(tr("&Log on Hit"), this, From 7ce703a4a82e8f94d06bc5bdb6b7bc881a595a4c Mon Sep 17 00:00:00 2001 From: mitaclaw <140017135+mitaclaw@users.noreply.github.com> Date: Mon, 5 Aug 2024 21:34:10 -0700 Subject: [PATCH 07/13] BranchWatchDialog: Refactor Context Menus Instead of one wildly complex context menu constructed lazily, now three manageable context menus are constructed proactively. --- .../DolphinQt/Debugger/BranchWatchDialog.cpp | 201 +++++++++--------- .../DolphinQt/Debugger/BranchWatchDialog.h | 9 +- 2 files changed, 112 insertions(+), 98 deletions(-) diff --git a/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp b/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp index 928a907e64..63c389cded 100644 --- a/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp +++ b/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp @@ -34,6 +34,7 @@ #include "Common/CommonTypes.h" #include "Common/FileUtil.h" #include "Common/IOFile.h" +#include "Common/Unreachable.h" #include "Core/ConfigManager.h" #include "Core/Core.h" #include "Core/Debugger/BranchWatch.h" @@ -414,6 +415,38 @@ BranchWatchDialog::BranchWatchDialog(Core::System& system, Core::BranchWatch& br m_control_toolbar->addWidget(group_box); } + // Table Context Menus + auto* const delete_action = new QAction(tr("&Delete"), this); + connect(delete_action, &QAction::triggered, this, &BranchWatchDialog::OnTableDelete); + + m_act_copy_address = new QAction(tr("&Copy Address"), this); + connect(m_act_copy_address, &QAction::triggered, this, &BranchWatchDialog::OnTableCopyAddress); + + m_act_insert_nop = new QAction(tr("Insert &NOP"), this); + connect(m_act_insert_nop, &QAction::triggered, this, &BranchWatchDialog::OnTableSetNOP); + + m_act_insert_blr = new QAction(tr("Insert &BLR"), this); + connect(m_act_insert_blr, &QAction::triggered, this, &BranchWatchDialog::OnTableSetBLR); + + m_mnu_set_breakpoint = new QMenu(tr("Set Brea&kpoint"), this); + m_act_break_on_hit = m_mnu_set_breakpoint->addAction( + tr("&Break on Hit"), this, &BranchWatchDialog::OnTableSetBreakpointBreak); + m_act_log_on_hit = m_mnu_set_breakpoint->addAction(tr("&Log on Hit"), this, + &BranchWatchDialog::OnTableSetBreakpointLog); + m_act_both_on_hit = m_mnu_set_breakpoint->addAction(tr("Break &and Log on Hit"), this, + &BranchWatchDialog::OnTableSetBreakpointBoth); + + m_mnu_table_context_origin = new QMenu(this); + m_mnu_table_context_origin->addActions( + {delete_action, m_act_insert_nop, m_act_copy_address, m_mnu_set_breakpoint->menuAction()}); + + m_mnu_table_context_destin_or_symbol = new QMenu(this); + m_mnu_table_context_destin_or_symbol->addActions( + {delete_action, m_act_insert_blr, m_act_copy_address, m_mnu_set_breakpoint->menuAction()}); + + m_mnu_table_context_other = new QMenu(this); + m_mnu_table_context_other->addAction(delete_action); + LoadQSettings(); // Column Visibility Menu @@ -1040,112 +1073,88 @@ void BranchWatchDialog::SetBreakpoints(bool break_on_hit, bool log_on_hit) const m_code_widget->Update(); } -QMenu* BranchWatchDialog::GetTableContextMenu(const QModelIndex& index) +void BranchWatchDialog::SetBreakpointMenuActionsIcons() const { - if (m_mnu_table_context == nullptr) + qsizetype bp_break_count = 0, bp_log_count = 0, bp_both_count = 0; + for (auto& breakpoints = m_system.GetPowerPC().GetBreakPoints(); + const QModelIndex& index : m_index_list_temp) { - 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); - - m_mnu_set_breakpoint = new QMenu(tr("Set Brea&kpoint"), this); - m_act_break_on_hit = m_mnu_set_breakpoint->addAction( - tr("&Break on Hit"), this, &BranchWatchDialog::OnTableSetBreakpointBreak); - m_act_log_on_hit = m_mnu_set_breakpoint->addAction(tr("&Log on Hit"), this, - &BranchWatchDialog::OnTableSetBreakpointLog); - m_act_both_on_hit = m_mnu_set_breakpoint->addAction( - tr("Break &and Log on Hit"), this, &BranchWatchDialog::OnTableSetBreakpointBoth); - m_mnu_table_context->addMenu(m_mnu_set_breakpoint); + if (const TBreakPoint* bp = breakpoints.GetRegularBreakpoint( + m_table_proxy->data(index, UserRole::ClickRole).value())) + { + if (bp->break_on_hit && bp->log_on_hit) + { + bp_both_count += 1; + continue; + } + bp_break_count += bp->break_on_hit; + bp_log_count += bp->log_on_hit; + } } + const qsizetype selected_row_count = m_index_list_temp.size(); + m_act_break_on_hit->setIconVisibleInMenu(bp_break_count != 0); + m_act_break_on_hit->setIcon(bp_break_count == selected_row_count ? m_icn_full : m_icn_partial); + m_act_log_on_hit->setIconVisibleInMenu(bp_log_count != 0); + m_act_log_on_hit->setIcon(bp_log_count == selected_row_count ? m_icn_full : m_icn_partial); + m_act_both_on_hit->setIconVisibleInMenu(bp_both_count != 0); + m_act_both_on_hit->setIcon(bp_both_count == selected_row_count ? m_icn_full : m_icn_partial); +} +QMenu* BranchWatchDialog::GetTableContextMenu(const QModelIndex& index) const +{ const bool core_initialized = Core::GetState(m_system) != Core::State::Uninitialized; - - bool supported_column = true; switch (index.column()) { + case Column::Instruction: + case Column::Condition: + return m_mnu_table_context_other; case Column::Origin: - m_act_insert_blr->setVisible(false); - m_act_insert_nop->setVisible(true); - m_act_insert_nop->setEnabled(core_initialized); - m_act_copy_address->setEnabled(true); - m_mnu_set_breakpoint->setEnabled(true); - break; + return GetTableContextMenu_Origin(core_initialized); case Column::Destination: - { - m_act_insert_nop->setVisible(false); - m_act_insert_blr->setVisible(true); - const bool all_branches_save_lr = - core_initialized && - 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); - m_mnu_set_breakpoint->setEnabled(true); - break; - } + return GetTableContextMenu_Destin(core_initialized); + case Column::RecentHits: + case Column::TotalHits: + return m_mnu_table_context_other; case Column::OriginSymbol: case Column::DestinSymbol: - { - m_act_insert_nop->setVisible(false); - m_act_insert_blr->setVisible(true); - const bool all_symbols_valid = - core_initialized && - 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); - m_mnu_set_breakpoint->setEnabled(all_symbols_valid); - break; + return GetTableContextMenu_Symbol(core_initialized); } - default: - m_act_insert_nop->setVisible(false); - m_act_insert_blr->setVisible(false); - supported_column = false; - break; - } - m_act_copy_address->setVisible(supported_column); - m_mnu_set_breakpoint->menuAction()->setVisible(supported_column); - // Setting breakpoints while nothing is being emulated is discouraged in the UI. - m_mnu_set_breakpoint->setEnabled(core_initialized); - - if (core_initialized && supported_column) - { - qsizetype bp_break_count = 0, bp_log_count = 0, bp_both_count = 0; - for (auto& breakpoints = m_system.GetPowerPC().GetBreakPoints(); - const QModelIndex& idx : m_index_list_temp) - { - if (const TBreakPoint* bp = breakpoints.GetRegularBreakpoint( - m_table_proxy->data(idx, UserRole::ClickRole).value())) - { - if (bp->break_on_hit && bp->log_on_hit) - { - bp_both_count += 1; - continue; - } - bp_break_count += bp->break_on_hit; - bp_log_count += bp->log_on_hit; - } - } - m_act_break_on_hit->setIconVisibleInMenu(bp_break_count != 0); - m_act_break_on_hit->setIcon(bp_break_count == m_index_list_temp.size() ? m_icn_full : - m_icn_partial); - m_act_log_on_hit->setIconVisibleInMenu(bp_log_count != 0); - m_act_log_on_hit->setIcon(bp_log_count == m_index_list_temp.size() ? m_icn_full : - m_icn_partial); - m_act_both_on_hit->setIconVisibleInMenu(bp_both_count != 0); - m_act_both_on_hit->setIcon(bp_both_count == m_index_list_temp.size() ? m_icn_full : - m_icn_partial); - } - - return m_mnu_table_context; + static_assert(Column::NumberOfColumns == 8); + Common::Unreachable(); +} + +QMenu* BranchWatchDialog::GetTableContextMenu_Origin(bool core_initialized) const +{ + SetBreakpointMenuActionsIcons(); + m_act_insert_nop->setEnabled(core_initialized); + m_act_copy_address->setEnabled(true); + m_mnu_set_breakpoint->setEnabled(true); + return m_mnu_table_context_origin; +} + +QMenu* BranchWatchDialog::GetTableContextMenu_Destin(bool core_initialized) const +{ + SetBreakpointMenuActionsIcons(); + const bool all_branches_save_lr = // Taking advantage of short-circuit evaluation here. + core_initialized && std::ranges::all_of(m_index_list_temp, [this](const QModelIndex& index) { + const QModelIndex sibling = index.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); + m_mnu_set_breakpoint->setEnabled(true); + return m_mnu_table_context_destin_or_symbol; +} + +QMenu* BranchWatchDialog::GetTableContextMenu_Symbol(bool core_initialized) const +{ + SetBreakpointMenuActionsIcons(); + const bool all_symbols_valid = + std::ranges::all_of(m_index_list_temp, [this](const QModelIndex& index) { + return m_table_proxy->data(index, UserRole::ClickRole).isValid(); + }); + m_act_insert_blr->setEnabled(core_initialized && all_symbols_valid); + m_act_copy_address->setEnabled(all_symbols_valid); + m_mnu_set_breakpoint->setEnabled(all_symbols_valid); + return m_mnu_table_context_destin_or_symbol; } diff --git a/Source/Core/DolphinQt/Debugger/BranchWatchDialog.h b/Source/Core/DolphinQt/Debugger/BranchWatchDialog.h index 35f89c8f87..87bc2c131f 100644 --- a/Source/Core/DolphinQt/Debugger/BranchWatchDialog.h +++ b/Source/Core/DolphinQt/Debugger/BranchWatchDialog.h @@ -118,7 +118,11 @@ private: void SetStubPatches(u32 value) const; void SetBreakpoints(bool break_on_hit, bool log_on_hit) const; - [[nodiscard]] QMenu* GetTableContextMenu(const QModelIndex& index); + void SetBreakpointMenuActionsIcons() const; + QMenu* GetTableContextMenu(const QModelIndex& index) const; + QMenu* GetTableContextMenu_Origin(bool core_initialized) const; + QMenu* GetTableContextMenu_Destin(bool core_initialized) const; + QMenu* GetTableContextMenu_Symbol(bool core_initialized) const; Core::System& m_system; Core::BranchWatch& m_branch_watch; @@ -134,7 +138,8 @@ private: QAction* m_act_break_on_hit; QAction* m_act_log_on_hit; QAction* m_act_both_on_hit; - QMenu* m_mnu_table_context = nullptr; + QMenu *m_mnu_table_context_origin, *m_mnu_table_context_destin_or_symbol, + *m_mnu_table_context_other; QMenu* m_mnu_column_visibility; QToolBar* m_control_toolbar; From 0c889c715d36b1c0817ac721ccefedf077814a19 Mon Sep 17 00:00:00 2001 From: mitaclaw <140017135+mitaclaw@users.noreply.github.com> Date: Sat, 31 Aug 2024 12:36:24 -0700 Subject: [PATCH 08/13] BranchWatchDialog: Const Correctness m_index_list_temp should not be imagined as a member of `BranchWatchDialog`, so it is now mutable to allow for more const member functions. --- .../DolphinQt/Debugger/BranchWatchDialog.cpp | 44 +++++++++---------- .../DolphinQt/Debugger/BranchWatchDialog.h | 42 +++++++++--------- 2 files changed, 43 insertions(+), 43 deletions(-) diff --git a/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp b/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp index 63c389cded..4ee474c5ee 100644 --- a/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp +++ b/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp @@ -104,7 +104,7 @@ public: } bool IsBranchTypeAllowed(UGeckoInstruction inst) const; - void SetInspected(const QModelIndex& index); + void SetInspected(const QModelIndex& index) const; private: const Core::BranchWatch& m_branch_watch; @@ -190,7 +190,7 @@ bool BranchWatchProxyModel::IsBranchTypeAllowed(UGeckoInstruction inst) const return false; } -void BranchWatchProxyModel::SetInspected(const QModelIndex& index) +void BranchWatchProxyModel::SetInspected(const QModelIndex& index) const { sourceModel()->SetInspected(mapToSource(index)); } @@ -535,7 +535,7 @@ void BranchWatchDialog::showEvent(QShowEvent* event) QDialog::showEvent(event); } -void BranchWatchDialog::OnStartPause(bool checked) +void BranchWatchDialog::OnStartPause(bool checked) const { if (checked) { @@ -672,22 +672,22 @@ void BranchWatchDialog::OnBranchNotOverwritten() UpdateStatus(); } -void BranchWatchDialog::OnWipeRecentHits() +void BranchWatchDialog::OnWipeRecentHits() const { m_table_model->OnWipeRecentHits(); } -void BranchWatchDialog::OnWipeInspection() +void BranchWatchDialog::OnWipeInspection() const { m_table_model->OnWipeInspection(); } -void BranchWatchDialog::OnTimeout() +void BranchWatchDialog::OnTimeout() const { Update(); } -void BranchWatchDialog::OnEmulationStateChanged(Core::State new_state) +void BranchWatchDialog::OnEmulationStateChanged(Core::State new_state) const { if (TimerCondition(m_branch_watch, new_state)) m_timer->start(BRANCH_WATCH_TOOL_TIMER_DELAY_MS); @@ -781,7 +781,7 @@ void BranchWatchDialog::OnToggleAutoSave(bool checked) m_autosave_filepath = filepath.toStdString(); } -void BranchWatchDialog::OnHideShowControls(bool checked) +void BranchWatchDialog::OnHideShowControls(bool checked) const { if (checked) m_control_toolbar->hide(); @@ -789,12 +789,12 @@ void BranchWatchDialog::OnHideShowControls(bool checked) m_control_toolbar->show(); } -void BranchWatchDialog::OnToggleIgnoreApploader(bool checked) +void BranchWatchDialog::OnToggleIgnoreApploader(bool checked) const { m_system.SetIsBranchWatchIgnoreApploader(checked); } -void BranchWatchDialog::OnTableClicked(const QModelIndex& index) +void BranchWatchDialog::OnTableClicked(const QModelIndex& index) const { const QVariant v = m_table_proxy->data(index, UserRole::ClickRole); switch (index.column()) @@ -811,7 +811,7 @@ void BranchWatchDialog::OnTableClicked(const QModelIndex& index) } } -void BranchWatchDialog::OnTableContextMenu(const QPoint& pos) +void BranchWatchDialog::OnTableContextMenu(const QPoint& pos) const { if (m_table_view->horizontalHeader()->hiddenSectionCount() == Column::NumberOfColumns) { @@ -827,12 +827,12 @@ void BranchWatchDialog::OnTableContextMenu(const QPoint& pos) m_index_list_temp.shrink_to_fit(); } -void BranchWatchDialog::OnTableHeaderContextMenu(const QPoint& pos) +void BranchWatchDialog::OnTableHeaderContextMenu(const QPoint& pos) const { m_mnu_column_visibility->exec(m_table_view->horizontalHeader()->mapToGlobal(pos)); } -void BranchWatchDialog::OnTableDelete() +void BranchWatchDialog::OnTableDelete() const { std::ranges::transform( m_index_list_temp, m_index_list_temp.begin(), @@ -847,7 +847,7 @@ void BranchWatchDialog::OnTableDelete() UpdateStatus(); } -void BranchWatchDialog::OnTableDeleteKeypress() +void BranchWatchDialog::OnTableDeleteKeypress() const { m_index_list_temp = m_table_view->selectionModel()->selectedRows(); OnTableDelete(); @@ -855,17 +855,17 @@ void BranchWatchDialog::OnTableDeleteKeypress() m_index_list_temp.shrink_to_fit(); } -void BranchWatchDialog::OnTableSetBLR() +void BranchWatchDialog::OnTableSetBLR() const { SetStubPatches(0x4e800020); } -void BranchWatchDialog::OnTableSetNOP() +void BranchWatchDialog::OnTableSetNOP() const { SetStubPatches(0x60000000); } -void BranchWatchDialog::OnTableCopyAddress() +void BranchWatchDialog::OnTableCopyAddress() const { auto iter = m_index_list_temp.begin(); if (iter == m_index_list_temp.end()) @@ -883,17 +883,17 @@ void BranchWatchDialog::OnTableCopyAddress() QApplication::clipboard()->setText(text); } -void BranchWatchDialog::OnTableSetBreakpointBreak() +void BranchWatchDialog::OnTableSetBreakpointBreak() const { SetBreakpoints(true, false); } -void BranchWatchDialog::OnTableSetBreakpointLog() +void BranchWatchDialog::OnTableSetBreakpointLog() const { SetBreakpoints(false, true); } -void BranchWatchDialog::OnTableSetBreakpointBoth() +void BranchWatchDialog::OnTableSetBreakpointBoth() const { SetBreakpoints(true, true); } @@ -955,14 +955,14 @@ void BranchWatchDialog::SaveQSettings() const m_table_view->horizontalHeader()->saveState()); } -void BranchWatchDialog::Update() +void BranchWatchDialog::Update() const { if (m_branch_watch.GetRecordingPhase() == Core::BranchWatch::Phase::Blacklist) UpdateStatus(); m_table_model->UpdateHits(); } -void BranchWatchDialog::UpdateStatus() +void BranchWatchDialog::UpdateStatus() const { switch (m_branch_watch.GetRecordingPhase()) { diff --git a/Source/Core/DolphinQt/Debugger/BranchWatchDialog.h b/Source/Core/DolphinQt/Debugger/BranchWatchDialog.h index 87bc2c131f..0c9eee8433 100644 --- a/Source/Core/DolphinQt/Debugger/BranchWatchDialog.h +++ b/Source/Core/DolphinQt/Debugger/BranchWatchDialog.h @@ -66,7 +66,7 @@ protected: void showEvent(QShowEvent* event) override; private: - void OnStartPause(bool checked); + void OnStartPause(bool checked) const; void OnClearBranchWatch(); void OnSave(); void OnSaveAs(); @@ -76,27 +76,27 @@ private: void OnCodePathNotTaken(); void OnBranchWasOverwritten(); void OnBranchNotOverwritten(); - void OnWipeRecentHits(); - void OnWipeInspection(); - void OnTimeout(); - void OnEmulationStateChanged(Core::State new_state); + void OnWipeRecentHits() const; + void OnWipeInspection() const; + void OnTimeout() const; + void OnEmulationStateChanged(Core::State new_state) const; void OnThemeChanged(); void OnHelp(); void OnToggleAutoSave(bool checked); - void OnHideShowControls(bool checked); - void OnToggleIgnoreApploader(bool checked); + void OnHideShowControls(bool checked) const; + void OnToggleIgnoreApploader(bool checked) const; - void OnTableClicked(const QModelIndex& index); - void OnTableContextMenu(const QPoint& pos); - void OnTableHeaderContextMenu(const QPoint& pos); - void OnTableDelete(); - void OnTableDeleteKeypress(); - void OnTableSetBLR(); - void OnTableSetNOP(); - void OnTableCopyAddress(); - void OnTableSetBreakpointBreak(); - void OnTableSetBreakpointLog(); - void OnTableSetBreakpointBoth(); + void OnTableClicked(const QModelIndex& index) const; + void OnTableContextMenu(const QPoint& pos) const; + void OnTableHeaderContextMenu(const QPoint& pos) const; + void OnTableDelete() const; + void OnTableDeleteKeypress() const; + void OnTableSetBLR() const; + void OnTableSetNOP() const; + void OnTableCopyAddress() const; + void OnTableSetBreakpointBreak() const; + void OnTableSetBreakpointLog() const; + void OnTableSetBreakpointBoth() const; void ConnectSlots(); void DisconnectSlots(); @@ -107,10 +107,10 @@ private: public: // TODO: Step doesn't cause EmulationStateChanged to be emitted, so it has to call this manually. - void Update(); + void Update() const; private: - void UpdateStatus(); + void UpdateStatus() const; void UpdateIcons(); void Save(const Core::CPUThreadGuard& guard, const std::string& filepath); void Load(const Core::CPUThreadGuard& guard, const std::string& filepath); @@ -151,6 +151,6 @@ private: QIcon m_icn_full, m_icn_partial; - QModelIndexList m_index_list_temp; + mutable QModelIndexList m_index_list_temp; std::optional m_autosave_filepath; }; From f5e7b457731f57a8de132d21790fe90761d064ee Mon Sep 17 00:00:00 2001 From: mitaclaw <140017135+mitaclaw@users.noreply.github.com> Date: Thu, 8 Aug 2024 07:41:33 -0700 Subject: [PATCH 09/13] BranchWatchDialog: Listen For `clicked` Signal Rather Than `pressed` The latter signal was used by mistake, see PR #8263. --- Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp | 12 ++++++------ Source/Core/DolphinQt/Debugger/CodeWidget.cpp | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp b/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp index 4ee474c5ee..88ce34a8d2 100644 --- a/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp +++ b/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp @@ -256,16 +256,16 @@ BranchWatchDialog::BranchWatchDialog(Core::System& system, Core::BranchWatch& br m_btn_start_pause->setCheckable(true); m_btn_clear_watch = new QPushButton(tr("Clear Branch Watch"), nullptr); - connect(m_btn_clear_watch, &QPushButton::pressed, this, &BranchWatchDialog::OnClearBranchWatch); + connect(m_btn_clear_watch, &QPushButton::clicked, this, &BranchWatchDialog::OnClearBranchWatch); m_btn_clear_watch->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Expanding); m_btn_path_was_taken = new QPushButton(tr("Code Path Was Taken"), nullptr); - connect(m_btn_path_was_taken, &QPushButton::pressed, this, + connect(m_btn_path_was_taken, &QPushButton::clicked, this, &BranchWatchDialog::OnCodePathWasTaken); m_btn_path_was_taken->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Expanding); m_btn_path_not_taken = new QPushButton(tr("Code Path Not Taken"), nullptr); - connect(m_btn_path_not_taken, &QPushButton::pressed, this, + connect(m_btn_path_not_taken, &QPushButton::clicked, this, &BranchWatchDialog::OnCodePathNotTaken); m_btn_path_not_taken->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Expanding); @@ -388,17 +388,17 @@ BranchWatchDialog::BranchWatchDialog(Core::System& system, Core::BranchWatch& br { // Misc. Controls m_btn_was_overwritten = new QPushButton(tr("Branch Was Overwritten"), nullptr); - connect(m_btn_was_overwritten, &QPushButton::pressed, this, + connect(m_btn_was_overwritten, &QPushButton::clicked, this, &BranchWatchDialog::OnBranchWasOverwritten); m_btn_was_overwritten->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Expanding); m_btn_not_overwritten = new QPushButton(tr("Branch Not Overwritten"), nullptr); - connect(m_btn_not_overwritten, &QPushButton::pressed, this, + connect(m_btn_not_overwritten, &QPushButton::clicked, this, &BranchWatchDialog::OnBranchNotOverwritten); m_btn_not_overwritten->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Expanding); m_btn_wipe_recent_hits = new QPushButton(tr("Wipe Recent Hits"), nullptr); - connect(m_btn_wipe_recent_hits, &QPushButton::pressed, this, + connect(m_btn_wipe_recent_hits, &QPushButton::clicked, this, &BranchWatchDialog::OnWipeRecentHits); m_btn_wipe_recent_hits->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Expanding); m_btn_wipe_recent_hits->setEnabled(false); diff --git a/Source/Core/DolphinQt/Debugger/CodeWidget.cpp b/Source/Core/DolphinQt/Debugger/CodeWidget.cpp index 36d3d92284..5c92975f39 100644 --- a/Source/Core/DolphinQt/Debugger/CodeWidget.cpp +++ b/Source/Core/DolphinQt/Debugger/CodeWidget.cpp @@ -180,7 +180,7 @@ void CodeWidget::ConnectWidgets() }); connect(m_search_callstack, &QLineEdit::textChanged, this, &CodeWidget::UpdateCallstack); - connect(m_branch_watch, &QPushButton::pressed, this, &CodeWidget::OnBranchWatchDialog); + connect(m_branch_watch, &QPushButton::clicked, this, &CodeWidget::OnBranchWatchDialog); connect(m_symbols_list, &QListWidget::itemPressed, this, &CodeWidget::OnSelectSymbol); connect(m_callstack_list, &QListWidget::itemPressed, this, &CodeWidget::OnSelectCallstack); From e4500b57984d0a83049e43ca6c6ef2b6a739bd11 Mon Sep 17 00:00:00 2001 From: mitaclaw <140017135+mitaclaw@users.noreply.github.com> Date: Sat, 31 Aug 2024 15:10:04 -0700 Subject: [PATCH 10/13] BranchWatchDialog: Improve Branch Was/Not Overwritten Buttons Giving a warning if these are used when not usable is bad UX. --- Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp b/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp index 88ce34a8d2..39e7748df4 100644 --- a/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp +++ b/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp @@ -644,11 +644,6 @@ void BranchWatchDialog::OnCodePathNotTaken() void BranchWatchDialog::OnBranchWasOverwritten() { - if (Core::GetState(m_system) == Core::State::Uninitialized) - { - ModalMessageBox::warning(this, tr("Error"), tr("Core is uninitialized.")); - return; - } { const Core::CPUThreadGuard guard{m_system}; m_table_model->OnBranchWasOverwritten(guard); @@ -659,11 +654,6 @@ void BranchWatchDialog::OnBranchWasOverwritten() void BranchWatchDialog::OnBranchNotOverwritten() { - if (Core::GetState(m_system) == Core::State::Uninitialized) - { - ModalMessageBox::warning(this, tr("Error"), tr("Core is uninitialized.")); - return; - } { const Core::CPUThreadGuard guard{m_system}; m_table_model->OnBranchNotOverwritten(guard); @@ -689,6 +679,8 @@ void BranchWatchDialog::OnTimeout() const void BranchWatchDialog::OnEmulationStateChanged(Core::State new_state) const { + m_btn_was_overwritten->setEnabled(new_state != Core::State::Uninitialized); + m_btn_not_overwritten->setEnabled(new_state != Core::State::Uninitialized); if (TimerCondition(m_branch_watch, new_state)) m_timer->start(BRANCH_WATCH_TOOL_TIMER_DELAY_MS); else if (m_timer->isActive()) From 8bdfdc88b241e5f32177f434fc42f19dd9dd3c26 Mon Sep 17 00:00:00 2001 From: mitaclaw <140017135+mitaclaw@users.noreply.github.com> Date: Wed, 7 Aug 2024 02:39:17 -0700 Subject: [PATCH 11/13] Branch Watch Tool: Ignore Apploader Branch Hits Concurrency Fix Also removed worthless `Start` and `Pause` helpers from `Core::BranchWatch`. --- Source/Core/Core/Boot/Boot_BS2Emu.cpp | 4 ++-- Source/Core/Core/Debugger/BranchWatch.h | 4 +--- Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp | 12 +++--------- 3 files changed, 6 insertions(+), 14 deletions(-) diff --git a/Source/Core/Core/Boot/Boot_BS2Emu.cpp b/Source/Core/Core/Boot/Boot_BS2Emu.cpp index 891e7fc324..1f331fbe54 100644 --- a/Source/Core/Core/Boot/Boot_BS2Emu.cpp +++ b/Source/Core/Core/Boot/Boot_BS2Emu.cpp @@ -163,7 +163,7 @@ bool CBoot::RunApploader(Core::System& system, const Core::CPUThreadGuard& guard const bool resume_branch_watch = branch_watch.GetRecordingActive(); if (system.IsBranchWatchIgnoreApploader()) - branch_watch.Pause(); + branch_watch.SetRecordingActive(guard, false); // Call iAppLoaderEntry. DEBUG_LOG_FMT(BOOT, "Call iAppLoaderEntry"); @@ -226,7 +226,7 @@ bool CBoot::RunApploader(Core::System& system, const Core::CPUThreadGuard& guard // return ppc_state.pc = ppc_state.gpr[3]; - branch_watch.SetRecordingActive(resume_branch_watch); + branch_watch.SetRecordingActive(guard, resume_branch_watch); return true; } diff --git a/Source/Core/Core/Debugger/BranchWatch.h b/Source/Core/Core/Debugger/BranchWatch.h index fd4cd158ea..32dc28853d 100644 --- a/Source/Core/Core/Debugger/BranchWatch.h +++ b/Source/Core/Core/Debugger/BranchWatch.h @@ -117,9 +117,7 @@ public: using SelectionInspection = BranchWatchSelectionInspection; bool GetRecordingActive() const { return m_recording_active; } - void SetRecordingActive(bool active) { m_recording_active = active; } - void Start() { SetRecordingActive(true); } - void Pause() { SetRecordingActive(false); } + void SetRecordingActive(const CPUThreadGuard& guard, bool active) { m_recording_active = active; } void Clear(const CPUThreadGuard& guard); void Save(const CPUThreadGuard& guard, std::FILE* file) const; diff --git a/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp b/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp index 39e7748df4..aef6f18cc3 100644 --- a/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp +++ b/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp @@ -516,7 +516,6 @@ BranchWatchDialog::~BranchWatchDialog() } static constexpr int BRANCH_WATCH_TOOL_TIMER_DELAY_MS = 100; -static constexpr int BRANCH_WATCH_TOOL_TIMER_PAUSE_ONESHOT_MS = 200; static bool TimerCondition(const Core::BranchWatch& branch_watch, Core::State state) { @@ -537,23 +536,18 @@ void BranchWatchDialog::showEvent(QShowEvent* event) void BranchWatchDialog::OnStartPause(bool checked) const { + m_branch_watch.SetRecordingActive(Core::CPUThreadGuard{m_system}, checked); if (checked) { - m_branch_watch.Start(); m_btn_start_pause->setText(tr("Pause Branch Watch")); - // Restart the timer if the situation calls for it, but always turn off single-shot. - m_timer->setSingleShot(false); if (Core::GetState(m_system) > Core::State::Paused) m_timer->start(BRANCH_WATCH_TOOL_TIMER_DELAY_MS); } else { - m_branch_watch.Pause(); m_btn_start_pause->setText(tr("Start Branch Watch")); - // Schedule one last update in the future in case Branch Watch is in the middle of a hit. - if (Core::GetState(m_system) > Core::State::Paused) - m_timer->setInterval(BRANCH_WATCH_TOOL_TIMER_PAUSE_ONESHOT_MS); - m_timer->setSingleShot(true); + if (m_timer->isActive()) + m_timer->stop(); } Update(); } From 8f76a32be4af4ecd02839e05acfe1519deb36551 Mon Sep 17 00:00:00 2001 From: mitaclaw <140017135+mitaclaw@users.noreply.github.com> Date: Tue, 6 Aug 2024 05:04:50 -0700 Subject: [PATCH 12/13] Branch Watch Tool: New Conditional Branch Inspection Tools Invert conditions, invert decrement checks, and make conditional branches unconditional. USnapshotMetadata in prior versions of Dolphin is forward-compatible with these changes (tested on x86_64). --- Source/Core/Core/Debugger/BranchWatch.cpp | 4 +- Source/Core/Core/Debugger/BranchWatch.h | 2 + Source/Core/Core/PowerPC/Gekko.h | 8 ++ .../DolphinQt/Debugger/BranchWatchDialog.cpp | 128 +++++++++++++++--- .../DolphinQt/Debugger/BranchWatchDialog.h | 12 +- .../Debugger/BranchWatchTableModel.cpp | 127 +++++++++-------- .../Debugger/BranchWatchTableModel.h | 5 + 7 files changed, 199 insertions(+), 87 deletions(-) diff --git a/Source/Core/Core/Debugger/BranchWatch.cpp b/Source/Core/Core/Debugger/BranchWatch.cpp index 73e6892258..4ea0559cd9 100644 --- a/Source/Core/Core/Debugger/BranchWatch.cpp +++ b/Source/Core/Core/Debugger/BranchWatch.cpp @@ -38,14 +38,14 @@ union USnapshotMetadata using Inspection = BranchWatch::SelectionInspection; using StorageType = unsigned long long; - static_assert(Inspection::EndOfEnumeration == Inspection{(1u << 3) + 1}); + static_assert(Inspection::EndOfEnumeration == Inspection{(1u << 5) + 1}); StorageType hex; BitField<0, 1, bool, StorageType> is_virtual; BitField<1, 1, bool, StorageType> condition; BitField<2, 1, bool, StorageType> is_selected; - BitField<3, 4, Inspection, StorageType> inspection; + BitField<3, 6, Inspection, StorageType> inspection; USnapshotMetadata() : hex(0) {} explicit USnapshotMetadata(bool is_virtual_, bool condition_, bool is_selected_, diff --git a/Source/Core/Core/Debugger/BranchWatch.h b/Source/Core/Core/Debugger/BranchWatch.h index 32dc28853d..5644a979ba 100644 --- a/Source/Core/Core/Debugger/BranchWatch.h +++ b/Source/Core/Core/Debugger/BranchWatch.h @@ -63,6 +63,8 @@ enum class BranchWatchSelectionInspection : u8 SetDestinBLR = 1u << 1, SetOriginSymbolBLR = 1u << 2, SetDestinSymbolBLR = 1u << 3, + InvertBranchOption = 1u << 4, // Used for both conditions and decrement checks. + MakeUnconditional = 1u << 5, EndOfEnumeration, }; diff --git a/Source/Core/Core/PowerPC/Gekko.h b/Source/Core/Core/PowerPC/Gekko.h index 86c6dd8632..12d6a6bcc8 100644 --- a/Source/Core/Core/PowerPC/Gekko.h +++ b/Source/Core/Core/PowerPC/Gekko.h @@ -294,6 +294,14 @@ union UGeckoInstruction }; }; +// Precondition: inst is a bx, bcx, bclrx, or bcctrx instruction. +constexpr bool BranchIsConditional(UGeckoInstruction inst) +{ + if (inst.OPCD == 18) // bx + return false; + return (inst.BO & 0b10100) != 0b10100; // 1z1zz - Branch always +} + // Used in implementations of rlwimi, rlwinm, and rlwnm inline u32 MakeRotationMask(u32 mb, u32 me) { diff --git a/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp b/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp index aef6f18cc3..8e740daffd 100644 --- a/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp +++ b/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp @@ -105,6 +105,8 @@ public: bool IsBranchTypeAllowed(UGeckoInstruction inst) const; void SetInspected(const QModelIndex& index) const; + const Core::BranchWatchSelectionValueType& + GetBranchWatchSelection(const QModelIndex& index) const; private: const Core::BranchWatch& m_branch_watch; @@ -157,34 +159,25 @@ bool BranchWatchProxyModel::filterAcceptsRow(int source_row, const QModelIndex&) return true; } -static constexpr bool BranchSavesLR(UGeckoInstruction inst) -{ - DEBUG_ASSERT(inst.OPCD == 18 || inst.OPCD == 16 || - (inst.OPCD == 19 && (inst.SUBOP10 == 16 || inst.SUBOP10 == 528))); - // Every branch instruction uses the same LK field. - return inst.LK; -} - bool BranchWatchProxyModel::IsBranchTypeAllowed(UGeckoInstruction inst) const { - const bool lr_saved = BranchSavesLR(inst); switch (inst.OPCD) { case 18: - return lr_saved ? m_bl : m_b; + return inst.LK ? m_bl : m_b; case 16: - return lr_saved ? m_bcl : m_bc; + return inst.LK ? m_bcl : m_bc; case 19: switch (inst.SUBOP10) { case 16: if ((inst.BO & 0b10100) == 0b10100) // 1z1zz - Branch always - return lr_saved ? m_blrl : m_blr; - return lr_saved ? m_bclrl : m_bclr; + return inst.LK ? m_blrl : m_blr; + return inst.LK ? m_bclrl : m_bclr; case 528: if ((inst.BO & 0b10100) == 0b10100) // 1z1zz - Branch always - return lr_saved ? m_bctrl : m_bctr; - return lr_saved ? m_bcctrl : m_bcctr; + return inst.LK ? m_bctrl : m_bctr; + return inst.LK ? m_bcctrl : m_bcctr; } } return false; @@ -195,6 +188,12 @@ void BranchWatchProxyModel::SetInspected(const QModelIndex& index) const sourceModel()->SetInspected(mapToSource(index)); } +const Core::BranchWatchSelectionValueType& +BranchWatchProxyModel::GetBranchWatchSelection(const QModelIndex& index) const +{ + return sourceModel()->GetBranchWatchSelection(mapToSource(index)); +} + BranchWatchDialog::BranchWatchDialog(Core::System& system, Core::BranchWatch& branch_watch, PPCSymbolDB& ppc_symbol_db, CodeWidget* code_widget, QWidget* parent) @@ -419,6 +418,18 @@ BranchWatchDialog::BranchWatchDialog(Core::System& system, Core::BranchWatch& br auto* const delete_action = new QAction(tr("&Delete"), this); connect(delete_action, &QAction::triggered, this, &BranchWatchDialog::OnTableDelete); + m_act_invert_condition = new QAction(tr("Invert &Condition"), this); + connect(m_act_invert_condition, &QAction::triggered, this, + &BranchWatchDialog::OnTableInvertCondition); + + m_act_invert_decrement_check = new QAction(tr("Invert &Decrement Check"), this); + connect(m_act_invert_decrement_check, &QAction::triggered, this, + &BranchWatchDialog::OnTableInvertDecrementCheck); + + m_act_make_unconditional = new QAction(tr("Make &Unconditional"), this); + connect(m_act_make_unconditional, &QAction::triggered, this, + &BranchWatchDialog::OnTableMakeUnconditional); + m_act_copy_address = new QAction(tr("&Copy Address"), this); connect(m_act_copy_address, &QAction::triggered, this, &BranchWatchDialog::OnTableCopyAddress); @@ -436,6 +447,13 @@ BranchWatchDialog::BranchWatchDialog(Core::System& system, Core::BranchWatch& br m_act_both_on_hit = m_mnu_set_breakpoint->addAction(tr("Break &and Log on Hit"), this, &BranchWatchDialog::OnTableSetBreakpointBoth); + m_mnu_table_context_instruction = new QMenu(this); + m_mnu_table_context_instruction->addActions( + {delete_action, m_act_invert_condition, m_act_invert_decrement_check}); + + m_mnu_table_context_condition = new QMenu(this); + m_mnu_table_context_condition->addActions({delete_action, m_act_make_unconditional}); + m_mnu_table_context_origin = new QMenu(this); m_mnu_table_context_origin->addActions( {delete_action, m_act_insert_nop, m_act_copy_address, m_mnu_set_breakpoint->menuAction()}); @@ -734,6 +752,15 @@ void BranchWatchDialog::OnHelp() "destination symbol columns, these actions will only be enabled if every row in the " "selection has a symbol." "\n\n" + "If the instruction column of a row selection is right-clicked, an action to invert the " + "branch instruction's condition and an action to invert the branch instruction's " + "decrement check will be available, but only if the branch instruction is a conditional " + "one." + "\n\n" + "If the condition column of a row selection is right-clicked, an action to make the " + "branch instruction unconditional will be available, but only if the branch instruction " + "is a conditional one." + "\n\n" "If the origin column of a row selection is right-clicked, an action to replace the " "branch instruction at the origin(s) with a NOP instruction (No Operation) will be " "available." @@ -851,6 +878,33 @@ void BranchWatchDialog::OnTableSetNOP() const SetStubPatches(0x60000000); } +void BranchWatchDialog::OnTableInvertCondition() const +{ + SetEditPatches([](u32 hex) { + UGeckoInstruction inst = hex; + inst.BO ^= 0b01000; + return inst.hex; + }); +} + +void BranchWatchDialog::OnTableInvertDecrementCheck() const +{ + SetEditPatches([](u32 hex) { + UGeckoInstruction inst = hex; + inst.BO ^= 0b00010; + return inst.hex; + }); +} + +void BranchWatchDialog::OnTableMakeUnconditional() const +{ + SetEditPatches([](u32 hex) { + UGeckoInstruction inst = hex; + inst.BO = 0b10100; // 1z1zz - Branch always + return inst.hex; + }); +} + void BranchWatchDialog::OnTableCopyAddress() const { auto iter = m_index_list_temp.begin(); @@ -1047,6 +1101,21 @@ void BranchWatchDialog::SetStubPatches(u32 value) const m_code_widget->Update(); } +void BranchWatchDialog::SetEditPatches(u32 (*transform)(u32)) const +{ + auto& debug_interface = m_system.GetPowerPC().GetDebugInterface(); + for (const Core::CPUThreadGuard guard(m_system); const QModelIndex& index : m_index_list_temp) + { + const Core::BranchWatchCollectionKey& k = + m_table_proxy->GetBranchWatchSelection(index).collection_ptr->first; + // This function assumes patches apply to the origin address, unlike SetStubPatches. + debug_interface.SetPatch(guard, k.origin_addr, transform(k.original_inst.hex)); + m_table_proxy->SetInspected(index); + } + // TODO: Same issue as SetStubPatches. + m_code_widget->Update(); +} + void BranchWatchDialog::SetBreakpoints(bool break_on_hit, bool log_on_hit) const { auto& breakpoints = m_system.GetPowerPC().GetBreakPoints(); @@ -1092,8 +1161,9 @@ QMenu* BranchWatchDialog::GetTableContextMenu(const QModelIndex& index) const switch (index.column()) { case Column::Instruction: + return GetTableContextMenu_Instruction(core_initialized); case Column::Condition: - return m_mnu_table_context_other; + return GetTableContextMenu_Condition(core_initialized); case Column::Origin: return GetTableContextMenu_Origin(core_initialized); case Column::Destination: @@ -1109,6 +1179,29 @@ QMenu* BranchWatchDialog::GetTableContextMenu(const QModelIndex& index) const Common::Unreachable(); } +QMenu* BranchWatchDialog::GetTableContextMenu_Instruction(bool core_initialized) const +{ + const bool all_branches_conditional = // Taking advantage of short-circuit evaluation here. + core_initialized && std::ranges::all_of(m_index_list_temp, [this](const QModelIndex& index) { + return BranchIsConditional( + m_table_proxy->GetBranchWatchSelection(index).collection_ptr->first.original_inst); + }); + m_act_invert_condition->setEnabled(all_branches_conditional); + m_act_invert_decrement_check->setEnabled(all_branches_conditional); + return m_mnu_table_context_instruction; +} + +QMenu* BranchWatchDialog::GetTableContextMenu_Condition(bool core_initialized) const +{ + const bool all_branches_conditional = // Taking advantage of short-circuit evaluation here. + core_initialized && std::ranges::all_of(m_index_list_temp, [this](const QModelIndex& index) { + return BranchIsConditional( + m_table_proxy->GetBranchWatchSelection(index).collection_ptr->first.original_inst); + }); + m_act_make_unconditional->setEnabled(all_branches_conditional); + return m_mnu_table_context_condition; +} + QMenu* BranchWatchDialog::GetTableContextMenu_Origin(bool core_initialized) const { SetBreakpointMenuActionsIcons(); @@ -1123,8 +1216,7 @@ QMenu* BranchWatchDialog::GetTableContextMenu_Destin(bool core_initialized) cons SetBreakpointMenuActionsIcons(); const bool all_branches_save_lr = // Taking advantage of short-circuit evaluation here. core_initialized && std::ranges::all_of(m_index_list_temp, [this](const QModelIndex& index) { - const QModelIndex sibling = index.siblingAtColumn(Column::Instruction); - return BranchSavesLR(m_table_proxy->data(sibling, UserRole::ClickRole).value()); + return m_table_proxy->GetBranchWatchSelection(index).collection_ptr->first.original_inst.LK; }); m_act_insert_blr->setEnabled(all_branches_save_lr); m_act_copy_address->setEnabled(true); diff --git a/Source/Core/DolphinQt/Debugger/BranchWatchDialog.h b/Source/Core/DolphinQt/Debugger/BranchWatchDialog.h index 0c9eee8433..9d7d08833e 100644 --- a/Source/Core/DolphinQt/Debugger/BranchWatchDialog.h +++ b/Source/Core/DolphinQt/Debugger/BranchWatchDialog.h @@ -93,6 +93,9 @@ private: void OnTableDeleteKeypress() const; void OnTableSetBLR() const; void OnTableSetNOP() const; + void OnTableInvertCondition() const; + void OnTableInvertDecrementCheck() const; + void OnTableMakeUnconditional() const; void OnTableCopyAddress() const; void OnTableSetBreakpointBreak() const; void OnTableSetBreakpointLog() const; @@ -116,10 +119,13 @@ private: void Load(const Core::CPUThreadGuard& guard, const std::string& filepath); void AutoSave(const Core::CPUThreadGuard& guard); void SetStubPatches(u32 value) const; + void SetEditPatches(u32 (*transform)(u32)) const; void SetBreakpoints(bool break_on_hit, bool log_on_hit) const; void SetBreakpointMenuActionsIcons() const; QMenu* GetTableContextMenu(const QModelIndex& index) const; + QMenu* GetTableContextMenu_Instruction(bool core_initialized) const; + QMenu* GetTableContextMenu_Condition(bool core_initialized) const; QMenu* GetTableContextMenu_Origin(bool core_initialized) const; QMenu* GetTableContextMenu_Destin(bool core_initialized) const; QMenu* GetTableContextMenu_Symbol(bool core_initialized) const; @@ -131,6 +137,9 @@ 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_invert_condition; + QAction* m_act_invert_decrement_check; + QAction* m_act_make_unconditional; QAction* m_act_insert_nop; QAction* m_act_insert_blr; QAction* m_act_copy_address; @@ -138,7 +147,8 @@ private: QAction* m_act_break_on_hit; QAction* m_act_log_on_hit; QAction* m_act_both_on_hit; - QMenu *m_mnu_table_context_origin, *m_mnu_table_context_destin_or_symbol, + QMenu *m_mnu_table_context_instruction, *m_mnu_table_context_condition, + *m_mnu_table_context_origin, *m_mnu_table_context_destin_or_symbol, *m_mnu_table_context_other; QMenu* m_mnu_column_visibility; diff --git a/Source/Core/DolphinQt/Debugger/BranchWatchTableModel.cpp b/Source/Core/DolphinQt/Debugger/BranchWatchTableModel.cpp index be0c095d53..2a3f4cb3d1 100644 --- a/Source/Core/DolphinQt/Debugger/BranchWatchTableModel.cpp +++ b/Source/Core/DolphinQt/Debugger/BranchWatchTableModel.cpp @@ -195,6 +195,12 @@ void BranchWatchTableModel::SetInspected(const QModelIndex& index) const int row = index.row(); switch (index.column()) { + case Column::Instruction: + SetInspected(index, Core::BranchWatchSelectionInspection::InvertBranchOption); + return; + case Column::Condition: + SetInspected(index, Core::BranchWatchSelectionInspection::MakeUnconditional); + return; case Column::Origin: SetOriginInspected(m_branch_watch.GetSelection()[row].collection_ptr->first.origin_addr); return; @@ -210,35 +216,35 @@ void BranchWatchTableModel::SetInspected(const QModelIndex& index) } } +void BranchWatchTableModel::SetInspected(const QModelIndex& index, + Core::BranchWatchSelectionInspection inspection) +{ + static const QList roles = {Qt::FontRole, Qt::ForegroundRole}; + m_branch_watch.SetSelectedInspected(index.row(), inspection); + emit dataChanged(index, index, roles); +} + void BranchWatchTableModel::SetOriginInspected(u32 origin_addr) { - using Inspection = Core::BranchWatchSelectionInspection; - static const QList roles = {Qt::FontRole, Qt::ForegroundRole}; - const Core::BranchWatch::Selection& selection = m_branch_watch.GetSelection(); for (std::size_t i = 0; i < selection.size(); ++i) { if (selection[i].collection_ptr->first.origin_addr != origin_addr) continue; - m_branch_watch.SetSelectedInspected(i, Inspection::SetOriginNOP); - const QModelIndex index = createIndex(static_cast(i), Column::Origin); - emit dataChanged(index, index, roles); + SetInspected(createIndex(static_cast(i), Column::Origin), + Core::BranchWatchSelectionInspection::SetOriginNOP); } } void BranchWatchTableModel::SetDestinInspected(u32 destin_addr, bool nested) { - using Inspection = Core::BranchWatchSelectionInspection; - static const QList roles = {Qt::FontRole, Qt::ForegroundRole}; - const Core::BranchWatch::Selection& selection = m_branch_watch.GetSelection(); for (std::size_t i = 0; i < selection.size(); ++i) { if (selection[i].collection_ptr->first.destin_addr != destin_addr) continue; - m_branch_watch.SetSelectedInspected(i, Inspection::SetDestinBLR); - const QModelIndex index = createIndex(static_cast(i), Column::Destination); - emit dataChanged(index, index, roles); + SetInspected(createIndex(static_cast(i), Column::Destination), + Core::BranchWatchSelectionInspection::SetDestinBLR); } if (nested) @@ -248,23 +254,18 @@ void BranchWatchTableModel::SetDestinInspected(u32 destin_addr, bool nested) void BranchWatchTableModel::SetSymbolInspected(u32 symbol_addr, bool nested) { - using Inspection = Core::BranchWatchSelectionInspection; - static const QList roles = {Qt::FontRole, Qt::ForegroundRole}; - for (qsizetype i = 0; i < m_symbol_list.size(); ++i) { const SymbolListValueType& value = m_symbol_list[i]; if (value.origin_addr.isValid() && value.origin_addr.value() == symbol_addr) { - m_branch_watch.SetSelectedInspected(i, Inspection::SetOriginSymbolBLR); - const QModelIndex index = createIndex(i, Column::OriginSymbol); - emit dataChanged(index, index, roles); + SetInspected(createIndex(static_cast(i), Column::OriginSymbol), + Core::BranchWatchSelectionInspection::SetOriginSymbolBLR); } if (value.destin_addr.isValid() && value.destin_addr.value() == symbol_addr) { - m_branch_watch.SetSelectedInspected(i, Inspection::SetDestinSymbolBLR); - const QModelIndex index = createIndex(i, Column::DestinSymbol); - emit dataChanged(index, index, roles); + SetInspected(createIndex(static_cast(i), Column::DestinSymbol), + Core::BranchWatchSelectionInspection::SetDestinSymbolBLR); } } @@ -273,6 +274,13 @@ void BranchWatchTableModel::SetSymbolInspected(u32 symbol_addr, bool nested) SetDestinInspected(symbol_addr, true); } +const Core::BranchWatchSelectionValueType& +BranchWatchTableModel::GetBranchWatchSelection(const QModelIndex& index) const +{ + ASSERT(index.isValid()); + return m_branch_watch.GetSelection()[index.row()]; +} + void BranchWatchTableModel::PrefetchSymbols() { if (m_branch_watch.GetRecordingPhase() != Core::BranchWatch::Phase::Reduction) @@ -306,25 +314,14 @@ static QString GetInstructionMnemonic(u32 hex) return QString::fromLatin1(disas.data(), split); } -static bool BranchIsUnconditional(UGeckoInstruction inst) -{ - if (inst.OPCD == 18) // bx - return true; - // If BranchWatch is doing its job, the input will be only bcx, bclrx, and bcctrx instructions. - DEBUG_ASSERT(inst.OPCD == 16 || (inst.OPCD == 19 && (inst.SUBOP10 == 16 || inst.SUBOP10 == 528))); - if ((inst.BO & 0b10100) == 0b10100) // 1z1zz - Branch always - return true; - return false; -} - static QString GetConditionString(const Core::BranchWatch::Selection::value_type& value, const Core::BranchWatch::Collection::value_type* kv) { if (value.condition == false) return BranchWatchTableModel::tr("false"); - if (BranchIsUnconditional(kv->first.original_inst)) - return QStringLiteral(""); - return BranchWatchTableModel::tr("true"); + if (BranchIsConditional(kv->first.original_inst)) + return BranchWatchTableModel::tr("true"); + return QStringLiteral(""); } QVariant BranchWatchTableModel::DisplayRoleData(const QModelIndex& index) const @@ -361,21 +358,25 @@ QVariant BranchWatchTableModel::DisplayRoleData(const QModelIndex& index) const QVariant BranchWatchTableModel::FontRoleData(const QModelIndex& index) const { m_font.setBold([&]() -> bool { + using Inspection = Core::BranchWatchSelectionInspection; + const auto get_bit_test = [this, &index](Inspection inspection_mask) -> bool { + const Inspection inspection = m_branch_watch.GetSelection()[index.row()].inspection; + return (inspection & inspection_mask) != Inspection{}; + }; switch (index.column()) { - using Inspection = Core::BranchWatchSelectionInspection; + case Column::Instruction: + return get_bit_test(Inspection::InvertBranchOption); + case Column::Condition: + return get_bit_test(Inspection::MakeUnconditional); case Column::Origin: - return (m_branch_watch.GetSelection()[index.row()].inspection & Inspection::SetOriginNOP) != - Inspection{}; + return get_bit_test(Inspection::SetOriginNOP); case Column::Destination: - return (m_branch_watch.GetSelection()[index.row()].inspection & Inspection::SetDestinBLR) != - Inspection{}; + return get_bit_test(Inspection::SetDestinBLR); case Column::OriginSymbol: - return (m_branch_watch.GetSelection()[index.row()].inspection & - Inspection::SetOriginSymbolBLR) != Inspection{}; + return get_bit_test(Inspection::SetOriginSymbolBLR); case Column::DestinSymbol: - return (m_branch_watch.GetSelection()[index.row()].inspection & - Inspection::SetDestinSymbolBLR) != Inspection{}; + return get_bit_test(Inspection::SetDestinSymbolBLR); } // Importantly, this code path avoids subscripting the selection to get an inspection value. return false; @@ -406,31 +407,25 @@ QVariant BranchWatchTableModel::TextAlignmentRoleData(const QModelIndex& index) QVariant BranchWatchTableModel::ForegroundRoleData(const QModelIndex& index) const { + using Inspection = Core::BranchWatchSelectionInspection; + const auto get_brush_v = [this, &index](Inspection inspection_mask) -> QVariant { + const Inspection inspection = m_branch_watch.GetSelection()[index.row()].inspection; + return (inspection & inspection_mask) != Inspection{} ? QBrush(Qt::red) : QVariant(); + }; switch (index.column()) { - using Inspection = Core::BranchWatchSelectionInspection; + case Column::Instruction: + return get_brush_v(Inspection::InvertBranchOption); + case Column::Condition: + return get_brush_v(Inspection::MakeUnconditional); case Column::Origin: - { - const Inspection inspection = m_branch_watch.GetSelection()[index.row()].inspection; - return (inspection & Inspection::SetOriginNOP) != Inspection{} ? QBrush(Qt::red) : QVariant(); - } + return get_brush_v(Inspection::SetOriginNOP); case Column::Destination: - { - const Inspection inspection = m_branch_watch.GetSelection()[index.row()].inspection; - return (inspection & Inspection::SetDestinBLR) != Inspection{} ? QBrush(Qt::red) : QVariant(); - } + return get_brush_v(Inspection::SetDestinBLR); case Column::OriginSymbol: - { - const Inspection inspection = m_branch_watch.GetSelection()[index.row()].inspection; - return (inspection & Inspection::SetOriginSymbolBLR) != Inspection{} ? QBrush(Qt::red) : - QVariant(); - } + return get_brush_v(Inspection::SetOriginSymbolBLR); case Column::DestinSymbol: - { - const Inspection inspection = m_branch_watch.GetSelection()[index.row()].inspection; - return (inspection & Inspection::SetDestinSymbolBLR) != Inspection{} ? QBrush(Qt::red) : - QVariant(); - } + return get_brush_v(Inspection::SetDestinSymbolBLR); } // Importantly, this code path avoids subscripting the selection to get an inspection value. return QVariant(); @@ -465,9 +460,9 @@ static int GetConditionInteger(const Core::BranchWatch::Selection::value_type& v { if (value.condition == false) return 0; - if (BranchIsUnconditional(kv->first.original_inst)) - return 2; - return 1; + if (BranchIsConditional(kv->first.original_inst)) + return 1; + return 2; } QVariant BranchWatchTableModel::SortRoleData(const QModelIndex& index) const diff --git a/Source/Core/DolphinQt/Debugger/BranchWatchTableModel.h b/Source/Core/DolphinQt/Debugger/BranchWatchTableModel.h index 616c2da023..c1cfca2415 100644 --- a/Source/Core/DolphinQt/Debugger/BranchWatchTableModel.h +++ b/Source/Core/DolphinQt/Debugger/BranchWatchTableModel.h @@ -15,6 +15,8 @@ namespace Core { class BranchWatch; +enum class BranchWatchSelectionInspection : u8; +struct BranchWatchSelectionValueType; class CPUThreadGuard; class System; } // namespace Core @@ -106,9 +108,12 @@ public: void UpdateHits(); void SetInspected(const QModelIndex& index); + const Core::BranchWatchSelectionValueType& + GetBranchWatchSelection(const QModelIndex& index) const; const SymbolList& GetSymbolList() const { return m_symbol_list; } private: + void SetInspected(const QModelIndex& index, Core::BranchWatchSelectionInspection inspection); void SetOriginInspected(u32 origin_addr); void SetDestinInspected(u32 destin_addr, bool nested); void SetSymbolInspected(u32 symbol_addr, bool nested); From 2a20e6e3df6d2b3fef46573dad0ef90a4dff4345 Mon Sep 17 00:00:00 2001 From: mitaclaw <140017135+mitaclaw@users.noreply.github.com> Date: Sun, 4 Aug 2024 05:44:44 -0700 Subject: [PATCH 13/13] Branch Watch Tool: Toolbar Visiblity Menu Adds the ability to hide unneeded features of the controls toolbar. --- .../DolphinQt/Debugger/BranchWatchDialog.cpp | 40 +++++++++++++++++-- .../DolphinQt/Debugger/BranchWatchDialog.h | 2 + 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp b/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp index 8e740daffd..9e89a42d41 100644 --- a/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp +++ b/Source/Core/DolphinQt/Debugger/BranchWatchDialog.cpp @@ -321,7 +321,7 @@ BranchWatchDialog::BranchWatchDialog(Core::System& system, Core::BranchWatch& br group_box->setLayout(layout); group_box->setAlignment(Qt::AlignHCenter); - m_control_toolbar->addWidget(group_box); + m_act_branch_type_filters = m_control_toolbar->addWidget(group_box); } { // Origin and Destination Filter Options @@ -353,7 +353,7 @@ BranchWatchDialog::BranchWatchDialog(Core::System& system, Core::BranchWatch& br group_box->setLayout(layout); group_box->setAlignment(Qt::AlignHCenter); - m_control_toolbar->addWidget(group_box); + m_act_origin_destin_filters = m_control_toolbar->addWidget(group_box); } { // Condition Filter Options @@ -382,7 +382,7 @@ BranchWatchDialog::BranchWatchDialog(Core::System& system, Core::BranchWatch& br group_box->setLayout(layout); group_box->setAlignment(Qt::AlignHCenter); - m_control_toolbar->addWidget(group_box); + m_act_condition_filters = m_control_toolbar->addWidget(group_box); } { // Misc. Controls @@ -411,7 +411,7 @@ BranchWatchDialog::BranchWatchDialog(Core::System& system, Core::BranchWatch& br group_box->setLayout(layout); group_box->setAlignment(Qt::AlignHCenter); - m_control_toolbar->addWidget(group_box); + m_act_misc_controls = m_control_toolbar->addWidget(group_box); } // Table Context Menus @@ -486,6 +486,21 @@ BranchWatchDialog::BranchWatchDialog(Core::System& system, Core::BranchWatch& br } } + // Toolbar Visibility Menu + auto* const toolbar_visibility_menu = new QMenu(this); + { + const auto routine = [toolbar_visibility_menu](const QString& text, QAction* toolbar_action) { + auto* const menu_action = + toolbar_visibility_menu->addAction(text, toolbar_action, &QAction::setVisible); + menu_action->setChecked(toolbar_action->isVisible()); + menu_action->setCheckable(true); + }; + routine(tr("&Branch Type"), m_act_branch_type_filters); + routine(tr("&Origin and Destination"), m_act_origin_destin_filters); + routine(tr("&Condition"), m_act_condition_filters); + routine(tr("&Misc. Controls"), m_act_misc_controls); + } + // Menu Bar auto* const menu_bar = new QMenuBar(nullptr); menu_bar->setNativeMenuBar(false); @@ -512,6 +527,7 @@ BranchWatchDialog::BranchWatchDialog(Core::System& system, Core::BranchWatch& br connect(act_ignore_apploader, &QAction::toggled, this, &BranchWatchDialog::OnToggleIgnoreApploader); menu->addMenu(m_mnu_column_visibility)->setText(tr("Column &Visibility")); + menu->addMenu(toolbar_visibility_menu)->setText(tr("&Toolbar Visibility")); menu->addAction(tr("Wipe &Inspection Data"), this, &BranchWatchDialog::OnWipeInspection); menu->addAction(tr("&Help"), this, &BranchWatchDialog::OnHelp); } @@ -985,6 +1001,14 @@ void BranchWatchDialog::LoadQSettings() restoreGeometry(settings.value(QStringLiteral("branchwatchdialog/geometry")).toByteArray()); m_table_view->horizontalHeader()->restoreState( // Restore column visibility state. settings.value(QStringLiteral("branchwatchdialog/tableheader/state")).toByteArray()); + m_act_branch_type_filters->setVisible( + !settings.value(QStringLiteral("branchwatchdialog/toolbar/branch_type_hidden")).toBool()); + m_act_origin_destin_filters->setVisible( + !settings.value(QStringLiteral("branchwatchdialog/toolbar/origin_destin_hidden")).toBool()); + m_act_condition_filters->setVisible( + !settings.value(QStringLiteral("branchwatchdialog/toolbar/condition_hidden")).toBool()); + m_act_misc_controls->setVisible( + !settings.value(QStringLiteral("branchwatchdialog/toolbar/misc_controls_hidden")).toBool()); } void BranchWatchDialog::SaveQSettings() const @@ -993,6 +1017,14 @@ void BranchWatchDialog::SaveQSettings() const settings.setValue(QStringLiteral("branchwatchdialog/geometry"), saveGeometry()); settings.setValue(QStringLiteral("branchwatchdialog/tableheader/state"), m_table_view->horizontalHeader()->saveState()); + settings.setValue(QStringLiteral("branchwatchdialog/toolbar/branch_type_hidden"), + !m_act_branch_type_filters->isVisible()); + settings.setValue(QStringLiteral("branchwatchdialog/toolbar/origin_destin_hidden"), + !m_act_origin_destin_filters->isVisible()); + settings.setValue(QStringLiteral("branchwatchdialog/toolbar/condition_hidden"), + !m_act_condition_filters->isVisible()); + settings.setValue(QStringLiteral("branchwatchdialog/toolbar/misc_controls_hidden"), + !m_act_misc_controls->isVisible()); } void BranchWatchDialog::Update() const diff --git a/Source/Core/DolphinQt/Debugger/BranchWatchDialog.h b/Source/Core/DolphinQt/Debugger/BranchWatchDialog.h index 9d7d08833e..9de8e1e36b 100644 --- a/Source/Core/DolphinQt/Debugger/BranchWatchDialog.h +++ b/Source/Core/DolphinQt/Debugger/BranchWatchDialog.h @@ -153,6 +153,8 @@ private: QMenu* m_mnu_column_visibility; QToolBar* m_control_toolbar; + QAction *m_act_branch_type_filters, *m_act_origin_destin_filters, *m_act_condition_filters, + *m_act_misc_controls; QTableView* m_table_view; BranchWatchProxyModel* m_table_proxy; BranchWatchTableModel* m_table_model;