-
Notifications
You must be signed in to change notification settings - Fork 719
[css-values] Consider removing asymptotic special-cases for tan() #8527
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
Comments
Given that all of |
Right, but at least how every engine implements this right now implies doing maths in An alternative here could be to specify some epsilon for which values very close to |
I don't have a strong opinion on preserving this behavior. I don't recall the exact details, but it was part of some (seemingly?) widely-accepted convention for tangents. But JS can't represent these at all, indeed. The only thing making me want to preserve this is the note at the end of the section:
I think it would be good to preserve this if possible? But it's not a strong need; if this ends up being too awkward or weird to do, I'm fine abandoning it and leaving the undef values explicitly undefined. |
This comment was marked as duplicate.
This comment was marked as duplicate.
er, I guess my note duplicated what Tab already said; I skimmed their post too quickly. :) Sorry. Just to illustrate the roundtripping thing... here's a trivial data-uri testcase to test the expectation that Testcase:
Reference:
Chromium/WebKit render both of these with "c" at the bottom. Firefox renders the testcase with "c" at the top. [EDIT: I should note, I'm using trunk-ish versions of all engines. It looks like the official Chrome release at least doesn't render the testcases above with any rotation at all. The trunk-ish versions that I'm testing are: |
I think you should wrap them in a |
I intended to, but that doesn't seem to make a difference to my local testing, actually. So I'll leave them as-is. :) The WPT that @CanadaHonk referred to is discussed in more detail on https://bugzilla.mozilla.org/show_bug.cgi?id=1820673 - there's a WPT https://wpt.fyi/results/css/css-values/acos-asin-atan-atan2-computed.html that fails only-in-Firefox since it's testing this exact issue (though, as a side note, it happens to be testing it using the value |
Context: #7441
Precisely,
But radians are fine because (from https://en.cppreference.com/w/cpp/numeric/math/tan) "no common floating-point representation is able to represent π/2 exactly, thus there is no value of the argument for which a pole error occurs" when using radians. It would make sense to undefine all units if radians were the canonical unit, but they are not.
Sounds good.
Or you could use an algorithm for computing the tangent based on degrees. It's fine to have a radian-based implementation, but then before converting to radians you have to handle the special cases that won't work well in radians.
No, you are supposed to start from degrees which are the canonical unit, you can convert to radians as an intermediate step during calculations, but then don't convert back to degrees...
Not opposed to that, but it should be values very close to |
Let's not overstate - nothing in the spec requires doing calculations in any particular unit. Simplification will convert all values to the canonical unit, and so it usually makes sense to do the calcs in that unit just for simplicity, but nothing requires that. But indeed, if you are generally resolving calculations in the canonical unit, then a 90deg angle is exactly representable in floating point.
Indeed, |
The CSS Working Group just discussed
The full IRC log of that discussion |
Done. Behavior at asymptotes is explicitly undefined; I've rephrased the old text to be SHOULD-level, and only applying to impls that can exactly represent the asymptotic values. |
See resolved CSS spec issue: w3c/csswg-drafts#8527
Uh oh!
There was an error while loading. Please reload this page.
https://w3c.github.io/csswg-drafts/css-values/#trig-infinities has:
Given:
I'd prefer to remove the special-case for degrees here. Wdyt @tabatkins?
The text was updated successfully, but these errors were encountered: