Fixed "Offset Map" action for infinite maps

When used without selection it would clip the map to its boundaries,
which is not expected in case of infinite maps.

Closes #1866

NEWS.md

@@ -4,12 +4,13 @@
 * Fixed hang when loading map file with empty compressed layer data
 * Fixed selection of tile stamp to work on mouse click
 * Fixed tools not being up to date on modifier keys after activation
+* Fixed "Offset Map" action for infinite maps (#1866)
 * Templates view: Keep template centered when resizing view
 * Tile Collision Editor: Keep tile centered when resizing view
 * Tile Collision Editor: Display tool info text in status bar
-* JSON plugin: Fixed reading of infinite maps
-* libtiled-java: Fixed some bugs (by Henry Wang)
-* libtiled-java: Fixed tile offset value not being considered (by digitalhoax)
+* JSON plugin: Fixed reading of infinite maps (#1858)
+* libtiled-java: Fixed some bugs (by Henry Wang, #1840)
+* libtiled-java: Fixed tile offset value not being considered (by digitalhoax, #1863)
 
 ### Tiled 1.1.1 (4 January 2018)
 

src/libtiled/objectgroup.cpp

@@ -159,19 +159,24 @@ void ObjectGroup::offsetObjects(const QPointF &offset,
                                 const QRectF &bounds,
                                 bool wrapX, bool wrapY)
 {
+    if (offset.isNull())
+        return;
+
+    const bool boundsValid = bounds.isValid();
+
     for (MapObject *object : mObjects) {
         const QPointF objectCenter = object->bounds().center();
-        if (!bounds.contains(objectCenter))
+        if (boundsValid && !bounds.contains(objectCenter))
             continue;
 
         QPointF newCenter(objectCenter + offset);
 
-        if (wrapX && bounds.width() > 0) {
+        if (wrapX && boundsValid) {
             qreal nx = std::fmod(newCenter.x() - bounds.left(), bounds.width());
             newCenter.setX(bounds.left() + (nx < 0 ? bounds.width() + nx : nx));
         }
 
-        if (wrapY && bounds.height() > 0) {
+        if (wrapY && boundsValid) {
             qreal ny = std::fmod(newCenter.y() - bounds.top(), bounds.height());
             newCenter.setY(bounds.top() + (ny < 0 ? bounds.height() + ny : ny));
         }

src/libtiled/tilelayer.cpp

@@ -574,10 +574,20 @@ void TileLayer::resize(const QSize &size, const QPoint &offset)
     setSize(size);
 }
 
+static int clampWrap(int value, int min, int max)
+{
+    int v = value - min;
+    int d = max - min;
+    return (v < 0 ? (v + 1) % d + d - 1 : v % d) + min;
+}
+
 void TileLayer::offsetTiles(const QPoint &offset,
                             const QRect &bounds,
                             bool wrapX, bool wrapY)
 {
+    if (offset.isNull())
+        return;
+
     QScopedPointer<TileLayer> newLayer(clone());
 
     for (int y = bounds.top(); y <= bounds.bottom(); ++y) {
@@ -587,20 +597,12 @@ void TileLayer::offsetTiles(const QPoint &offset,
             int oldY = y - offset.y();
 
             // Wrap x value that will be pulled from
-            if (wrapX && bounds.width() > 0) {
-                while (oldX < bounds.left())
-                    oldX += bounds.width();
-                while (oldX > bounds.right())
-                    oldX -= bounds.width();
-            }
+            if (wrapX)
+                oldX = clampWrap(oldX, bounds.left(), bounds.right() + 1);
 
             // Wrap y value that will be pulled from
-            if (wrapY && bounds.height() > 0) {
-                while (oldY < bounds.top())
-                    oldY += bounds.height();
-                while (oldY > bounds.bottom())
-                    oldY -= bounds.height();
-            }
+            if (wrapY)
+                oldY = clampWrap(oldY, bounds.top(), bounds.bottom() + 1);
 
             // Set the new tile
             if (bounds.contains(oldX, oldY))
@@ -614,6 +616,34 @@ void TileLayer::offsetTiles(const QPoint &offset,
     mBounds = newLayer->mBounds;
 }
 
+void TileLayer::offsetTiles(const QPoint &offset)
+{
+    QScopedPointer<TileLayer> newLayer(new TileLayer(QString(), 0, 0, 0, 0));
+
+    // Process only the allocated chunks
+    QHashIterator<QPoint, Chunk> it(mChunks);
+    while (it.hasNext()) {
+        it.next();
+
+        const QPoint p = it.key();
+        const Chunk &chunk = it.value();
+        const QRect r(p.x() * CHUNK_SIZE,
+                      p.y() * CHUNK_SIZE,
+                      CHUNK_SIZE, CHUNK_SIZE);
+
+        for (int y = r.top(); y <= r.bottom(); ++y) {
+            for (int x = r.left(); x <= r.right(); ++x) {
+                int newX = x + offset.x();
+                int newY = y + offset.y();
+                newLayer->setCell(newX, newY, chunk.cellAt(x - r.left(), y - r.top()));
+            }
+        }
+    }
+
+    mChunks = newLayer->mChunks;
+    mBounds = newLayer->mBounds;
+}
+
 bool TileLayer::canMergeWith(Layer *other) const
 {
     return other->isTileLayer();

src/libtiled/tilelayer.h

@@ -481,6 +481,13 @@ class TILEDSHARED_EXPORT TileLayer : public Layer
                      const QRect &bounds,
                      bool wrapX, bool wrapY);
 
+    /**
+     * Offsets the tiles in this layer by \a offset.
+     *
+     * \sa ObjectGroup::offsetObjects()
+     */
+    void offsetTiles(const QPoint &offset);
+
     bool canMergeWith(Layer *other) const override;
     Layer *mergedWith(Layer *other) const override;
 

src/tiled/mapdocument.cpp

@@ -358,17 +358,12 @@ void MapDocument::offsetMap(const QList<Layer*> &layers,
     if (layers.empty())
         return;
 
-    if (layers.size() == 1) {
-        mUndoStack->push(new OffsetLayer(this, layers.first(), offset,
+    mUndoStack->beginMacro(tr("Offset Map"));
+    for (auto layer : layers) {
+        mUndoStack->push(new OffsetLayer(this, layer, offset,
                                          bounds, wrapX, wrapY));
-    } else {
-        mUndoStack->beginMacro(tr("Offset Map"));
-        for (auto layer : layers) {
-            mUndoStack->push(new OffsetLayer(this, layer, offset,
-                                             bounds, wrapX, wrapY));
-        }
-        mUndoStack->endMacro();
     }
+    mUndoStack->endMacro();
 }
 
 /**

src/tiled/offsetlayer.cpp

@@ -31,9 +31,18 @@
 
 #include <QCoreApplication>
 
+#include "qtcompat_p.h"
+
 using namespace Tiled;
 using namespace Tiled::Internal;
 
+/**
+ * Creates an undo command that offsets the layer at \a index by \a offset,
+ * within \a bounds, and can optionally wrap on the x or y axis.
+ *
+ * If \a bounds is empty, the \a offset is applied everywhere and the wrapping
+ * is ignored.
+ */
 OffsetLayer::OffsetLayer(MapDocument *mapDocument,
                          Layer *layer,
                          const QPoint &offset,
@@ -50,11 +59,14 @@ OffsetLayer::OffsetLayer(MapDocument *mapDocument,
     switch (mOriginalLayer->layerType()) {
     case Layer::TileLayerType:
         mOffsetLayer = layer->clone();
-        static_cast<TileLayer*>(mOffsetLayer)->offsetTiles(offset, bounds, wrapX, wrapY);
+        if (bounds.isEmpty())
+            static_cast<TileLayer*>(mOffsetLayer)->offsetTiles(offset);
+        else
+            static_cast<TileLayer*>(mOffsetLayer)->offsetTiles(offset, bounds, wrapX, wrapY);
         break;
     case Layer::ObjectGroupType:
         mOffsetLayer = layer->clone();
-        // fall through
+        Q_FALLTHROUGH();
     case Layer::ImageLayerType:
     case Layer::GroupLayerType: {
         // These layers need offset and bounds converted to pixel units

src/tiled/offsetlayer.h

@@ -39,10 +39,6 @@ class MapDocument;
 class OffsetLayer : public QUndoCommand
 {
 public:
-    /**
-     * Creates an undo command that offsets the layer at \a index by \a offset,
-     * within \a bounds, and can optionally wrap on the x or y axis.
-     */
     OffsetLayer(MapDocument *mapDocument,
                 Layer *layer,
                 const QPoint &offset,

src/tiled/offsetmapdialog.cpp

@@ -37,15 +37,17 @@ OffsetMapDialog::OffsetMapDialog(MapDocument *mapDocument, QWidget *parent)
     mUi->setupUi(this);
     setWindowFlags(windowFlags() & ~Qt::WindowContextHelpButtonHint);
 
-    if (mMapDocument->selectedArea().isEmpty())
-        disableBoundsSelectionCurrentArea();
-    else
-        mUi->boundsSelection->setCurrentIndex(1);
-
-    if (mMapDocument->map()->infinite()) {
-        mUi->wrapX->setEnabled(false);
-        mUi->wrapY->setEnabled(false);
+    if (mMapDocument->selectedArea().isEmpty()) {
+        setBoundsSelection(WholeMap);
+        mUi->boundsSelection->setEnabled(false);
+    } else {
+        setBoundsSelection(CurrentSelectionArea);
     }
+
+    boundsSelectionChanged();   // updates wrap checkboxes
+
+    connect(mUi->boundsSelection, SIGNAL(currentIndexChanged(int)),
+            this, SLOT(boundsSelectionChanged()));
 }
 
 OffsetMapDialog::~OffsetMapDialog()
@@ -78,22 +80,20 @@ QList<Layer *> OffsetMapDialog::affectedLayers() const
     return layers;
 }
 
+/**
+ * Returns the bounding rect that is to be affected by the offset operation.
+ *
+ * For infinite maps, when not using the currently selected area, the returned
+ * rect is empty.
+ */
 QRect OffsetMapDialog::affectedBoundingRect() const
 {
     QRect boundingRect;
 
     switch (boundsSelection()) {
     case WholeMap:
-        boundingRect = QRect(QPoint(0, 0), mMapDocument->map()->size());
-
-        if (mMapDocument->map()->infinite()) {
-            LayerIterator iterator(mMapDocument->map());
-
-            while (Layer *layer = iterator.next())
-                if (TileLayer *tileLayer = dynamic_cast<TileLayer*>(layer))
-                    boundingRect = boundingRect.united(tileLayer->bounds());
-
-        }
+        if (!mMapDocument->map()->infinite())
+            boundingRect = QRect(QPoint(0, 0), mMapDocument->map()->size());
         break;
     case CurrentSelectionArea: {
         const QRegion &selection = mMapDocument->selectedArea();
@@ -129,6 +129,18 @@ OffsetMapDialog::BoundsSelection OffsetMapDialog::boundsSelection() const
     return CurrentSelectionArea;
 }
 
+void OffsetMapDialog::setBoundsSelection(BoundsSelection boundsSelection)
+{
+    switch (boundsSelection) {
+    case WholeMap:
+        mUi->boundsSelection->setCurrentIndex(0);
+        break;
+    case CurrentSelectionArea:
+        mUi->boundsSelection->setCurrentIndex(1);
+        break;
+    }
+}
+
 QPoint OffsetMapDialog::offset() const
 {
     return QPoint(mUi->xOffset->value(), mUi->yOffset->value());
@@ -144,10 +156,20 @@ bool OffsetMapDialog::wrapY() const
     return mUi->wrapY->isChecked();
 }
 
-void OffsetMapDialog::disableBoundsSelectionCurrentArea()
+void OffsetMapDialog::boundsSelectionChanged()
 {
-    mUi->boundsSelection->setEnabled(false);
-    mUi->boundsSelection->setCurrentIndex(0);
+    bool wrapEnabled = true;
+
+    if (boundsSelection() == WholeMap && mMapDocument->map()->infinite())
+        wrapEnabled = false;
+
+    mUi->wrapX->setEnabled(wrapEnabled);
+    mUi->wrapY->setEnabled(wrapEnabled);
+
+    if (!wrapEnabled) {
+        mUi->wrapX->setChecked(false);
+        mUi->wrapY->setChecked(false);
+    }
 }
 
 } // namespace Internal

src/tiled/offsetmapdialog.h

@@ -42,7 +42,7 @@ class OffsetMapDialog : public QDialog
 public:
     OffsetMapDialog(MapDocument *mapDocument, QWidget *parent = nullptr);
 
-    ~OffsetMapDialog();
+    ~OffsetMapDialog() override;
 
     QList<Layer*> affectedLayers() const;
     QRect affectedBoundingRect() const;
@@ -51,6 +51,9 @@ class OffsetMapDialog : public QDialog
     bool wrapX() const;
     bool wrapY() const;
 
+private slots:
+    void boundsSelectionChanged();
+
 private:
     enum LayerSelection {
         AllVisibleLayers,
@@ -64,9 +67,9 @@ class OffsetMapDialog : public QDialog
     };
 
     LayerSelection layerSelection() const;
-    BoundsSelection boundsSelection() const;
 
-    void disableBoundsSelectionCurrentArea();
+    BoundsSelection boundsSelection() const;
+    void setBoundsSelection(BoundsSelection boundsSelection);
 
     Ui::OffsetMapDialog *mUi;
     MapDocument *mMapDocument;