Closed Bug 480098 Opened 12 years ago Closed 12 years ago

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

Categories

(Core :: Graphics, defect)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: jtd, Assigned: karlt)

References

()

Details

(Keywords: fixed1.9.1)

Attachments

(1 file, 1 obsolete file)

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.
I can reproduce attachment 364058 [details] when viewing attachment 364053 [details].
Thanks for the testcase.
Assignee: nobody → mozbugz
Attached patch correct unitsSplinter Review
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.
Attachment #364190 - Flags: review?(roc)
(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].
(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.
(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.
Attached image screenshot with patch (obsolete) —
Here is a screenshot of the testcase from a build with the patch from attachment 364190 [details] [diff] [review] included.
(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.
Attachment #364524 - Attachment description: scrennshot with patch → screenshot with patch
Attachment #364524 - Attachment is obsolete: true
Karl, is this ready for checkin?

One more testcase:
http://people.mozilla.org/~jdaggett/font-face/synthetic-weight-style.html
(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.
Whiteboard: [needs landing]
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.
http://hg.mozilla.org/mozilla-central/rev/b34586dde1b5
Blocks: 458169
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing] → [needs 191 landing]
Whiteboard: [needs 191 landing] → [needs 191 approval]
Comment on attachment 364190 [details] [diff] [review]
correct units

Requesting 1.9.1 approval for a small change covered by reftests.
Attachment #364190 - Flags: approval1.9.1?
Attachment #364190 - Flags: approval1.9.1? → approval1.9.1+
Pushed to 1.9.1
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/16ff7ab20ac5
Keywords: fixed1.9.1
Whiteboard: [needs 191 approval]
You need to log in before you can comment on or make changes to this bug.