Map Object resizing, v3!

[mauvecow]

Depends on pull requests #591 and #592, so you might want to get through them first to make the comparison less unsightly. Replaces pull #485. Does the following:

• Map renderer supports scaling of cells.
• Map reader and object tile creation will initialize the size of the object to the tile's size.
• Removed handles from mapobjectitem.cpp.
• In objectselectiontool.cpp, added a set of 8 ResizeHandles, and changed the Corner enum to AnchorPosition, representing the edge centers as well.
• When a single object is selected, resizing is handled in object space.
• When multiple objects are selected, resizing is handled in world space.

Caveats:

• Negative scaling factors are not allowed.

[bjorn]

Sorry, my fixes in commit a2b030a are conflicting with these changes. Could you please rebase these commits on top of the latest master?

[callagga]

@mauvecow - Hi. I'm very interested in a version of tiled that allow one to resize the images one adds to object layers, i.e. with the "Insert Tile" action on an object layer. Would version provide this? Any tips or links re how to build a Mac version for this version? Regards Greg

[callagga]

@bjorn what's the next step to get this change merged in?

[bjorn]

@callagga Time constraints. I got a lot going on with work, family and several hobby projects. This is a big change that will take time to review in detail and will likely need some additional work before it can be considered stable. I also do not want it to delay a possible stable release further, so before merging this in I want to make a Tiled 0.10 release.

Regarding compiling Tiled on Mac OS X, it should be quite straight-forward, though I'm personally having some strange issue when trying to compile it with Qt 5. I'm just happy the Qt 4 daily builds are still somewhat working, even though it warns a lot about not supporting my version of OS X during the compile. I need to look into that sometime, but meantime just try to follow the wiki page about compiling Tiled and if you get stuck somewhere, report your problem and ask for help. So start at https://github.com/bjorn/tiled/wiki/Contributing-to-Tiled.

[callagga]

@bjorn - ok thanks for the background - hard to know sometimes if something that seems simply on the surface really is. I'll have a look at the build info. Mind if I ask:

a) what does the green notice " All is well — The Travis CI build passed" mean? Does this imply the patch tested ok?

b) follow on question from this is whether this patch/pull request works at a basic level? (i..e if you happen to know & there's issues, then I won't try to spend the time frying to build it - just if you happened to have tried it)

[bjorn]

a) what does the green notice " All is well — The Travis CI build passed" mean? Does this imply the patch tested ok?

That just means it compiled fine. No relevant autotests currently exist.

b) follow on question from this is whether this patch/pull request works at a basic level? (i..e if you happen to know & there's issues, then I won't try to spend the time frying to build it - just if you happened to have tried it)

It's been a while since I tried it so I can't remember any exact issues right now. I think it works fine in general, though.

[obeea]

Maybe I didn't merge all the changes in correctly but I'm finding some strange side effects with the resizing code.
I added a rectangle at 0,0, width and height = 64pixels, tile grid = 32x32, snap to grid enabled.

I dragged the centre right handle to size it by one tile (32pixels) but although it appeared to snap into place, the width is now 94.52 instead of 96 (64+32).
The more I resize it the further out it goes out of align.

When selecting multiple objects the same things happens but it also doesn't restrict the horizontal and vertical movement when using a one axis resize handle e.g. centre right, should only size on the X axis but it actually resizes on both.

I rechecked the committed changes, so I think I have transposed them correctly.
Maybe you could confirm if you can reproduce it, or whether it is just me.

Thanks

[bjorn]

@obeea If you want to make sure they're not issues introduced by your merging efforts, then check out exactly the branch @mauvecow requested to pull (his mapobjectresizev3 branch) and see how that behaves. Your feedback is very welcome, since it saves me some time and maybe @mauvecow could still fix some of the issues you find.

[obeea]

I did as you suggested and can confirm that these issues remain.

1. Resize does not snap to grid correctly (as detailed in my earlier post).
2. Resize multiple objects does not consider the resize handle position and always scales on both axis.
   I'll let you know if I find anything else.

Thanks.

[bjorn]

2. Resize multiple objects does not consider the resize handle position and always scales on both axis. I'll let you know if I find anything else.

The reason for this is probably because it has no special-case for single items or groups of non-rotated items. Because for groups of items that have different individual rotation, this kind of resizing is not possible with Tiled because of the kind of transformation that would need to be applied.

[obeea]

