From cc22f1a5585a732f3c040eb972bf8d1af132c66f Mon Sep 17 00:00:00 2001 From: TryTwo Date: Wed, 6 Apr 2022 22:50:05 -0700 Subject: [PATCH 1/2] MemoryWidget add dual views for two separate column types. Force first column to be Hex32. --- .../DolphinQt/Debugger/MemoryViewWidget.cpp | 160 ++++++++++++------ .../DolphinQt/Debugger/MemoryViewWidget.h | 5 +- .../Core/DolphinQt/Debugger/MemoryWidget.cpp | 11 +- Source/Core/DolphinQt/Debugger/MemoryWidget.h | 1 + 4 files changed, 125 insertions(+), 52 deletions(-) diff --git a/Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp b/Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp index cfa488f6cb..29c62bfe7f 100644 --- a/Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp +++ b/Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp @@ -91,31 +91,32 @@ static int GetTypeSize(MemoryViewWidget::Type type) static int GetCharacterCount(MemoryViewWidget::Type type) { + // Max number of characters +1 for spacing between columns. switch (type) { case MemoryViewWidget::Type::ASCII: - return 1; - case MemoryViewWidget::Type::Hex8: return 2; - case MemoryViewWidget::Type::Unsigned8: + case MemoryViewWidget::Type::Hex8: return 3; + case MemoryViewWidget::Type::Unsigned8: + return 4; case MemoryViewWidget::Type::Hex16: case MemoryViewWidget::Type::Signed8: - return 4; - case MemoryViewWidget::Type::Unsigned16: return 5; - case MemoryViewWidget::Type::Signed16: + case MemoryViewWidget::Type::Unsigned16: return 6; + case MemoryViewWidget::Type::Signed16: + return 7; case MemoryViewWidget::Type::Hex32: - return 8; - case MemoryViewWidget::Type::Float32: return 9; + case MemoryViewWidget::Type::Float32: + return 11; case MemoryViewWidget::Type::Double: case MemoryViewWidget::Type::Unsigned32: case MemoryViewWidget::Type::Signed32: - return 10; + return 12; default: - return 8; + return 10; } } @@ -128,7 +129,10 @@ void MemoryViewWidget::Update() const int data_columns = m_bytes_per_row / GetTypeSize(m_type); - setColumnCount(2 + data_columns); + if (m_dual_view) + setColumnCount(2 + 2 * data_columns + (m_bytes_per_row == 8 ? 1 : 0)); + else + setColumnCount(2 + data_columns); if (rowCount() == 0) setRowCount(1); @@ -179,33 +183,68 @@ void MemoryViewWidget::Update() continue; } + } - bool row_breakpoint = true; + int starting_column = 2; + + if (m_dual_view) + { + // Match left columns to number of right columns. + Type left_type = Type::Hex32; + if (GetTypeSize(m_type) == 1) + left_type = Type::Hex8; + else if (GetTypeSize(m_type) == 2) + left_type = Type::Hex16; + + UpdateColumns(left_type, starting_column); + + int column_count = m_bytes_per_row / GetTypeSize(left_type); + + // Update column width + for (int i = starting_column; i < starting_column + column_count; i++) + setColumnWidth(i, m_font_width * GetCharacterCount(left_type)); + + starting_column += column_count; + } + + UpdateColumns(m_type, starting_column); + UpdateBreakpointTags(); + + setColumnWidth(0, rowHeight(0)); + + for (int i = starting_column; i < columnCount(); i++) + setColumnWidth(i, m_font_width * GetCharacterCount(m_type)); + + viewport()->update(); + update(); +} + +void MemoryViewWidget::UpdateColumns(Type type, int first_column) +{ + const int data_columns = m_bytes_per_row / GetTypeSize(type); + const AddressSpace::Accessors* accessors = AddressSpace::GetAccessors(m_address_space); + + auto text_alignment = Qt::AlignLeft; + if (type == Type::Signed32 || type == Type::Unsigned32 || type == Type::Signed16 || + type == Type::Unsigned16 || type == Type::Signed8 || type == Type::Unsigned8) + text_alignment = Qt::AlignRight; + + for (int i = 0; i < rowCount(); i++) + { + u32 row_address = item(i, 1)->data(Qt::UserRole).toUInt(); + if (!accessors->IsValidAddress(row_address)) + continue; auto update_values = [&](auto value_to_string) { for (int c = 0; c < data_columns; c++) { auto* cell_item = new QTableWidgetItem; cell_item->setFlags(Qt::ItemIsEnabled | Qt::ItemIsSelectable); + cell_item->setTextAlignment(text_alignment); - if (m_type == Type::Signed32 || m_type == Type::Unsigned32 || m_type == Type::Signed16 || - m_type == Type::Unsigned16 || m_type == Type::Signed8 || m_type == Type::Unsigned8) - cell_item->setTextAlignment(Qt::AlignRight); + const u32 cell_address = row_address + c * GetTypeSize(type); - const u32 cell_address = row_address + c * GetTypeSize(m_type); - - // GetMemCheck is more accurate than OverlapsMemcheck, unless standard alginments are - // enforced. - if (m_address_space == AddressSpace::Type::Effective && - PowerPC::memchecks.GetMemCheck(cell_address, GetTypeSize(m_type)) != nullptr) - { - cell_item->setBackground(Qt::red); - } - else - { - row_breakpoint = false; - } - setItem(i, 2 + c, cell_item); + setItem(i, first_column + c, cell_item); if (accessors->IsValidAddress(cell_address)) { @@ -219,7 +258,7 @@ void MemoryViewWidget::Update() } } }; - switch (m_type) + switch (type) { case Type::Hex8: update_values([&accessors](u32 address) { @@ -295,29 +334,50 @@ void MemoryViewWidget::Update() }); break; } + } +} +void MemoryViewWidget::UpdateBreakpointTags() +{ + for (int i = 0; i < rowCount(); i++) + { + bool row_breakpoint = true; + + for (int c = 2; c < columnCount(); c++) + { + // Pull address from cell itself, helpful for dual column view. + auto cell = item(i, c); + u32 address = cell->data(Qt::UserRole).toUInt(); + + if (address == 0) + { + row_breakpoint = false; + continue; + } + + // In dual view the only sizes that dont match up on both left and right views are for + // Double, which uses two columns of hex32. + int type_size = GetTypeSize(m_type); + if (m_dual_view && m_type == Type::Double && c < 4) + type_size = 4; + + if (m_address_space == AddressSpace::Type::Effective && + PowerPC::memchecks.GetMemCheck(address, type_size) != nullptr) + { + cell->setBackground(Qt::red); + } + else + { + row_breakpoint = false; + } + } if (row_breakpoint) { - bp_item->setData(Qt::DecorationRole, Resources::GetScaledThemeIcon("debugger_breakpoint") - .pixmap(QSize(rowHeight(0) - 3, rowHeight(0) - 3))); + item(i, 0)->setData(Qt::DecorationRole, + Resources::GetScaledThemeIcon("debugger_breakpoint") + .pixmap(QSize(rowHeight(0) - 3, rowHeight(0) - 3))); } } - - setColumnWidth(0, rowHeight(0)); - - // Number of characters possible. - int max_length = GetCharacterCount(m_type); - - // Column width is the max number of characters + 1 or 2 for the space between columns. A longer - // length means less columns, so a bigger spacing is fine. - max_length += max_length < 8 ? 1 : 2; - const int width = m_font_width * max_length; - - for (int i = 2; i < columnCount(); i++) - setColumnWidth(i, width); - - viewport()->update(); - update(); } void MemoryViewWidget::SetAddressSpace(AddressSpace::Type address_space) @@ -335,12 +395,12 @@ AddressSpace::Type MemoryViewWidget::GetAddressSpace() const { return m_address_space; } +void MemoryViewWidget::SetDisplay(Type type, int bytes_per_row, int alignment, bool dual_view) -void MemoryViewWidget::SetDisplay(Type type, int bytes_per_row, int alignment) { m_type = type; m_bytes_per_row = bytes_per_row; - + m_dual_view = dual_view; if (alignment == 0) m_alignment = GetTypeSize(type); else diff --git a/Source/Core/DolphinQt/Debugger/MemoryViewWidget.h b/Source/Core/DolphinQt/Debugger/MemoryViewWidget.h index b3dfe6e086..c128c1f183 100644 --- a/Source/Core/DolphinQt/Debugger/MemoryViewWidget.h +++ b/Source/Core/DolphinQt/Debugger/MemoryViewWidget.h @@ -48,7 +48,7 @@ public: void SetAddressSpace(AddressSpace::Type address_space); AddressSpace::Type GetAddressSpace() const; - void SetDisplay(Type type, int bytes_per_row, int alignment); + void SetDisplay(Type type, int bytes_per_row, int alignment, bool dual_view); void SetBPType(BPType type); void SetAddress(u32 address); @@ -70,6 +70,8 @@ private: void OnContextMenu(); void OnCopyAddress(); void OnCopyHex(); + void UpdateBreakpointTags(); + void UpdateColumns(Type type, int first_column); AddressSpace::Type m_address_space{}; Type m_type = Type::Hex8; @@ -82,4 +84,5 @@ private: int m_font_vspace = 0; int m_bytes_per_row = 16; int m_alignment = 16; + bool m_dual_view = false; }; diff --git a/Source/Core/DolphinQt/Debugger/MemoryWidget.cpp b/Source/Core/DolphinQt/Debugger/MemoryWidget.cpp index 0576420883..3bff61f66e 100644 --- a/Source/Core/DolphinQt/Debugger/MemoryWidget.cpp +++ b/Source/Core/DolphinQt/Debugger/MemoryWidget.cpp @@ -215,9 +215,12 @@ void MemoryWidget::CreateWidgets() m_row_length_combo->addItem(tr("8 Bytes"), 8); m_row_length_combo->addItem(tr("16 Bytes"), 16); + m_dual_check = new QCheckBox(tr("Dual View")); + displaytype_layout->addWidget(m_display_combo); displaytype_layout->addWidget(m_align_combo); displaytype_layout->addWidget(m_row_length_combo); + displaytype_layout->addWidget(m_dual_check); // MBP options auto* bp_group = new QGroupBox(tr("Memory breakpoint options")); @@ -318,6 +321,8 @@ void MemoryWidget::ConnectWidgets() &MemoryWidget::OnDisplayChanged); } + connect(m_dual_check, &QCheckBox::toggled, this, &MemoryWidget::OnDisplayChanged); + for (auto* radio : {m_bp_read_write, m_bp_read_only, m_bp_write_only}) connect(radio, &QRadioButton::toggled, this, &MemoryWidget::OnBPTypeChanged); @@ -431,6 +436,10 @@ void MemoryWidget::OnDisplayChanged() const auto type = static_cast(m_display_combo->currentData().toInt()); int bytes_per_row = m_row_length_combo->currentData().toInt(); int alignment; + bool dual_view = m_dual_check->isChecked(); + + if (dual_view) + bytes_per_row = 4; if (type == MemoryViewWidget::Type::Double && bytes_per_row == 4) bytes_per_row = 8; @@ -443,7 +452,7 @@ void MemoryWidget::OnDisplayChanged() else alignment = m_align_combo->currentData().toInt(); - m_memory_view->SetDisplay(type, bytes_per_row, alignment); + m_memory_view->SetDisplay(type, bytes_per_row, alignment, dual_view); SaveSettings(); } diff --git a/Source/Core/DolphinQt/Debugger/MemoryWidget.h b/Source/Core/DolphinQt/Debugger/MemoryWidget.h index b3a8d75186..ff0070ddb5 100644 --- a/Source/Core/DolphinQt/Debugger/MemoryWidget.h +++ b/Source/Core/DolphinQt/Debugger/MemoryWidget.h @@ -98,6 +98,7 @@ private: QComboBox* m_display_combo; QComboBox* m_align_combo; QComboBox* m_row_length_combo; + QCheckBox* m_dual_check; QPushButton* m_set_value; QPushButton* m_from_file; QPushButton* m_dump_mram; From a7111e3910a696f93774717cb3be0acd1549bd2d Mon Sep 17 00:00:00 2001 From: TryTwo Date: Sun, 17 Apr 2022 00:47:05 -0700 Subject: [PATCH 2/2] Dual View any size. --- .../DolphinQt/Debugger/MemoryViewWidget.cpp | 102 +++++++++++------- .../DolphinQt/Debugger/MemoryViewWidget.h | 3 +- .../Core/DolphinQt/Debugger/MemoryWidget.cpp | 6 +- 3 files changed, 70 insertions(+), 41 deletions(-) diff --git a/Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp b/Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp index 29c62bfe7f..6ecce440f2 100644 --- a/Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp +++ b/Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp @@ -64,7 +64,7 @@ void MemoryViewWidget::UpdateFont() Update(); } -static int GetTypeSize(MemoryViewWidget::Type type) +constexpr int GetTypeSize(MemoryViewWidget::Type type) { switch (type) { @@ -83,38 +83,41 @@ static int GetTypeSize(MemoryViewWidget::Type type) case MemoryViewWidget::Type::Float32: return 4; case MemoryViewWidget::Type::Double: + case MemoryViewWidget::Type::Hex64: return 8; default: return 1; } } -static int GetCharacterCount(MemoryViewWidget::Type type) +constexpr int GetCharacterCount(MemoryViewWidget::Type type) { // Max number of characters +1 for spacing between columns. switch (type) { - case MemoryViewWidget::Type::ASCII: + case MemoryViewWidget::Type::ASCII: // A return 2; - case MemoryViewWidget::Type::Hex8: + case MemoryViewWidget::Type::Hex8: // Byte = FF return 3; - case MemoryViewWidget::Type::Unsigned8: + case MemoryViewWidget::Type::Unsigned8: // UCHAR_MAX = 255 return 4; - case MemoryViewWidget::Type::Hex16: - case MemoryViewWidget::Type::Signed8: + case MemoryViewWidget::Type::Hex16: // 2 Bytes = FFFF + case MemoryViewWidget::Type::Signed8: // CHAR_MIN = -128 return 5; - case MemoryViewWidget::Type::Unsigned16: + case MemoryViewWidget::Type::Unsigned16: // USHORT_MAX = 65535 return 6; - case MemoryViewWidget::Type::Signed16: + case MemoryViewWidget::Type::Signed16: // SHORT_MIN = -32768 return 7; - case MemoryViewWidget::Type::Hex32: + case MemoryViewWidget::Type::Hex32: // 4 Bytes = FFFFFFFF return 9; - case MemoryViewWidget::Type::Float32: + case MemoryViewWidget::Type::Float32: // Rounded and Negative FLT_MAX = -3.403e+38 + case MemoryViewWidget::Type::Unsigned32: // UINT_MAX = 4294967295 return 11; - case MemoryViewWidget::Type::Double: - case MemoryViewWidget::Type::Unsigned32: - case MemoryViewWidget::Type::Signed32: + case MemoryViewWidget::Type::Double: // Rounded and Negative DBL_MAX = -1.798e+308 + case MemoryViewWidget::Type::Signed32: // INT_MIN = -2147483648 return 12; + case MemoryViewWidget::Type::Hex64: // For dual_view + Double. 8 Bytes = FFFFFFFFFFFFFFFF + return 17; default: return 10; } @@ -130,7 +133,7 @@ void MemoryViewWidget::Update() const int data_columns = m_bytes_per_row / GetTypeSize(m_type); if (m_dual_view) - setColumnCount(2 + 2 * data_columns + (m_bytes_per_row == 8 ? 1 : 0)); + setColumnCount(2 + 2 * data_columns); else setColumnCount(2 + data_columns); @@ -195,15 +198,21 @@ void MemoryViewWidget::Update() left_type = Type::Hex8; else if (GetTypeSize(m_type) == 2) left_type = Type::Hex16; + else if (GetTypeSize(m_type) == 8) + left_type = Type::Hex64; UpdateColumns(left_type, starting_column); - int column_count = m_bytes_per_row / GetTypeSize(left_type); + const int column_count = m_bytes_per_row / GetTypeSize(left_type); // Update column width - for (int i = starting_column; i < starting_column + column_count; i++) + for (int i = starting_column; i < starting_column + column_count - 1; i++) setColumnWidth(i, m_font_width * GetCharacterCount(left_type)); + // Extra spacing between dual views. + setColumnWidth(starting_column + column_count - 1, + m_font_width * (GetCharacterCount(left_type) + 2)); + starting_column += column_count; } @@ -212,7 +221,7 @@ void MemoryViewWidget::Update() setColumnWidth(0, rowHeight(0)); - for (int i = starting_column; i < columnCount(); i++) + for (int i = starting_column; i <= columnCount(); i++) setColumnWidth(i, m_font_width * GetCharacterCount(m_type)); viewport()->update(); @@ -221,13 +230,18 @@ void MemoryViewWidget::Update() void MemoryViewWidget::UpdateColumns(Type type, int first_column) { + if (Core::GetState() != Core::State::Paused) + return; + const int data_columns = m_bytes_per_row / GetTypeSize(type); const AddressSpace::Accessors* accessors = AddressSpace::GetAccessors(m_address_space); auto text_alignment = Qt::AlignLeft; if (type == Type::Signed32 || type == Type::Unsigned32 || type == Type::Signed16 || type == Type::Unsigned16 || type == Type::Signed8 || type == Type::Unsigned8) + { text_alignment = Qt::AlignRight; + } for (int i = 0; i < rowCount(); i++) { @@ -285,6 +299,12 @@ void MemoryViewWidget::UpdateColumns(Type type, int first_column) return QStringLiteral("%1").arg(value, 8, 16, QLatin1Char('0')); }); break; + case Type::Hex64: + update_values([&accessors](u32 address) { + const u64 value = accessors->ReadU64(address); + return QStringLiteral("%1").arg(value, 16, 16, QLatin1Char('0')); + }); + break; case Type::Unsigned8: update_values( [&accessors](u32 address) { return QString::number(accessors->ReadU8(address)); }); @@ -339,9 +359,12 @@ void MemoryViewWidget::UpdateColumns(Type type, int first_column) void MemoryViewWidget::UpdateBreakpointTags() { + if (Core::GetState() != Core::State::Paused) + return; + for (int i = 0; i < rowCount(); i++) { - bool row_breakpoint = true; + bool row_breakpoint = false; for (int c = 2; c < columnCount(); c++) { @@ -356,21 +379,15 @@ void MemoryViewWidget::UpdateBreakpointTags() } // In dual view the only sizes that dont match up on both left and right views are for - // Double, which uses two columns of hex32. - int type_size = GetTypeSize(m_type); - if (m_dual_view && m_type == Type::Double && c < 4) - type_size = 4; - + // Double, which uses two or four columns of hex32. if (m_address_space == AddressSpace::Type::Effective && - PowerPC::memchecks.GetMemCheck(address, type_size) != nullptr) + PowerPC::memchecks.GetMemCheck(address, GetTypeSize(m_type)) != nullptr) { + row_breakpoint = true; cell->setBackground(Qt::red); } - else - { - row_breakpoint = false; - } } + if (row_breakpoint) { item(i, 0)->setData(Qt::DecorationRole, @@ -467,16 +484,28 @@ u32 MemoryViewWidget::GetContextAddress() const void MemoryViewWidget::ToggleRowBreakpoint(bool row) { - TMemCheck check; + if (m_address_space != AddressSpace::Type::Effective) + return; const u32 addr = row ? m_base_address : GetContextAddress(); - const auto length = row ? m_bytes_per_row : GetTypeSize(m_type); + const auto length = GetTypeSize(m_type); + const int breaks = row ? (m_bytes_per_row / length) : 1; + bool overlap = false; - if (m_address_space == AddressSpace::Type::Effective) + // Row breakpoint should either remove any breakpoint left on the row, or activate all + // breakpoints. + if (row && PowerPC::memchecks.OverlapsMemcheck(addr, m_bytes_per_row)) + overlap = true; + + for (int i = 0; i < breaks; i++) { - if (!PowerPC::memchecks.OverlapsMemcheck(addr, length)) + u32 address = addr + length * i; + TMemCheck* check_ptr = PowerPC::memchecks.GetMemCheck(address, length); + + if (check_ptr == nullptr && !overlap) { - check.start_address = addr; + TMemCheck check; + check.start_address = address; check.end_address = check.start_address + length - 1; check.is_ranged = length > 0; check.is_break_on_read = (m_bp_type == BPType::ReadOnly || m_bp_type == BPType::ReadWrite); @@ -486,9 +515,10 @@ void MemoryViewWidget::ToggleRowBreakpoint(bool row) PowerPC::memchecks.Add(check); } - else + else if (check_ptr != nullptr) { - PowerPC::memchecks.Remove(addr); + // Using the pointer fixes misaligned breakpoints (0x11 breakpoint in 0x10 aligned view). + PowerPC::memchecks.Remove(check_ptr->start_address); } } diff --git a/Source/Core/DolphinQt/Debugger/MemoryViewWidget.h b/Source/Core/DolphinQt/Debugger/MemoryViewWidget.h index c128c1f183..0faaa23b49 100644 --- a/Source/Core/DolphinQt/Debugger/MemoryViewWidget.h +++ b/Source/Core/DolphinQt/Debugger/MemoryViewWidget.h @@ -21,6 +21,7 @@ public: Hex8 = 1, Hex16, Hex32, + Hex64, Unsigned8, Unsigned16, Unsigned32, @@ -74,7 +75,7 @@ private: void UpdateColumns(Type type, int first_column); AddressSpace::Type m_address_space{}; - Type m_type = Type::Hex8; + Type m_type = Type::Hex32; BPType m_bp_type = BPType::ReadWrite; bool m_do_log = true; u32 m_context_address; diff --git a/Source/Core/DolphinQt/Debugger/MemoryWidget.cpp b/Source/Core/DolphinQt/Debugger/MemoryWidget.cpp index 3bff61f66e..8ae1d9aefd 100644 --- a/Source/Core/DolphinQt/Debugger/MemoryWidget.cpp +++ b/Source/Core/DolphinQt/Debugger/MemoryWidget.cpp @@ -215,6 +215,7 @@ void MemoryWidget::CreateWidgets() m_row_length_combo->addItem(tr("8 Bytes"), 8); m_row_length_combo->addItem(tr("16 Bytes"), 16); + m_row_length_combo->setCurrentIndex(2); m_dual_check = new QCheckBox(tr("Dual View")); displaytype_layout->addWidget(m_display_combo); @@ -438,14 +439,11 @@ void MemoryWidget::OnDisplayChanged() int alignment; bool dual_view = m_dual_check->isChecked(); - if (dual_view) - bytes_per_row = 4; - if (type == MemoryViewWidget::Type::Double && bytes_per_row == 4) bytes_per_row = 8; // Alignment: First (fixed) option equals bytes per row. 'currentData' is correct for other - // options. Type-based must be calculated in memoryviewwidget and is left at 0. No alignment is + // options. Type-based must be calculated in memoryviewwidget and is left at 0. "No alignment" is // equivalent to a value of 1. if (m_align_combo->currentIndex() == 0) alignment = bytes_per_row;