Skip to content

Commit

Permalink
[CSS Math Functions] round() is incorrect when number is a multiple o…
Browse files Browse the repository at this point in the history 
…f step

https://bugs.webkit.org/show_bug.cgi?id=259702
rdar://113233588

Reviewed by Simon Fraser.

In Safari 16.6, round(up, 10px, 5px) resolves to 15px instead of 10px as it should.

From https://drafts.csswg.org/css-values-4/#round-func:
> If A is exactly equal to an integer multiple of B, round() resolves to A exactly

Also add WPT for https://drafts.csswg.org/css-values/#round-infinities

* LayoutTests/imported/w3c/web-platform-tests/css/css-values/round-mod-rem-computed-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-values/round-mod-rem-computed.html:
* Source/WebCore/platform/calc/CalcOperator.h:
(WebCore::evaluateCalcExpression):

Canonical link: https://commits.webkit.org/266504@main
  • Loading branch information
@nt1m
nt1m committed Aug 2, 2023
1 parent d2a72f5 commit 58d1ff8
Show file tree
Hide file tree
Showing 3 changed files with 183 additions and 11 deletions.
114 changes: 110 additions & 4 deletions ...tTests/imported/w3c/web-platform-tests/css/css-values/round-mod-rem-computed-expected.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,38 @@ PASS rem(1,1) should be used-value-equivalent to 0
PASS calc(round(100,10)) should be used-value-equivalent to 100
PASS calc(round(up, 101,10)) should be used-value-equivalent to 110
PASS calc(round(down, 106,10)) should be used-value-equivalent to 100
PASS calc(round(to-zero,105, 10)) should be used-value-equivalent to 100
PASS calc(round(to-zero,-105, 10)) should be used-value-equivalent to -100
PASS calc(round(-100,10)) should be used-value-equivalent to -100
PASS calc(round(up, -103,10)) should be used-value-equivalent to -100
PASS calc(round(to-zero, 105, 10)) should be used-value-equivalent to 100
PASS calc(round(to-zero, -105, 10)) should be used-value-equivalent to -100
PASS calc(round(-100, 10)) should be used-value-equivalent to -100
PASS calc(round(up, -103, 10)) should be used-value-equivalent to -100
PASS round(up, 0, 5) should be used-value-equivalent to 0
PASS round(down, 0, 5) should be used-value-equivalent to 0
PASS round(nearest, 0, 5) should be used-value-equivalent to 0
PASS round(to-zero, 0, 5) should be used-value-equivalent to 0
PASS round(up, 5, 5) should be used-value-equivalent to 5
PASS round(down, 5, 5) should be used-value-equivalent to 5
PASS round(nearest, 5, 5) should be used-value-equivalent to 5
PASS round(to-zero, 5, 5) should be used-value-equivalent to 5
PASS round(up, -5, 5) should be used-value-equivalent to -5
PASS round(down, -5, 5) should be used-value-equivalent to -5
PASS round(nearest, -5, 5) should be used-value-equivalent to -5
PASS round(to-zero, -5, 5) should be used-value-equivalent to -5
PASS round(up, 10, 5) should be used-value-equivalent to 10
PASS round(down, 10, 5) should be used-value-equivalent to 10
PASS round(nearest, 10, 5) should be used-value-equivalent to 10
PASS round(to-zero, 10, 5) should be used-value-equivalent to 10
PASS round(up, -10, 5) should be used-value-equivalent to -10
PASS round(down, -10, 5) should be used-value-equivalent to -10
PASS round(nearest, -10, 5) should be used-value-equivalent to -10
PASS round(to-zero, -10, 5) should be used-value-equivalent to -10
PASS round(up, 20, 5) should be used-value-equivalent to 20
PASS round(down, 20, 5) should be used-value-equivalent to 20
PASS round(nearest, 20, 5) should be used-value-equivalent to 20
PASS round(to-zero, 20, 5) should be used-value-equivalent to 20
PASS round(up, -20, 5) should be used-value-equivalent to -20
PASS round(down, -20, 5) should be used-value-equivalent to -20
PASS round(nearest, -20, 5) should be used-value-equivalent to -20
PASS round(to-zero, -20, 5) should be used-value-equivalent to -20
PASS mod(18,5) should be used-value-equivalent to 3
PASS rem(18,5) should be used-value-equivalent to 3
PASS mod(-140,-90) should be used-value-equivalent to -50
Expand Down Expand Up @@ -119,4 +147,82 @@ PASS rem(-18vw,5vw) should be used-value-equivalent to -3vw
PASS calc(round(1px + 0%, 1px + 0%)) should be used-value-equivalent to 1px
PASS calc(mod(3px + 0%, 2px + 0%)) should be used-value-equivalent to 1px
PASS calc(rem(3px + 0%, 2px + 0%)) should be used-value-equivalent to 1px
PASS round(0, 0) should be used-value-equivalent to calc(NaN)
PASS round(-0, 0) should be used-value-equivalent to calc(NaN)
PASS round(Infinity, 0) should be used-value-equivalent to calc(NaN)
PASS round(-Infinity, 0) should be used-value-equivalent to calc(NaN)
PASS round(-4, 0) should be used-value-equivalent to calc(NaN)
PASS round(4, 0) should be used-value-equivalent to calc(NaN)
PASS round(Infinity, Infinity) should be used-value-equivalent to calc(NaN)
PASS round(-Infinity, -Infinity) should be used-value-equivalent to calc(NaN)
PASS round(Infinity, -Infinity) should be used-value-equivalent to calc(NaN)
PASS round(-Infinity, Infinity) should be used-value-equivalent to calc(NaN)
PASS mod(0, 0) should be used-value-equivalent to calc(NaN)
PASS mod(-0, 0) should be used-value-equivalent to calc(NaN)
PASS mod(Infinity, 0) should be used-value-equivalent to calc(NaN)
PASS mod(-Infinity, 0) should be used-value-equivalent to calc(NaN)
PASS mod(-4, 0) should be used-value-equivalent to calc(NaN)
PASS mod(4, 0) should be used-value-equivalent to calc(NaN)
PASS mod(Infinity, Infinity) should be used-value-equivalent to calc(NaN)
PASS mod(-Infinity, -Infinity) should be used-value-equivalent to calc(NaN)
PASS mod(Infinity, -Infinity) should be used-value-equivalent to calc(NaN)
PASS mod(-Infinity, Infinity) should be used-value-equivalent to calc(NaN)
PASS rem(0, 0) should be used-value-equivalent to calc(NaN)
PASS rem(-0, 0) should be used-value-equivalent to calc(NaN)
PASS rem(Infinity, 0) should be used-value-equivalent to calc(NaN)
PASS rem(-Infinity, 0) should be used-value-equivalent to calc(NaN)
PASS rem(-4, 0) should be used-value-equivalent to calc(NaN)
PASS rem(4, 0) should be used-value-equivalent to calc(NaN)
PASS rem(Infinity, Infinity) should be used-value-equivalent to calc(NaN)
PASS rem(-Infinity, -Infinity) should be used-value-equivalent to calc(NaN)
PASS rem(Infinity, -Infinity) should be used-value-equivalent to calc(NaN)
PASS rem(-Infinity, Infinity) should be used-value-equivalent to calc(NaN)
PASS round(up, Infinity, 4) should be used-value-equivalent to calc(Infinity)
PASS round(up, -Infinity, 4) should be used-value-equivalent to calc(-Infinity)
PASS round(up, Infinity, -4) should be used-value-equivalent to calc(Infinity)
PASS round(up, -Infinity, -4) should be used-value-equivalent to calc(-Infinity)
PASS round(down, Infinity, 4) should be used-value-equivalent to calc(Infinity)
PASS round(down, -Infinity, 4) should be used-value-equivalent to calc(-Infinity)
PASS round(down, Infinity, -4) should be used-value-equivalent to calc(Infinity)
PASS round(down, -Infinity, -4) should be used-value-equivalent to calc(-Infinity)
PASS round(nearest, Infinity, 4) should be used-value-equivalent to calc(Infinity)
PASS round(nearest, -Infinity, 4) should be used-value-equivalent to calc(-Infinity)
PASS round(nearest, Infinity, -4) should be used-value-equivalent to calc(Infinity)
PASS round(nearest, -Infinity, -4) should be used-value-equivalent to calc(-Infinity)
PASS round(to-zero, Infinity, 4) should be used-value-equivalent to calc(Infinity)
PASS round(to-zero, -Infinity, 4) should be used-value-equivalent to calc(-Infinity)
PASS round(to-zero, Infinity, -4) should be used-value-equivalent to calc(Infinity)
PASS round(to-zero, -Infinity, -4) should be used-value-equivalent to calc(-Infinity)
PASS round(nearest, 0, Infinity) should be used-value-equivalent to 0
PASS round(nearest, 4, Infinity) should be used-value-equivalent to 0
PASS round(nearest, -0, Infinity) should be used-value-equivalent to calc(-0)
PASS round(nearest, -4, Infinity) should be used-value-equivalent to calc(-0)
PASS round(nearest, 0, -Infinity) should be used-value-equivalent to 0
PASS round(nearest, 4, -Infinity) should be used-value-equivalent to 0
PASS round(nearest, -0, -Infinity) should be used-value-equivalent to calc(-0)
PASS round(nearest, -4, -Infinity) should be used-value-equivalent to calc(-0)
PASS round(to-zero, 0, Infinity) should be used-value-equivalent to 0
PASS round(to-zero, 4, Infinity) should be used-value-equivalent to 0
PASS round(to-zero, -0, Infinity) should be used-value-equivalent to calc(-0)
PASS round(to-zero, -4, Infinity) should be used-value-equivalent to calc(-0)
PASS round(to-zero, 0, -Infinity) should be used-value-equivalent to 0
PASS round(to-zero, 4, -Infinity) should be used-value-equivalent to 0
PASS round(to-zero, -0, -Infinity) should be used-value-equivalent to calc(-0)
PASS round(to-zero, -4, -Infinity) should be used-value-equivalent to calc(-0)
PASS round(up, 1, Infinity) should be used-value-equivalent to calc(Infinity)
PASS round(up, 0, Infinity) should be used-value-equivalent to 0
PASS round(up, -1, Infinity) should be used-value-equivalent to calc(-0)
PASS round(up, 1, -Infinity) should be used-value-equivalent to calc(Infinity)
PASS round(up, 0, -Infinity) should be used-value-equivalent to 0
PASS round(up, -1, -Infinity) should be used-value-equivalent to calc(-0)
PASS round(down, 1, Infinity) should be used-value-equivalent to calc(-0)
PASS round(down, 0, Infinity) should be used-value-equivalent to 0
PASS round(down, -1, Infinity) should be used-value-equivalent to calc(-Infinity)
PASS round(down, 1, -Infinity) should be used-value-equivalent to calc(-0)
PASS round(down, 0, -Infinity) should be used-value-equivalent to 0
PASS round(down, -1, -Infinity) should be used-value-equivalent to calc(-Infinity)
PASS mod(-0, Infinity) should be used-value-equivalent to calc(NaN)
PASS mod(0, -Infinity) should be used-value-equivalent to calc(NaN)
PASS mod(-4, Infinity) should be used-value-equivalent to calc(NaN)
PASS mod(4, -Infinity) should be used-value-equivalent to calc(NaN)

