Merge pull request #8337 from CookiePLMonster/log-widget-improvements

Log widget improvements

Source/Core/Common/FixedSizeQueue.h

@@ -6,6 +6,7 @@
 
 #include <array>
 #include <cstddef>
+#include <type_traits>
 #include <utility>
 
 // STL-look-a-like interface, but name is mixed case to distinguish it clearly from the
@@ -19,6 +20,9 @@ class FixedSizeQueue
 public:
   void clear()
   {
+    if constexpr (!std::is_trivial_v<T>)
+      storage = {};
+
     head = 0;
     tail = 0;
     count = 0;
@@ -26,31 +30,47 @@ public:
 
   void push(T t)
   {
+    if (count == N)
+      head = (head + 1) % N;
+    else
+      count++;
+
     storage[tail] = std::move(t);
-    tail++;
-    if (tail == N)
-      tail = 0;
-    count++;
+    tail = (tail + 1) % N;
+  }
+
+  template <class... Args>
+  void emplace(Args&&... args)
+  {
+    if (count == N)
+      head = (head + 1) % N;
+    else
+      count++;
+
+    storage[tail] = T(std::forward<Args>(args)...);
+    tail = (tail + 1) % N;
   }
 
   void pop()
   {
-    head++;
-    if (head == N)
-      head = 0;
+    if constexpr (!std::is_trivial_v<T>)
+      storage[head] = {};
+
+    head = (head + 1) % N;
     count--;
   }
 
   T pop_front()
   {
-    T& temp = storage[head];
+    T temp = std::move(front());
     pop();
-    return std::move(temp);
+    return temp;
   }
 
-  T& front() { return storage[head]; }
-  const T& front() const { return storage[head]; }
-  size_t size() const { return count; }
+  T& front() noexcept { return storage[head]; }
+  const T& front() const noexcept { return storage[head]; }
+  size_t size() const noexcept { return count; }
+  bool empty() const noexcept { return size() == 0; }
 
 private:
   std::array<T, N> storage;

Source/Core/DolphinQt/Config/LogWidget.cpp

@@ -8,27 +8,27 @@
 #include <QComboBox>
 #include <QFont>
 #include <QFontDatabase>
-#include <QGroupBox>
+#include <QGridLayout>
+#include <QPlainTextEdit>
 #include <QPushButton>
-#include <QScrollBar>
-#include <QTextEdit>
 #include <QTimer>
-#include <QVBoxLayout>
-
-#include "Common/FileUtil.h"
-#include "Common/StringUtil.h"
 
 #include "Core/ConfigManager.h"
 
-#include "DolphinQt/QtUtils/QueueOnObject.h"
 #include "DolphinQt/Settings.h"
 
 // Delay in ms between calls of UpdateLog()
 constexpr int UPDATE_LOG_DELAY = 100;
 // Maximum lines to process at a time
-constexpr int MAX_LOG_LINES = 200;
+constexpr size_t MAX_LOG_LINES_TO_UPDATE = 200;
 // Timestamp length
-constexpr int TIMESTAMP_LENGTH = 10;
+constexpr size_t TIMESTAMP_LENGTH = 10;
+
+// A helper function to construct QString from std::string_view in one line
+static QString QStringFromStringView(std::string_view str)
+{
+  return QString::fromUtf8(str.data(), static_cast<int>(str.size()));
+}
 
 LogWidget::LogWidget(QWidget* parent) : QDockWidget(parent), m_timer(new QTimer(this))
 {
@@ -44,7 +44,12 @@ LogWidget::LogWidget(QWidget* parent) : QDockWidget(parent), m_timer(new QTimer(
   ConnectWidgets();
 
   connect(m_timer, &QTimer::timeout, this, &LogWidget::UpdateLog);
-  m_timer->start(UPDATE_LOG_DELAY);
+  connect(this, &QDockWidget::visibilityChanged, [this](bool visible) {
+    if (visible)
+      m_timer->start(UPDATE_LOG_DELAY);
+    else
+      m_timer->stop();
+  });
 
   connect(&Settings::Instance(), &Settings::DebugFontChanged, this, &LogWidget::UpdateFont);
 
@@ -60,36 +65,47 @@ LogWidget::~LogWidget()
 
 void LogWidget::UpdateLog()
 {
-  std::lock_guard<std::mutex> lock(m_log_mutex);
-
-  if (m_log_queue.empty())
-    return;
-
-  auto* vscroll = m_log_text->verticalScrollBar();
-  auto* hscroll = m_log_text->horizontalScrollBar();
-
-  // If the vertical scrollbar is within 50 units of the maximum value, count it as being at the
-  // bottom
-  bool vscroll_bottom = vscroll->maximum() - vscroll->value() < 50;
-
-  int old_horizontal = hscroll->value();
-  int old_vertical = vscroll->value();
-
-  for (int i = 0; !m_log_queue.empty() && i < MAX_LOG_LINES; i++)
+  std::vector<LogEntry> elements_to_push;
   {
-    m_log_text->append(m_log_queue.front());
-    m_log_queue.pop();
+    std::lock_guard lock(m_log_mutex);
+    if (m_log_ring_buffer.empty())
+      return;
+
+    elements_to_push.reserve(std::min(MAX_LOG_LINES_TO_UPDATE, m_log_ring_buffer.size()));
+
+    for (size_t i = 0; !m_log_ring_buffer.empty() && i < MAX_LOG_LINES_TO_UPDATE; i++)
+      elements_to_push.push_back(std::move(m_log_ring_buffer.pop_front()));
   }
 
-  if (hscroll->value() != old_horizontal)
-    hscroll->setValue(old_horizontal);
-
-  if (vscroll->value() != old_vertical)
+  for (auto& line : elements_to_push)
   {
-    if (vscroll_bottom)
-      vscroll->setValue(vscroll->maximum());
-    else
-      vscroll->setValue(old_vertical);
+    const char* color = "white";
+    switch (std::get<LogTypes::LOG_LEVELS>(line))
+    {
+    case LogTypes::LOG_LEVELS::LERROR:
+      color = "red";
+      break;
+    case LogTypes::LOG_LEVELS::LWARNING:
+      color = "yellow";
+      break;
+    case LogTypes::LOG_LEVELS::LNOTICE:
+      color = "lime";
+      break;
+    case LogTypes::LOG_LEVELS::LINFO:
+      color = "cyan";
+      break;
+    case LogTypes::LOG_LEVELS::LDEBUG:
+      color = "lightgrey";
+      break;
+    }
+
+    const std::string_view str_view(std::get<std::string>(line));
+
+    m_log_text->appendHtml(
+        QStringLiteral("%1 <span style=\"color: %2; white-space: pre\">%3</span>")
+            .arg(QStringFromStringView(str_view.substr(0, TIMESTAMP_LENGTH)),
+                 QString::fromUtf8(color),
+                 QStringFromStringView(str_view.substr(TIMESTAMP_LENGTH)).toHtmlEscaped()));
   }
 }
 
@@ -115,8 +131,7 @@ void LogWidget::UpdateFont()
 void LogWidget::CreateWidgets()
 {
   // Log
-  m_tab_log = new QWidget;
-  m_log_text = new QTextEdit;
+  m_log_text = new QPlainTextEdit;
   m_log_wrap = new QCheckBox(tr("Word Wrap"));
   m_log_font = new QComboBox;
   m_log_clear = new QPushButton(tr("Clear"));
@@ -124,7 +139,6 @@ void LogWidget::CreateWidgets()
   m_log_font->addItems({tr("Default Font"), tr("Monospaced Font"), tr("Selected Font")});
 
   auto* log_layout = new QGridLayout;
-  m_tab_log->setLayout(log_layout);
   log_layout->addWidget(m_log_wrap, 0, 0);
   log_layout->addWidget(m_log_font, 0, 1);
   log_layout->addWidget(m_log_clear, 0, 2);
@@ -136,6 +150,8 @@ void LogWidget::CreateWidgets()
   setWidget(widget);
 
   m_log_text->setReadOnly(true);
+  m_log_text->setUndoRedoEnabled(false);
+  m_log_text->setMaximumBlockCount(MAX_LOG_LINES);
 
   QPalette palette = m_log_text->palette();
   palette.setColor(QPalette::Base, Qt::black);
@@ -145,13 +161,15 @@ void LogWidget::CreateWidgets()
 
 void LogWidget::ConnectWidgets()
 {
-  connect(m_log_clear, &QPushButton::clicked, m_log_text, &QTextEdit::clear);
+  connect(m_log_clear, &QPushButton::clicked, [this] {
+    m_log_text->clear();
+    m_log_ring_buffer.clear();
+  });
   connect(m_log_wrap, &QCheckBox::toggled, this, &LogWidget::SaveSettings);
   connect(m_log_font, static_cast<void (QComboBox::*)(int)>(&QComboBox::currentIndexChanged), this,
           &LogWidget::SaveSettings);
   connect(this, &QDockWidget::topLevelChanged, this, &LogWidget::SaveSettings);
-  connect(&Settings::Instance(), &Settings::LogVisibilityChanged, this,
-          [this](bool visible) { setHidden(!visible); });
+  connect(&Settings::Instance(), &Settings::LogVisibilityChanged, this, &LogWidget::setVisible);
 }
 
 void LogWidget::LoadSettings()
@@ -163,7 +181,8 @@ void LogWidget::LoadSettings()
 
   // Log - Wrap Lines
   m_log_wrap->setChecked(settings.value(QStringLiteral("logging/wraplines")).toBool());
-  m_log_text->setLineWrapMode(m_log_wrap->isChecked() ? QTextEdit::WidgetWidth : QTextEdit::NoWrap);
+  m_log_text->setLineWrapMode(m_log_wrap->isChecked() ? QPlainTextEdit::WidgetWidth :
+                                                        QPlainTextEdit::NoWrap);
 
   // Log - Font Selection
   // Currently "Debugger Font" is not supported as there is no Qt Debugger, defaulting to Monospace
@@ -180,7 +199,8 @@ void LogWidget::SaveSettings()
 
   // Log - Wrap Lines
   settings.setValue(QStringLiteral("logging/wraplines"), m_log_wrap->isChecked());
-  m_log_text->setLineWrapMode(m_log_wrap->isChecked() ? QTextEdit::WidgetWidth : QTextEdit::NoWrap);
+  m_log_text->setLineWrapMode(m_log_wrap->isChecked() ? QPlainTextEdit::WidgetWidth :
+                                                        QPlainTextEdit::NoWrap);
 
   // Log - Font Selection
   settings.setValue(QStringLiteral("logging/font"), m_log_font->currentIndex());
@@ -189,40 +209,13 @@ void LogWidget::SaveSettings()
 
 void LogWidget::Log(LogTypes::LOG_LEVELS level, const char* text)
 {
-  // The text has to be copied here as it will be deallocated after this method has returned
-  std::string str(text);
+  size_t text_length = strlen(text);
+  while (text_length > 0 && text[text_length - 1] == '\n')
+    text_length--;
 
-  QueueOnObject(this, [this, level, str]() mutable {
-    std::lock_guard<std::mutex> lock(m_log_mutex);
-
-    const char* color = "white";
-
-    switch (level)
-    {
-    case LogTypes::LOG_LEVELS::LERROR:
-      color = "red";
-      break;
-    case LogTypes::LOG_LEVELS::LWARNING:
-      color = "yellow";
-      break;
-    case LogTypes::LOG_LEVELS::LNOTICE:
-      color = "lime";
-      break;
-    case LogTypes::LOG_LEVELS::LINFO:
-      color = "cyan";
-      break;
-    case LogTypes::LOG_LEVELS::LDEBUG:
-      color = "lightgrey";
-      break;
-    }
-
-    StringPopBackIf(&str, '\n');
-    m_log_queue.push(
-        QStringLiteral("%1 <span style=\"color: %2; white-space: pre\">%3</span>")
-            .arg(QString::fromStdString(str.substr(0, TIMESTAMP_LENGTH)),
-                 QString::fromStdString(color),
-                 QString::fromStdString(str.substr(TIMESTAMP_LENGTH)).toHtmlEscaped()));
-  });
+  std::lock_guard lock(m_log_mutex);
+  m_log_ring_buffer.emplace(std::piecewise_construct, std::forward_as_tuple(text, text_length),
+                            std::forward_as_tuple(level));
 }
 
 void LogWidget::closeEvent(QCloseEvent*)

Source/Core/DolphinQt/Config/LogWidget.h

@@ -7,16 +7,16 @@
 #include <QDockWidget>
 
 #include <mutex>
-#include <queue>
+#include <string>
 
+#include "Common/FixedSizeQueue.h"
 #include "Common/Logging/LogManager.h"
 
 class QCheckBox;
 class QCloseEvent;
 class QComboBox;
+class QPlainTextEdit;
 class QPushButton;
-class QVBoxLayout;
-class QTextEdit;
 class QTimer;
 
 class LogWidget final : public QDockWidget, LogListener
@@ -43,12 +43,15 @@ private:
   QCheckBox* m_log_wrap;
   QComboBox* m_log_font;
   QPushButton* m_log_clear;
-  QVBoxLayout* m_main_layout;
-  QTextEdit* m_log_text;
-  QWidget* m_tab_log;
+  QPlainTextEdit* m_log_text;
 
   QTimer* m_timer;
 
+  using LogEntry = std::pair<std::string, LogTypes::LOG_LEVELS>;
+
+  // Maximum number of lines to show in log viewer
+  static constexpr int MAX_LOG_LINES = 5000;
+
   std::mutex m_log_mutex;
-  std::queue<QString> m_log_queue;
+  FixedSizeQueue<LogEntry, MAX_LOG_LINES> m_log_ring_buffer;
 };

Source/UnitTests/Common/FixedSizeQueueTest.cpp

@@ -31,3 +31,85 @@ TEST(FixedSizeQueue, Simple)
 
   EXPECT_EQ(0u, q.size());
 }
+
+TEST(FixedSizeQueue, RingBuffer)
+{
+  // Testing if queue works when used as a ring buffer
+  FixedSizeQueue<int, 5> q;
+
+  EXPECT_EQ(0u, q.size());
+
+  q.push(0);
+  q.push(1);
+  q.push(2);
+  q.push(3);
+  q.push(4);
+  q.push(5);
+
+  EXPECT_EQ(5u, q.size());
+  EXPECT_EQ(1, q.pop_front());
+
+  EXPECT_EQ(4u, q.size());
+}
+
+// Local classes cannot have static fields,
+// therefore this has to be declared in global scope.
+class NonTrivialTypeTestData
+{
+public:
+  static inline int num_objects = 0;
+  static inline int total_constructed = 0;
+  static inline int default_constructed = 0;
+  static inline int total_destructed = 0;
+
+  NonTrivialTypeTestData()
+  {
+    num_objects++;
+    total_constructed++;
+    default_constructed++;
+  }
+
+  NonTrivialTypeTestData(int /*val*/)
+  {
+    num_objects++;
+    total_constructed++;
+  }
+
+  ~NonTrivialTypeTestData()
+  {
+    num_objects--;
+    total_destructed++;
+  }
+};
+
+TEST(FixedSizeQueue, NonTrivialTypes)
+{
+  // Testing if construction/destruction of non-trivial types happens as expected
+  FixedSizeQueue<NonTrivialTypeTestData, 2> q;
+
+  EXPECT_EQ(0u, q.size());
+
+  EXPECT_EQ(2, NonTrivialTypeTestData::num_objects);
+  EXPECT_EQ(2, NonTrivialTypeTestData::total_constructed);
+  EXPECT_EQ(2, NonTrivialTypeTestData::default_constructed);
+  EXPECT_EQ(0, NonTrivialTypeTestData::total_destructed);
+
+  q.emplace(4);
+  q.emplace(6);
+  q.emplace(8);
+
+  EXPECT_EQ(2, NonTrivialTypeTestData::num_objects);
+  EXPECT_EQ(2 + 3, NonTrivialTypeTestData::total_constructed);
+  EXPECT_EQ(2, NonTrivialTypeTestData::default_constructed);
+  EXPECT_EQ(3, NonTrivialTypeTestData::total_destructed);
+  EXPECT_EQ(2u, q.size());
+
+  q.pop();
+  q.pop();
+
+  EXPECT_EQ(2, NonTrivialTypeTestData::num_objects);
+  EXPECT_EQ(2 + 3 + 2, NonTrivialTypeTestData::total_constructed);
+  EXPECT_EQ(2 + 2, NonTrivialTypeTestData::default_constructed);
+  EXPECT_EQ(3 + 2, NonTrivialTypeTestData::total_destructed);
+  EXPECT_EQ(0u, q.size());
+}