1371196 - font-weight should be rounded to nearest value (not floored)

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Brian Birtles (:birtles)]

I was investigating why dom/smil/test/test_smilMappedAttrFromTo.xhtml fails with Stylo for the following test:

 new AnimTestcaseFromTo("normal", "bold",
                        { fromComp: "400", midComp: "500", toComp: "700" }),

with the following error:

  font-weight: checking value halfway through animation - got "600", expected "500"

Sure, enough, looking at the Gecko code, at 50% of the way between 400 and 700, Gecko produces 500.

The code is as follows:

> double interpolatedValue = aCoeff1 * double(aValue1.GetIntValue()) +
>                            aCoeff2 * double(aValue2.GetIntValue());

i.e. = 0.5 * 400.0 + 0.5 * 700.0
     = 200.0 + 350.0
     = 550.0

> int32_t result = floor(interpolatedValue + 0.5);

i.e. = floor(550.5)
     = 550.0

> if (aProperty == eCSSProperty_font_weight) {
>   if (result < 100) {
>     result = 100;
>   } else if (result > 900) {
>     result = 900;
>   }
>   result -= result % 100;
> } else {
>   result = RestrictValue(aProperty, result);
> }

i.e. result = 550 - 550 % 100
            = 550 - 50
            = 500

That is, we round down.

The spec, however, says:

  font weight: interpolated via discrete steps (multiples of 100). The 
  interpolation happens in real number space and is converted to an integer by
  rounding to the nearest multiple of 100, with values halfway between
  multiples of 100 rounded towards positive infinity.[1]

It looks like the code that does the rounding down was implemented in bug 528234 in Nov 2009 but the spec was updated in Mar 2012[2].

We should update Gecko to no longer floor but round and update the tests so Stylo passes too.

[1] https://drafts.csswg.org/css-transitions/#animtype-font-weight
[2] https://github.com/w3c/csswg-drafts/commit/00c62861095c9c3b911f3af0747790440a8e6467

Comment 1

[Brian Birtles (:birtles)]

Attachment: Bug 1371196 - Round font-weight when interpolating, don't floor;

The spec says,

 font weight: interpolated via discrete steps (multiples of 100). The
 interpolation happens in real number space and is converted to an integer by
 rounding to the nearest multiple of 100, with values halfway between
 multiples of 100 rounded towards positive infinity.[1]

However, our implementation pre-dates this spec text (bug 528234 landed Nov 2009
whereas the spec was updated in Mar 2012[2]).

This patch rounds the result by simply adding 50 to the result before we floor
it (which is good enough in this case because we don't need to worry about
negative values).

It also slightly simplifies the logic by re-using Clamp from MathAlgorithms.h.

[1] https://drafts.csswg.org/css-transitions/#animtype-font-weight
[2] https://github.com/w3c/csswg-drafts/commit/00c62861095c9c3b911f3af0747790440a8e6467

Review commit: https://reviewboard.mozilla.org/r/147414/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/147414/

Comment 2

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 8875995 [details]
Bug 1371196 - Round font-weight when interpolating, don't floor;

https://reviewboard.mozilla.org/r/147414/#review151634

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/52c02dbc1e23
Round font-weight when interpolating, don't floor; r=hiro

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/52c02dbc1e23