78 changes: 71 additions & 7 deletions LayoutTests/imported/w3c/web-platform-tests/css/css-values/round-mod-rem-computed.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,24 @@
test_math_used('mod(1,1)', '0', {type:'number'});
test_math_used('rem(1,1)', '0', {type:'number'});

//Test basic round
// Test basic round
test_math_used('calc(round(100,10))', '100', {type:'number'});
test_math_used('calc(round(up, 101,10))', '110', {type:'number'});
test_math_used('calc(round(down, 106,10))', '100', {type:'number'});
test_math_used('calc(round(to-zero,105, 10))', '100', {type:'number'});
test_math_used('calc(round(to-zero,-105, 10))', '-100', {type:'number'});
test_math_used('calc(round(-100,10))', '-100', {type:'number'});
test_math_used('calc(round(up, -103,10))', '-100', {type:'number'});
test_math_used('calc(round(to-zero, 105, 10))', '100', {type:'number'});
test_math_used('calc(round(to-zero, -105, 10))', '-100', {type:'number'});
test_math_used('calc(round(-100, 10))', '-100', {type:'number'});
test_math_used('calc(round(up, -103, 10))', '-100', {type:'number'});

//Test basic mod/rem
// Test round when first number is a multiple of the second number.
for (let number of [0, 5, -5, 10, -10, 20, -20]) {
test_math_used(`round(up, ${number}, 5)`, `${number}`, {type:'number'});
test_math_used(`round(down, ${number}, 5)`, `${number}`, {type:'number'});
test_math_used(`round(nearest, ${number}, 5)`, `${number}`, {type:'number'});
test_math_used(`round(to-zero, ${number}, 5)`, `${number}`, {type:'number'});
}

