480098 - [@font-face] synthetic bolding/italics applied incorrectly to downloadable fonts on Linux

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[John Daggett (:jtd)]

Loading the testcase attached to bug 468387, all the fonts on the page are downloadable fonts, in this case the M+ fonts (see layout/reftests/fonts/mplus).  The three columns on the left use a single family with 7 different weights defined.  The three columns on the right use the regular face but define @font-face rules for each weight and for italic/oblique.  So all the text in the right three columns should appear in 400-weight, non-slanted text.

There are two problems under Linux:

* 600/700 weight rows, left three columns are too bold (synthetic?)
* 700/900 weight rows, right three columns are synthetic bold

See the screenshot attached to bug 468387.

Comment 1

[Karl Tomlinson (:karlt)]

I can reproduce attachment 364058 [details] when viewing attachment 364053 [details].
Thanks for the testcase.

Comment 2

[Karl Tomlinson (:karlt)]

Attachment: correct units

gfxFontconfigUtils::FcWeightForBaseWeight(PRInt8 aBaseWeight) expects a weight between 0 and 10, which was a unit chosen to align with what gfxFontStyle:ComputeWeightAndOffset() provides.

However, gfxFontEntry::mWeight changed from these units to CSS absolute weights (well before I wrote gfxFcFontEntry::AdjustPatternToCSS).

gfxMixedFontFamily::FindWeightsForStyle uses gfxFontEntry::mWeight/100, so I assume there is no need to deal with CSS weights that are not divisible by 100.

Comment 3

[John Daggett (:jtd)]

(In reply to comment #2)
> gfxMixedFontFamily::FindWeightsForStyle uses gfxFontEntry::mWeight/100, so I
> assume there is no need to deal with CSS weights that are not divisible by 100.

Since these faces are defined via @font-face rules, there's no way to get a weight that is not a multiple of 100 in the inclusive range [100 .. 900].

Comment 4

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

(In reply to comment #3)
> Since these faces are defined via @font-face rules, there's no way to get a
> weight that is not a multiple of 100 in the inclusive range [100 .. 900].

It's probably good to document that with an NS_ASSERTION or NS_ABORT_IF_FALSE.

Comment 5

[Bill Gianopoulos [:WG9s]]

(In reply to comment #2)
> Created an attachment (id=364190) [details]
> correct units
Even with the patch applied, the 700 and 900 weight lines are bolder than the 400 weight line.  As I understand the testcase, the red portion of each line should be identical and they should all be 400 weight.

Comment 6

[Bill Gianopoulos [:WG9s]]

Attachment: screenshot with patch

Here is a screenshot of the testcase from a build with the patch from attachment 364190 [details] [diff] [review] included.

Comment 7

[Bill Gianopoulos [:WG9s]]

(In reply to comment #5)
> (In reply to comment #2)
> > Created an attachment (id=364190) [details] [details]
> > correct units
> Even with the patch applied, the 700 and 900 weight lines are bolder than the
> 400 weight line.  As I understand the testcase, the red portion of each line
> should be identical and they should all be 400 weight.

Oddly, I seem to no longer be able to reproduce this issue.

Comment 8

[John Daggett (:jtd)]

Karl, is this ready for checkin?

One more testcase:
http://people.mozilla.org/~jdaggett/font-face/synthetic-weight-style.html

Comment 9

[Karl Tomlinson (:karlt)]

(In reply to comment #8)
> Karl, is this ready for checkin?

Yes.

> One more testcase:
> http://people.mozilla.org/~jdaggett/font-face/synthetic-weight-style.html

That passes with the patch.

Comment 10

[John Daggett (:jtd)]

With the fix for bug 480267 I added a reftest along the lines of the test in comment 8.  You'll need to swizzle the condition for that test when you check this in.

Comment 11

[Karl Tomlinson (:karlt)]

http://hg.mozilla.org/mozilla-central/rev/b34586dde1b5

Comment 12

[Karl Tomlinson (:karlt)]

Comment on attachment 364190 [details] [diff] [review]
correct units

Requesting 1.9.1 approval for a small change covered by reftests.

Comment 13

[John Daggett (:jtd)]

Pushed to 1.9.1
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/16ff7ab20ac5