Merge pull request #10612 from AdmiralCurtiss/memory-widget-refactoring-2

Qt/MemoryViewWidget: Replace the click-to-center behavior with a scrollbar.

Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp

@@ -5,10 +5,12 @@
 
 #include <QApplication>
 #include <QClipboard>
+#include <QHBoxLayout>
 #include <QHeaderView>
 #include <QMenu>
 #include <QMouseEvent>
 #include <QScrollBar>
+#include <QTableWidget>
 #include <QtGlobal>
 
 #include <cmath>
@@ -32,20 +34,120 @@ constexpr auto USER_ROLE_IS_ROW_BREAKPOINT_CELL = Qt::UserRole;
 constexpr auto USER_ROLE_CELL_ADDRESS = Qt::UserRole + 1;
 constexpr auto USER_ROLE_HAS_VALUE = Qt::UserRole + 2;
 
-MemoryViewWidget::MemoryViewWidget(QWidget* parent) : QTableWidget(parent)
+// 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.
-  horizontalHeader()->hide();
+constexpr int SCROLLBAR_MINIMUM = 0;
-  verticalHeader()->hide();
+constexpr int SCROLLBAR_PAGESTEP = 250;
-  setVerticalScrollBarPolicy(Qt::ScrollBarAlwaysOff);
+constexpr int SCROLLBAR_MAXIMUM = 20000;
-  setShowGrid(false);
+constexpr int SCROLLBAR_CENTER = SCROLLBAR_MAXIMUM / 2;
 
-  setContextMenuPolicy(Qt::CustomContextMenu);
+class MemoryViewTable final : public QTableWidget
+{
+public:
+  explicit MemoryViewTable(MemoryViewWidget* parent) : QTableWidget(parent), m_view(parent)
+  {
+    horizontalHeader()->hide();
+    verticalHeader()->hide();
+    setVerticalScrollBarPolicy(Qt::ScrollBarAlwaysOff);
+    setShowGrid(false);
+    setContextMenuPolicy(Qt::CustomContextMenu);
+    setSelectionMode(SingleSelection);
+
+    connect(this, &MemoryViewTable::customContextMenuRequested, m_view,
+            &MemoryViewWidget::OnContextMenu);
+  }
+
+  void resizeEvent(QResizeEvent*) override { m_view->Update(); }
+
+  void keyPressEvent(QKeyEvent* event) override
+  {
+    switch (event->key())
+    {
+    case Qt::Key_Up:
+      m_view->m_address -= m_view->m_bytes_per_row;
+      m_view->Update();
+      return;
+    case Qt::Key_Down:
+      m_view->m_address += m_view->m_bytes_per_row;
+      m_view->Update();
+      return;
+    case Qt::Key_PageUp:
+      m_view->m_address -= this->rowCount() * m_view->m_bytes_per_row;
+      m_view->Update();
+      return;
+    case Qt::Key_PageDown:
+      m_view->m_address += this->rowCount() * m_view->m_bytes_per_row;
+      m_view->Update();
+      return;
+    default:
+      QWidget::keyPressEvent(event);
+      break;
+    }
+  }
+
+  void wheelEvent(QWheelEvent* event) override
+  {
+    auto delta =
+        -static_cast<int>(std::round((event->angleDelta().y() / (SCROLL_FRACTION_DEGREES * 8))));
+
+    if (delta == 0)
+      return;
+
+    m_view->m_address += delta * m_view->m_bytes_per_row;
+    m_view->Update();
+  }
+
+  void mousePressEvent(QMouseEvent* event) override
+  {
+    if (event->button() != Qt::LeftButton)
+      return;
+
+    auto* item = this->itemAt(event->pos());
+    if (!item)
+      return;
+
+    if (item->data(USER_ROLE_IS_ROW_BREAKPOINT_CELL).toBool())
+    {
+      const u32 address = item->data(USER_ROLE_CELL_ADDRESS).toUInt();
+      m_view->ToggleBreakpoint(address, true);
+      m_view->Update();
+    }
+    else
+    {
+      QTableWidget::mousePressEvent(event);
+    }
+  }
+
+private:
+  MemoryViewWidget* m_view;
+};
+
+MemoryViewWidget::MemoryViewWidget(QWidget* parent) : QWidget(parent)
+{
+  auto* layout = new QHBoxLayout();
+  layout->setContentsMargins(0, 0, 0, 0);
+
+  m_table = new MemoryViewTable(this);
+  layout->addWidget(m_table);
+
+  // Since the Memory View is infinitely long -- it wraps around -- we can't use a normal scroll
+  // bar, so this initializes a custom one that is always centered but otherwise still behaves more
+  // or less like a regular scrollbar.
+  m_scrollbar = new QScrollBar(this);
+  m_scrollbar->setRange(SCROLLBAR_MINIMUM, SCROLLBAR_MAXIMUM);
+  m_scrollbar->setPageStep(SCROLLBAR_PAGESTEP);
+  m_scrollbar->setValue(SCROLLBAR_CENTER);
+  connect(m_scrollbar, &QScrollBar::actionTriggered, this,
+          &MemoryViewWidget::ScrollbarActionTriggered);
+  connect(m_scrollbar, &QScrollBar::sliderReleased, this,
+          &MemoryViewWidget::ScrollbarSliderReleased);
+  layout->addWidget(m_scrollbar);
+
+  this->setLayout(layout);
 
   connect(&Settings::Instance(), &Settings::DebugFontChanged, this, &MemoryViewWidget::UpdateFont);
   connect(&Settings::Instance(), &Settings::EmulationStateChanged, this, [this] { Update(); });
   connect(Host::GetInstance(), &Host::UpdateDisasmDialog, this, &MemoryViewWidget::Update);
-  connect(this, &MemoryViewWidget::customContextMenuRequested, this,
-          &MemoryViewWidget::OnContextMenu);
   connect(&Settings::Instance(), &Settings::ThemeChanged, this, &MemoryViewWidget::Update);
 
   // Also calls update.
@@ -64,7 +166,7 @@ void MemoryViewWidget::UpdateFont()
 #else
   m_font_width = fm.width(QLatin1Char('0'));
 #endif
-  setFont(Settings::Instance().GetDebugFont());
+  m_table->setFont(Settings::Instance().GetDebugFont());
   Update();
 }
 
@@ -129,7 +231,7 @@ constexpr int GetCharacterCount(MemoryViewWidget::Type type)
 
 void MemoryViewWidget::Update()
 {
-  clearSelection();
+  m_table->clearSelection();
 
   u32 address = m_address;
   address = Common::AlignDown(address, m_alignment);
@@ -137,28 +239,29 @@ void MemoryViewWidget::Update()
   const int data_columns = m_bytes_per_row / GetTypeSize(m_type);
 
   if (m_dual_view)
-    setColumnCount(2 + 2 * data_columns);
+    m_table->setColumnCount(2 + 2 * data_columns);
   else
-    setColumnCount(2 + data_columns);
+    m_table->setColumnCount(2 + data_columns);
 
-  if (rowCount() == 0)
+  if (m_table->rowCount() == 0)
-    setRowCount(1);
+    m_table->setRowCount(1);
 
   // This sets all row heights and determines horizontal ascii spacing.
-  verticalHeader()->setDefaultSectionSize(m_font_vspace - 1);
+  m_table->verticalHeader()->setDefaultSectionSize(m_font_vspace - 1);
-  verticalHeader()->setMinimumSectionSize(m_font_vspace - 1);
+  m_table->verticalHeader()->setMinimumSectionSize(m_font_vspace - 1);
-  horizontalHeader()->setMinimumSectionSize(m_font_width * 2);
+  m_table->horizontalHeader()->setMinimumSectionSize(m_font_width * 2);
 
   const AddressSpace::Accessors* accessors = AddressSpace::GetAccessors(m_address_space);
 
   // Calculate (roughly) how many rows will fit in our table
-  int rows = std::round((height() / static_cast<float>(rowHeight(0))) - 0.25);
+  const int rows =
+      std::round((m_table->height() / static_cast<float>(m_table->rowHeight(0))) - 0.25);
 
-  setRowCount(rows);
+  m_table->setRowCount(rows);
 
   for (int i = 0; i < rows; i++)
   {
-    u32 row_address = address - ((rowCount() / 2) * m_bytes_per_row) + i * m_bytes_per_row;
+    u32 row_address = address - ((m_table->rowCount() / 2) * m_bytes_per_row) + i * m_bytes_per_row;
 
     auto* bp_item = new QTableWidgetItem;
     bp_item->setFlags(Qt::ItemIsEnabled);
@@ -166,7 +269,7 @@ void MemoryViewWidget::Update()
     bp_item->setData(USER_ROLE_CELL_ADDRESS, row_address);
     bp_item->setData(USER_ROLE_HAS_VALUE, false);
 
-    setItem(i, 0, bp_item);
+    m_table->setItem(i, 0, bp_item);
 
     auto* row_item =
         new QTableWidgetItem(QStringLiteral("%1").arg(row_address, 8, 16, QLatin1Char('0')));
@@ -176,22 +279,22 @@ void MemoryViewWidget::Update()
     row_item->setData(USER_ROLE_CELL_ADDRESS, row_address);
     row_item->setData(USER_ROLE_HAS_VALUE, false);
 
-    setItem(i, 1, row_item);
+    m_table->setItem(i, 1, row_item);
 
     if (row_address == address)
       row_item->setSelected(true);
 
     if (Core::GetState() != Core::State::Paused || !accessors->IsValidAddress(row_address))
     {
-      for (int c = 2; c < columnCount(); c++)
+      for (int c = 2; c < m_table->columnCount(); c++)
       {
         auto* item = new QTableWidgetItem(QStringLiteral("-"));
-        item->setFlags(Qt::ItemIsEnabled);
+        item->setFlags(Qt::ItemIsEnabled | Qt::ItemIsSelectable);
         item->setData(USER_ROLE_IS_ROW_BREAKPOINT_CELL, false);
         item->setData(USER_ROLE_CELL_ADDRESS, row_address);
         item->setData(USER_ROLE_HAS_VALUE, false);
 
-        setItem(i, c, item);
+        m_table->setItem(i, c, item);
       }
 
       continue;
@@ -217,11 +320,11 @@ void MemoryViewWidget::Update()
 
     // Update column width
     for (int i = starting_column; i < starting_column + column_count - 1; i++)
-      setColumnWidth(i, m_font_width * GetCharacterCount(left_type));
+      m_table->setColumnWidth(i, m_font_width * GetCharacterCount(left_type));
 
     // Extra spacing between dual views.
-    setColumnWidth(starting_column + column_count - 1,
+    m_table->setColumnWidth(starting_column + column_count - 1,
-                   m_font_width * (GetCharacterCount(left_type) + 2));
+                            m_font_width * (GetCharacterCount(left_type) + 2));
 
     starting_column += column_count;
   }
@@ -229,12 +332,13 @@ void MemoryViewWidget::Update()
   UpdateColumns(m_type, starting_column);
   UpdateBreakpointTags();
 
-  setColumnWidth(0, rowHeight(0));
+  m_table->setColumnWidth(0, m_table->rowHeight(0));
 
-  for (int i = starting_column; i <= columnCount(); i++)
+  for (int i = starting_column; i <= m_table->columnCount(); i++)
-    setColumnWidth(i, m_font_width * GetCharacterCount(m_type));
+    m_table->setColumnWidth(i, m_font_width * GetCharacterCount(m_type));
 
-  viewport()->update();
+  m_table->viewport()->update();
+  m_table->update();
   update();
 }
 
@@ -253,9 +357,9 @@ void MemoryViewWidget::UpdateColumns(Type type, int first_column)
     text_alignment = Qt::AlignRight;
   }
 