// Test basic mod/rem
test_math_used('mod(18,5)', '3', {type:'number'});
test_math_used('rem(18,5)', '3', {type:'number'});
test_math_used('mod(-140,-90)', '-50', {type:'number'});
Expand All @@ -33,7 +41,7 @@
test_math_used('mod(140,-90)', '-40', {type:'number'});
test_math_used('rem(140,-90)', '50', {type:'number'});

//Test basic calculations
// Test basic calculations
test_math_used('calc(round(round(100,10), 10))', '100', {type:'number'});
test_math_used('calc(round(up, round(100,10) + 1,10))', '110', {type:'number'});
test_math_used('calc(round(down, round(100,10) + 2 * 3,10))', '100', {type:'number'});
Expand Down Expand Up @@ -147,4 +155,60 @@
test_math_used('calc(mod(3px + 0%, 2px + 0%))', '1px');
test_math_used('calc(rem(3px + 0%, 2px + 0%))', '1px');

// In round(A, B), if B is 0, the result is NaN. If A and B are both infinite, the result is NaN.
// In mod(A, B) or rem(A, B), if B is 0, the result is NaN. If A is infinite, the result is NaN.
for (let operator of ['round', 'mod', 'rem']) {
test_math_used(`${operator}(0, 0)`, 'calc(NaN)', {type: 'number'});
test_math_used(`${operator}(-0, 0)`, 'calc(NaN)', {type: 'number'});
test_math_used(`${operator}(Infinity, 0)`, 'calc(NaN)', {type: 'number'});
test_math_used(`${operator}(-Infinity, 0)`, 'calc(NaN)', {type: 'number'});
test_math_used(`${operator}(-4, 0)`, 'calc(NaN)', {type: 'number'});
test_math_used(`${operator}(4, 0)`, 'calc(NaN)', {type: 'number'});
test_math_used(`${operator}(Infinity, Infinity)`, 'calc(NaN)', {type: 'number'});
test_math_used(`${operator}(-Infinity, -Infinity)`, 'calc(NaN)', {type: 'number'});
test_math_used(`${operator}(Infinity, -Infinity)`, 'calc(NaN)', {type: 'number'});
test_math_used(`${operator}(-Infinity, Infinity)`, 'calc(NaN)', {type: 'number'});
}

