From e493254f70033905c3f053cfa9dcf9b37b917a91 Mon Sep 17 00:00:00 2001 From: "Kanishka.." Date: Thu, 27 Aug 2026 05:02:08 +0530 Subject: [PATCH] Fixed view jumping when undoing map resizes and moves (#4572) When undoing a resize from the left/top edge, the resize gets undone but the whole map jumps in the view. Same problem when undoing a map move in a world, the other maps move in the view instead of the moved one going back. This happens because the view was only adjusted when the change was made through the tool, so undo/redo never adjusted it. Now the ResizeMap command reports the contents shift through the mapResized signal and MapScene adjusts the view when it happens. For maps that are in a world, refreshScene compensates for the change in world position instead. So the view stays steady no matter where the change came from. The shift, and the world position, are measured from the map's bounding rect, which handles the isometric origin depending on the map height and the half tile hexagonal and staggered contents jump when the stagger parity changes. The resize handle counts the new size from the size the map started at, instead of dividing the map bounds by the tile size. --- src/tiled/abstractworldtool.cpp | 10 ----- src/tiled/abstractworldtool.h | 2 - src/tiled/editablemap.cpp | 2 +- src/tiled/mapdocument.cpp | 14 ++++-- src/tiled/mapdocument.h | 5 ++- src/tiled/mapscene.cpp | 47 ++++++++++++++++++++ src/tiled/mapscene.h | 4 ++ src/tiled/resizemap.cpp | 23 +++++++++- src/tiled/resizemap.h | 3 ++ src/tiled/worldmovemaptool.cpp | 79 ++++++++++++++------------------- 10 files changed, 123 insertions(+), 66 deletions(-) diff --git a/src/tiled/abstractworldtool.cpp b/src/tiled/abstractworldtool.cpp index 8f850510f6..d0228137af 100644 --- a/src/tiled/abstractworldtool.cpp +++ b/src/tiled/abstractworldtool.cpp @@ -563,16 +563,6 @@ void AbstractWorldTool::setSelectionScreenRect(const QRect &rect) } } -// Move the camera back by offset, to keep the active map steady after it shifts -void AbstractWorldTool::recenterView(const QPoint &offset) -{ - if (offset.isNull()) - return; - - MapView *view = DocumentManager::instance()->viewForDocument(mapDocument()); - view->forceCenterOn(view->viewCenter() - offset); -} - } // namespace Tiled #include "moc_abstractworldtool.cpp" diff --git a/src/tiled/abstractworldtool.h b/src/tiled/abstractworldtool.h index 694daba761..cba1c5eeda 100644 --- a/src/tiled/abstractworldtool.h +++ b/src/tiled/abstractworldtool.h @@ -112,8 +112,6 @@ class AbstractWorldTool : public AbstractTool void setSelectionScreenRect(const QRect &rect); int resizeHandleNear(const QPointF &scenePos, MapDocument *&mapDocument) const; - void recenterView(const QPoint &offset); - bool mapCanBeMoved(MapDocument *mapDocument) const; bool mapCanBeResized(MapDocument *mapDocument) const; QRect mapRect(MapDocument *mapDocument) const; diff --git a/src/tiled/editablemap.cpp b/src/tiled/editablemap.cpp index 3ab640b5be..e3272d3699 100644 --- a/src/tiled/editablemap.cpp +++ b/src/tiled/editablemap.cpp @@ -492,7 +492,7 @@ QPointF EditableMap::tileToPixel(qreal x, qreal y) const void EditableMap::setSize(int width, int height) { if (auto doc = mapDocument()) { - push(new ResizeMap(doc, QSize(width, height))); + push(new ResizeMap(doc, QSize(width, height), QPoint())); } else if (!checkReadOnly()) { map()->setWidth(width); map()->setHeight(height); diff --git a/src/tiled/mapdocument.cpp b/src/tiled/mapdocument.cpp index 960c49bba4..f8c7666a98 100644 --- a/src/tiled/mapdocument.cpp +++ b/src/tiled/mapdocument.cpp @@ -451,13 +451,19 @@ void MapDocument::resizeMap(QSize size, QPoint offset, bool removeObjects) new TransformMapObjects(this, objectsToMove, states, command); } - new ResizeMap(this, size, command); + new ResizeMap(this, size, offset, command); new ChangeSelectedArea(this, movedSelection, command); + // A world positions a map by its bounding rect, so move by how much that + // rect shifts. The pixel offset moved isometric maps twice as far as their + // rect actually shifts + const QRect oldBounds = renderer()->boundingRect(QRect(QPoint(), map()->size())); + const QRect newBounds = renderer()->boundingRect(QRect(-offset, size)); + const QPoint boundsOffset = newBounds.topLeft() - oldBounds.topLeft(); + // Adjust world position if this map is part of any loaded worlds - if (!pixelOffset.isNull()) { + if (!boundsOffset.isNull()) { const QString &mapName = fileName(); - const QPoint offsetPixels = pixelOffset.toPoint(); for (const auto &worldDocument : WorldManager::instance().worlds()) { auto world = worldDocument->world(); @@ -466,7 +472,7 @@ void MapDocument::resizeMap(QSize size, QPoint offset, bool removeObjects) continue; // also skips maps matched via pattern const QPoint prevPos = world->mapRect(mapName).topLeft(); - const QPoint newPos = prevPos - offsetPixels; + const QPoint newPos = prevPos + boundsOffset; new SetMapPosInLoadedWorld(worldDocument->fileName(), mapName, prevPos, newPos, command); } } diff --git a/src/tiled/mapdocument.h b/src/tiled/mapdocument.h index 3c402ceed5..0537c731ec 100644 --- a/src/tiled/mapdocument.h +++ b/src/tiled/mapdocument.h @@ -322,9 +322,10 @@ class TILED_EDITOR_EXPORT MapDocument final : public Document void mapObjectPicked(MapObject *object); /** - * Emitted when the map size changes. + * Emitted when the map size changes. The screen offset tells by how much + * the contents were shifted during the resize, in screen coordinates. */ - void mapResized(); + void mapResized(QPointF screenOffset); void layerAdded(Layer *layer); void layerAboutToBeRemoved(GroupLayer *parentLayer, int index); diff --git a/src/tiled/mapscene.cpp b/src/tiled/mapscene.cpp index ae4f73fdba..e4e28f5c33 100644 --- a/src/tiled/mapscene.cpp +++ b/src/tiled/mapscene.cpp @@ -31,6 +31,7 @@ #include "map.h" #include "mapobject.h" #include "maprenderer.h" +#include "mapview.h" #include "objectgroup.h" #include "objecttemplate.h" #include "preferences.h" @@ -117,6 +118,8 @@ void MapScene::setMapDocument(MapDocument *mapDocument) this, [this] { update(); }); connect(mMapDocument, &MapDocument::tilesetReplaced, this, &MapScene::tilesetReplaced); + connect(mMapDocument, &MapDocument::mapResized, + this, &MapScene::mapResized); } refreshScene(); @@ -285,6 +288,36 @@ QPointF MapScene::parallaxOffset(const Layer &layer) const (1.0 - parallaxFactor.y()) * viewCenter.y()); } +/** + * When a resize shifted the map contents, move the view along by the same + * amount so the contents appear to stay in place. + */ +void MapScene::mapResized(QPointF screenOffset) +{ + if (screenOffset.isNull()) + return; + + // when the map has its own entry in a world, the resize also moves its + // world position and refreshScene handles the view, so nothing to do here + const QString &fileName = mMapDocument->fileName(); + if (auto worldDocument = WorldManager::instance().worldForMap(fileName)) + if (worldDocument->world()->mapIndex(fileName) >= 0) + return; + + translateViews(screenOffset); +} + +/** + * Moves all views on this scene along by the given delta. + */ +void MapScene::translateViews(const QPointF &delta) +{ + const auto sceneViews = views(); + for (QGraphicsView *view : sceneViews) + if (auto mapView = qobject_cast(view)) + mapView->forceCenterOn(mapView->viewCenter() + delta); +} + /** * Refreshes the map scene. */ @@ -293,6 +326,7 @@ void MapScene::refreshScene() QHash mapItems; if (!mMapDocument) { + mLastWorldPositionMapFile.clear(); mMapItems.swap(mapItems); qDeleteAll(mapItems); updateSceneRect(); @@ -307,6 +341,17 @@ void MapScene::refreshScene() const QPoint currentMapPosition = world->mapRect(currentMapFile).topLeft(); auto const contextMaps = world->contextMaps(currentMapFile); + // If the current map moved in its world (by a move or an undo), + // shift the view along so the map visibly moves instead of the + // world around it. + if (mLastWorldPositionMapFile == currentMapFile + && currentMapPosition != mLastWorldPosition) { + const QPoint delta = currentMapPosition - mLastWorldPosition; + translateViews(-delta); + } + mLastWorldPositionMapFile = currentMapFile; + mLastWorldPosition = currentMapPosition; + for (const WorldMapEntry &mapEntry : contextMaps) { MapDocumentPtr mapDocument; @@ -329,6 +374,8 @@ void MapScene::refreshScene() } } } else { + mLastWorldPositionMapFile.clear(); + auto mapItem = takeOrCreateMapItem(mMapDocument->sharedFromThis(), MapItem::Editable); mapItems.insert(mMapDocument, mapItem); } diff --git a/src/tiled/mapscene.h b/src/tiled/mapscene.h index 039f30cdac..d69048d63c 100644 --- a/src/tiled/mapscene.h +++ b/src/tiled/mapscene.h @@ -114,6 +114,8 @@ class MapScene : public QGraphicsScene void refreshScene(); void changeEvent(const ChangeEvent &change); + void mapResized(QPointF screenOffset); + void translateViews(const QPointF &delta); void repaintTileset(Tileset *tileset); void tilesetReplaced(int index, Tileset *tileset, Tileset *oldTileset); @@ -133,6 +135,8 @@ class MapScene : public QGraphicsScene MapDocument *mMapDocument = nullptr; QHash mMapItems; + QString mLastWorldPositionMapFile; + QPoint mLastWorldPosition; AbstractTool *mSelectedTool = nullptr; DebugDrawItem *mDebugDrawItem = nullptr; bool mUnderMouse = false; diff --git a/src/tiled/resizemap.cpp b/src/tiled/resizemap.cpp index 3469dc8ecd..cdd0fd05f5 100644 --- a/src/tiled/resizemap.cpp +++ b/src/tiled/resizemap.cpp @@ -22,6 +22,7 @@ #include "map.h" #include "mapdocument.h" +#include "maprenderer.h" #include @@ -29,12 +30,14 @@ namespace Tiled { ResizeMap::ResizeMap(MapDocument *mapDocument, QSize size, + QPoint offset, QUndoCommand *parent) : QUndoCommand(QCoreApplication::translate("Undo Commands", "Resize Map"), parent) , mMapDocument(mapDocument) , mSize(size) + , mOffset(offset) { } @@ -51,12 +54,28 @@ void ResizeMap::redo() void ResizeMap::swapSize() { Map *map = mMapDocument->map(); - QSize oldSize(map->width(), map->height()); + const MapRenderer *renderer = mMapDocument->renderer(); + + const QSize oldSize = map->size(); + + // Measure the bounding rect rather than the contents, since on hexagonal + // and staggered maps the contents jump half a tile whenever the stagger + // parity changes. Taken before the resize, because the isometric origin + // follows the map height. + const QRect oldBounds = renderer->boundingRect(QRect(QPoint(), oldSize)); + const QRect newBounds = renderer->boundingRect(QRect(-mOffset, mSize)); + const QPoint boundsOffset = newBounds.topLeft() - oldBounds.topLeft(); + map->setWidth(mSize.width()); map->setHeight(mSize.height()); mSize = oldSize; - emit mMapDocument->mapResized(); + // The view goes the other way, so the map stays where it was on screen. + emit mMapDocument->mapResized(-boundsOffset); + + // Shift the other way when this command is applied again, so that undo + // and redo each restore the previous state. + mOffset = -mOffset; } } // namespace Tiled diff --git a/src/tiled/resizemap.h b/src/tiled/resizemap.h index 5cff2dc118..842730c5c3 100644 --- a/src/tiled/resizemap.h +++ b/src/tiled/resizemap.h @@ -20,6 +20,7 @@ #pragma once +#include #include #include @@ -36,6 +37,7 @@ class ResizeMap : public QUndoCommand public: ResizeMap(MapDocument *mapDocument, QSize size, + QPoint offset, QUndoCommand *parent = nullptr); void undo() override; @@ -46,6 +48,7 @@ class ResizeMap : public QUndoCommand MapDocument *mMapDocument; QSize mSize; + QPoint mOffset; }; } // namespace Tiled diff --git a/src/tiled/worldmovemaptool.cpp b/src/tiled/worldmovemaptool.cpp index 0cd9ada986..081807521d 100644 --- a/src/tiled/worldmovemaptool.cpp +++ b/src/tiled/worldmovemaptool.cpp @@ -141,19 +141,13 @@ void WorldMoveMapTool::moveMap(MapDocument *document, QPoint moveBy) auto undoStack = worldDocument->undoStack(); undoStack->push(new SetMapRectCommand(worldDocument, document->fileName(), rect)); - - if (document == mapDocument()) { - // undo camera movement, by the actual snapped offset - recenterView(rect.topLeft() - prevRect.topLeft()); - } } void WorldMoveMapTool::updateResizingMap(const QPointF &pos, Qt::KeyboardModifiers modifiers) { const Map *map = mResizingMap->map(); - const int tileWidth = map->tileWidth(); - const int tileHeight = map->tileHeight(); + const MapRenderer *renderer = mResizingMap->renderer(); const QSize step = snapSize(mResizingMap); const HandleEdges edges = handleEdges[mResizeHandle]; @@ -165,37 +159,48 @@ void WorldMoveMapTool::updateResizingMap(const QPointF &pos, const QPoint delta = (pos - mDragStartScenePos).toPoint(); const bool snapToGrid = !(modifiers & Qt::ControlModifier); - const auto snap = [&](int value, int gridStep) { - return (snapToGrid && gridStep > 0) ? qRound(qreal(value) / gridStep) * gridStep - : value; + // snap the drag rather than the resulting edge, since the bounds of a + // staggered or hexagonal map don't line up with the grid and the edge + // would otherwise jump as soon as a handle is grabbed + const auto snap = [&](int amount, int gridStep) { + return (snapToGrid && gridStep > 0) ? qRound(qreal(amount) / gridStep) * gridStep + : amount; }; if (edges.left) - left = snap(left + delta.x(), step.width()); + left += snap(delta.x(), step.width()); if (edges.right) - right = snap(right + delta.x(), step.width()); + right += snap(delta.x(), step.width()); if (edges.top) - top = snap(top + delta.y(), step.height()); + top += snap(delta.y(), step.height()); if (edges.bottom) - bottom = snap(bottom + delta.y(), step.height()); - - // a map is always a whole number of tiles, so round to the nearest tile - const int newWidth = qMax(1, qRound(qreal(right - left) / tileWidth)); - const int newHeight = qMax(1, qRound(qreal(bottom - top) / tileHeight)); + bottom += snap(delta.y(), step.height()); + + // ask the renderer what one more column or row adds on screen, since that + // is only the tile size for orthogonal maps + const QSize mapSize = map->size(); + const QRect mapRect = renderer->boundingRect(QRect(QPoint(), mapSize)); + const QRect oneMoreColumn = renderer->boundingRect(QRect(QPoint(), mapSize + QSize(1, 0))); + const QRect oneMoreRow = renderer->boundingRect(QRect(QPoint(), mapSize + QSize(0, 1))); + const int columnPixels = qMax(1, oneMoreColumn.width() - mapRect.width()); + const int rowPixels = qMax(1, oneMoreRow.height() - mapRect.height()); + + // count from the size we started at, so grabbing a handle without dragging + // leaves the map as it is + const int widthDragged = right - left - mResizeStartWorldRect.width(); + const int heightDragged = bottom - top - mResizeStartWorldRect.height(); + const int newWidth = qMax(1, mapSize.width() + qRound(qreal(widthDragged) / columnPixels)); + const int newHeight = qMax(1, mapSize.height() + qRound(qreal(heightDragged) / rowPixels)); // the content only shifts when the left or top edge is the one being moved - mResizeOffset = QPoint(edges.left ? newWidth - map->width() : 0, - edges.top ? newHeight - map->height() : 0); + mResizeOffset = QPoint(edges.left ? newWidth - mapSize.width() : 0, + edges.top ? newHeight - mapSize.height() : 0); mResizeNewSize = QSize(newWidth, newHeight); - // preview the result, matching what resizeMap() will produce, using the - // renderer so the size is correct for isometric and other orientations - const MapRenderer *renderer = mResizingMap->renderer(); - const QPointF pixelOffset = renderer->tileToPixelCoords(QPointF()) - - renderer->tileToPixelCoords(-mResizeOffset); - const QPoint topLeft = mResizeStartWorldRect.topLeft() - pixelOffset.toPoint(); - const QSize previewSize = renderer->boundingRect(QRect(QPoint(), mResizeNewSize)).size(); - setSelectionScreenRect(QRect(topLeft, previewSize).translated(mResizeSceneOffset)); + // preview the result, positioned the way resizeMap() will position it + const QRect newBounds = renderer->boundingRect(QRect(-mResizeOffset, mResizeNewSize)); + const QPoint topLeft = mResizeStartWorldRect.topLeft() + newBounds.topLeft() - mapRect.topLeft(); + setSelectionScreenRect(QRect(topLeft, newBounds.size()).translated(mResizeSceneOffset)); setStatusInfo(tr("Resize map to %1 x %2").arg(newWidth).arg(newHeight)); } @@ -326,20 +331,9 @@ void WorldMoveMapTool::finishResizing() auto resizedMap = std::exchange(mResizingMap, nullptr); mResizeHandle = -1; - if (mResizeNewSize != resizedMap->map()->size() || !mResizeOffset.isNull()) { + if (mResizeNewSize != resizedMap->map()->size() || !mResizeOffset.isNull()) resizedMap->resizeMap(mResizeNewSize, mResizeOffset, false); - // keep the view steady by compensating for the content shift, which - // matches the pixelOffset applied by MapDocument::resizeMap (a map - // in a world has its position adjusted by exactly this amount) - if (resizedMap == mapDocument()) { - const MapRenderer *renderer = resizedMap->renderer(); - const QPointF pixelOffset = renderer->tileToPixelCoords(QPointF()) - - renderer->tileToPixelCoords(-mResizeOffset); - recenterView(-pixelOffset.toPoint()); - } - } - updateSelectionRectangle(); refreshCursor(); setStatusInfo(QString()); @@ -363,11 +357,6 @@ void WorldMoveMapTool::finishMoving() auto undoStack = worldDocument->undoStack(); undoStack->push(new SetMapRectCommand(worldDocument, draggedMap->fileName(), rect)); - - if (draggedMap == mapDocument()) { - // undo camera movement - view->forceCenterOn(view->viewCenter() - mDragOffset); - } } } else { // switch to the document