performance optimizations: text/ini tabs and advanced conflict list use views and custom models

call update() after feedFiles() to allow some tabs to have post-processing after feedFiles()

src/modinfodialog.cpp

@@ -35,6 +35,19 @@ using namespace MOShared;
 
 const int max_scan_for_context_menu = 50;
 
+
+int naturalCompare(const QString& a, const QString& b)
+{
+  static QCollator c = []{
+    QCollator c;
+    c.setNumericMode(true);
+    c.setCaseSensitivity(Qt::CaseInsensitive);
+    return c;
+  }();
+
+  return c.compare(a, b);
+}
+
 bool canPreviewFile(
   PluginContainer& pluginContainer, bool isArchive, const QString& filename)
 {
@@ -283,10 +296,15 @@ void ModInfoDialog::updateTabs(bool becauseOriginChanged)
 
     tabInfo.tab->setMod(m_mod, origin);
     tabInfo.tab->clear();
+  }
+
+
+  feedFiles(becauseOriginChanged);
+
+  for (auto& tabInfo : m_tabs) {
     tabInfo.tab->update();
   }
 
-  feedFiles(becauseOriginChanged);
   setTabsColors();
 }
 

src/modinfodialog.h

@@ -42,6 +42,8 @@ bool canUnhideFile(bool isArchive, const QString& filename);
 FileRenamer::RenameResults hideFile(FileRenamer& renamer, const QString &oldName);
 FileRenamer::RenameResults unhideFile(FileRenamer& renamer, const QString &oldName);
 
+int naturalCompare(const QString& a, const QString& b);
+
 
 /**
  * this is a larger dialog used to visualise information about the mod.

src/modinfodialog.ui

@@ -60,13 +60,16 @@
             </widget>
            </item>
            <item>
-            <widget class="QListWidget" name="textFileList">
+            <widget class="QListView" name="textFileList">
              <property name="toolTip">
               <string>A list of text-files in the mod directory.</string>
              </property>
              <property name="whatsThis">
               <string>A list of text-files in the mod directory like readmes. </string>
              </property>
+             <property name="uniformItemSizes">
+              <bool>true</bool>
+             </property>
             </widget>
            </item>
           </layout>
@@ -117,13 +120,16 @@
             </widget>
            </item>
            <item>
-            <widget class="QListWidget" name="iniFileList">
+            <widget class="QListView" name="iniFileList">
              <property name="toolTip">
               <string>This is a list of .ini files in the mod.</string>
              </property>
              <property name="whatsThis">
               <string>This is a list of .ini files in the mod. These are usually used to configure the behaviour of mods if there are configurable parameters.</string>
              </property>
+             <property name="uniformItemSizes">
+              <bool>true</bool>
+             </property>
             </widget>
            </item>
           </layout>
@@ -251,6 +257,9 @@ They usually contain optional functionality, see the readme.
 
 Most mods do not have optional esps, so chances are good you are looking at an empty list.</string>
              </property>
+             <property name="uniformItemSizes">
+              <bool>true</bool>
+             </property>
             </widget>
            </item>
           </layout>
@@ -375,6 +384,9 @@ Most mods do not have optional esps, so chances are good you are looking at an e
                 <property name="whatsThis">
                  <string>These are the mod files that are in the (virtual) data directory of your game and will thus be selectable in the esp list in the main window.</string>
                 </property>
+                <property name="uniformItemSizes">
+                 <bool>true</bool>
+                </property>
                </widget>
               </item>
              </layout>
@@ -477,9 +489,6 @@ text-align: left;</string>
                 <property name="sortingEnabled">
                  <bool>true</bool>
                 </property>
-                <property name="animated">
-                 <bool>true</bool>
-                </property>
                 <property name="columnCount">
                  <number>2</number>
                 </property>
@@ -553,9 +562,6 @@ text-align: left;</string>
                 <property name="sortingEnabled">
                  <bool>true</bool>
                 </property>
-                <property name="animated">
-                 <bool>true</bool>
-                </property>
                 <property name="columnCount">
                  <number>2</number>
                 </property>
@@ -629,9 +635,6 @@ text-align: left;</string>
                 <property name="sortingEnabled">
                  <bool>true</bool>
                 </property>
-                <property name="animated">
-                 <bool>true</bool>
-                </property>
                 <property name="columnCount">
                  <number>1</number>
                 </property>
@@ -696,7 +699,7 @@ text-align: left;</string>
                   <number>0</number>
                  </property>
                  <item>
-                  <widget class="QTreeWidget" name="conflictsAdvancedList">
+                  <widget class="QTreeView" name="conflictsAdvancedList">
                    <property name="contextMenuPolicy">
                     <enum>Qt::CustomContextMenu</enum>
                    </property>
@@ -709,27 +712,6 @@ text-align: left;</string>
                    <property name="sortingEnabled">
                     <bool>true</bool>
                    </property>
-                   <property name="columnCount">
-                    <number>3</number>
-                   </property>
-                   <attribute name="headerDefaultSectionSize">
-                    <number>243</number>
-                   </attribute>
-                   <column>
-                    <property name="text">
-                     <string>Overwrites</string>
-                    </property>
-                   </column>
-                   <column>
-                    <property name="text">
-                     <string>File</string>
-                    </property>
-                   </column>
-                   <column>
-                    <property name="text">
-                     <string>Overwritten by</string>
-                    </property>
-                   </column>
                   </widget>
                  </item>
                 </layout>

src/modinfodialogconflicts.cpp

@@ -13,19 +13,6 @@ using namespace MOBase;
 const int max_scan_for_context_menu = 50;
 
 
-int naturalCompare(const QString& a, const QString& b)
-{
-  static QCollator c = []{
-    QCollator c;
-    c.setNumericMode(true);
-    c.setCaseSensitivity(Qt::CaseInsensitive);
-    return c;
-  }();
-
-  return c.compare(a, b);
-}
-
-
 class ElideLeftDelegate : public QStyledItemDelegate
 {
 public:
@@ -581,6 +568,10 @@ bool GeneralConflictsTab::update()
   int numOverwrite = 0;
   int numOverwritten = 0;
 
+  QList<QTreeWidgetItem*> overwriteItems;
+  QList<QTreeWidgetItem*> overwrittenItems;
+  QList<QTreeWidgetItem*> noConflictItems;
+
   if (m_tab->origin() != nullptr) {
     const auto rootPath = m_tab->mod()->absolutePath();
 
@@ -594,19 +585,19 @@ bool GeneralConflictsTab::update()
 
       if (fileOrigin == m_tab->origin()->getID()) {
         if (!alternatives.empty()) {
-          ui->overwriteTree->addTopLevelItem(createOverwriteItem(
+          overwriteItems.append(createOverwriteItem(
             file->getIndex(), archive, fileName, relativeName, alternatives));
 
           ++numOverwrite;
         } else {
           // otherwise, put the file in the noconflict tree
-          ui->noConflictTree->addTopLevelItem(createNoConflictItem(
+          noConflictItems.append(createNoConflictItem(
             file->getIndex(), archive, fileName, relativeName));
 
           ++numNonConflicting;
         }
       } else {
-        ui->overwrittenTree->addTopLevelItem(createOverwrittenItem(
+        overwrittenItems.append(createOverwrittenItem(
           file->getIndex(), fileOrigin, archive, fileName, relativeName));
 
         ++numOverwritten;
@@ -614,6 +605,10 @@ bool GeneralConflictsTab::update()
     }
   }
 
+  ui->overwriteTree->addTopLevelItems(overwriteItems);
+  ui->overwrittenTree->addTopLevelItems(overwrittenItems);
+  ui->noConflictTree->addTopLevelItems(noConflictItems);
+
   ui->overwriteCount->display(numOverwrite);
   ui->overwrittenCount->display(numOverwritten);
   ui->noConflictCount->display(numNonConflicting);
@@ -691,10 +686,166 @@ void GeneralConflictsTab::onOverwrittenActivated(QTreeWidgetItem *item, int)
 }
 
 
+class AdvancedListModel : public QAbstractItemModel
+{
+public:
+  struct Item
+  {
+    QString before, relativeName, after;
+    FileEntry::Index index;
+    QString fileName;
+    bool hasAltOrigins;
+    QString altOrigin;
+    bool archive;
+  };
+
+  void clear()
+  {
+    m_items.clear();
+  }
+
+  void reserve(std::size_t s)
+  {
+    m_items.reserve(s);
+  }
+
+  QModelIndex index(int row, int col, const QModelIndex& ={}) const override
+  {
+    return createIndex(row, col);
+  }
+
+  QModelIndex parent(const QModelIndex&) const override
+  {
+    return {};
+  }
+
+  int rowCount(const QModelIndex& parent={}) const override
+  {
+    if (parent.isValid()) {
+      return 0;
+    }
+
+    return static_cast<int>(m_items.size());
+  }
+
+  int columnCount(const QModelIndex& ={}) const override
+  {
+    return 3;
+  }
+
+  QVariant data(const QModelIndex& index, int role) const override
+  {
+    if (role == Qt::DisplayRole) {
+      const auto row = index.row();
+      if (row < 0) {
+        return {};
+      }
+
+      const auto i = static_cast<std::size_t>(row);
+      if (i >= m_items.size()) {
+        return {};
+      }
+
+      const auto& item = m_items[i];
+
+      if (index.column() == 0) {
+        return item.before;
+      } else if (index.column() == 1) {
+        return item.relativeName;
+      } else if (index.column() == 2) {
+        return item.after;
+      }
+    }
+
+    return {};
+  }
+
+  QVariant headerData(int col, Qt::Orientation, int role) const
+  {
+    if (role == Qt::DisplayRole) {
+      if (col == 0) {
+        return tr("Overwrites");
+      } else if (col == 1) {
+        return tr("File");
+      } else if (col == 2) {
+        return tr("Overwritten by");
+      }
+    }
+
+    return {};
+  }
+
+  void sort(int col, Qt::SortOrder order=Qt::AscendingOrder)
+  {
+    // avoids branching on column/sort order while sorting
+    auto sortBeforeAsc = [](const auto& a, const auto& b) {
+      return (naturalCompare(a.before, b.before) < 0);
+    };
+
+    auto sortBeforeDesc = [](const auto& a, const auto& b) {
+      return (naturalCompare(a.before, b.before) > 0);
+    };
+
+    auto sortFileAsc = [](const auto& a, const auto& b) {
+      return (naturalCompare(a.relativeName, b.relativeName) < 0);
+    };
+
+    auto sortFileDesc = [](const auto& a, const auto& b) {
+      return (naturalCompare(a.relativeName, b.relativeName) > 0);
+    };
+
+    auto sortAfterAsc = [](const auto& a, const auto& b) {
+      return (naturalCompare(a.after, b.after) < 0);
+    };
+
+    auto sortAfterDesc = [](const auto& a, const auto& b) {
+      return (naturalCompare(a.after, b.after) > 0);
+    };
+
+    if (col == 0) {
+      if (order == Qt::AscendingOrder) {
+        std::sort(m_items.begin(), m_items.end(), sortBeforeAsc);
+      } else {
+        std::sort(m_items.begin(), m_items.end(), sortBeforeDesc);
+      }
+    } else if (col == 1) {
+      if (order == Qt::AscendingOrder) {
+        std::sort(m_items.begin(), m_items.end(), sortFileAsc);
+      } else {
+        std::sort(m_items.begin(), m_items.end(), sortFileDesc);
+      }
+    } else if (col == 2) {
+      if (order == Qt::AscendingOrder) {
+        std::sort(m_items.begin(), m_items.end(), sortAfterAsc);
+      } else {
+        std::sort(m_items.begin(), m_items.end(), sortAfterDesc);
+      }
+    }
+
+    emit layoutChanged({}, QAbstractItemModel::VerticalSortHint);
+  }
+
+  void add(Item item)
+  {
+    m_items.emplace_back(std::move(item));
+  }
+
+  void finished()
+  {
+    emit dataChanged(index(0, 0), index(0, rowCount()));
+  }
+
+private:
+  std::vector<Item> m_items;
+};
+
+
 AdvancedConflictsTab::AdvancedConflictsTab(
   ConflictsTab* tab, Ui::ModInfoDialog* pui, OrganizerCore& oc)
-    : m_tab(tab), ui(pui), m_core(oc)
+    : m_tab(tab), ui(pui), m_core(oc), m_model(new AdvancedListModel)
 {
+  ui->conflictsAdvancedList->setModel(m_model);
+
   // left-elide the overwrites column so that the nearest are visible
   ui->conflictsAdvancedList->setItemDelegateForColumn(
     0, new ElideLeftDelegate(ui->conflictsAdvancedList));
@@ -717,9 +868,9 @@ AdvancedConflictsTab::AdvancedConflictsTab(
     ui->conflictsAdvancedShowNearest, &QRadioButton::clicked,
     [&]{ update(); });
 
-  QObject::connect(
-    ui->conflictsAdvancedList, &QTreeWidget::customContextMenuRequested,
-    [&](const QPoint& p){ m_tab->showContextMenu(p, ui->conflictsAdvancedList); });
+  //QObject::connect(
+  //  ui->conflictsAdvancedList, &QTreeView::customContextMenuRequested,
+  //  [&](const QPoint& p){ m_tab->showContextMenu(p, ui->conflictsAdvancedList); });
 
   m_filter.set(ui->conflictsAdvancedFilter);
   m_filter.changed = [&]{ update(); };
@@ -727,7 +878,7 @@ AdvancedConflictsTab::AdvancedConflictsTab(
 
 void AdvancedConflictsTab::clear()
 {
-  ui->conflictsAdvancedList->clear();
+  m_model->clear();
 }
 
 void AdvancedConflictsTab::saveState(Settings& s)
@@ -776,7 +927,10 @@ void AdvancedConflictsTab::update()
   if (m_tab->origin() != nullptr) {
     const auto rootPath = m_tab->mod()->absolutePath();
 
-    for (const auto& file : m_tab->origin()->getFiles()) {
+    const auto& files = m_tab->origin()->getFiles();
+    m_model->reserve(files.size());
+
+    for (const auto& file : files) {
       const QString relativeName = QDir::fromNativeSeparators(ToQString(file->getRelativePath()));
       const QString fileName = relativeName.mid(0).prepend(rootPath);
 
@@ -784,18 +938,16 @@ void AdvancedConflictsTab::update()
       const int fileOrigin = file->getOrigin(archive);
       const auto& alternatives = file->getAlternatives();
 
-      auto* advancedItem = createItem(
+      addItem(
         file->getIndex(), fileOrigin, archive,
         fileName, relativeName, alternatives);
-
-      if (advancedItem) {
-        ui->conflictsAdvancedList->addTopLevelItem(advancedItem);
-      }
     }
+
+    m_model->finished();
   }
 }
 
-QTreeWidgetItem* AdvancedConflictsTab::createItem(
+void AdvancedConflictsTab::addItem(
   FileEntry::Index index, int fileOrigin, bool archive,
   const QString& fileName, const QString& relativeName,
   const MOShared::FileEntry::AlternativesVector& alternatives)
@@ -888,7 +1040,7 @@ QTreeWidgetItem* AdvancedConflictsTab::createItem(
     // if both before and after are empty, it means this file has no conflicts
     // at all, only display it if the user wants it
     if (!hasAlts) {
-      return nullptr;
+      return;
     }
   }
 
@@ -900,9 +1052,9 @@ QTreeWidgetItem* AdvancedConflictsTab::createItem(
     });
 
   if (!matched) {
-    return nullptr;
+    return;
   }
 
-  return new ConflictItem(
-    {before, relativeName, after}, index, fileName, hasAlts, "", archive);
+  m_model->add(
+    {before, relativeName, after, index, fileName, hasAlts, "", archive});
 }

src/modinfodialogconflicts.h

@@ -9,6 +9,7 @@
 
 class ConflictsTab;
 class OrganizerCore;
+class ConflictItem;
 
 class GeneralConflictsTab : public QObject
 {
@@ -60,6 +61,8 @@ private:
 };
 
 
+class AdvancedListModel;
+
 class AdvancedConflictsTab : public QObject
 {
   Q_OBJECT;
@@ -82,8 +85,9 @@ private:
   Ui::ModInfoDialog* ui;
   OrganizerCore& m_core;
   FilterWidget m_filter;
+  AdvancedListModel* m_model;
 
-  QTreeWidgetItem* createItem(
+  void addItem(
     MOShared::FileEntry::Index index, int fileOrigin, bool archive,
     const QString& fileName, const QString& relativeName,
     const MOShared::FileEntry::AlternativesVector& alternatives);

src/modinfodialogtextfiles.cpp

@@ -1,32 +1,109 @@
 #include "modinfodialogtextfiles.h"
 #include "ui_modinfodialog.h"
+#include "modinfodialog.h"
 #include <QMessageBox>
 
-class FileListItem : public QListWidgetItem
+class FileListModel : public QAbstractItemModel
 {
 public:
-  FileListItem(const QString& rootPath, QString fullPath)
-    : m_fullPath(std::move(fullPath))
+  void clear()
   {
-    setText(m_fullPath.mid(rootPath.length() + 1));
+    m_files.clear();
   }
 
-  const QString& fullPath() const
+  QModelIndex index(int row, int col, const QModelIndex& ={}) const override
   {
-    return m_fullPath;
+    return createIndex(row, col);
+  }
+
+  QModelIndex parent(const QModelIndex&) const override
+  {
+    return {};
+  }
+
+  int rowCount(const QModelIndex& ={}) const override
+  {
+    return static_cast<int>(m_files.size());
+  }
+
+  int columnCount(const QModelIndex& ={}) const override
+  {
+    return 1;
+  }
+
+  QVariant data(const QModelIndex& index, int role) const override
+  {
+    if (role == Qt::DisplayRole) {
+      const auto row = index.row();
+      if (row < 0) {
+        return {};
+      }
+
+      const auto i = static_cast<std::size_t>(row);
+      if (i >= m_files.size()) {
+        return {};
+      }
+
+      return m_files[i].text;
+    }
+
+    return {};
+  }
+
+  void add(const QString& rootPath, QString fullPath)
+  {
+    QString text = fullPath.mid(rootPath.length() + 1);
+    m_files.emplace_back(std::move(fullPath), std::move(text));
+  }
+
+  void finished()
+  {
+    std::sort(m_files.begin(), m_files.end(), [](const auto& a, const auto& b) {
+      return (naturalCompare(a.text, b.text) < 0);
+    });
+
+    emit dataChanged(index(0, 0), index(0, rowCount()));
+  }
+
+  QString fullPath(const QModelIndex& index) const
+  {
+    const auto row = index.row();
+    if (row < 0) {
+      return {};
+    }
+
+    const auto i = static_cast<std::size_t>(row);
+    if (i >= m_files.size()) {
+      return {};
+    }
+
+    return m_files[i].fullPath;
   }
 
 private:
-  QString m_fullPath;
+  struct File
+  {
+    QString fullPath;
+    QString text;
+
+    File(QString fp, QString t)
+      : fullPath(std::move(fp)), text(std::move(t))
+    {
+    }
+  };
+
+  std::deque<File> m_files;
 };
 
 
 GenericFilesTab::GenericFilesTab(
   OrganizerCore& oc, PluginContainer& plugin,
   QWidget* parent, Ui::ModInfoDialog* ui, int id,
-  QListWidget* list, QSplitter* sp, TextEditor* e)
-    : ModInfoDialogTab(oc, plugin, parent, ui, id), m_list(list), m_editor(e)
+  QListView* list, QSplitter* sp, TextEditor* e) :
+    ModInfoDialogTab(oc, plugin, parent, ui, id),
+    m_list(list), m_editor(e), m_model(new FileListModel)
 {
+  m_list->setModel(m_model);
   m_editor->setupToolbar();
 
   sp->setSizes({200, 1});
@@ -34,14 +111,14 @@ GenericFilesTab::GenericFilesTab(
   sp->setStretchFactor(1, 1);
 
   QObject::connect(
-    m_list, &QListWidget::currentItemChanged,
-    [&](auto* current, auto* previous){ onSelection(current, previous); });
+    m_list->selectionModel(), &QItemSelectionModel::currentRowChanged,
+    [&](auto current, auto previous){ onSelection(current, previous); });
 }
 
 void GenericFilesTab::clear()
 {
-  m_list->clear();
-  select(nullptr);
+  m_model->clear();
+  select({});
   setHasData(false);
 }
 
@@ -76,7 +153,7 @@ bool GenericFilesTab::feedFile(const QString& rootPath, const QString& fullPath)
 
   for (const auto* e : extensions) {
     if (wantsFile(rootPath, fullPath)) {
-      m_list->addItem(new FileListItem(rootPath, fullPath));
+      m_model->add(rootPath, fullPath);
       setHasData(true);
       return true;
     }
@@ -85,31 +162,31 @@ bool GenericFilesTab::feedFile(const QString& rootPath, const QString& fullPath)
   return false;
 }
 
-void GenericFilesTab::onSelection(
-  QListWidgetItem* current, QListWidgetItem* previous)
+void GenericFilesTab::update()
 {
-  auto* item = dynamic_cast<FileListItem*>(current);
-  if (!item) {
-    qCritical("TextFilesTab: item is not a FileListItem");
-    return;
-  }
-
-  if (!canClose()) {
-    m_list->setCurrentItem(previous, QItemSelectionModel::Current);
-    return;
-  }
-
-  select(item);
+  m_model->finished();
 }
 
-void GenericFilesTab::select(FileListItem* item)
+void GenericFilesTab::onSelection(
+  const QModelIndex& current, const QModelIndex& previous)
 {
-  if (item) {
-    m_editor->setEnabled(true);
-    m_editor->load(item->fullPath());
-  } else {
-    m_editor->setEnabled(false);
+  if (!canClose()) {
+    m_list->selectionModel()->select(previous, QItemSelectionModel::Current);
+    return;
   }
+
+  select(current);
+}
+
+void GenericFilesTab::select(const QModelIndex& index)
+{
+  if (!index.isValid()) {
+    m_editor->setEnabled(false);
+    return;
+  }
+
+  m_editor->setEnabled(true);
+  m_editor->load(m_model->fullPath(index));
 }
 
 

src/modinfodialogtextfiles.h

@@ -3,9 +3,10 @@
 
 #include "modinfodialogtab.h"
 #include <QSplitter>
-#include <QListWidget>
+#include <QListView>
 
 class FileListItem;
+class FileListModel;
 class TextEditor;
 
 class GenericFilesTab : public ModInfoDialogTab
@@ -16,21 +17,23 @@ public:
   void clear() override;
   bool canClose() override;
   bool feedFile(const QString& rootPath, const QString& fullPath) override;
+  void update() override;
 
 protected:
-  QListWidget* m_list;
+  QListView* m_list;
   TextEditor* m_editor;
+  FileListModel* m_model;
 
   GenericFilesTab(
     OrganizerCore& oc, PluginContainer& plugin,
     QWidget* parent, Ui::ModInfoDialog* ui, int id,
-    QListWidget* list, QSplitter* splitter, TextEditor* editor);
+    QListView* list, QSplitter* splitter, TextEditor* editor);
 
   virtual bool wantsFile(const QString& rootPath, const QString& fullPath) const = 0;
 
 private:
-  void onSelection(QListWidgetItem* current, QListWidgetItem* previous);
-  void select(FileListItem* item);
+  void onSelection(const QModelIndex& current, const QModelIndex& previous);
+  void select(const QModelIndex& index);
 };