// In round(A, B), if A is infinite but B is finite, the result is the same infinity.
for (let roundingStrategy of ['up', 'down', 'nearest', 'to-zero']) {
test_math_used(`round(${roundingStrategy}, Infinity, 4)`, 'calc(Infinity)', {type: 'number'});
test_math_used(`round(${roundingStrategy}, -Infinity, 4)`, 'calc(-Infinity)', {type: 'number'});
test_math_used(`round(${roundingStrategy}, Infinity, -4)`, 'calc(Infinity)', {type: 'number'});
test_math_used(`round(${roundingStrategy}, -Infinity, -4)`, 'calc(-Infinity)', {type: 'number'});
}

// If A is finite but B is infinite, the result depends on the <rounding-strategy> and the sign of A:
// nearest & to-zero: If A is positive or 0⁺, return 0⁺. Otherwise, return 0⁻.
for (let roundingStrategy of ['nearest', 'to-zero']) {
test_math_used(`round(${roundingStrategy}, 0, Infinity)`, '0', {type: 'number'});
test_math_used(`round(${roundingStrategy}, 4, Infinity)`, '0', {type: 'number'});
test_math_used(`round(${roundingStrategy}, -0, Infinity)`, 'calc(-0)', {type: 'number'});
test_math_used(`round(${roundingStrategy}, -4, Infinity)`, 'calc(-0)', {type: 'number'});
test_math_used(`round(${roundingStrategy}, 0, -Infinity)`, '0', {type: 'number'});
test_math_used(`round(${roundingStrategy}, 4, -Infinity)`, '0', {type: 'number'});
test_math_used(`round(${roundingStrategy}, -0, -Infinity)`, 'calc(-0)', {type: 'number'});
test_math_used(`round(${roundingStrategy}, -4, -Infinity)`, 'calc(-0)', {type: 'number'});
}

