1570236 - calc(<int>) works differently from <int> if the variable is not representable as a 32-bit float.

Status: NEW

(Core :: CSS Parsing and Computation, defect, P3)

Description

[mildlyinteresting]

Attachment: z-index-bug.html

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:68.0) Gecko/20100101 Firefox/68.0

Steps to reproduce:

I have the following css

z-index: calc(var(--value) - 1)

where value is 2147483646, that is 1 less than the maximum signed int for 32 bits. The calculation is not working.

Attached a simple HTML with a reproduction scenario

Actual results:

The z-index is not working with the calc() function, but it works perfectly if the number is inserted directly in the CSS declaration.

From my tests, it seems that the maximum value for calc() is now 2147483583

Expected results:

The calc() function returns the correct number and the z-index is set correctly.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

This is not about var() but about calc(): calc(2147483646) doesn't work, but 2147483646 does... I'll take a look.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Ok, just got back to this. The explanation for this is easy:

The CSS parser tokenizes number tokens using 32-bit floats, but keeps track of the integer value as well.

The non-calc path for z-index requires integers (that is, z-index: 1.0 is invalid per spec). However z-index: calc(1.0) is perfectly valid.

From https://drafts.csswg.org/css-values-4/#calc-range:

Additionally, if a math function that resolves to <number> is used somewhere that only accepts <integer>, the computed value and used value are rounded to the nearest integer, in the same manner as clamping, above. The rounding method must be the same as is used for animations of integer values.

(calc(1.0) is such a function).

So for simplicity, even though there are only integers involved, we do the calc math and storage in floating-point numbers, and round at the end.

Issue is, 2147483646 is not a representable 32-bit float, and the closest one is 2147483648, which is outside of the i32 range. So when casted back to i32 it overflows and ta-da, there you go, a negative number.

So, possible fixes:

• Do a manual saturating floating point to integer conversion here. This will at least avoid the surprising behavior of the value turning negative. It's the least-effort one, but I think it may not fix the arithmetic case (i.e., what happens when you substract 1 to that number. I think it'd remain out of range for integers). Worth trying and testing it though.

• Changing the CSS parser to tokenize floats as f64 (and thus values::*::Number and such to store them, probably? Though we could also special-case calc here and truncate elsewhere). That'd avoid the precision issue in the first place that is causing the overflow. Probably worth it, but also pretty big change. (saschanaz hit a similar precision issue working on DOMMatrix). If we want to keep the precision all the way we'd grow style structs somewhat significantly in memory, which is unfortunate.

• Teach calc() to do integer arithmetic as much as possible. This implies not forgetting of int_value here, and handle subtractions and summations (and possibly multiplications?) as integer arithmetic if the two source tokens were integers. This is not too hard, I think...

I'm not sure 100% what the best path forward is... Cam / Simon, any strong preference or something else I've missed?

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Also, https://github.com/rust-lang/rust/issues/63171 made debugging this extra-fun, since the value rust was showing me back was in range for an i32.

Comment 4

[Simon Sapin (:SimonSapin)]

Issue is, 2147483646 is not a representable 32-bit float, and the closest one is 2147483648, which is outside of the i32 range. So when casted back to i32 it overflows and ta-da, there you go, a negative number.

For extra fun, this conversion might actually be Undefined Behavior: https://github.com/rust-lang/rust/issues/10184

I don’t have a strong opinion on the preferable approach here, though. What do other engines do?