Skip to content

Commit

Permalink
AutoMapping: Optimized reloading of rule maps
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bjorn committed Jan 29, 2025
1 parent ce589a1 commit a593a2a
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 19 deletions.
2 changes: 1 addition & 1 deletion src/tiled/automapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -674,7 +674,7 @@ void AutoMapper::setupRules()
#endif
}

void AutoMapper::prepareAutoMap(AutoMappingContext &context)
void AutoMapper::prepareAutoMap(AutoMappingContext &context) const
{
setupWorkMapLayers(context);

Expand Down
2 changes: 1 addition & 1 deletion src/tiled/automapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion src/tiled/automapperwrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
using namespace Tiled;

AutoMapperWrapper::AutoMapperWrapper(MapDocument *mapDocument,
const QVector<AutoMapper *> &autoMappers,
const QVector<const AutoMapper *> &autoMappers,
const QRegion &where,
const TileLayer *touchedLayer)
: PaintTileLayer(mapDocument)
Expand Down
2 changes: 1 addition & 1 deletion src/tiled/automapperwrapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class AutoMapperWrapper : public PaintTileLayer
{
public:
AutoMapperWrapper(MapDocument *mapDocument,
const QVector<AutoMapper*> &autoMappers,
const QVector<const AutoMapper *> &autoMappers,
const QRegion &where,
const TileLayer *touchedLayer = nullptr);
};
Expand Down
33 changes: 22 additions & 11 deletions src/tiled/automappingmanager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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<AutoMapper*> autoMappers;
autoMappers.reserve(mAutoMappers.size());
for (const auto &autoMapper : mAutoMappers) {
QVector<const AutoMapper*> 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())
Expand All @@ -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;
}

Expand Down Expand Up @@ -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<Map> rules { readMap(filePath, &errorString) };

Expand All @@ -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;
Expand Down Expand Up @@ -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();
}

Expand Down
17 changes: 13 additions & 4 deletions src/tiled/automappingmanager.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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 references by the rules file a new AutoMapper is
* setup. In this map we store all loaded AutoMappers instances.
*/
std::vector<std::unique_ptr<AutoMapper>> mAutoMappers;
std::unordered_map<QString, std::unique_ptr<AutoMapper>> 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<const AutoMapper*> mActiveAutoMappers;

/**
* This tells you if the rules for the current map document were already
Expand Down

0 comments on commit a593a2a

Please sign in to comment.