Summary: | Implement abs,sign calc functions | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Nikos Mouchtaris <nmouchtaris> | ||||||||||||||||||
Component: | New Bugs | Assignee: | Nikos Mouchtaris <nmouchtaris> | ||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||
Severity: | Normal | CC: | clopez, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, jonlee, macpherson, menard, simon.fraser, webkit-bug-importer, youennf | ||||||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||||
Attachments: |
|
Description
Nikos Mouchtaris
2021-09-01 17:57:47 PDT
Created attachment 437098 [details]
Patch
Created attachment 438167 [details]
Patch
This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess Created attachment 438306 [details]
Patch
Created attachment 438540 [details]
Patch
Comment on attachment 438540 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=438540&action=review > Source/WebCore/ChangeLog:14 > + Added support for calc functions sign and abs. Involved adding new css keywords and handling > + for parsing calc expression and computing the resulting value. Spec for these functions: > + https://drafts.csswg.org/css-values-4/#sign-funcs. This paragraph should go above the Tests line. > Source/WebCore/css/calc/CSSCalcExpressionNodeParser.cpp:198 > // TODO: clamp, sin, cos, tan, asin, acos, atan, atan2, pow, sqrt, hypot This list is no longer correct. > Source/WebCore/css/calc/CSSCalcOperationNode.cpp:466 > + if (calcOperator() == CalcOperator::Sign) > + combinedUnitType = CSSUnitType::CSS_NUMBER; The spec says "The abs(A) function contains one calculation A, and returns the absolute value of A, as the same type as the input" so I'm not sure this is right. > Source/WebCore/css/calc/CSSCalcOperationNode.cpp:1108 > + if (children.size() != 1) > + return std::numeric_limits<double>::quiet_NaN(); > + if (children[0] > 0) > + return 1; > + if (children[0] < 0) > + return -1; > + return children[0]; Does this work for -0? > Source/WebCore/platform/calc/CalcExpressionOperation.cpp:117 > + if (m_children.size() != 1) > + return std::numeric_limits<double>::quiet_NaN(); > + if (m_children[0]->evaluate(maxValue) > 0) > + return 1; > + if (m_children[0]->evaluate(maxValue) < 0) > + return -1; > + return m_children[0]->evaluate(maxValue); Same question about -0 > LayoutTests/imported/w3c/web-platform-tests/css/css-values/signs-abs-computed.html:15 > +test_math_used('sign(-1)', '-1', {type:'number', approx:0.1}); Test with -0 > LayoutTests/imported/w3c/web-platform-tests/css/css-values/signs-abs-computed.html:31 > +test_math_used('calc(sign(0))', '0', {type:'number', approx:0.1}); Need to also test with inputs that are lengths, angles etc to test the "as the same type as the input" part. Created attachment 438733 [details]
Patch
Created attachment 438860 [details]
Patch
Created attachment 439112 [details]
abs/sign functions
Created attachment 439169 [details]
abs/sign functions
Committed r283063 (242121@main): <https://commits.webkit.org/242121@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 439169 [details]. |