-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[Issue 838] Update ASCeilPixelValue and ASRoundPixelValue #864
Conversation
Layouts can come back for 3x devices as a repeating decimal. Floats/doubles have a hard time representing these values and often the last digit or two will be garbage. This garbage can result in unexpected values from `ceil` or `round`. I try to fix this by subtracting `FLT_EPSILON` from the value before calling `ceil` or `round` TextureGroup#838
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
If you have time, please update ASFloorPixelValue
as well. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes sense for ceil()
but I don't see the same reasoning applying to round()
Yes I think |
I am not a smart man… is it worth using ASCeilPixelValue? It seems like it's asking for this kind of trouble, and subpixel clipping of content isn't a serious risk. Anyway, I support landing this change. I can't think of how it could produce bad results in practice. |
Yes fair warning I am quite naive as to how this is used in practice! But am pretty sure if you apply this to |
OK I see the example in the comments cites |
@wiseoldduck I think you are right -- We probably shouldn't do anything to round. I'll remove that. @Adlai-Holler When creating an ASLayout, it uses I don't love this solution, but it with 3x devices in particular floats/doubles will try to represent a value they can't leading to garbage at some point. If we treat that garbage as true then we will see weird issues like these. |
Good change. |
…up#864) * [Issue 838] Update ASCeilPixelValue and ASRoundPixelValue Layouts can come back for 3x devices as a repeating decimal. Floats/doubles have a hard time representing these values and often the last digit or two will be garbage. This garbage can result in unexpected values from `ceil` or `round`. I try to fix this by subtracting `FLT_EPSILON` from the value before calling `ceil` or `round` TextureGroup#838 * addressed comments on the pr
Layouts can come back for 3x devices as a repeating decimal. Floats/doubles have a hard time representing these values and often the last digit or two will be garbage. This garbage can result in unexpected values from
ceil
orround
. I try to fix this by subtractingFLT_EPSILON
from the value before callingceil
orround
#838