HaikuArchives / ArtPaint

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

Width/Height attributes only set on first save & off by one #424

Closed humdingerb closed 1 year ago

humdingerb commented 1 year ago
humdingerb commented 1 year ago

As it turns out, this isn't ArtPaint's fault. How can it, Artpaint never writes those attributes... It seems to be Tracker's new thumbnailer. I'll file a ticket at Trac.

dsizzle commented 1 year ago

I saw the code review… I wonder if your fix should be floor(Width() + 0.5) to round up any fractional Width values instead of IntegerWidth() + 1 which will always add 1? (Same for height of course)

humdingerb commented 1 year ago

Doesn't the use of IntegerWidth() already get rid of any fractions?

dsizzle commented 1 year ago

It does but I don’t know if it’s rounded up or down. I guess I can look at the source code…

dsizzle commented 1 year ago

Looks like it doesn’t handle rounding, it just returns the integer version. This just returns the part before the decimal point (I.e. a Width() of 10.999999 will return an IntegerWidth() of 10)

dsizzle commented 1 year ago

See haiku/headers/os/interface/Size.h

dsizzle commented 1 year ago

Actually maybe a better fix overall would be to change the IntegerWidth and IntegerHeight functions to add 0.5 to the width and height which would then go the rounding

so like:

return (int32)(width + 0.5);

since width and height should only be > 0 (this doesn’t work for negative numbers)

...but then again it might break anything that currently relies on the current behavior... so maybe don't fix Size.h. It is what it is. I updated my previous comment above; meant to say floor instead of ceil.

I commented on the code review - I didn't realize I had the power to comment on other peoples' reviews. I feel like a superhero or something.

dsizzle commented 1 year ago

I see waddlesplash disagrees with me - but it is good to know that width is pixel center-to-pixel center...so two pixels wide = width of 1. Might explain some of the weird off-by-ones in ArtPaint!

humdingerb commented 1 year ago

Yes, that center-to center has been a fun source of issues since BeOS days. :) Might be the reason for 1 pixel selections not working...