551871 - normal weight (400) doesn't map to medium (500) correctly

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[John Daggett (:jtd)]

As noted in the CSS3 Fonts spec, when a normal (400) weight face doesn't exist and font-weight is set to normal, 500 is checked first, then weights < 400.  Our current code doesn't check for a 500 weight in the missing 400 case.

With the attached testpage, the medium (500) face should be used in both cases.

Comment 1

[John Daggett (:jtd)]

Attachment: patch, v.0.1, fixup weight mapping with reftests

Comment 2

[John Daggett (:jtd)]

Attachment: GDI fake bolding pattern

Screenshot showing how the combination of LOGFONT weight and weight as specified in the usWeightClass value of the OS/2 table of the font affects rendering.

Basically, GDI uses an 'n + 2' rule, if the weight in the LOGFONT is <font weight> + 200 or more, it bolds.  This means it will bold a 100-weight font at 400 and a 700-weight font at 900.

To produce this I generated a set of fonts with weights 100-900 based on a single original.  Then I tweaked the GDI font code to always pass the font entry weight directly into the LOGFONT struct.  So to make fake bolding work correctly (i.e. consistent with DirectWrite/Mac) we need to read the usWeightClass and specify the LOGFONT weight based on mNeedsBold and the underlying usWeightClass weight.  Argh.

Comment 3

[John Daggett (:jtd)]

Attachment: patch, v.0.2b, fixup weight mapping

Refreshed.

Comment 4

[John Daggett (:jtd)]

Attachment: patch, v.0.2c, fixup weight mapping with more extensive reftests

This consists of mostly tightening up the code in gfxFontFamily::FindFontForStyle to assure that it matches weights correctly.  It includes a number of new reftests to test various weight configurations within families.

Comment 5

[Jonathan Kew [:jfkthame]]

Comment on attachment 459317 [details] [diff] [review]
patch, v.0.2c, fixup weight mapping with more extensive reftests

Looks good; the logic seems to make sense (though it always takes me a while to figure out what's going on with this weight-matching stuff).

>+        for (j = matchBaseWeight + direction; j < 10 && j > 0 && wghtSteps <= absDistance; j += direction) {

This line is awfully long.

Comment 6

[John Daggett (:jtd)]

Comment on attachment 459317 [details] [diff] [review]
patch, v.0.2c, fixup weight mapping with more extensive reftests

This is a low impact change that adds more robust testing of our font weight matching code.  We need this to pass CSS 2.1 tests:

http://test.csswg.org/suites/css2.1/20100701/html4/font-weight-normal-001.htm
http://test.csswg.org/suites/css2.1/20100701/html4/font-weight-normal-001-ref.htm
http://test.csswg.org/suites/css2.1/20100701/html4/font-weight-bolder-001.htm
http://test.csswg.org/suites/css2.1/20100701/html4/font-weight-bolder-001-ref.htm
http://test.csswg.org/suites/css2.1/20100701/html4/font-weight-lighter-001.htm
http://test.csswg.org/suites/css2.1/20100701/html4/font-weight-lighter-001-ref.htm

Note: these tests won't display correctly without CSSTest fonts installed:

http://www.w3.org/Style/CSS/Test/Fonts/CSSTest/CSSTest.zip

Comment 7

[John Daggett (:jtd)]

Pushed to trunk (w/ changes based on review comment)
http://hg.mozilla.org/mozilla-central/rev/66a256d340da