Yes that is a good point, it would be more complicated to check the rotation for each object in the group and resize accordingly.
I think the problem for the single object resizing is also down to rotation. Although it snaps the difference in movement to the grid, it then calculates a scaling factor based on the rotation and new size and results in some slightly lower values, which for non-rotated objects is not desirable.
Maybe the compromise would be to simplify the calculation for non-rotated objects.

[obeea]

I had a play today and was able to get the singleitem resize code to snap to the grid, I really don't know if this is the best way to do it so I've included the code here for reference.
Also, I noticed a bug in the polygon resize code where it would exponentially increase the number of points in the polygon rather than replace the existing points. Again the fix is below for reference.
Hope this helps.

void ObjectSelectionTool::updateResizingSingleItem(const QPointF &pos,
                                                   Qt::KeyboardModifiers modifiers)
{

    MapObjectItem *objectItem = *mMovingItems.begin();
    MapRenderer *renderer = mapDocument()->renderer();
    QPointF diff = pos - mResizingOrigin;
    QPointF startDiff = mStart - mResizingOrigin;

    diff = snapToGrid(diff, modifiers);

    // Most calculations in this function occur in object space, so
    // we transform the scaling factors from world space.
    qreal rotation = objectItem->rotation() * M_PI / -180;
    const qreal sn = std::sin(rotation);
    const qreal cs = std::cos(rotation);

    if (rotation != 0)  {
        diff = QPointF(diff.x() * cs - diff.y() * sn, diff.x() * sn + diff.y() * cs);
        startDiff = QPointF(startDiff.x() * cs - startDiff.y() * sn, startDiff.x() * sn + startDiff.y() * cs);
    }

    // Calculate scaling factor.
    QSizeF scalingFactor(qMax((qreal)0, diff.x() / startDiff.x()),
                         qMax((qreal)0, diff.y() / startDiff.y()));

    if (mResizingLimitHorizontal)
        scalingFactor.setWidth(1);
    if (mResizingLimitVertical)
        scalingFactor.setHeight(1);

    // Convert relative position into object space, scale,
    // and then convert back to world space.
    const QPointF oldRelPos = mOldObjectItemPositions.at(0) - mResizingOrigin;

    const QPointF objectRelPos(oldRelPos.x() * cs - oldRelPos.y() * sn,
                               oldRelPos.x() * sn + oldRelPos.y() * cs);

    const QPointF scaledRelPos(objectRelPos.x() * scalingFactor.width(),
                               objectRelPos.y() * scalingFactor.height());

    const QPointF newRelPos(scaledRelPos.x() * cs + scaledRelPos.y() * sn,
                            scaledRelPos.x() * -sn + scaledRelPos.y() * cs);

    const QPointF newScreenPos = mResizingOrigin + newRelPos;
    const QPointF newPos = renderer->screenToPixelCoords(newScreenPos);
    const QSizeF origSize = mOldObjectSizes.at(0);
    QSizeF newSize(origSize.width() * scalingFactor.width(),
                         origSize.height() * scalingFactor.height());

    //make sure new size aligns to grid
    QPointF nSize = QPointF(newSize.width(),newSize.height());
    nSize = snapToGrid(nSize, modifiers);
    newSize.setWidth(nSize.x());
    newSize.setHeight(nSize.y());

    if (objectItem->mapObject()->polygon().isEmpty() == false) {
        const QPolygonF &oldPolygon = mOldObjectPolygons.at(0);
        QPolygonF newPolygon(oldPolygon.size());
        for (int n = 0; n < oldPolygon.size(); ++n) {
            const QPointF point(oldPolygon[n]);
            const QPointF newPoint(point.x() * scalingFactor.width(),
                                   point.y() * scalingFactor.height());
//            newPolygon += newPoint;
            newPolygon[n] = newPoint; // replace point
        }
        objectItem->mapObject()->setPolygon(newPolygon);
    }

    objectItem->resizeObject(newSize);
    objectItem->setPos(newScreenPos);
    objectItem->mapObject()->setPosition(newPos);
}

[obeea]

apologies for the formatting, are there special tags I can use to format code?
Edit : Figured it out, looks better now.

[Ablu]

You can use

    ~~~c++

    ~~~

for more fancy code :)

Like this:

    if (rotation != 0)  {
        diff = QPointF(diff.x() * cs - diff.y() * sn, diff.x() * sn + diff.y() * cs);
        startDiff = QPointF(startDiff.x() * cs - startDiff.y() * sn, startDiff.x() * sn + startDiff.y() * cs);
    }

[bjorn]

