Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AutoMapping: Optimized reloading of rule maps #4165

Merged
merged 2 commits into from
Jan 30, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
5 changes: 2 additions & 3 deletions src/tiled/automapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -143,10 +143,9 @@ AutoMappingContext::AutoMappingContext(MapDocument *mapDocument)
}


AutoMapper::AutoMapper(std::unique_ptr<Map> rulesMap, const QRegularExpression &mapNameFilter)
AutoMapper::AutoMapper(std::unique_ptr<Map> rulesMap)
: mRulesMap(std::move(rulesMap))
, mRulesMapRenderer(MapRenderer::create(mRulesMap.get()))
, mMapNameFilter(mapNameFilter)
{
setupRuleMapProperties();

Expand Down Expand Up @@ -674,7 +673,7 @@ void AutoMapper::setupRules()
#endif
}

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

Expand Down
12 changes: 2 additions & 10 deletions src/tiled/automapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
#include <QList>
#include <QMap>
#include <QRegion>
#include <QRegularExpression>
#include <QSet>
#include <QString>
#include <QVector>
Expand Down Expand Up @@ -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<Map> rulesMap, const QRegularExpression &mapNameFilter = {});
explicit AutoMapper(std::unique_ptr<Map> rulesMap);
~AutoMapper();

QString rulesMapFileName() const;
const QRegularExpression &mapNameFilter() const;

/**
* Checks if the passed \a ruleLayerName is used as input layer in this
Expand All @@ -351,7 +349,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 Expand Up @@ -467,7 +465,6 @@ class TILED_EDITOR_EXPORT AutoMapper
*/
const std::unique_ptr<Map> mRulesMap;
const std::unique_ptr<MapRenderer> mRulesMapRenderer;
const QRegularExpression mMapNameFilter;

RuleMapSetup mRuleMapSetup;

Expand All @@ -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
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
101 changes: 61 additions & 40 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 @@ -110,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.
Expand All @@ -136,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<AutoMapper*> autoMappers;
autoMappers.reserve(mAutoMappers.size());
for (const auto &autoMapper : mAutoMappers) {
const auto &mapNameFilter = autoMapper->mapNameFilter();

QVector<const AutoMapper*> autoMappers;
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.get());
if (const AutoMapper *autoMapper = findAutoMapper(ruleMap.filePath))
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 All @@ -165,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.
*
Expand All @@ -184,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)
Expand Down Expand Up @@ -252,33 +270,30 @@ bool AutomappingManager::loadRulesFile(const QString &filePath)
return ret;
}

bool AutomappingManager::loadRuleMap(const QString &filePath)
std::unique_ptr<AutoMapper> AutomappingManager::loadRuleMap(const QString &filePath)
{
QString errorString;
std::unique_ptr<Map> 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> autoMapper { new AutoMapper(std::move(rules), mMapNameFilter) };
mWatcher.addPath(filePath);

auto autoMapper = std::make_unique<AutoMapper>(std::move(rulesMap));

mWarning += autoMapper->warningString();
const QString error = autoMapper->errorString();
if (error.isEmpty()) {
mAutoMappers.push_back(std::move(autoMapper));
mWatcher.addPath(filePath);
} else {
if (!error.isEmpty())
mError += error;
}

return true;
return autoMapper;
}

/**
Expand Down Expand Up @@ -339,15 +354,21 @@ void AutomappingManager::refreshRulesFile(const QString &ruleFileOverride)

void AutomappingManager::cleanUp()
{
mAutoMappers.clear();
mRuleMapReferences.clear();
mLoaded = false;
if (!mWatcher.files().isEmpty())
mWatcher.removePaths(mWatcher.files());
}

void AutomappingManager::onFileChanged()
void AutomappingManager::onFileChanged(const QString &path)
{
cleanUp();
// Make sure the changed file will be reloaded
mLoadedAutoMappers.erase(path);

// File will be re-added when it is still relevant
mWatcher.removePath(path);

// Re-parse the rules file(s) when any of them changed
if (path.endsWith(QLatin1String(".txt"), Qt::CaseInsensitive))
cleanUp();
}

#include "moc_automappingmanager.cpp"
29 changes: 23 additions & 6 deletions src/tiled/automappingmanager.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
#include <QString>

#include <memory>
#include <vector>
#include <unordered_map>

namespace Tiled {

Expand All @@ -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.
Expand Down Expand Up @@ -82,11 +88,13 @@ class AutomappingManager : public QObject
private:
void onRegionEdited(const QRegion &where, TileLayer *touchedLayer);
void onMapFileNameChanged();
void onFileChanged();
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<AutoMapper> loadRuleMap(const QString &filePath);

/**
* Applies automapping to the region \a where.
Expand All @@ -107,10 +115,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::unordered_map<QString, std::unique_ptr<AutoMapper>> mLoadedAutoMappers;

/**
* 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<std::unique_ptr<AutoMapper>> mAutoMappers;
QVector<RuleMapReference> mRuleMapReferences;

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