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

[css-images] compute/round the <angle> value for image-orientation property is not consistent #1206

Closed
chenpighead opened this issue Apr 11, 2017 · 7 comments

Comments

@chenpighead
Copy link

@chenpighead chenpighead commented Apr 11, 2017

Quote from the spec:

The computed value of the property is calculated by rounding the specified angle to the nearest quarter-turn (90deg, 100grad, .25turn, etc.), rounding away from 0 (that is, 45deg is rounded to 90deg, while -45deg is rounded to -90deg), then moduloing the value by 1 turn (360deg, 400grad, etc.).

As to the rounding away from 0, the example says that -45deg is rounded to -90deg. What about 315deg (the equivalence of -45deg)? Should we round it to 360deg (equivalence of 0deg) to align the rounding away from 0 stuff? This is a bit confusing to me.

If we intent to always make -45deg (and 315deg) always round to -90deg, then IMO this worth some more words in the spec. Since there are only four possible rounding edges (45deg, 135deg, 225deg, 315deg), maybe we could also specify all of them in the spec. instead of just putting 2 examples.

@LeaVerou
Copy link
Member

@LeaVerou LeaVerou commented Apr 12, 2017

-45deg should definitely round to the same angle as 315deg, so this looks like a bug.

@LeaVerou LeaVerou changed the title [css-images] compute/round the <angle> value for image-orientation property is a bit confusing [css-images] compute/round the <angle> value for image-orientation property is not consistent Apr 12, 2017
@tabatkins
Copy link
Member

@tabatkins tabatkins commented Apr 12, 2017

Yeah, it's a bug. There's no greater reasoning behind which way to round; I just needed an unambiguous answer. We can just swap the order of the round/mod to be mod/round instead, that'll take care of things.

@LeaVerou
Copy link
Member

@LeaVerou LeaVerou commented Apr 12, 2017

We can just swap the order of the round/mod to be mod/round instead, that'll take care of things.

Heh, I was about to post the exact same thing, then I realized it wouldn't work. 315deg mod 1turn is still 315deg...

@tabatkins
Copy link
Member

@tabatkins tabatkins commented Apr 12, 2017

done in ebdeb61

@tabatkins tabatkins closed this Apr 12, 2017
@tabatkins
Copy link
Member

@tabatkins tabatkins commented Apr 12, 2017

? It definitely works. It changes -45deg to round to 0deg (same as 315deg does), tho.

@LeaVerou
Copy link
Member

@LeaVerou LeaVerou commented Apr 12, 2017

Let me know if I'm missing something here:
-45 mod 360 is still -45, which rounds to -90 (due to the "rounding away from 0" clause).
315 mod 360 is still 315, which rounds to 360 = 0 (due to the same "rounding away from 0" clause).

@tabatkins
Copy link
Member

@tabatkins tabatkins commented Apr 12, 2017

No, that's using the bizarro broken version of modulus popularized by C. I made it clear in the spec text that we're using the correct version of modulus, which restricts it to the half-open range [0deg, 360deg).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants