Merge pull request #11261 from TryTwo/PR_MemoryView_Auto_Update

MemoryView auto-update while running and color recently changed cells.

1 files changed, 258 insertions, 81 deletions

diff --git a/Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp b/Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp
index fcf57c8acf..9f5bd628ff 100644
--- a/Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp
+++ b/Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp
@@ -8,12 +8,14 @@
 
 #include <QApplication>
 #include <QClipboard>
+#include <QColorDialog>
 #include <QHBoxLayout>
 #include <QHeaderView>
+#include <QLineEdit>
 #include <QMenu>
 #include <QMouseEvent>
 #include <QScrollBar>
-#include <QSignalBlocker>
+#include <QStyledItemDelegate>
 #include <QTableWidget>
 #include <QtGlobal>
 
@@ -42,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.
@@ -52,6 +55,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<QLineEdit*>(editor)->text();
+  emit editFinished(index.row(), index.column(), input);
+}
+
 class MemoryViewTable final : public QTableWidget
 {
 public:
@@ -66,6 +77,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,13 +94,17 @@ 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
   {
     QTableWidget::resizeEvent(event);
-    m_view->CreateTable();
+    // Remakes table if vertically resized
+    const int rows = std::round((height() / static_cast<float>(rowHeight(0))) - 0.25);
+    if (rows != rowCount())
+      m_view->UpdateDispatcher(MemoryViewWidget::UpdateType::Full);
   }
 
   void keyPressEvent(QKeyEvent* event) override
@@ -93,24 +113,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
@@ -122,7 +139,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
@@ -138,7 +155,6 @@ public:
     {
       const u32 address = item->data(USER_ROLE_CELL_ADDRESS).toUInt();
       m_view->ToggleBreakpoint(address, true);
-      m_view->Update();
     }
     else
     {
@@ -146,9 +162,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<MemoryViewWidget::Type>(item->data(USER_ROLE_VALUE_TYPE).toInt());
     std::vector<u8> bytes = m_view->ConvertTextToBytes(type, text);
@@ -156,17 +172,18 @@ public:
     u32 address = item->data(USER_ROLE_CELL_ADDRESS).toUInt();
     u32 end_address = address + static_cast<u32>(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();
+    m_view->UpdateDispatcher(MemoryViewWidget::UpdateType::Values);
   }
 
 private:
@@ -198,13 +215,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());
@@ -220,7 +249,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)
@@ -282,6 +311,42 @@ constexpr int GetCharacterCount(MemoryViewWidget::Type type)
   }
 }
 
+void MemoryViewWidget::UpdateDispatcher(UpdateType type)
+{
+  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.
+  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;
+  case UpdateType::Auto:
+    // Values were captured on CPU thread while doing a callback.
+    if (m_values.size() != 0)
+      UpdateColumns();
+  default:
+    break;
+  }
+}
+
 void MemoryViewWidget::CreateTable()
 {
   m_table->clearContents();
@@ -292,8 +357,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);
@@ -388,17 +451,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;
-
-  const QSignalBlocker blocker(m_table);
   m_table->clearSelection();
 
   // Update addresses
@@ -406,6 +462,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);
@@ -426,69 +485,139 @@ 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);
     }
   }
 
-  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;
-
-  const QSignalBlocker blocker(m_table);
-
   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<Type>(cell_item->data(USER_ROLE_VALUE_TYPE).toInt());
+      std::optional<QString> 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);
+
+      // 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 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);
+      }
     }
   }
 }
 
+// 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();
+  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)
+std::optional<QString> 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)
   {
@@ -550,7 +679,7 @@ QString MemoryViewWidget::ValueToString(const Core::CPUThreadGuard& guard, u32 a
     return string;
   }
   default:
-    return INVALID_MEMORY;
+    return std::nullopt;
   }
 }
 
@@ -606,7 +735,7 @@ void MemoryViewWidget::SetAddressSpace(AddressSpace::Type address_space)
   }
 
   m_address_space = address_space;
-  Update();
+  UpdateDispatcher(UpdateType::Addresses);
 }
 
 AddressSpace::Type MemoryViewWidget::GetAddressSpace() const
@@ -777,7 +906,55 @@ void MemoryViewWidget::SetDisplay(Type type, int bytes_per_row, int alignment, b
   else
     m_alignment = alignment;
 
-  CreateTable();
+  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)
@@ -793,7 +970,7 @@ void MemoryViewWidget::SetAddress(u32 address)
 
   m_address = address;
 
-  Update();
+  UpdateDispatcher(UpdateType::Addresses);
 }
 
 void MemoryViewWidget::SetBPLoggingEnabled(bool enabled)
@@ -916,7 +1093,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
   {
@@ -931,7 +1108,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);
   }