@mauvecow So, 0.10 is out now and the master branch would be open for larger changes like this one. Does it make sense for me to try finding time for a somewhat detailed review and would you have time to fix things? Also, what do you think about @obeea's feedback?

[mauvecow]

Will get on reviewing this over the weekend(I hope). Some notes:

• Didn't know about the vertex duplication bug, will review that in detail.
• Resize/Scaling snapping was wonky originally too IIRC. Will also check that, but suspecting it's just a floating point rounding error that needs to be bumped up to a double or something.

It's been long enough that I've forgotten everything so it will take some time to get a good look at it.

[mauvecow]

Rebased to the current master, added the polygon vertex fix from above. Thanks, totally didn't notice.

The resizing issue only exists for non-tile objects, and it is tied to the boundingRect() function causing the resizing origins to be placed. I remember this being an issue with object names being applied incorrectly as well. Waiting on feedback on this before fixing this up. And yeah I'll squash this all down next time.

[bjorn]

I remember this being an issue with object names being applied incorrectly as well. Waiting on feedback on this before fixing this up. And yeah I'll squash this all down next time.

I totally think object names should not affect the bounding rect. In fact, I think the rendering of object names should be taken out of the map renderer and instead it should be done with separate items on the scene. But, this will be quite a change so it should probably be done separately.

[bjorn]

I've merged master into this branch and pushed the result to wip/mapobjectresizev3. Fixing this up and making it ready to merge back to master will be one of my objectives for Tiled 0.12, which I will work on in the coming month.

So far, it looks like you've done an amazing job, @mauvecow! I'm sorry it has taken me so long to finally work towards merging this.

I'll document here the issues I'm aware of that I think should be looked at before merging it into master:

• Switching map orientation leaves the handles in the wrong place (this wasn't possible back when you worked on this).
• There's this problem related the name display for non-tile objects.
• It's too easy to get into the situation where you resize your object down to 0-size, and then you can't make it bigger anymore. I think either it should allow making it bigger starting from 0-size or we should prevent the size from going below 1.
• The resize behavior looks quite broken for isometric maps.

Either before or after merging to master, but before releasing, I'd prefer to also tackle the following:

• Add a modifier key for keeping the aspect ratio (Ctrl in Inkscape).
• Add a modifier key for putting the resize origin in the center (Shift in Inkscape).
• The resize handles should look a lot nicer.
• The mouse cursors are not pointing in the right direction for single rotated objects. I'm thinking if we make the handles arrows themselves we do not actually need these mouse cursors (like in Inkscape).

[ntwest]

This is exciting news! I just discovered tiled and have been having great fun making maps for D&D. With object resizing this will be a first-class RPG map editor!

[mauvecow]

It's been so long I've actually sort of forgotten everything I'd done here. (the project that uses this is, uh, still underway, just taking forever)

Sounds like you've got a fair degree of it handled. Is there anything you want me to review? It's kind of a bad time for me to hit this up quickly though.

[bjorn]

@mauvecow I may have some questions later. It'll still be one week till I really have time to work on this. Of course I understand you will have forgotten most of this patch by now, and so have I only faint memories of earlier review rounds we made. You've had this patch in progress since almost 2 years now. :-/

[bjorn]

I've pushed the progress so far to the wip/mapobjectresizev3. Not many issues down, but I've got a much better idea of the situation and could fix at least one of the problems with resizing (commit ff671fb). Unfortunately, Tiled has grown several inconsitencies related to this over the years which have to be dealt with now.

[bjorn]

I pushed some mostly visual improvements in change 333df0e. Now it looks like this:

I'm considering whether this looks maybe a little too much still and to put the rotation below a mode switch as done in Inkscape.

[bjorn]

It's merged! The isometric projection as well as the varying object origins (top-left for most objects, bottom-left for tile objects, bottom-center for tile objects on isometric maps...) made it all quite challenging, but I'm relatively happy with the resulting code (commit 42b564d).

So all known issues with this patch have been resolved, but some more testing will be in order. The Windows daily build including these changes is already available and an OS X one should be coming soon, as well as rebuilds of the other daily packages.

Feature-wise, it's still lacking a mode to keep the aspect ratio, which I think is quite important when working with images.

[bjorn]

Control modifier added for keeping aspect ration in commit 66db601. Snap toggle moved to Alt during resize.

[kheftel-old]

woohoo!

[mauvecow]

Glad to hear it finally made it in. Now to finish the game I was working on that used it...