From 4cf19725b26c415d1e8aaca30a1fbbc923a309cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thorbj=C3=B8rn=20Lindeijer?= Date: Wed, 29 Jan 2025 18:44:05 +0100 Subject: [PATCH 1/2] AutoMapping: Optimized reloading of rule maps By only deleting the AutoMapper instances for changed rule map files, the reloading becomes much faster since it can just reuse the still loaded instances. --- src/tiled/automapper.cpp | 2 +- src/tiled/automapper.h | 2 +- src/tiled/automapperwrapper.cpp | 2 +- src/tiled/automapperwrapper.h | 2 +- src/tiled/automappingmanager.cpp | 33 +++++++++++++++++++++----------- src/tiled/automappingmanager.h | 17 ++++++++++++---- 6 files changed, 39 insertions(+), 19 deletions(-) diff --git a/src/tiled/automapper.cpp b/src/tiled/automapper.cpp index addfcddccb..f44c6634f9 100644 --- a/src/tiled/automapper.cpp +++ b/src/tiled/automapper.cpp @@ -674,7 +674,7 @@ void AutoMapper::setupRules() #endif } -void AutoMapper::prepareAutoMap(AutoMappingContext &context) +void AutoMapper::prepareAutoMap(AutoMappingContext &context) const { setupWorkMapLayers(context); diff --git a/src/tiled/automapper.h b/src/tiled/automapper.h index a8ca7da548..a869d5a9fe 100644 --- a/src/tiled/automapper.h +++ b/src/tiled/automapper.h @@ -351,7 +351,7 @@ class TILED_EDITOR_EXPORT AutoMapper * painful to keep these data structures up to date all time. (indices of * layers of the working map) */ - void prepareAutoMap(AutoMappingContext &context); + void prepareAutoMap(AutoMappingContext &context) const; /** * Here is done all the AutoMapping. diff --git a/src/tiled/automapperwrapper.cpp b/src/tiled/automapperwrapper.cpp index 3fc4ae1735..749b5fbce8 100644 --- a/src/tiled/automapperwrapper.cpp +++ b/src/tiled/automapperwrapper.cpp @@ -36,7 +36,7 @@ using namespace Tiled; AutoMapperWrapper::AutoMapperWrapper(MapDocument *mapDocument, - const QVector &autoMappers, + const QVector &autoMappers, const QRegion &where, const TileLayer *touchedLayer) : PaintTileLayer(mapDocument) diff --git a/src/tiled/automapperwrapper.h b/src/tiled/automapperwrapper.h index ae2bba35b8..a83a76a2b5 100644 --- a/src/tiled/automapperwrapper.h +++ b/src/tiled/automapperwrapper.h @@ -40,7 +40,7 @@ class AutoMapperWrapper : public PaintTileLayer { public: AutoMapperWrapper(MapDocument *mapDocument, - const QVector &autoMappers, + const QVector &autoMappers, const QRegion &where, const TileLayer *touchedLayer = nullptr); }; diff --git a/src/tiled/automappingmanager.cpp b/src/tiled/automappingmanager.cpp index b84070d0f4..7f9a095f5b 100644 --- a/src/tiled/automappingmanager.cpp +++ b/src/tiled/automappingmanager.cpp @@ -26,7 +26,6 @@ #include "logginginterface.h" #include "map.h" #include "mapdocument.h" -#include "preferences.h" #include "project.h" #include "projectmanager.h" #include "tilelayer.h" @@ -138,12 +137,12 @@ void AutomappingManager::autoMapInternal(const QRegion &where, // Determine the list of AutoMappers that is relevant for this map const QString mapFileName = QFileInfo(mMapDocument->fileName()).fileName(); - QVector autoMappers; - autoMappers.reserve(mAutoMappers.size()); - for (const auto &autoMapper : mAutoMappers) { + QVector autoMappers; + autoMappers.reserve(mActiveAutoMappers.size()); + for (auto autoMapper : mActiveAutoMappers) { const auto &mapNameFilter = autoMapper->mapNameFilter(); if (!mapNameFilter.isValid() || mapNameFilter.match(mapFileName).hasMatch()) - autoMappers.append(autoMapper.get()); + autoMappers.append(autoMapper); } if (autoMappers.isEmpty()) @@ -154,7 +153,7 @@ void AutomappingManager::autoMapInternal(const QRegion &where, if (touchedLayer) { if (std::none_of(autoMappers.cbegin(), autoMappers.cend(), - [=] (AutoMapper *autoMapper) { return autoMapper->ruleLayerNameUsed(touchedLayer->name()); })) + [=] (const AutoMapper *autoMapper) { return autoMapper->ruleLayerNameUsed(touchedLayer->name()); })) return; } @@ -254,6 +253,12 @@ bool AutomappingManager::loadRulesFile(const QString &filePath) bool AutomappingManager::loadRuleMap(const QString &filePath) { + auto it = mLoadedAutoMappers.find(filePath); + if (it != mLoadedAutoMappers.end()) { + mActiveAutoMappers.push_back(it->second.get()); + return true; + } + QString errorString; std::unique_ptr rules { readMap(filePath, &errorString) }; @@ -272,7 +277,9 @@ bool AutomappingManager::loadRuleMap(const QString &filePath) mWarning += autoMapper->warningString(); const QString error = autoMapper->errorString(); if (error.isEmpty()) { - mAutoMappers.push_back(std::move(autoMapper)); + auto autoMapperPtr = autoMapper.get(); + mLoadedAutoMappers.insert(std::make_pair(filePath, std::move(autoMapper))); + mActiveAutoMappers.push_back(autoMapperPtr); mWatcher.addPath(filePath); } else { mError += error; @@ -339,14 +346,18 @@ void AutomappingManager::refreshRulesFile(const QString &ruleFileOverride) void AutomappingManager::cleanUp() { - mAutoMappers.clear(); + mActiveAutoMappers.clear(); mLoaded = false; - if (!mWatcher.files().isEmpty()) - mWatcher.removePaths(mWatcher.files()); } -void AutomappingManager::onFileChanged() +void AutomappingManager::onFileChanged(const QString &path) { + // Make sure the changed file will be reloaded + mLoadedAutoMappers.erase(path); + + // File will be re-added when it is still relevant + mWatcher.removePath(path); + cleanUp(); } diff --git a/src/tiled/automappingmanager.h b/src/tiled/automappingmanager.h index 52f0750f6d..6f2bf7ebdd 100644 --- a/src/tiled/automappingmanager.h +++ b/src/tiled/automappingmanager.h @@ -82,7 +82,7 @@ class AutomappingManager : public QObject private: void onRegionEdited(const QRegion &where, TileLayer *touchedLayer); void onMapFileNameChanged(); - void onFileChanged(); + void onFileChanged(const QString &path); bool loadFile(const QString &filePath); bool loadRulesFile(const QString &filePath); @@ -107,10 +107,19 @@ class AutomappingManager : public QObject MapDocument *mMapDocument = nullptr; /** - * For each new file of rules a new AutoMapper is setup. In this vector we - * can store all of the AutoMappers in order. + * For each rule map referenced by the rules file a new AutoMapper is + * setup. In this map we store all loaded AutoMappers instances. */ - std::vector> mAutoMappers; + std::unordered_map> mLoadedAutoMappers; + + /** + * The active list of AutoMapper instances, in the order they were + * encountered in the rules file. + * + * Some loaded rule maps might not be active, and some might be active + * multiple times. + */ + std::vector mActiveAutoMappers; /** * This tells you if the rules for the current map document were already From 339b504123cb2c190ac7a6d9e7b7cdcb36fc3af3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thorbj=C3=B8rn=20Lindeijer?= Date: Thu, 30 Jan 2025 16:09:35 +0100 Subject: [PATCH 2/2] AutoMapping: Load rule maps on-demand This optimizes AutoMapping operations by not loading rule maps which do not apply to the current map based on file name filters. Also, re-parsing of the rules.txt file(s) now only happens when any of such files were changed, not when rule maps have changed. --- NEWS.md | 1 + src/tiled/automapper.cpp | 3 +- src/tiled/automapper.h | 10 +--- src/tiled/automappingmanager.cpp | 94 ++++++++++++++++++-------------- src/tiled/automappingmanager.h | 16 ++++-- 5 files changed, 67 insertions(+), 57 deletions(-) diff --git a/NEWS.md b/NEWS.md index 10271d4731..8cd392051f 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,6 +2,7 @@ * Added support for SVG 1.2 / CSS blending modes to layers (#3932) * AutoMapping: Don't match rules based on empty input indexes +* AutoMapping: Optimized reloading of rule maps and load rule maps on-demand * Raised minimum supported Qt version from 5.12 to 5.15 ### Tiled 1.11.2 (28 Jan 2025) diff --git a/src/tiled/automapper.cpp b/src/tiled/automapper.cpp index f44c6634f9..08b257457e 100644 --- a/src/tiled/automapper.cpp +++ b/src/tiled/automapper.cpp @@ -143,10 +143,9 @@ AutoMappingContext::AutoMappingContext(MapDocument *mapDocument) } -AutoMapper::AutoMapper(std::unique_ptr rulesMap, const QRegularExpression &mapNameFilter) +AutoMapper::AutoMapper(std::unique_ptr rulesMap) : mRulesMap(std::move(rulesMap)) , mRulesMapRenderer(MapRenderer::create(mRulesMap.get())) - , mMapNameFilter(mapNameFilter) { setupRuleMapProperties(); diff --git a/src/tiled/automapper.h b/src/tiled/automapper.h index a869d5a9fe..2dfc382cf4 100644 --- a/src/tiled/automapper.h +++ b/src/tiled/automapper.h @@ -31,7 +31,6 @@ #include #include #include -#include #include #include #include @@ -333,11 +332,10 @@ class TILED_EDITOR_EXPORT AutoMapper * @param rulesMap The map containing the AutoMapping rules. The * AutoMapper takes ownership of this map. */ - AutoMapper(std::unique_ptr rulesMap, const QRegularExpression &mapNameFilter = {}); + explicit AutoMapper(std::unique_ptr rulesMap); ~AutoMapper(); QString rulesMapFileName() const; - const QRegularExpression &mapNameFilter() const; /** * Checks if the passed \a ruleLayerName is used as input layer in this @@ -467,7 +465,6 @@ class TILED_EDITOR_EXPORT AutoMapper */ const std::unique_ptr mRulesMap; const std::unique_ptr mRulesMapRenderer; - const QRegularExpression mMapNameFilter; RuleMapSetup mRuleMapSetup; @@ -490,9 +487,4 @@ class TILED_EDITOR_EXPORT AutoMapper const TileLayer dummy; // used in case input layers are missing }; -inline const QRegularExpression &AutoMapper::mapNameFilter() const -{ - return mMapNameFilter; -} - } // namespace Tiled diff --git a/src/tiled/automappingmanager.cpp b/src/tiled/automappingmanager.cpp index 7f9a095f5b..16944204a1 100644 --- a/src/tiled/automappingmanager.cpp +++ b/src/tiled/automappingmanager.cpp @@ -109,21 +109,6 @@ void AutomappingManager::autoMapInternal(const QRegion &where, const bool automatic = touchedLayer != nullptr; - if (!mLoaded) { - if (mRulesFile.isEmpty()) { - mError = tr("No AutoMapping rules provided. Save the map or refer to a rule file in the project properties."); - emit errorsOccurred(automatic); - return; - } - - if (loadFile(mRulesFile)) { - mLoaded = true; - } else { - emit errorsOccurred(automatic); - return; - } - } - // Even if no AutoMapper instance will be executed, we still want to report // any warnings or errors that might have been reported while interpreting // the rule maps. @@ -135,14 +120,29 @@ void AutomappingManager::autoMapInternal(const QRegion &where, emit errorsOccurred(automatic); }); + if (!mLoaded) { + if (mRulesFile.isEmpty()) { + mError = tr("No AutoMapping rules provided. Save the map or refer to a rule file in the project properties."); + return; + } + + if (!loadFile(mRulesFile)) + return; + + mLoaded = true; + } + // Determine the list of AutoMappers that is relevant for this map const QString mapFileName = QFileInfo(mMapDocument->fileName()).fileName(); + QVector autoMappers; - autoMappers.reserve(mActiveAutoMappers.size()); - for (auto autoMapper : mActiveAutoMappers) { - const auto &mapNameFilter = autoMapper->mapNameFilter(); + autoMappers.reserve(mRuleMapReferences.size()); + + for (auto &ruleMap : std::as_const(mRuleMapReferences)) { + const auto &mapNameFilter = ruleMap.mapNameFilter; if (!mapNameFilter.isValid() || mapNameFilter.match(mapFileName).hasMatch()) - autoMappers.append(autoMapper); + if (const AutoMapper *autoMapper = findAutoMapper(ruleMap.filePath)) + autoMappers.append(autoMapper); } if (autoMappers.isEmpty()) @@ -164,6 +164,24 @@ void AutomappingManager::autoMapInternal(const QRegion &where, mMapDocument->undoStack()->push(aw); } +/** + * Returns the AutoMapper instance for the given rules file, loading it if + * necessary. Returns nullptr if the file could not be loaded. + */ +const AutoMapper *AutomappingManager::findAutoMapper(const QString &filePath) +{ + auto it = mLoadedAutoMappers.find(filePath); + if (it != mLoadedAutoMappers.end()) + return it->second.get(); + + auto autoMapper = loadRuleMap(filePath); + if (!autoMapper) + return nullptr; + + auto result = mLoadedAutoMappers.emplace(filePath, std::move(autoMapper)); + return result.first->second.get(); +} + /** * This function parses a rules file or loads a rules map file. * @@ -183,7 +201,8 @@ bool AutomappingManager::loadFile(const QString &filePath) return loadRulesFile(filePath); } - return loadRuleMap(filePath); + mRuleMapReferences.append(RuleMapReference { filePath, mMapNameFilter }); + return true; } bool AutomappingManager::loadRulesFile(const QString &filePath) @@ -251,41 +270,30 @@ bool AutomappingManager::loadRulesFile(const QString &filePath) return ret; } -bool AutomappingManager::loadRuleMap(const QString &filePath) +std::unique_ptr AutomappingManager::loadRuleMap(const QString &filePath) { - auto it = mLoadedAutoMappers.find(filePath); - if (it != mLoadedAutoMappers.end()) { - mActiveAutoMappers.push_back(it->second.get()); - return true; - } - QString errorString; - std::unique_ptr rules { readMap(filePath, &errorString) }; - - if (!rules) { + auto rulesMap = readMap(filePath, &errorString); + if (!rulesMap) { QString error = tr("Opening rules map '%1' failed: %2") .arg(filePath, errorString); ERROR(error); mError += error; mError += QLatin1Char('\n'); - return false; + return {}; } - std::unique_ptr autoMapper { new AutoMapper(std::move(rules), mMapNameFilter) }; + mWatcher.addPath(filePath); + + auto autoMapper = std::make_unique(std::move(rulesMap)); mWarning += autoMapper->warningString(); const QString error = autoMapper->errorString(); - if (error.isEmpty()) { - auto autoMapperPtr = autoMapper.get(); - mLoadedAutoMappers.insert(std::make_pair(filePath, std::move(autoMapper))); - mActiveAutoMappers.push_back(autoMapperPtr); - mWatcher.addPath(filePath); - } else { + if (!error.isEmpty()) mError += error; - } - return true; + return autoMapper; } /** @@ -346,7 +354,7 @@ void AutomappingManager::refreshRulesFile(const QString &ruleFileOverride) void AutomappingManager::cleanUp() { - mActiveAutoMappers.clear(); + mRuleMapReferences.clear(); mLoaded = false; } @@ -358,7 +366,9 @@ void AutomappingManager::onFileChanged(const QString &path) // File will be re-added when it is still relevant mWatcher.removePath(path); - cleanUp(); + // Re-parse the rules file(s) when any of them changed + if (path.endsWith(QLatin1String(".txt"), Qt::CaseInsensitive)) + cleanUp(); } #include "moc_automappingmanager.cpp" diff --git a/src/tiled/automappingmanager.h b/src/tiled/automappingmanager.h index 6f2bf7ebdd..77b6400127 100644 --- a/src/tiled/automappingmanager.h +++ b/src/tiled/automappingmanager.h @@ -30,7 +30,7 @@ #include #include -#include +#include namespace Tiled { @@ -39,6 +39,12 @@ class TileLayer; class AutoMapper; class MapDocument; +struct RuleMapReference +{ + QString filePath; + QRegularExpression mapNameFilter; +}; + /** * This class is a superior class to the AutoMapper and AutoMapperWrapper class. * It uses these classes to do the whole automapping process. @@ -84,9 +90,11 @@ class AutomappingManager : public QObject void onMapFileNameChanged(); void onFileChanged(const QString &path); + const AutoMapper *findAutoMapper(const QString &filePath); + bool loadFile(const QString &filePath); bool loadRulesFile(const QString &filePath); - bool loadRuleMap(const QString &filePath); + std::unique_ptr loadRuleMap(const QString &filePath); /** * Applies automapping to the region \a where. @@ -113,13 +121,13 @@ class AutomappingManager : public QObject std::unordered_map> mLoadedAutoMappers; /** - * The active list of AutoMapper instances, in the order they were + * The active list of rule map references, in the order they were * encountered in the rules file. * * Some loaded rule maps might not be active, and some might be active * multiple times. */ - std::vector mActiveAutoMappers; + QVector mRuleMapReferences; /** * This tells you if the rules for the current map document were already