-  for (int i = 0; i < rowCount(); i++)
+  for (int i = 0; i < m_table->rowCount(); i++)
   {
-    u32 row_address = item(i, 1)->data(USER_ROLE_CELL_ADDRESS).toUInt();
+    u32 row_address = m_table->item(i, 1)->data(USER_ROLE_CELL_ADDRESS).toUInt();
     if (!accessors->IsValidAddress(row_address))
       continue;
 
@@ -268,7 +372,7 @@ void MemoryViewWidget::UpdateColumns(Type type, int first_column)
 
         const u32 cell_address = row_address + c * GetTypeSize(type);
 
-        setItem(i, first_column + c, cell_item);
+        m_table->setItem(i, first_column + c, cell_item);
 
         if (accessors->IsValidAddress(cell_address))
         {
@@ -279,7 +383,6 @@ void MemoryViewWidget::UpdateColumns(Type type, int first_column)
         }
         else
         {
-          cell_item->setFlags({});
           cell_item->setText(QStringLiteral("-"));
           cell_item->setData(USER_ROLE_IS_ROW_BREAKPOINT_CELL, false);
           cell_item->setData(USER_ROLE_CELL_ADDRESS, cell_address);
@@ -377,14 +480,14 @@ void MemoryViewWidget::UpdateBreakpointTags()
   if (Core::GetState() != Core::State::Paused)
     return;
 
-  for (int i = 0; i < rowCount(); i++)
+  for (int i = 0; i < m_table->rowCount(); i++)
   {
     bool row_breakpoint = false;
 
-    for (int c = 2; c < columnCount(); c++)
+    for (int c = 2; c < m_table->columnCount(); c++)
     {
       // Pull address from cell itself, helpful for dual column view.
-      auto cell = item(i, c);
+      auto cell = m_table->item(i, c);
       u32 address = cell->data(USER_ROLE_CELL_ADDRESS).toUInt();
 
       if (address == 0)
@@ -405,9 +508,10 @@ void MemoryViewWidget::UpdateBreakpointTags()
 
     if (row_breakpoint)
     {
-      item(i, 0)->setData(Qt::DecorationRole,
+      m_table->item(i, 0)->setData(
-                          Resources::GetScaledThemeIcon("debugger_breakpoint")
+          Qt::DecorationRole,
-                              .pixmap(QSize(rowHeight(0) - 3, rowHeight(0) - 3)));
+          Resources::GetScaledThemeIcon("debugger_breakpoint")
+              .pixmap(QSize(m_table->rowHeight(0) - 3, m_table->rowHeight(0) - 3)));
     }
   }
 }
@@ -427,8 +531,8 @@ 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, bool dual_view)
 {
   m_type = type;
   m_bytes_per_row = bytes_per_row;
@@ -461,37 +565,6 @@ void MemoryViewWidget::SetBPLoggingEnabled(bool enabled)
   m_do_log = enabled;
 }
 
-void MemoryViewWidget::resizeEvent(QResizeEvent*)
-{
-  Update();
-}
-
-void MemoryViewWidget::keyPressEvent(QKeyEvent* event)
-{
-  switch (event->key())
-  {
-  case Qt::Key_Up:
-    m_address -= 16;
-    Update();
-    return;
-  case Qt::Key_Down:
-    m_address += 16;
-    Update();
-    return;
-  case Qt::Key_PageUp:
-    m_address -= rowCount() * 16;
-    Update();
-    return;
-  case Qt::Key_PageDown:
-    m_address += rowCount() * 16;
-    Update();
-    return;
-  default:
-    QWidget::keyPressEvent(event);
-    break;
-  }
-}
-
 void MemoryViewWidget::ToggleBreakpoint(u32 addr, bool row)
 {
   if (m_address_space != AddressSpace::Type::Effective)
@@ -535,35 +608,6 @@ void MemoryViewWidget::ToggleBreakpoint(u32 addr, bool row)
   Update();
 }
 
-void MemoryViewWidget::wheelEvent(QWheelEvent* event)
-{
-  auto delta =
-      -static_cast<int>(std::round((event->angleDelta().y() / (SCROLL_FRACTION_DEGREES * 8))));
-
-  if (delta == 0)
-    return;
-
-  m_address += delta * 16;
-  Update();
-}
-
-void MemoryViewWidget::mousePressEvent(QMouseEvent* event)
-{
-  if (event->button() != Qt::LeftButton)
-    return;
-
-  auto* item = itemAt(event->pos());
-  if (!item)
-    return;
-
-  const u32 address = item->data(USER_ROLE_CELL_ADDRESS).toUInt();
-  if (item->data(USER_ROLE_IS_ROW_BREAKPOINT_CELL).toBool())
-    ToggleBreakpoint(address, true);
-  else
-    SetAddress(address);
-  Update();
-}
-
 void MemoryViewWidget::OnCopyAddress(u32 addr)
 {
   QApplication::clipboard()->setText(QStringLiteral("%1").arg(addr, 8, 16, QLatin1Char('0')));
@@ -582,7 +626,7 @@ void MemoryViewWidget::OnCopyHex(u32 addr)
 
 void MemoryViewWidget::OnContextMenu(const QPoint& pos)
 {
-  auto* item_selected = itemAt(pos);
+  auto* item_selected = m_table->itemAt(pos);
 
   // We don't have a meaningful context menu to show for when the user right-clicks either free
   // space in the table or the row breakpoint cell.
@@ -604,7 +648,7 @@ void MemoryViewWidget::OnContextMenu(const QPoint& pos)
 
   auto* copy_value = menu->addAction(tr("Copy Value"), this, [this, &pos] {
     // Re-fetch the item in case the underlying table has refreshed since the menu was opened.
-    auto* item = itemAt(pos);
+    auto* item = m_table->itemAt(pos);
     if (item && item->data(USER_ROLE_HAS_VALUE).toBool())
       QApplication::clipboard()->setText(item->text());
   });
@@ -625,3 +669,39 @@ void MemoryViewWidget::OnContextMenu(const QPoint& pos)
 
   menu->exec(QCursor::pos());
 }
+
+void MemoryViewWidget::ScrollbarActionTriggered(int action)
+{
+  const int difference = m_scrollbar->sliderPosition() - m_scrollbar->value();
+  if (difference == 0)
+    return;
+
+  if (m_scrollbar->isSliderDown())
+  {
+    // User is currently dragging the scrollbar.
+    // Adjust the memory view by the exact drag difference.
+    SetAddress(m_address + difference * m_bytes_per_row);
+  }
+  else
+  {
+    if (std::abs(difference) == 1)
+    {
+      // User clicked the arrows at the top or bottom, go up/down one row.
+      SetAddress(m_address + difference * m_bytes_per_row);
+    }
+    else
+    {
+      // User clicked the free part of the scrollbar, go up/down one page.
+      SetAddress(m_address + (difference < 0 ? -1 : 1) * m_bytes_per_row * m_table->rowCount());
+    }
+
+    // Manually reset the draggable part of the bar back to the center.
+    m_scrollbar->setSliderPosition(SCROLLBAR_CENTER);
+  }
+}
+
+void MemoryViewWidget::ScrollbarSliderReleased()
+{
+  // Reset the draggable part of the bar back to the center.
+  m_scrollbar->setValue(SCROLLBAR_CENTER);
+}

Source/Core/DolphinQt/Debugger/MemoryViewWidget.h

@@ -3,18 +3,21 @@
 
 #pragma once
 
-#include <QTableWidget>
+#include <QWidget>
 
 #include "Common/CommonTypes.h"
 
 class QPoint;
+class QScrollBar;
 
 namespace AddressSpace
 {
 enum class Type;
 }
 
-class MemoryViewWidget : public QTableWidget
+class MemoryViewTable;
+
+class MemoryViewWidget final : public QWidget
 {
   Q_OBJECT
 public:
@@ -56,11 +59,6 @@ public:
 
   void SetBPLoggingEnabled(bool enabled);
 
-  void resizeEvent(QResizeEvent*) override;
-  void keyPressEvent(QKeyEvent* event) override;
-  void mousePressEvent(QMouseEvent* event) override;
-  void wheelEvent(QWheelEvent* event) override;
-
 signals:
   void BreakpointsChanged();
   void ShowCode(u32 address);
@@ -72,7 +70,11 @@ private:
   void OnCopyHex(u32 addr);
   void UpdateBreakpointTags();
   void UpdateColumns(Type type, int first_column);
+  void ScrollbarActionTriggered(int action);
+  void ScrollbarSliderReleased();
 
+  MemoryViewTable* m_table;
+  QScrollBar* m_scrollbar;
   AddressSpace::Type m_address_space{};
   Type m_type = Type::Hex32;
   BPType m_bp_type = BPType::ReadWrite;
@@ -83,4 +85,6 @@ private:
   int m_bytes_per_row = 16;
   int m_alignment = 16;
   bool m_dual_view = false;
+
+  friend class MemoryViewTable;
 };