search

HaikuArchives / ArtPaint

ArtPaint is a painting and image processing program.
https://haikuarchives.github.io/ArtPaint/
29 stars 18 forks source link

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

Closed dsizzle closed 12 months ago

dsizzle commented 12 months ago

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 commented 12 months ago

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:

Walter_the_OS_shear

And the result is 3 pixels wider.

dsizzle commented 12 months ago

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 commented 12 months ago

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

humdingerb commented 12 months ago

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 commented 12 months ago

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 commented 12 months ago

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

dsizzle commented 12 months ago

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 commented 12 months ago

👍

dsizzle commented 12 months ago

merged!