From 12377465499100699f6148d6378992d4c39db226 Mon Sep 17 00:00:00 2001 From: AnyOldName3 Date: Sun, 6 Apr 2025 01:31:05 +0100 Subject: [PATCH 1/7] Be more careful when we tell Qt that data has changed Unchecking files only changes whether they're checked, and doesn't completely rearrange the table and change the number of elements it has, so we only need to change the check state, not the whole layout. It's way faster to just query all the data once after setting a content list than it is to query the data for all files between the old and new location of a file when we change any file's location in the load order. --- components/contentselector/model/contentmodel.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/components/contentselector/model/contentmodel.cpp b/components/contentselector/model/contentmodel.cpp index fe26d37b97..d7222f92f3 100644 --- a/components/contentselector/model/contentmodel.cpp +++ b/components/contentselector/model/contentmodel.cpp @@ -725,7 +725,6 @@ void ContentSelectorModel::ContentModel::setContentList(const QStringList& fileL if (filePosition < previousPosition) { mFiles.move(filePosition, previousPosition); - emit dataChanged(index(filePosition, 0, QModelIndex()), index(previousPosition, 0, QModelIndex())); } else { @@ -734,6 +733,7 @@ void ContentSelectorModel::ContentModel::setContentList(const QStringList& fileL } } checkForLoadOrderErrors(); + emit dataChanged(index(0, 0), index(rowCount(), columnCount())); } void ContentSelectorModel::ContentModel::checkForLoadOrderErrors() @@ -900,7 +900,6 @@ ContentSelectorModel::ContentFileList ContentSelectorModel::ContentModel::checke void ContentSelectorModel::ContentModel::uncheckAll() { - emit layoutAboutToBeChanged(); mCheckedFiles.clear(); - emit layoutChanged(); + emit dataChanged(index(0, 0), index(rowCount(), columnCount()), { Qt::CheckStateRole, Qt::UserRole + 1 }); } From 7bad2864d9354cc611390f88dc0c96376e81f581 Mon Sep 17 00:00:00 2001 From: AnyOldName3 Date: Sun, 6 Apr 2025 02:40:42 +0100 Subject: [PATCH 2/7] Reuse QIcon This saves more than 15% of launcher startup time on my machine (after the prior improvements - it's way less without those) --- apps/launcher/datafilespage.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/launcher/datafilespage.cpp b/apps/launcher/datafilespage.cpp index 9bb54fdb8e..36532a7d84 100644 --- a/apps/launcher/datafilespage.cpp +++ b/apps/launcher/datafilespage.cpp @@ -351,6 +351,8 @@ void Launcher::DataFilesPage::populateFileViews(const QString& contentModelName) if (!resourcesVfs.isEmpty()) directories.insert(0, { resourcesVfs }); + QIcon containsDataIcon(":/images/openmw-plugin.png"); + std::unordered_set visitedDirectories; for (const Config::SettingValue& currentDir : directories) { @@ -402,7 +404,7 @@ void Launcher::DataFilesPage::populateFileViews(const QString& contentModelName) // Add a "data file" icon if the directory contains a content file if (mSelector->containsDataFiles(currentDir.value)) { - item->setIcon(QIcon(":/images/openmw-plugin.png")); + item->setIcon(containsDataIcon); tooltip << tr("Contains content file(s)"); } From 973282e471154b7821ffbfaa6ad62d8a19668722 Mon Sep 17 00:00:00 2001 From: AnyOldName3 Date: Sun, 6 Apr 2025 02:45:28 +0100 Subject: [PATCH 3/7] Optimise ContentSelectorModel::ContentModel::item This saves about 5% of remaining launcher startup time Not using fileProperty avoids loads of QVariant conversions. --- components/contentselector/model/contentmodel.cpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/components/contentselector/model/contentmodel.cpp b/components/contentselector/model/contentmodel.cpp index d7222f92f3..4ec7324e5d 100644 --- a/components/contentselector/model/contentmodel.cpp +++ b/components/contentselector/model/contentmodel.cpp @@ -78,14 +78,10 @@ ContentSelectorModel::EsmFile* ContentSelectorModel::ContentModel::item(int row) } const ContentSelectorModel::EsmFile* ContentSelectorModel::ContentModel::item(const QString& name) const { - EsmFile::FileProperty fp = EsmFile::FileProperty_FileName; - - if (name.contains('/')) - fp = EsmFile::FileProperty_FilePath; - + bool path = name.contains('/'); for (const EsmFile* file : mFiles) { - if (name.compare(file->fileProperty(fp).toString(), Qt::CaseInsensitive) == 0) + if (name.compare(path ? file->filePath() : file->fileName(), Qt::CaseInsensitive) == 0) return file; } return nullptr; From e779f115efc5fd88ae3cbaca479c93ea9c998749 Mon Sep 17 00:00:00 2001 From: AnyOldName3 Date: Mon, 7 Apr 2025 16:11:27 +0100 Subject: [PATCH 4/7] Exclude directories from containsDataFiles Also include capo's microoptimisation even though it doesn't make things any faster. --- components/contentselector/model/contentmodel.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/components/contentselector/model/contentmodel.cpp b/components/contentselector/model/contentmodel.cpp index 4ec7324e5d..9c161cf0ff 100644 --- a/components/contentselector/model/contentmodel.cpp +++ b/components/contentselector/model/contentmodel.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -548,15 +549,13 @@ void ContentSelectorModel::ContentModel::addFiles(const QString& path, bool newf bool ContentSelectorModel::ContentModel::containsDataFiles(const QString& path) { - QDir dir(path); QStringList filters; filters << "*.esp" << "*.esm" << "*.omwgame" << "*.omwaddon"; - dir.setNameFilters(filters); - - return dir.entryList().count() != 0; + QDirIterator it(path, filters, QDir::Files | QDir::NoDotAndDotDot); + return it.hasNext(); } void ContentSelectorModel::ContentModel::clearFiles() From d6b61f1f54b70b8e30b11e09c82c3181688d2dc8 Mon Sep 17 00:00:00 2001 From: AnyOldName3 Date: Tue, 8 Apr 2025 00:34:45 +0100 Subject: [PATCH 5/7] Sprinkle some const& QStringView required more fighting as loads of call sites take a const& --- components/contentselector/model/esmfile.hpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/components/contentselector/model/esmfile.hpp b/components/contentselector/model/esmfile.hpp index 28b4bd2822..d040dbd04d 100644 --- a/components/contentselector/model/esmfile.hpp +++ b/components/contentselector/model/esmfile.hpp @@ -48,18 +48,18 @@ namespace ContentSelectorModel void addGameFile(const QString& name) { mGameFiles.append(name); } QVariant fileProperty(const FileProperty prop) const; - QString fileName() const { return mFileName; } - QString author() const { return mAuthor; } + const QString& fileName() const { return mFileName; } + const QString& author() const { return mAuthor; } QDateTime modified() const { return mModified; } - QString formatVersion() const { return mVersion; } - QString filePath() const { return mPath; } + const QString& formatVersion() const { return mVersion; } + const QString& filePath() const { return mPath; } bool builtIn() const { return mBuiltIn; } bool fromAnotherConfigFile() const { return mFromAnotherConfigFile; } bool isMissing() const { return mPath.isEmpty(); } /// @note Contains file names, not paths. const QStringList& gameFiles() const { return mGameFiles; } - QString description() const { return mDescription; } + const QString& description() const { return mDescription; } QString toolTip() const { if (isMissing()) From 894ea4ba626914ee56f5763f1ec5b0ab02a27d54 Mon Sep 17 00:00:00 2001 From: AnyOldName3 Date: Tue, 8 Apr 2025 01:19:24 +0100 Subject: [PATCH 6/7] Don't precompute load order errors after every change It's much slower than doing it on demand as it only takes a microsecond, but for a really big load order, there are hundreds of thousands of intermediate calls before everything's set up and we can draw the GUI. --- .../contentselector/model/contentmodel.cpp | 32 ++++--------------- .../contentselector/model/contentmodel.hpp | 4 --- .../contentselector/view/contentselector.cpp | 2 -- 3 files changed, 7 insertions(+), 31 deletions(-) diff --git a/components/contentselector/model/contentmodel.cpp b/components/contentselector/model/contentmodel.cpp index 9c161cf0ff..5f0d41d38c 100644 --- a/components/contentselector/model/contentmodel.cpp +++ b/components/contentselector/model/contentmodel.cpp @@ -307,7 +307,6 @@ bool ContentSelectorModel::ContentModel::setData(const QModelIndex& index, const { setCheckState(file->filePath(), success); emit dataChanged(index, index); - checkForLoadOrderErrors(); } else return success; @@ -422,7 +421,6 @@ bool ContentSelectorModel::ContentModel::dropMimeData( dataChanged(index(minRow, 0), index(maxRow, 0)); // at this point we know that drag and drop has finished. - checkForLoadOrderErrors(); return true; } @@ -702,12 +700,13 @@ void ContentSelectorModel::ContentModel::setNonUserContent(const QStringList& fi bool ContentSelectorModel::ContentModel::isLoadOrderError(const EsmFile* file) const { - return mPluginsWithLoadOrderError.contains(file->filePath()); + int index = indexFromItem(file).row(); + auto errors = checkForLoadOrderErrors(file, index); + return !errors.empty(); } void ContentSelectorModel::ContentModel::setContentList(const QStringList& fileList) { - mPluginsWithLoadOrderError.clear(); int previousPosition = -1; for (const QString& filepath : fileList) { @@ -727,27 +726,9 @@ void ContentSelectorModel::ContentModel::setContentList(const QStringList& fileL } } } - checkForLoadOrderErrors(); emit dataChanged(index(0, 0), index(rowCount(), columnCount())); } -void ContentSelectorModel::ContentModel::checkForLoadOrderErrors() -{ - for (int row = 0; row < mFiles.count(); ++row) - { - EsmFile* file = mFiles.at(row); - bool isRowInError = checkForLoadOrderErrors(file, row).count() != 0; - if (isRowInError) - { - mPluginsWithLoadOrderError.insert(file->filePath()); - } - else - { - mPluginsWithLoadOrderError.remove(file->filePath()); - } - } -} - QList ContentSelectorModel::ContentModel::checkForLoadOrderErrors( const EsmFile* file, int row) const { @@ -786,11 +767,12 @@ QList ContentSelectorModel::ContentModel:: QString ContentSelectorModel::ContentModel::toolTip(const EsmFile* file) const { - if (isLoadOrderError(file)) + int index = indexFromItem(file).row(); + auto errors = checkForLoadOrderErrors(file, index); + if (!errors.empty()) { QString text(""); - int index = indexFromItem(item(file->filePath())).row(); - for (const LoadOrderError& error : checkForLoadOrderErrors(file, index)) + for (const LoadOrderError& error : errors) { assert(error.errorCode() != LoadOrderError::ErrorCode::ErrorCode_None); diff --git a/components/contentselector/model/contentmodel.hpp b/components/contentselector/model/contentmodel.hpp index 467a9c032a..3eba939125 100644 --- a/components/contentselector/model/contentmodel.hpp +++ b/components/contentselector/model/contentmodel.hpp @@ -69,9 +69,6 @@ namespace ContentSelectorModel void refreshModel(); - /// Checks all plug-ins for load order errors and updates mPluginsWithLoadOrderError with plug-ins with issues - void checkForLoadOrderErrors(); - private: void addFile(EsmFile* file); @@ -89,7 +86,6 @@ namespace ContentSelectorModel QStringList mNonUserContent; std::set mCheckedFiles; QHash mNewFiles; - QSet mPluginsWithLoadOrderError; QString mEncoding; QIcon mWarningIcon; QIcon mErrorIcon; diff --git a/components/contentselector/view/contentselector.cpp b/components/contentselector/view/contentselector.cpp index bce136b335..0be6e7c023 100644 --- a/components/contentselector/view/contentselector.cpp +++ b/components/contentselector/view/contentselector.cpp @@ -211,7 +211,6 @@ void ContentSelectorView::ContentSelector::addFiles(const QString& path, bool ne ui->gameFileView->setCurrentIndex(0); mContentModel->uncheckAll(); - mContentModel->checkForLoadOrderErrors(); } void ContentSelectorView::ContentSelector::sortFiles() @@ -254,7 +253,6 @@ void ContentSelectorView::ContentSelector::slotCurrentGameFileIndexChanged(int i oldIndex = index; setGameFileSelected(index, true); - mContentModel->checkForLoadOrderErrors(); } emit signalCurrentGamefileIndexChanged(index); From 096759435a40f94f5098db9a379d20d12704efd4 Mon Sep 17 00:00:00 2001 From: AnyOldName3 Date: Wed, 9 Apr 2025 01:36:52 +0100 Subject: [PATCH 7/7] Add progress bars where the launcher can be limited by IO I tested this with a USB3 external hard drive. These two places were the only ones where we're IO-bound and block the main thread, so they're the only ones that need progress bars. If trying to replicate this test, then it's important to unplug the hard drive between each repeat. Apparently Windows is excellent at disk caching these days as it takes a minute and a half to start the launcher with Total Overhaul on this drive when it's just been plugged in, but less time than the first launch after a reboot on an NVME drive once the cache has been warmed up. --- apps/launcher/datafilespage.cpp | 7 +++++++ components/config/gamesettings.cpp | 8 ++++++++ 2 files changed, 15 insertions(+) diff --git a/apps/launcher/datafilespage.cpp b/apps/launcher/datafilespage.cpp index 36532a7d84..16ece6d34c 100644 --- a/apps/launcher/datafilespage.cpp +++ b/apps/launcher/datafilespage.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include @@ -353,9 +354,15 @@ void Launcher::DataFilesPage::populateFileViews(const QString& contentModelName) QIcon containsDataIcon(":/images/openmw-plugin.png"); + QProgressDialog progressBar("Adding data directories", {}, 0, directories.count(), this); + progressBar.setWindowModality(Qt::WindowModal); + progressBar.setValue(0); + std::unordered_set visitedDirectories; for (const Config::SettingValue& currentDir : directories) { + progressBar.setValue(progressBar.value() + 1); + if (!visitedDirectories.insert(currentDir.value).second) continue; diff --git a/components/config/gamesettings.cpp b/components/config/gamesettings.cpp index f318cec4a4..36373f8f35 100644 --- a/components/config/gamesettings.cpp +++ b/components/config/gamesettings.cpp @@ -1,6 +1,7 @@ #include "gamesettings.hpp" #include +#include #include #include @@ -37,8 +38,13 @@ void Config::GameSettings::validatePaths() mDataDirs.clear(); + QProgressDialog progressBar("Validating paths", {}, 0, paths.count() + 1); + progressBar.setWindowModality(Qt::WindowModal); + progressBar.setValue(0); + for (const auto& dataDir : paths) { + progressBar.setValue(progressBar.value() + 1); if (QDir(dataDir.value).exists()) { SettingValue copy = dataDir; @@ -50,6 +56,8 @@ void Config::GameSettings::validatePaths() // Do the same for data-local const QString& local = mSettings.value(QString("data-local")).value; + progressBar.setValue(progressBar.value() + 1); + if (!local.isEmpty() && QDir(local).exists()) { mDataLocal = QDir(local).canonicalPath();