RESOLVED FIXED 189445
Correctly interpret from angle for conic gradients
https://bugs.webkit.org/show_bug.cgi?id=189445
Summary Correctly interpret from angle for conic gradients
Megan Gardner
Reported 2018-09-07 17:52:04 PDT
Correctly interperate from angle for conic gradients
Attachments
Patch (6.90 KB, patch)
2018-09-07 17:58 PDT, Megan Gardner
no flags
Patch (6.89 KB, patch)
2018-09-07 18:12 PDT, Megan Gardner
no flags
Megan Gardner
Comment 1 2018-09-07 17:58:41 PDT
Megan Gardner
Comment 2 2018-09-07 17:59:28 PDT
Tim Horton
Comment 3 2018-09-07 18:02:39 PDT
interperate is not a word
Megan Gardner
Comment 4 2018-09-07 18:12:16 PDT
Simon Fraser (smfr)
Comment 5 2018-09-08 12:23:21 PDT
Comment on attachment 349224 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=349224&action=review > Source/WebCore/css/CSSGradientValue.cpp:1439 > - angle = m_angle->floatValue(CSSPrimitiveValue::CSS_DEG); > + angle = m_angle->floatValue(CSSPrimitiveValue::CSS_RAD); How do you know this is radians? It looks like consumeAngle() can use whatever units the source specified.
Megan Gardner
Comment 6 2018-09-10 14:56:52 PDT
Because this is what is fed to CG, and CG wants radians. It's the only thing it's used for.
WebKit Commit Bot
Comment 7 2018-09-10 15:23:13 PDT
Comment on attachment 349224 [details] Patch Clearing flags on attachment: 349224 Committed r235868: <https://trac.webkit.org/changeset/235868>
WebKit Commit Bot
Comment 8 2018-09-10 15:23:15 PDT
All reviewed patches have been landed. Closing bug.
Simon Fraser (smfr)
Comment 9 2018-09-10 15:28:04 PDT
(In reply to Megan Gardner from comment #6) > Because this is what is fed to CG, and CG wants radians. It's the only thing > it's used for. But this is cross-platform code. I think ConicData needs to rename 'angle' to 'angleRadians' or at least have a comments saying that it's Radians. Ideally, we'd have a typedef.
Megan Gardner
Comment 10 2018-09-10 17:53:23 PDT
Follow up fix for Simon's naming request https://trac.webkit.org/changeset/235877/webkit
Note You need to log in before you can comment on or make changes to this bug.