FashionFreedom / Seamly2D

Open source patternmaking software to democratize fashion.
https://seamly.io
GNU General Public License v3.0
616 stars 111 forks source link

Image origin tool #1133

Closed Onetchou closed 4 months ago

Onetchou commented 4 months ago

Fixes #1099

It's now possible to change the origin point of images.

Right click on an image -> change origin: image

Click on the new origin point: image

The (x,y) position of the image is now the position of this point: image

And the image is rotated around this point: image

what-the-diff[bot] commented 4 months ago

PR Summary

Hello there! Our developer team is on fire! 🚀 Here's what we've been doing to improve the project:

Such exciting times! Can't wait for you to try out these changes! Stay tuned for more enhancements coming your way! 😄

Onetchou commented 4 months ago

Mmmmm don't know exactly what I touched that could cause this fail in the SeamlyMe tests: image

DSCaskey commented 4 months ago

Mmmmm don't know exactly what I touched that could cause this fail in the SeamlyMe tests:

I'm not sure either. I got the similar error with a previous PR... and I'm sure it's the same thing with a PR I just made. Something seems to have changed somewhere as some of the test measurement files seem to be missing? I'll need to dig into the test and commit history to see what's missing or not getting copied.

DSCaskey commented 4 months ago

It's got to be an issue in the ci.

I just ran the collection test locally and the tests pass... after moving the test/collectiontest/bin folders & test to seamly2D/bin. The test needs some qt libs & the platforms folder

DSCaskey commented 4 months ago

The test run is giving this warning:

image

Looking up the warning I found this:

Screenshot 2024-07-02 083448

and the script is using:

image

Could this explain why the tests work sometimes and fail other times?

DSCaskey commented 4 months ago

Hey @Onetchou... I merged develop in to fix the tests. Once you make the requested change I'll approve & merge. :)

Onetchou commented 4 months ago

@DSCaskey

It's one of those semantic things. While setting the origin is changing the origin, changing the origin doesn't neccessarily mean you're setting the origin... you could be changing the origin transform, but not setting the origin.

I was about to make the change, but I'm puzzled... Doesn't "setting the origin" implies that there was no origin before? Because the tool is only moving the origin, changing its position, the origin was there before, at the top left of the image... Same for "select the origin", it would imply that the origin was not selected before, but it was already there.

What do you think?

DSCaskey commented 4 months ago

I was about to make the change, but I'm puzzled... Doesn't "setting the origin" implies that there was no origin before? Because the tool is only moving the origin, changing its position, the origin was there before, at the top left of the image... Same for "select the origin", it would imply that the origin was not selected before, but it was already there.

What do you think?

Actually as I think about it more... just to throw you a curve, Technically "Move origin" is a more asccurate descriptor. :) Again "Change" leaves me with "change from what"? "Select" makes sense in that it tells a user they have to select a point, as well as it matches the existing code routine names.

I think I like "Move origin"... it implies an origin exists and that it's moving - ie changing. :)

DSCaskey commented 4 months ago

@Onetchou

I know what I don't like about "Change origin"... we're not changing the origin as it implies... we're changing the origin's position, where "Change origin position" is too long. IMO "Move origin" conveys the same meaning more concisely. :)

Onetchou commented 4 months ago

@Onetchou

I know what I don't like about "Change origin"... we're not changing the origin as it implies... we're changing the origin's position, where "Change origin position" is too long. IMO "Move origin" conveys the same meaning more concisely. :)

Deal! 😆 I'll change it to "Move origin"