1493181 - The size of the 'ch' unit may be inappropriately rounded up when converting to appUnits

Status: RESOLVED FIXED

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

Description

[Jonathan Kew [:jfkthame]]

Attachment: testcase that shows failures on mac/win/android platforms

When converting the width of a font's '0' glyph from floating-point device pixels into integer appUnits for use as the basis of the CSS 'ch' unit, we apply the ceiling function rather than rounding, here:
https://searchfox.org/mozilla-central/rev/0b8ed772d24605d7cb44c1af6d59e4ca023bd5f5/layout/style/ServoBindings.cpp#2559

This tends to sometimes make 'ch' fractionally (one appUnit) larger than would be expected, affecting layout when sizes are expressed in multiples of 'ch' units.

See attached testcase, which shows failures on a few of the test elements -- exactly which ones varies if you zoom in/out, as this affects where the rounding errors occur -- on macOS, Windows and Android; only Linux seems immune (probably because glyph advances are pixel-rounded by the font backend).

Resolving this will be required in order to reliably fix bug 1432198, as it can affect the computation of tab positions.

Comment 1

[Jonathan Kew [:jfkthame]]

Attachment: Test for expected width of CSS 'ch' unit

Reftest form of the testcase here, which currently fails across all platforms except Linux.

Comment 2

[Jonathan Kew [:jfkthame]]

Attachment: Use rounding instead of ceiling when computing 'ch' width; update test expectations accordingly

The trivial fix of changing ceil() to round() resolves the problem; and as a nice bonus, it also fixes a bunch of other currently-failing testcases, so we can remove those failure annotations as well.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/70160e10c604
Test for expected width of CSS 'ch' unit. r=manishearth
https://hg.mozilla.org/integration/mozilla-inbound/rev/0b11e726fe9c
Use rounding instead of ceiling when computing 'ch' width; update test expectations accordingly. r=manishearth

Comment 4

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/70160e10c604
https://hg.mozilla.org/mozilla-central/rev/0b11e726fe9c