// up: If A is positive (not zero), return +∞. If A is 0⁺, return 0⁺. Otherwise, return 0⁻.
test_math_used('round(up, 1, Infinity)', 'calc(Infinity)', {type: 'number'});
test_math_used('round(up, 0, Infinity)', '0', {type: 'number'});
test_math_used('round(up, -1, Infinity)', 'calc(-0)', {type: 'number'});
test_math_used('round(up, 1, -Infinity)', 'calc(Infinity)', {type: 'number'});
test_math_used('round(up, 0, -Infinity)', '0', {type: 'number'});
test_math_used('round(up, -1, -Infinity)', 'calc(-0)', {type: 'number'});
// down: If A is negative (not zero), return −∞. If A is 0⁻, return 0⁻. Otherwise, return 0⁺.
test_math_used('round(down, 1, Infinity)', 'calc(-0)', {type: 'number'});
test_math_used('round(down, 0, Infinity)', '0', {type: 'number'});
test_math_used('round(down, -1, Infinity)', 'calc(-Infinity)', {type: 'number'});
test_math_used('round(down, 1, -Infinity)', 'calc(-0)', {type: 'number'});
test_math_used('round(down, 0, -Infinity)', '0', {type: 'number'});
test_math_used('round(down, -1, -Infinity)', 'calc(-Infinity)', {type: 'number'});

// In mod(A, B) only, if B is infinite and A has opposite sign to B (including an oppositely-signed zero), the result is NaN.
test_math_used('mod(-0, Infinity)', 'calc(NaN)', {type: 'number'});
test_math_used('mod(0, -Infinity)', 'calc(NaN)', {type: 'number'});
test_math_used('mod(-4, Infinity)', 'calc(NaN)', {type: 'number'});
test_math_used('mod(4, -Infinity)', 'calc(NaN)', {type: 'number'});
</script>
2 changes: 2 additions & 0 deletions Source/WebCore/platform/calc/CalcOperator.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ template <typename T, typename Function>
double evaluateCalcExpression(CalcOperator calcOperator, const Vector<T>& children, Function&& evaluate)
{
auto getNearestMultiples = [](double a, double b) -> std::pair<double, double> {
if (!std::fmod(a, b))
return { a, a };
double lower = std::floor(a / std::abs(b)) * std::abs(b);
double upper = lower + std::abs(b);
return { lower, upper };
Expand Down

0 comments on commit 58d1ff8

Please sign in to comment.