Closed
Bug 480098
Opened 15 years ago
Closed 15 years ago
[@font-face] synthetic bolding/italics applied incorrectly to downloadable fonts on Linux
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: jtd, Assigned: karlt)
References
()
Details
(Keywords: fixed1.9.1)
Attachments
(1 file, 1 obsolete file)
2.59 KB,
patch
|
roc
:
review+
vlad
:
approval1.9.1+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•15 years ago
|
||
I can reproduce attachment 364058 [details] when viewing attachment 364053 [details]. Thanks for the testcase.
Assignee: nobody → mozbugz
Assignee | ||
Comment 2•15 years ago
|
||
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)
Attachment #364190 -
Flags: review?(roc) → review+
Reporter | ||
Comment 3•15 years ago
|
||
(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.
Comment 5•15 years ago
|
||
(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•15 years ago
|
||
Here is a screenshot of the testcase from a build with the patch from attachment 364190 [details] [diff] [review] included.
Comment 7•15 years ago
|
||
(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.
Updated•15 years ago
|
Attachment #364524 -
Attachment description: scrennshot with patch → screenshot with patch
Attachment #364524 -
Attachment is obsolete: true
Reporter | ||
Comment 8•15 years ago
|
||
Karl, is this ready for checkin? One more testcase: http://people.mozilla.org/~jdaggett/font-face/synthetic-weight-style.html
Assignee | ||
Comment 9•15 years ago
|
||
(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]
Reporter | ||
Comment 10•15 years ago
|
||
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.
Assignee | ||
Comment 11•15 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/b34586dde1b5
Blocks: 458169
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing] → [needs 191 landing]
Assignee | ||
Updated•15 years ago
|
Whiteboard: [needs 191 landing] → [needs 191 approval]
Assignee | ||
Comment 12•15 years ago
|
||
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+
Reporter | ||
Comment 13•15 years ago
|
||
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.
Description
•