AutoMapping: Ignore rules with empty input or output regions (#4115)

Rules with an empty input region were causing issues, because they don't have a valid bounding rectangle. They are also commonly created by accident. If intended as a rule that always matches, such a rule can be made by placing the special "Ignore" tile in the rule's input. Rules with empty output region are no longer matched because they would not produce any output anyway. As a drive-by, AutoMapper no longer derives from QObject, which appears to have only been done to access the tr() function. This is now made available using Q_DECLARE_TR_FUNCTIONS instead. Closes #3834
mapeditor · Jan 9, 2025 · 6aad5ea · 6aad5ea
1 parent 7725ed2
commit 6aad5ea
Show file tree

Hide file tree

Showing 8 changed files with 54 additions and 7 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -6,6 +6,7 @@
 * Scripting: Added `Tileset.transformationFlags` (#3753)
 * Scripting: Added `Dialog.addRadioButtonGroup` for selecting one of a list of mutually exclusive options (#4107)
 * Scripting: Made `currentWangSet` and `currentWangColorIndex` properties writeable (#4105)
+* AutoMapping: Ignore rules with empty input or output regions (#3834)
 * Fixed saving/loading of custom properties set on worlds (#4025)
 * Fixed issue with placing tile objects after switching maps (#3497)
 * Fixed crash when accessing a world through a symlink (#4042)

diff --git a/src/tiled/automapper.cpp b/src/tiled/automapper.cpp
@@ -639,9 +639,16 @@ void AutoMapper::setupRules()
     mRules.reserve(combinedRegions.size());
 
     for (const QRegion &combinedRegion : combinedRegions) {
+        QRegion inputRegion = combinedRegion & regionInput;
+        QRegion outputRegion = combinedRegion & regionOutput;
+
+        // Skip rules where either input or output region is empty
+        if (inputRegion.isEmpty() || outputRegion.isEmpty())
+            continue;
+
         Rule &rule = mRules.emplace_back();
-        rule.inputRegion = combinedRegion & regionInput;
-        rule.outputRegion = combinedRegion & regionOutput;
+        rule.inputRegion = std::move(inputRegion);
+        rule.outputRegion = std::move(outputRegion);
         rule.options = mRuleOptions;
 
         for (const auto &optionsArea : setup.mRuleOptionsAreas)
@@ -1474,5 +1481,3 @@ void AutoMapper::addWarning(const QString &message, std::function<void ()> callb
 }
 
 } // namespace Tiled
-
-#include "moc_automapper.cpp"
diff --git a/src/tiled/automapper.h b/src/tiled/automapper.h
@@ -27,6 +27,7 @@
 #include "tilelayer.h"
 #include "tileset.h"
 
+#include <QCoreApplication>
 #include <QList>
 #include <QMap>
 #include <QRegion>
@@ -275,9 +276,9 @@ struct TILED_EDITOR_EXPORT AutoMappingContext
  * - copy regions of Maps (multiple Layers, the layerlist is a
  *                         lookup-table for matching the Layers)
  */
-class TILED_EDITOR_EXPORT AutoMapper : public QObject
+class TILED_EDITOR_EXPORT AutoMapper
 {
-    Q_OBJECT
+    Q_DECLARE_TR_FUNCTIONS(Tiled::AutoMapper)
 
 public:
     struct Options
@@ -333,7 +334,7 @@ class TILED_EDITOR_EXPORT AutoMapper : public QObject
      *                 AutoMapper takes ownership of this map.
      */
     AutoMapper(std::unique_ptr<Map> rulesMap, const QRegularExpression &mapNameFilter = {});
-    ~AutoMapper() override;
+    ~AutoMapper();
 
     QString rulesMapFileName() const;
     const QRegularExpression &mapNameFilter() const;

diff --git a/tests/automapping/ignore-empty-region/map-result.tmx b/tests/automapping/ignore-empty-region/map-result.tmx
@@ -0,0 +1,10 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<map version="1.11" tiledversion="1.11.0" orientation="orthogonal" renderorder="right-down" width="2" height="2" tilewidth="16" tileheight="16" infinite="0" nextlayerid="6" nextobjectid="1">
+ <tileset firstgid="1" source="../spr_test_tileset.tsx"/>
+ <layer id="1" name="set" width="2" height="2">
+  <data encoding="csv">
+2,2,
+2,2
+</data>
+ </layer>
+</map>
diff --git a/tests/automapping/ignore-empty-region/map.tmx b/tests/automapping/ignore-empty-region/map.tmx
@@ -0,0 +1,10 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<map version="1.11" tiledversion="1.11.0" orientation="orthogonal" renderorder="right-down" width="2" height="2" tilewidth="16" tileheight="16" infinite="0" nextlayerid="6" nextobjectid="1">
+ <tileset firstgid="1" source="../spr_test_tileset.tsx"/>
+ <layer id="1" name="set" width="2" height="2">
+  <data encoding="csv">
+2,2,
+2,2
+</data>
+ </layer>
+</map>
diff --git a/tests/automapping/ignore-empty-region/rules.tmx b/tests/automapping/ignore-empty-region/rules.tmx
@@ -0,0 +1,18 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<map version="1.11" tiledversion="1.11.0" orientation="orthogonal" renderorder="right-down" width="5" height="3" tilewidth="16" tileheight="16" infinite="0" nextlayerid="12" nextobjectid="2">
+ <tileset firstgid="1" source="../spr_test_tileset.tsx"/>
+ <layer id="6" name="input_set" width="5" height="3">
+  <data encoding="csv">
+0,0,0,0,0,
+0,3,0,0,0,
+0,0,0,0,0
+</data>
+ </layer>
+ <layer id="2" name="output_set" width="5" height="3">
+  <data encoding="csv">
+0,0,0,0,0,
+0,0,0,4,0,
+0,0,0,0,0
+</data>
+ </layer>
+</map>
diff --git a/tests/automapping/ignore-empty-region/rules.txt b/tests/automapping/ignore-empty-region/rules.txt
@@ -0,0 +1 @@
+./rules.tmx
diff --git a/tests/automapping/test_automapping.cpp b/tests/automapping/test_automapping.cpp
@@ -22,6 +22,7 @@ void test_AutoMapping::autoMap_data()
 {
     QTest::addColumn<QString>("directory");
 
+    QTest::newRow("ignore-empty-region") << QStringLiteral("ignore-empty-region");
     QTest::newRow("ignore-flip") << QStringLiteral("ignore-flip");
     QTest::newRow("infinite-target-map") << QStringLiteral("infinite-target-map");
     QTest::newRow("inputnot") << QStringLiteral("inputnot");