Bug 230073 – Implement round,mod,rem functions for calc

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Nikos Mouchtaris]

Implement round,mod,rem functions for calc

[Nikos Mouchtaris]

Created attachment 437687 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/83174669>

[Nikos Mouchtaris]

Created attachment 438495 [details]
Patch

[EWS Watchlist]

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

[Nikos Mouchtaris]

Created attachment 438539 [details]
Patch

[Nikos Mouchtaris]

Created attachment 438888 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 438888 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=438888&action=review

> Source/WebCore/ChangeLog:6
> +        Reviewed by NOBODY (OOPS!).

Add blank line below here.

> Source/WebCore/css/calc/CSSCalcOperationNode.cpp:451
> +        LOG_WITH_STREAM(Calc, stream << "Failed to create round node because unable to determine category from " << prettyPrintNodes(values));

round -> step

> Source/WebCore/platform/calc/CalcExpressionOperation.cpp:37
> +    double lowerB = std::floor(a / std::abs(b))*std::abs(b);
> +    double upperB = lowerB + std::abs(b);

Maybe avoid calling std::abs(b) threee times. Also space around *

> Source/WebCore/platform/calc/CalcExpressionOperation.cpp:145
> +        return std::numeric_limits<double>::quiet_NaN();

Should this ASSERT_NOT_REACHED?

> Source/WebCore/platform/calc/CalcExpressionOperation.cpp:151
> +        auto upperB = ret.second;
> +        return upperB;

No need for upperB

> Source/WebCore/platform/calc/CalcExpressionOperation.cpp:157
> +        auto lowerB = ret.first;

Ditto

> Source/WebCore/platform/calc/CalcExpressionOperation.cpp:163
> +        auto ret = getNearestMultiples(m_children[0]->evaluate(maxValue), m_children[1]->evaluate(maxValue));

Put m_children[0]->evaluate(maxValue) etc into local variables?

> Source/WebCore/platform/calc/CalcExpressionOperation.cpp:166
> +        return std::abs(upperB-m_children[0]->evaluate(maxValue)) <= std::abs(m_children[1]->evaluate(maxValue)) / 2 ? upperB : lowerB;

space around -

[Nikos Mouchtaris]

Created attachment 439224 [details]
round/mod/rem functions

[EWS]

Committed r283073 (242131@main): <https://commits.webkit.org/242131@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 439224 [details].