Skip to content
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

Crop/Scale Canvas Maniplators: fixed off-by-1 errors #630

Merged
merged 1 commit into from
Oct 7, 2023

Conversation

dsizzle
Copy link
Collaborator

@dsizzle dsizzle commented Oct 5, 2023

New fix for #552 - now you can crop to 1x1 and it's actually 1x1, and I believe other crops are accurate too.

Also fixed Canvas->Resize since it's kind of the same thing (and had the same problem).

I didn't mess with Edit->Scale; I think it's also off by 1 but it's a lot hairier in general.

@humdingerb
Copy link
Member

Crash when invoking Canvas|Resize... twice without changing anything but confirming both times with "OK" button:
ArtPaint-11210-debug-05-10-2023-14-30-15.report.txt

No crash when cropping twice.

Otherwise it does indeed seem to fix the cropping one-off.
There's still something off with resizing. I don't have the time to test thorougly ATM, but here's one thing:

  • Canvas | Resize...
  • Uncheck "Lock"
  • Increase width by 1

Walter_the_OS_shear

And the result is 3 pixels wider.

@dsizzle
Copy link
Collaborator Author

dsizzle commented Oct 5, 2023

I fixed the weird shearing - and it seems to correctly only expand by 1 pixel in width.

Still looking into the crash. Seems like it's related to trying to change the size when clicking Ok instead of not doing anything.

@dsizzle
Copy link
Collaborator Author

dsizzle commented Oct 5, 2023

I think I fixed the crash as well - but I will try to test more.

@humdingerb
Copy link
Member

Looks good!

Can you remind me why there's a "Left" and "Top" in the Resize panel? For cropping, it works - it de/increases the canvas size. For resizing it does nothing.
It could have a use, if entering e.g. 100 in "Top" decreased the "Height" by 100 (respecting the "Lock" checkbox to decrease the width accordingly).

@dsizzle
Copy link
Collaborator Author

dsizzle commented Oct 6, 2023

Honestly I don’t recall. I think it was just for consistency. But yeah, I guess it is kinda pointless. Even if it worked as you suggested, it’s no different from decreasing the width or height.

@humdingerb
Copy link
Member

True. At max. it saves you from mental arithmatic when having to subtract, say 283 from 1292 for the new width.

@dsizzle
Copy link
Collaborator Author

dsizzle commented Oct 6, 2023

Ok, after sleeping on it, I think I remember why we have the Left/Top - it's simply because of the resize box. So when you move the upper left handle, it does behave as you describe.

Anyway, since this seems to have fixed the original issue, I'll go ahead and merge it later.

@humdingerb
Copy link
Member

👍

@dsizzle dsizzle merged commit a83fa84 into HaikuArchives:master Oct 7, 2023
1 of 2 checks passed
@dsizzle dsizzle deleted the crop-scale-off1 branch October 7, 2023 14:16
@dsizzle
Copy link
Collaborator Author

dsizzle commented Oct 7, 2023

merged!

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

Successfully merging this pull request may close these issues.

2 participants