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()

7 files changed, 338 insertions, 100 deletions

diff --git a/src/modinfodialog.cpp b/src/modinfodialog.cpp
index 4c169cc4..a407e9b7 100644
--- a/src/modinfodialog.cpp
+++ b/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();
-    tabInfo.tab->update();
   }
 
+
   feedFiles(becauseOriginChanged);
+
+  for (auto& tabInfo : m_tabs) {
+    tabInfo.tab->update();
+  }
+
   setTabsColors();
 }
 
diff --git a/src/modinfodialog.h b/src/modinfodialog.h
index 31ea5536..e814dcf4 100644
--- a/src/modinfodialog.h
+++ b/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.

diff --git a/src/modinfodialog.ui b/src/modinfodialog.ui
index 40b3c7b4..5defcd57 100644
--- a/src/modinfodialog.ui
+++ b/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>

diff --git a/src/modinfodialogconflicts.cpp b/src/modinfodialogconflicts.cpp
index 7f297ec3..55fbf2e5 100644
--- a/src/modinfodialogconflicts.cpp
+++ b/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});
 }
diff --git a/src/modinfodialogconflicts.h b/src/modinfodialogconflicts.h
index 1f82a7c0..f499d1a4 100644
--- a/src/modinfodialogconflicts.h
+++ b/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);
diff --git a/src/modinfodialogtextfiles.cpp b/src/modinfodialogtextfiles.cpp
index 7c8e84c7..08b8c988 100644
--- a/src/modinfodialogtextfiles.cpp
+++ b/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;
-  }
+  m_model->finished();
+}
 
+void GenericFilesTab::onSelection(
+  const QModelIndex& current, const QModelIndex& previous)
+{
   if (!canClose()) {
-    m_list->setCurrentItem(previous, QItemSelectionModel::Current);
+    m_list->selectionModel()->select(previous, QItemSelectionModel::Current);
     return;
   }
 
-  select(item);
+  select(current);
 }
 
-void GenericFilesTab::select(FileListItem* item)
+void GenericFilesTab::select(const QModelIndex& index)
 {
-  if (item) {
-    m_editor->setEnabled(true);
-    m_editor->load(item->fullPath());
-  } else {
+  if (!index.isValid()) {
     m_editor->setEnabled(false);
+    return;
   }
+
+  m_editor->setEnabled(true);
+  m_editor->load(m_model->fullPath(index));
 }
 
 
diff --git a/src/modinfodialogtextfiles.h b/src/modinfodialogtextfiles.h
index 75f31d88..d879c2bd 100644
--- a/src/modinfodialogtextfiles.h
+++ b/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);
 };