From 2e006d97876b794cad2c4c5f6e6c9d3f37f5df88 Mon Sep 17 00:00:00 2001 From: TryTwo Date: Wed, 22 Nov 2023 00:51:27 -0700 Subject: [PATCH 1/6] MemoryViewWidget: Refactor. Remove OnItemChanged signal and QSignalBlocker - replace with a signal that is only sent at the correct time. --- .../DolphinQt/Debugger/MemoryViewWidget.cpp | 44 ++++++++++++------- .../DolphinQt/Debugger/MemoryViewWidget.h | 16 +++++++ 2 files changed, 43 insertions(+), 17 deletions(-) diff --git a/Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp b/Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp index fcf57c8acf..333bb46496 100644 --- a/Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp +++ b/Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp @@ -10,10 +10,11 @@ #include #include #include +#include #include #include #include -#include +#include #include #include @@ -52,6 +53,14 @@ constexpr int SCROLLBAR_CENTER = SCROLLBAR_MAXIMUM / 2; const QString INVALID_MEMORY = QStringLiteral("-"); +void TableEditDelegate::setModelData(QWidget* editor, QAbstractItemModel* model, + const QModelIndex& index) const +{ + // Triggers on placing data into a cell. Editor has the text to be input, index has the location. + const QString input = qobject_cast(editor)->text(); + emit editFinished(index.row(), index.column(), input); +} + class MemoryViewTable final : public QTableWidget { public: @@ -66,6 +75,11 @@ public: setSelectionMode(NoSelection); setTextElideMode(Qt::TextElideMode::ElideNone); + // Route user's direct cell inputs to an editFinished signal. Much better than an itemChanged + // signal and QSignalBlock juggling. + TableEditDelegate* table_edit_delegate = new TableEditDelegate(this); + setItemDelegate(table_edit_delegate); + // Prevent colors from changing based on focus. QPalette palette(m_view->palette()); palette.setBrush(QPalette::Inactive, QPalette::Highlight, palette.brush(QPalette::Highlight)); @@ -78,7 +92,8 @@ public: connect(this, &MemoryViewTable::customContextMenuRequested, m_view, &MemoryViewWidget::OnContextMenu); - connect(this, &MemoryViewTable::itemChanged, this, &MemoryViewTable::OnItemChanged); + connect(table_edit_delegate, &TableEditDelegate::editFinished, this, + &MemoryViewTable::OnDirectTableEdit); } void resizeEvent(QResizeEvent* event) override @@ -146,9 +161,9 @@ public: } } - void OnItemChanged(QTableWidgetItem* item) + void OnDirectTableEdit(const int row, const int column, const QString& text) { - QString text = item->text(); + QTableWidgetItem* item = this->item(row, column); MemoryViewWidget::Type type = static_cast(item->data(USER_ROLE_VALUE_TYPE).toInt()); std::vector bytes = m_view->ConvertTextToBytes(type, text); @@ -156,16 +171,16 @@ public: u32 address = item->data(USER_ROLE_CELL_ADDRESS).toUInt(); u32 end_address = address + static_cast(bytes.size()) - 1; AddressSpace::Accessors* accessors = AddressSpace::GetAccessors(m_view->GetAddressSpace()); - - const Core::CPUThreadGuard guard(m_view->m_system); - - if (!bytes.empty() && accessors->IsValidAddress(guard, address) && - accessors->IsValidAddress(guard, end_address)) { - for (const u8 c : bytes) - accessors->WriteU8(guard, address++, c); - } + const Core::CPUThreadGuard guard(m_view->m_system); + if (!bytes.empty() && accessors->IsValidAddress(guard, address) && + accessors->IsValidAddress(guard, end_address)) + { + for (const u8 c : bytes) + accessors->WriteU8(guard, address++, c); + } + } m_view->Update(); } @@ -292,8 +307,6 @@ void MemoryViewWidget::CreateTable() m_table->verticalHeader()->setMinimumSectionSize(m_font_vspace); m_table->horizontalHeader()->setMinimumSectionSize(m_font_width * 2); - const QSignalBlocker blocker(m_table); - // Set column and row parameters. // Span is the number of unique memory values covered in one row. const int data_span = m_bytes_per_row / GetTypeSize(m_type); @@ -398,7 +411,6 @@ void MemoryViewWidget::Update() if (m_table->item(1, 1) == nullptr) return; - const QSignalBlocker blocker(m_table); m_table->clearSelection(); // Update addresses @@ -464,8 +476,6 @@ void MemoryViewWidget::UpdateColumns(const Core::CPUThreadGuard* guard) if (m_table->item(1, 1) == nullptr) return; - const QSignalBlocker blocker(m_table); - for (int i = 0; i < m_table->rowCount(); i++) { for (int c = 0; c < m_data_columns; c++) diff --git a/Source/Core/DolphinQt/Debugger/MemoryViewWidget.h b/Source/Core/DolphinQt/Debugger/MemoryViewWidget.h index 06f5d06afd..f8620472bc 100644 --- a/Source/Core/DolphinQt/Debugger/MemoryViewWidget.h +++ b/Source/Core/DolphinQt/Debugger/MemoryViewWidget.h @@ -3,6 +3,7 @@ #pragma once +#include #include #include "Common/CommonTypes.h" @@ -22,6 +23,21 @@ class CPUThreadGuard; class System; } // namespace Core +// Captures direct editing of the table. +class TableEditDelegate : public QStyledItemDelegate +{ + Q_OBJECT + +public: + explicit TableEditDelegate(QObject* parent) : QStyledItemDelegate(parent) {} + + void setModelData(QWidget* editor, QAbstractItemModel* model, + const QModelIndex& index) const override; + +signals: + void editFinished(const int row, const int column, const QString& text) const; +}; + class MemoryViewTable; class MemoryViewWidget final : public QWidget From 3edb5acccab983b08a181356c75177a635ad3c0b Mon Sep 17 00:00:00 2001 From: TryTwo Date: Mon, 24 Jun 2024 13:57:33 -0700 Subject: [PATCH 2/6] MemoryViewWidget: Refactor updates using a dispatch function. Isolate memory reads from table updates. Preparations for auto update while a game is running. --- .../DolphinQt/Debugger/MemoryViewWidget.cpp | 186 ++++++++++++------ .../DolphinQt/Debugger/MemoryViewWidget.h | 16 +- .../Core/DolphinQt/Debugger/MemoryWidget.cpp | 2 +- 3 files changed, 140 insertions(+), 64 deletions(-) diff --git a/Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp b/Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp index 333bb46496..abe179e96e 100644 --- a/Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp +++ b/Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp @@ -99,7 +99,10 @@ public: void resizeEvent(QResizeEvent* event) override { QTableWidget::resizeEvent(event); - m_view->CreateTable(); + // Remakes table if vertically resized + const int rows = std::round((height() / static_cast(rowHeight(0))) - 0.25); + if (rows != rowCount()) + m_view->UpdateDispatcher(MemoryViewWidget::UpdateType::Full); } void keyPressEvent(QKeyEvent* event) override @@ -108,24 +111,21 @@ public: { case Qt::Key_Up: m_view->m_address -= m_view->m_bytes_per_row; - m_view->Update(); - return; + break; case Qt::Key_Down: m_view->m_address += m_view->m_bytes_per_row; - m_view->Update(); - return; + break; case Qt::Key_PageUp: m_view->m_address -= this->rowCount() * m_view->m_bytes_per_row; - m_view->Update(); - return; + break; case Qt::Key_PageDown: m_view->m_address += this->rowCount() * m_view->m_bytes_per_row; - m_view->Update(); - return; + break; default: QWidget::keyPressEvent(event); - break; + return; } + m_view->UpdateDispatcher(MemoryViewWidget::UpdateType::Addresses); } void wheelEvent(QWheelEvent* event) override @@ -137,7 +137,7 @@ public: return; m_view->m_address += delta * m_view->m_bytes_per_row; - m_view->Update(); + m_view->UpdateDispatcher(MemoryViewWidget::UpdateType::Addresses); } void mousePressEvent(QMouseEvent* event) override @@ -153,7 +153,6 @@ public: { const u32 address = item->data(USER_ROLE_CELL_ADDRESS).toUInt(); m_view->ToggleBreakpoint(address, true); - m_view->Update(); } else { @@ -181,7 +180,8 @@ public: accessors->WriteU8(guard, address++, c); } } - m_view->Update(); + + m_view->UpdateDispatcher(MemoryViewWidget::UpdateType::Values); } private: @@ -213,13 +213,25 @@ MemoryViewWidget::MemoryViewWidget(Core::System& system, QWidget* parent) this->setLayout(layout); connect(&Settings::Instance(), &Settings::DebugFontChanged, this, &MemoryViewWidget::UpdateFont); - connect(&Settings::Instance(), &Settings::EmulationStateChanged, this, - qOverload<>(&MemoryViewWidget::UpdateColumns)); connect(Host::GetInstance(), &Host::PPCBreakpointsChanged, this, - qOverload<>(&MemoryViewWidget::Update)); - connect(Host::GetInstance(), &Host::UpdateDisasmDialog, this, - qOverload<>(&MemoryViewWidget::UpdateColumns)); - connect(&Settings::Instance(), &Settings::ThemeChanged, this, &MemoryViewWidget::Update); + &MemoryViewWidget::UpdateBreakpointTags); + connect(&Settings::Instance(), &Settings::EmulationStateChanged, this, [this] { + // UpdateDisasmDialog currently catches pauses, no need to signal it twice. + if (Core::GetState(m_system) != Core::State::Paused) + UpdateDispatcher(UpdateType::Values); + }); + connect(Host::GetInstance(), &Host::UpdateDisasmDialog, this, [this] { + // Disasm spam will break updates while running. Only need it for things like steps when paused + // and breaks which trigger a pause. + if (Core::GetState(m_system) != Core::State::Running) + UpdateDispatcher(UpdateType::Values); + }); + + // CPU Thread to Main Thread. + connect(this, &MemoryViewWidget::AutoUpdate, this, + [this] { UpdateDispatcher(UpdateType::Auto); }); + connect(&Settings::Instance(), &Settings::ThemeChanged, this, + [this] { UpdateDispatcher(UpdateType::Full); }); // Also calls create table. UpdateFont(Settings::Instance().GetDebugFont()); @@ -235,7 +247,7 @@ void MemoryViewWidget::UpdateFont(const QFont& font) m_font_width = fm.horizontalAdvance(QLatin1Char('0')); m_table->setFont(font); - CreateTable(); + UpdateDispatcher(UpdateType::Full); } constexpr int GetTypeSize(MemoryViewWidget::Type type) @@ -297,6 +309,33 @@ constexpr int GetCharacterCount(MemoryViewWidget::Type type) } } +void MemoryViewWidget::UpdateDispatcher(UpdateType type) +{ + if (!isVisible()) + return; + + // Check if table needs to be created. + if (m_table->item(2, 1) == nullptr) + type = UpdateType::Full; + + switch (type) + { + case UpdateType::Full: + CreateTable(); + [[fallthrough]]; + case UpdateType::Addresses: + Update(); + [[fallthrough]]; + case UpdateType::Values: + if (Core::GetState(m_system) == Core::State::Paused) + GetValues(); + UpdateColumns(); + break; + default: + break; + } +} + void MemoryViewWidget::CreateTable() { m_table->clearContents(); @@ -401,16 +440,10 @@ void MemoryViewWidget::CreateTable() const int width = m_font_width * GetCharacterCount(m_type); for (int i = start_fill; i < total_columns; i++) m_table->setColumnWidth(i, width); - - Update(); } void MemoryViewWidget::Update() { - // Check if table is created - if (m_table->item(1, 1) == nullptr) - return; - m_table->clearSelection(); // Update addresses @@ -418,6 +451,9 @@ void MemoryViewWidget::Update() u32 row_address = address - (m_table->rowCount() / 2) * m_bytes_per_row; const int data_span = m_bytes_per_row / GetTypeSize(m_type); + m_address_range.first = row_address; + m_address_range.second = row_address + m_table->rowCount() * m_bytes_per_row - 1; + for (int i = 0; i < m_table->rowCount(); i++, row_address += m_bytes_per_row) { auto* bp_item = m_table->item(i, 0); @@ -441,58 +477,84 @@ void MemoryViewWidget::Update() } } - UpdateColumns(); UpdateBreakpointTags(); - - m_table->viewport()->update(); - m_table->update(); - update(); } void MemoryViewWidget::UpdateColumns() { - if (!isVisible()) - return; - - // Check if table is created - if (m_table->item(1, 1) == nullptr) - return; - - if (Core::GetState(m_system) == Core::State::Paused) - { - const Core::CPUThreadGuard guard(m_system); - UpdateColumns(&guard); - } - else - { - // If the core is running, blank out the view of memory instead of reading anything. - UpdateColumns(nullptr); - } -} - -void MemoryViewWidget::UpdateColumns(const Core::CPUThreadGuard* guard) -{ - // Check if table is created - if (m_table->item(1, 1) == nullptr) - return; - for (int i = 0; i < m_table->rowCount(); i++) { for (int c = 0; c < m_data_columns; c++) { auto* cell_item = m_table->item(i, c + MISC_COLUMNS); + if (!cell_item) + return; + const u32 cell_address = cell_item->data(USER_ROLE_CELL_ADDRESS).toUInt(); const Type type = static_cast(cell_item->data(USER_ROLE_VALUE_TYPE).toInt()); + std::optional new_text; - cell_item->setText(guard ? ValueToString(*guard, cell_address, type) : INVALID_MEMORY); + // Dual view auto sets the type of the left-side based on m_type. Only time type and + // m_type differ. + if (type != m_type) + { + new_text = m_values_dual_view.empty() || !m_values_dual_view.contains(cell_address) ? + std::nullopt : + m_values_dual_view.at(cell_address); + } + else + { + new_text = m_values.empty() || !m_values.contains(cell_address) ? std::nullopt : + m_values.at(cell_address); + } // Set search address to selected / colored if (cell_address == m_address_highlight) cell_item->setSelected(true); + + // It gets a bit complicated, because invalid addresses becoming valid should not be + // colored. + if (!new_text.has_value()) + cell_item->setText(INVALID_MEMORY); + else + cell_item->setText(new_text.value()); } } } +void MemoryViewWidget::GetValues() +{ + m_values.clear(); + m_values_dual_view.clear(); + + // Check for dual view type + Type type = Type::Null; + + if (m_dual_view) + { + if (GetTypeSize(m_type) == 1) + type = Type::Hex8; + else if (GetTypeSize(m_type) == 2) + type = Type::Hex16; + else if (GetTypeSize(m_type) == 8) + type = Type::Hex64; + else + type = Type::Hex32; + } + + // Grab memory values as QStrings + Core::CPUThreadGuard guard(m_system); + + for (u32 address = m_address_range.first; address <= m_address_range.second; + address += GetTypeSize(m_type)) + { + m_values.insert(std::pair(address, ValueToString(guard, address, m_type))); + + if (m_dual_view) + m_values_dual_view.insert(std::pair(address, ValueToString(guard, address, type))); + } +} + // May only be called if we have taken on the role of the CPU thread QString MemoryViewWidget::ValueToString(const Core::CPUThreadGuard& guard, u32 address, Type type) { @@ -616,7 +678,7 @@ void MemoryViewWidget::SetAddressSpace(AddressSpace::Type address_space) } m_address_space = address_space; - Update(); + UpdateDispatcher(UpdateType::Addresses); } AddressSpace::Type MemoryViewWidget::GetAddressSpace() const @@ -787,7 +849,7 @@ void MemoryViewWidget::SetDisplay(Type type, int bytes_per_row, int alignment, b else m_alignment = alignment; - CreateTable(); + UpdateDispatcher(UpdateType::Full); } void MemoryViewWidget::SetBPType(BPType type) @@ -803,7 +865,7 @@ void MemoryViewWidget::SetAddress(u32 address) m_address = address; - Update(); + UpdateDispatcher(UpdateType::Addresses); } void MemoryViewWidget::SetBPLoggingEnabled(bool enabled) @@ -926,7 +988,7 @@ void MemoryViewWidget::ScrollbarActionTriggered(int action) // User is currently dragging the scrollbar. // Adjust the memory view by the exact drag difference. m_address += difference * m_bytes_per_row; - Update(); + UpdateDispatcher(UpdateType::Addresses); } else { @@ -941,7 +1003,7 @@ void MemoryViewWidget::ScrollbarActionTriggered(int action) m_address += (difference < 0 ? -1 : 1) * m_bytes_per_row * m_table->rowCount(); } - Update(); + UpdateDispatcher(UpdateType::Addresses); // Manually reset the draggable part of the bar back to the center. m_scrollbar->setSliderPosition(SCROLLBAR_CENTER); } diff --git a/Source/Core/DolphinQt/Debugger/MemoryViewWidget.h b/Source/Core/DolphinQt/Debugger/MemoryViewWidget.h index f8620472bc..584ce4fd42 100644 --- a/Source/Core/DolphinQt/Debugger/MemoryViewWidget.h +++ b/Source/Core/DolphinQt/Debugger/MemoryViewWidget.h @@ -2,6 +2,7 @@ // SPDX-License-Identifier: GPL-2.0-or-later #pragma once +#include #include #include @@ -70,10 +71,20 @@ public: WriteOnly }; + enum class UpdateType + { + Full, + Addresses, + Values, + Auto, + }; + explicit MemoryViewWidget(Core::System& system, QWidget* parent = nullptr); void CreateTable(); + void UpdateDispatcher(UpdateType type = UpdateType::Addresses); void Update(); + void GetValues(); void UpdateFont(const QFont& font); void ToggleBreakpoint(u32 addr, bool row); @@ -97,7 +108,6 @@ private: void OnCopyHex(u32 addr); void UpdateBreakpointTags(); void UpdateColumns(); - void UpdateColumns(const Core::CPUThreadGuard* guard); void ScrollbarActionTriggered(int action); void ScrollbarSliderReleased(); QString ValueToString(const Core::CPUThreadGuard& guard, u32 address, Type type); @@ -111,6 +121,9 @@ private: BPType m_bp_type = BPType::ReadWrite; bool m_do_log = true; u32 m_address = 0x80000000; + std::pair m_address_range; + std::map> m_values; + std::map> m_values_dual_view; u32 m_address_highlight = 0; int m_font_width = 0; int m_font_vspace = 0; @@ -118,6 +131,7 @@ private: int m_alignment = 16; int m_data_columns; bool m_dual_view = false; + QColor m_highlight_color = QColor(120, 255, 255, 100); friend class MemoryViewTable; }; diff --git a/Source/Core/DolphinQt/Debugger/MemoryWidget.cpp b/Source/Core/DolphinQt/Debugger/MemoryWidget.cpp index 847124acda..200219e0b6 100644 --- a/Source/Core/DolphinQt/Debugger/MemoryWidget.cpp +++ b/Source/Core/DolphinQt/Debugger/MemoryWidget.cpp @@ -352,7 +352,7 @@ void MemoryWidget::Update() if (!isVisible()) return; - m_memory_view->Update(); + m_memory_view->UpdateDispatcher(MemoryViewWidget::UpdateType::Addresses); update(); } From 32e135e6a94de9aba7e08ef0b32fac35f4647ab3 Mon Sep 17 00:00:00 2001 From: TryTwo Date: Mon, 24 Jun 2024 14:18:41 -0700 Subject: [PATCH 3/6] MemoryViewWidget: Add OnFrameEnd callback to auto-update memory while a game is running. --- .../DolphinQt/Debugger/MemoryViewWidget.cpp | 23 ++++++++++++++++- .../DolphinQt/Debugger/MemoryViewWidget.h | 3 +++ .../Core/DolphinQt/Debugger/MemoryWidget.cpp | 25 +++++++++++++++++++ Source/Core/DolphinQt/Debugger/MemoryWidget.h | 6 +++++ 4 files changed, 56 insertions(+), 1 deletion(-) diff --git a/Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp b/Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp index abe179e96e..9928632273 100644 --- a/Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp +++ b/Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp @@ -311,7 +311,12 @@ constexpr int GetCharacterCount(MemoryViewWidget::Type type) void MemoryViewWidget::UpdateDispatcher(UpdateType type) { - if (!isVisible()) + std::unique_lock lock(m_updating, std::defer_lock); + + // A full update may change parameters like row count, so make sure it goes through. + if (type == UpdateType::Full) + lock.lock(); + else if (!isVisible() || !lock.try_lock()) return; // Check if table needs to be created. @@ -331,6 +336,10 @@ void MemoryViewWidget::UpdateDispatcher(UpdateType type) GetValues(); UpdateColumns(); break; + case UpdateType::Auto: + // Values were captured on CPU thread while doing a callback. + if (m_values.size() != 0) + UpdateColumns(); default: break; } @@ -522,6 +531,18 @@ void MemoryViewWidget::UpdateColumns() } } +// Always runs on CPU thread from a callback. +void MemoryViewWidget::UpdateOnFrameEnd() +{ + std::unique_lock lock(m_updating, std::try_to_lock); + if (lock) + { + GetValues(); + // Should not directly trigger widget updates on a cpu thread. Signal main thread to do it. + emit AutoUpdate(); + } +} + void MemoryViewWidget::GetValues() { m_values.clear(); diff --git a/Source/Core/DolphinQt/Debugger/MemoryViewWidget.h b/Source/Core/DolphinQt/Debugger/MemoryViewWidget.h index 584ce4fd42..a1e2af09ea 100644 --- a/Source/Core/DolphinQt/Debugger/MemoryViewWidget.h +++ b/Source/Core/DolphinQt/Debugger/MemoryViewWidget.h @@ -84,6 +84,7 @@ public: void CreateTable(); void UpdateDispatcher(UpdateType type = UpdateType::Addresses); void Update(); + void UpdateOnFrameEnd(); void GetValues(); void UpdateFont(const QFont& font); void ToggleBreakpoint(u32 addr, bool row); @@ -99,6 +100,7 @@ public: void SetBPLoggingEnabled(bool enabled); signals: + void AutoUpdate(); void ShowCode(u32 address); void RequestWatch(QString name, u32 address); @@ -131,6 +133,7 @@ private: int m_alignment = 16; int m_data_columns; bool m_dual_view = false; + std::mutex m_updating; QColor m_highlight_color = QColor(120, 255, 255, 100); friend class MemoryViewTable; diff --git a/Source/Core/DolphinQt/Debugger/MemoryWidget.cpp b/Source/Core/DolphinQt/Debugger/MemoryWidget.cpp index 200219e0b6..d15a99d6ba 100644 --- a/Source/Core/DolphinQt/Debugger/MemoryWidget.cpp +++ b/Source/Core/DolphinQt/Debugger/MemoryWidget.cpp @@ -340,13 +340,38 @@ void MemoryWidget::ConnectWidgets() void MemoryWidget::closeEvent(QCloseEvent*) { Settings::Instance().SetMemoryVisible(false); + RemoveAfterFrameEventCallback(); } void MemoryWidget::showEvent(QShowEvent* event) { + RegisterAfterFrameEventCallback(); Update(); } +void MemoryWidget::hideEvent(QHideEvent* event) +{ + RemoveAfterFrameEventCallback(); +} + +void MemoryWidget::RegisterAfterFrameEventCallback() +{ + m_vi_end_field_event = VIEndFieldEvent::Register([this] { AutoUpdateTable(); }, "MemoryWidget"); +} + +void MemoryWidget::RemoveAfterFrameEventCallback() +{ + m_vi_end_field_event.reset(); +} + +void MemoryWidget::AutoUpdateTable() +{ + if (!isVisible()) + return; + + m_memory_view->UpdateOnFrameEnd(); +} + void MemoryWidget::Update() { if (!isVisible()) diff --git a/Source/Core/DolphinQt/Debugger/MemoryWidget.h b/Source/Core/DolphinQt/Debugger/MemoryWidget.h index 75fe40d5f8..de5c71e28c 100644 --- a/Source/Core/DolphinQt/Debugger/MemoryWidget.h +++ b/Source/Core/DolphinQt/Debugger/MemoryWidget.h @@ -9,6 +9,7 @@ #include #include "Common/CommonTypes.h" +#include "VideoCommon/VideoEvents.h" class MemoryViewWidget; class QCheckBox; @@ -76,7 +77,11 @@ private: void FindValue(bool next); void closeEvent(QCloseEvent*) override; + void hideEvent(QHideEvent* event) override; void showEvent(QShowEvent* event) override; + void RegisterAfterFrameEventCallback(); + void RemoveAfterFrameEventCallback(); + void AutoUpdateTable(); Core::System& m_system; @@ -109,4 +114,5 @@ private: QRadioButton* m_bp_read_only; QRadioButton* m_bp_write_only; QCheckBox* m_bp_log_check; + Common::EventHook m_vi_end_field_event; }; From 6d8f40c245e4c17cb1bf3a0967e9e93227b98a14 Mon Sep 17 00:00:00 2001 From: TryTwo Date: Mon, 24 Jun 2024 14:21:31 -0700 Subject: [PATCH 4/6] MemoryViewWidget: Reduce amount of unnecessary update calls. --- Source/Core/DolphinQt/Debugger/MemoryWidget.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/Source/Core/DolphinQt/Debugger/MemoryWidget.cpp b/Source/Core/DolphinQt/Debugger/MemoryWidget.cpp index d15a99d6ba..43b04090e7 100644 --- a/Source/Core/DolphinQt/Debugger/MemoryWidget.cpp +++ b/Source/Core/DolphinQt/Debugger/MemoryWidget.cpp @@ -66,9 +66,6 @@ MemoryWidget::MemoryWidget(Core::System& system, QWidget* parent) connect(&Settings::Instance(), &Settings::DebugModeToggled, this, [this](bool enabled) { setHidden(!enabled || !Settings::Instance().IsMemoryVisible()); }); - connect(&Settings::Instance(), &Settings::EmulationStateChanged, this, &MemoryWidget::Update); - connect(Host::GetInstance(), &Host::UpdateDisasmDialog, this, &MemoryWidget::Update); - LoadSettings(); ConnectWidgets(); From 7b191921348c3689488c5559c1a0f8e6111751e0 Mon Sep 17 00:00:00 2001 From: TryTwo Date: Sun, 19 Jan 2025 23:24:08 -0700 Subject: [PATCH 5/6] MemoryViewWidget: Color recently changed memory when auto updating. --- .../DolphinQt/Debugger/MemoryViewWidget.cpp | 92 ++++++++++++++++++- .../DolphinQt/Debugger/MemoryViewWidget.h | 4 +- 2 files changed, 91 insertions(+), 5 deletions(-) diff --git a/Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp b/Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp index 9928632273..9f5bd628ff 100644 --- a/Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp +++ b/Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp @@ -8,6 +8,7 @@ #include #include +#include #include #include #include @@ -43,6 +44,7 @@ constexpr int MISC_COLUMNS = 2; constexpr auto USER_ROLE_IS_ROW_BREAKPOINT_CELL = Qt::UserRole; constexpr auto USER_ROLE_CELL_ADDRESS = Qt::UserRole + 1; constexpr auto USER_ROLE_VALUE_TYPE = Qt::UserRole + 2; +constexpr auto USER_ROLE_VALID_ADDRESS = Qt::UserRole + 3; // Numbers for the scrollbar. These affect how much big the draggable part of the scrollbar is, how // smooth it scrolls, and how much memory it traverses while dragging. @@ -483,6 +485,10 @@ void MemoryViewWidget::Update() item_address = row_address + c * GetTypeSize(m_type); item->setData(USER_ROLE_CELL_ADDRESS, item_address); + + // Reset highlighting. + item->setBackground(Qt::transparent); + item->setData(USER_ROLE_VALID_ADDRESS, false); } } @@ -521,12 +527,41 @@ void MemoryViewWidget::UpdateColumns() if (cell_address == m_address_highlight) cell_item->setSelected(true); + // Color recently changed items. + QColor bcolor = cell_item->background().color(); + const bool valid = cell_item->data(USER_ROLE_VALID_ADDRESS).toBool(); + // It gets a bit complicated, because invalid addresses becoming valid should not be // colored. if (!new_text.has_value()) + { + cell_item->setBackground(Qt::transparent); + cell_item->setData(USER_ROLE_VALID_ADDRESS, false); cell_item->setText(INVALID_MEMORY); - else + } + else if (!valid) + { + // Wasn't valid on last update, is valid now. + cell_item->setData(USER_ROLE_VALID_ADDRESS, true); cell_item->setText(new_text.value()); + } + else if (bcolor.rgb() != m_highlight_color.rgb() && bcolor != Qt::transparent) + { + // Filter out colors that shouldn't change, such as breakpoints. + cell_item->setText(new_text.value()); + } + else if (cell_item->text() != new_text.value()) + { + // Cell changed, apply highlighting. + cell_item->setBackground(m_highlight_color); + cell_item->setText(new_text.value()); + } + else if (bcolor.alpha() > 0) + { + // Fade out highlighting each frame. + bcolor.setAlpha(bcolor.alpha() - 1); + cell_item->setBackground(bcolor); + } } } } @@ -577,11 +612,12 @@ void MemoryViewWidget::GetValues() } // May only be called if we have taken on the role of the CPU thread -QString MemoryViewWidget::ValueToString(const Core::CPUThreadGuard& guard, u32 address, Type type) +std::optional MemoryViewWidget::ValueToString(const Core::CPUThreadGuard& guard, + u32 address, Type type) { const AddressSpace::Accessors* accessors = AddressSpace::GetAccessors(m_address_space); if (!accessors->IsValidAddress(guard, address)) - return INVALID_MEMORY; + return std::nullopt; switch (type) { @@ -643,7 +679,7 @@ QString MemoryViewWidget::ValueToString(const Core::CPUThreadGuard& guard, u32 a return string; } default: - return INVALID_MEMORY; + return std::nullopt; } } @@ -873,6 +909,54 @@ void MemoryViewWidget::SetDisplay(Type type, int bytes_per_row, int alignment, b UpdateDispatcher(UpdateType::Full); } +void MemoryViewWidget::ToggleHighlights(bool enabled) +{ + // m_highlight_color should hold the current highlight color even when disabled, so it can + // be used if re-enabled. If modifying the enabled alpha, change in .h file as well. + if (enabled) + { + m_highlight_color.setAlpha(100); + } + else + { + // Treated as being interchangable with Qt::transparent. + m_highlight_color.setAlpha(0); + + // Immediately remove highlights when paused. + for (int i = 0; i < m_table->rowCount(); i++) + { + for (int c = 0; c < m_data_columns; c++) + m_table->item(i, c + MISC_COLUMNS)->setBackground(m_highlight_color); + } + } +} + +void MemoryViewWidget::SetHighlightColor() +{ + // Could allow custom alphas to be set, which would change fade-out rate. + QColor color = QColorDialog::getColor(m_highlight_color); + if (!color.isValid()) + return; + + const bool enabled = m_highlight_color.alpha() != 0; + m_highlight_color = color; + m_highlight_color.setAlpha(enabled ? 100 : 0); + if (!enabled) + return; + + // Immediately update colors. Only useful for playing with colors while paused. + for (int i = 0; i < m_table->rowCount(); i++) + { + for (int c = 0; c < m_data_columns; c++) + { + auto* item = m_table->item(i, c + MISC_COLUMNS); + // Get current cell alpha state. + color.setAlpha(item->background().color().alpha()); + m_table->item(i, c + MISC_COLUMNS)->setBackground(color); + } + } +} + void MemoryViewWidget::SetBPType(BPType type) { m_bp_type = type; diff --git a/Source/Core/DolphinQt/Debugger/MemoryViewWidget.h b/Source/Core/DolphinQt/Debugger/MemoryViewWidget.h index a1e2af09ea..29d006f7db 100644 --- a/Source/Core/DolphinQt/Debugger/MemoryViewWidget.h +++ b/Source/Core/DolphinQt/Debugger/MemoryViewWidget.h @@ -93,6 +93,8 @@ public: void SetAddressSpace(AddressSpace::Type address_space); AddressSpace::Type GetAddressSpace() const; void SetDisplay(Type type, int bytes_per_row, int alignment, bool dual_view); + void ToggleHighlights(bool enabled); + void SetHighlightColor(); void SetBPType(BPType type); void SetAddress(u32 address); void SetFocus() const; @@ -112,7 +114,7 @@ private: void UpdateColumns(); void ScrollbarActionTriggered(int action); void ScrollbarSliderReleased(); - QString ValueToString(const Core::CPUThreadGuard& guard, u32 address, Type type); + std::optional ValueToString(const Core::CPUThreadGuard& guard, u32 address, Type type); Core::System& m_system; From 0b8301ff97241c8718a4c6421e28602f89982848 Mon Sep 17 00:00:00 2001 From: TryTwo Date: Sun, 19 Jan 2025 23:24:38 -0700 Subject: [PATCH 6/6] MemoryViewWidget: Add auto update toggle. --- .../Core/DolphinQt/Debugger/MemoryWidget.cpp | 36 ++++++++++++++++--- Source/Core/DolphinQt/Debugger/MemoryWidget.h | 2 ++ 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/Source/Core/DolphinQt/Debugger/MemoryWidget.cpp b/Source/Core/DolphinQt/Debugger/MemoryWidget.cpp index 43b04090e7..4d06e74b10 100644 --- a/Source/Core/DolphinQt/Debugger/MemoryWidget.cpp +++ b/Source/Core/DolphinQt/Debugger/MemoryWidget.cpp @@ -66,6 +66,13 @@ MemoryWidget::MemoryWidget(Core::System& system, QWidget* parent) connect(&Settings::Instance(), &Settings::DebugModeToggled, this, [this](bool enabled) { setHidden(!enabled || !Settings::Instance().IsMemoryVisible()); }); + connect(this, &QDockWidget::visibilityChanged, this, [this](bool visible) { + // Stop auto-update if MemoryView is tabbed out. + if (visible && m_auto_update_enabled) + RegisterAfterFrameEventCallback(); + else + RemoveAfterFrameEventCallback(); + }); LoadSettings(); ConnectWidgets(); @@ -247,12 +254,34 @@ void MemoryWidget::CreateWidgets() // Sidebar top menu QMenuBar* menubar = new QMenuBar(sidebar); menubar->setNativeMenuBar(false); + QMenu* menu_views = new QMenu(tr("&View"), menubar); + menubar->addMenu(menu_views); QMenu* menu_import = new QMenu(tr("&Import"), menubar); menu_import->addAction(tr("&Load file to current address"), this, &MemoryWidget::OnSetValueFromFile); menubar->addMenu(menu_import); + auto* auto_update_action = + menu_views->addAction(tr("Auto update memory values"), this, [this](bool checked) { + m_auto_update_enabled = checked; + if (checked) + RegisterAfterFrameEventCallback(); + else + RemoveAfterFrameEventCallback(); + }); + auto_update_action->setCheckable(true); + auto_update_action->setChecked(true); + + auto* highlight_update_action = + menu_views->addAction(tr("Highlight recently changed values"), this, + [this](bool checked) { m_memory_view->ToggleHighlights(checked); }); + highlight_update_action->setCheckable(true); + highlight_update_action->setChecked(true); + + menu_views->addAction(tr("Highlight color"), this, + [this] { m_memory_view->SetHighlightColor(); }); + QMenu* menu_export = new QMenu(tr("&Export"), menubar); menu_export->addAction(tr("Dump &MRAM"), this, &MemoryWidget::OnDumpMRAM); menu_export->addAction(tr("Dump &ExRAM"), this, &MemoryWidget::OnDumpExRAM); @@ -342,7 +371,9 @@ void MemoryWidget::closeEvent(QCloseEvent*) void MemoryWidget::showEvent(QShowEvent* event) { - RegisterAfterFrameEventCallback(); + if (m_auto_update_enabled) + RegisterAfterFrameEventCallback(); + Update(); } @@ -363,9 +394,6 @@ void MemoryWidget::RemoveAfterFrameEventCallback() void MemoryWidget::AutoUpdateTable() { - if (!isVisible()) - return; - m_memory_view->UpdateOnFrameEnd(); } diff --git a/Source/Core/DolphinQt/Debugger/MemoryWidget.h b/Source/Core/DolphinQt/Debugger/MemoryWidget.h index de5c71e28c..9104e0516c 100644 --- a/Source/Core/DolphinQt/Debugger/MemoryWidget.h +++ b/Source/Core/DolphinQt/Debugger/MemoryWidget.h @@ -115,4 +115,6 @@ private: QRadioButton* m_bp_write_only; QCheckBox* m_bp_log_check; Common::EventHook m_vi_end_field_event; + + bool m_auto_update_enabled = true; };