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

RESOLVED FIXED in Firefox 55

Status

()

defect
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: birtles, Assigned: birtles)

Tracking

(Blocks 1 bug)

Trunk
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(1 attachment)

Assignee

Description

2 years ago
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
Assignee

Updated

2 years ago
Assignee: nobody → bbirtles
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)

Comment 2

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

https://reviewboard.mozilla.org/r/147414/#review151634
Attachment #8875995 - Flags: review?(hikezoe) → review+

Comment 3

2 years ago
Pushed by bbirtles@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/52c02dbc1e23
Round font-weight when interpolating, don't floor; r=hiro

Comment 4

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/52c02dbc1e23
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.