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

Map Object resizing, v3! #593

Merged
merged 6 commits into from Apr 9, 2015

Conversation

Projects
None yet
7 participants
@mauvecow
Contributor

mauvecow commented Jan 19, 2014

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

This comment has been minimized.

Show comment
Hide comment
@bjorn

bjorn Feb 1, 2014

Owner

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

Owner

bjorn commented Feb 1, 2014

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

@callagga

This comment has been minimized.

Show comment
Hide comment
@callagga

callagga Apr 18, 2014

@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

@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

This comment has been minimized.

Show comment
Hide comment
@callagga

callagga Apr 21, 2014

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

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

@bjorn

This comment has been minimized.

Show comment
Hide comment
@bjorn

bjorn Apr 21, 2014

Owner

@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.

Owner

bjorn commented Apr 21, 2014

@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

This comment has been minimized.

Show comment
Hide comment
@callagga

callagga Apr 21, 2014

@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 - 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

This comment has been minimized.

Show comment
Hide comment
@bjorn

bjorn Apr 21, 2014

Owner

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.

Owner

bjorn commented Apr 21, 2014

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

This comment has been minimized.

Show comment
Hide comment
@obeea

obeea Jul 16, 2014

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.

image

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

obeea commented Jul 16, 2014

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.

image

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

This comment has been minimized.

Show comment
Hide comment
@bjorn

bjorn Jul 16, 2014

Owner

@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.

Owner

bjorn commented Jul 16, 2014

@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

This comment has been minimized.

Show comment
Hide comment
@obeea

obeea Jul 17, 2014

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.

obeea commented Jul 17, 2014

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

This comment has been minimized.

Show comment
Hide comment
@bjorn

bjorn Jul 17, 2014

Owner
  1. 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.

Owner

bjorn commented Jul 17, 2014

  1. 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

This comment has been minimized.

Show comment
Hide comment
@obeea

obeea Jul 17, 2014

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 commented Jul 17, 2014

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

This comment has been minimized.

Show comment
Hide comment
@obeea

obeea Jul 17, 2014

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 commented Jul 17, 2014

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

This comment has been minimized.

Show comment
Hide comment
@obeea

obeea Jul 17, 2014

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

obeea commented Jul 17, 2014

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

@Ablu

This comment has been minimized.

Show comment
Hide comment
@Ablu

Ablu Jul 17, 2014

Contributor

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);
    }
Contributor

Ablu commented Jul 17, 2014

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

This comment has been minimized.

Show comment
Hide comment
@bjorn

bjorn Sep 24, 2014

Owner

@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?

Owner

bjorn commented Sep 24, 2014

@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

This comment has been minimized.

Show comment
Hide comment
@mauvecow

mauvecow Sep 25, 2014

Contributor

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.

Contributor

mauvecow commented Sep 25, 2014

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

This comment has been minimized.

Show comment
Hide comment
@mauvecow

mauvecow Nov 3, 2014

Contributor

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.

Contributor

mauvecow commented Nov 3, 2014

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

This comment has been minimized.

Show comment
Hide comment
@bjorn

bjorn Nov 3, 2014

Owner

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.

Owner

bjorn commented Nov 3, 2014

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 bjorn modified the milestone: 0.12.0 Dec 30, 2014

@bjorn bjorn added 1 - Ready and removed 1 - Ready labels Feb 22, 2015

@bjorn

This comment has been minimized.

Show comment
Hide comment
@bjorn

bjorn Mar 29, 2015

Owner

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).
Owner

bjorn commented Mar 29, 2015

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 comment has been minimized.

Show comment
Hide comment
@ntwest

ntwest Mar 31, 2015

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!

ntwest commented Mar 31, 2015

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

This comment has been minimized.

Show comment
Hide comment
@mauvecow

mauvecow Apr 1, 2015

Contributor

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.

Contributor

mauvecow commented Apr 1, 2015

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

This comment has been minimized.

Show comment
Hide comment
@bjorn

bjorn Apr 1, 2015

Owner

@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. :-/

Owner

bjorn commented Apr 1, 2015

@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 bjorn self-assigned this Apr 3, 2015

@bjorn

This comment has been minimized.

Show comment
Hide comment
@bjorn

bjorn Apr 7, 2015

Owner

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.

Owner

bjorn commented Apr 7, 2015

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

This comment has been minimized.

Show comment
Hide comment
@bjorn

bjorn Apr 8, 2015

Owner

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

selection_002

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.

Owner

bjorn commented Apr 8, 2015

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

selection_002

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 bjorn merged commit 98f8c83 into bjorn:master Apr 9, 2015

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
@bjorn

This comment has been minimized.

Show comment
Hide comment
@bjorn

bjorn Apr 9, 2015

Owner

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.

Owner

bjorn commented Apr 9, 2015

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

This comment has been minimized.

Show comment
Hide comment
@bjorn

bjorn Apr 9, 2015

Owner

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

Owner

bjorn commented Apr 9, 2015

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

@kheftel

This comment has been minimized.

Show comment
Hide comment

kheftel commented Apr 9, 2015

woohoo!

@mauvecow

This comment has been minimized.

Show comment
Hide comment
@mauvecow

mauvecow Apr 9, 2015

Contributor

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

Contributor

mauvecow commented Apr 9, 2015

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment