Font rendering in OS X 10.9 Mavericks is much lighter and less legible with some sites

RESOLVED FIXED in mozilla31

Status

()

Core
Layout: Text
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: Mario Grgic, Assigned: jfkthame)

Tracking

24 Branch
mozilla31
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(22 attachments, 7 obsolete attachments)

242.72 KB, image/tiff
Details
242.91 KB, image/tiff
Details
163.45 KB, image/tiff
Details
2.45 KB, text/html
Details
1.32 KB, patch
Details | Diff | Splinter Review
144.84 KB, image/png
Details
1.27 KB, patch
davidb
: review+
Details | Diff | Splinter Review
2.37 KB, text/html
Details
70.90 KB, text/plain
Details
60.19 KB, text/plain
Details
66.10 KB, text/plain
Details
56.01 KB, text/plain
Details
15.36 KB, patch
jtd
: review-
Details | Diff | Splinter Review
71.08 KB, text/plain
Details
8.31 KB, patch
Details | Diff | Splinter Review
3.51 KB, patch
jtd
: review-
Details | Diff | Splinter Review
82.85 KB, text/plain
Details
67.86 KB, text/plain
Details
48.23 KB, text/plain
Details
39.80 KB, text/plain
Details
3.49 KB, text/plain
Details
5.23 KB, patch
jtd
: review+
Details | Diff | Splinter Review
(Reporter)

Description

5 years ago
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:24.0) Gecko/20100101 Firefox/24.0 (Beta/Release)
Build ID: 20130910160258

Steps to reproduce:

Font rendering in Mac OS X 10.9 and Firefox 24 is quite a bit lighter than it was with OS X 10.8. 

Some fonts are affected more than others and some sites as a consequence are much less legible. 

Good example illustrating this is http://appleinsider.com/ where article preview text (in Helvetica Neue) rendering is much lighter than it was in OS X 10.8.x.
(Reporter)

Comment 1

5 years ago
Created attachment 822838 [details]
Screenshot of text with FF24 on OS X 10.9 Mavericks
(Reporter)

Comment 2

5 years ago
Created attachment 822839 [details]
Screenshot of text with FF24 on OS X 10.8.5 Mountain Lion

Updated

5 years ago
Component: Untriaged → Layout: Text
Product: Firefox → Core
(Assignee)

Comment 3

5 years ago
What weights of Helvetica Neue are provided in 10.9?

On OS X 10.7, I'm seeing Helvetica Neue Light for that article preview text. What face is used on 10.9? (You can check the Fonts panel in the web inspector to find out.) It's possible 10.9 is shipping with a different set of weights in the font family.

Updated

5 years ago
Blocks: 883824
(Reporter)

Comment 4

5 years ago
Created attachment 823507 [details]
Difference between Light vs Thin version of font.
(Reporter)

Comment 5

5 years ago
Yes, the site specifies "Helvetica Neue Light", however, it looks like Firefox 24 on OS X 10.9 renders it as "Helvetica Neue Thin" instead. So, it could be Firefox is wrongly chooses Thin front instead of Light version.

In fact, if I select some text, paste it in Pages and select "Helvetica Neue Thin" it looks exactly like how Firefox 24 on OS X 10.9 renders it. However, it should be rendered in "Helvetica Neue Light" which is thicker than Light version. 

The attached image demonstrates the difference between these two fonts.
(Reporter)

Comment 6

5 years ago
By the way in OS X 10.9 Helvetica Neue is shipped in following versions:

Regular, Medium, Light, Thin, UltraLight, Italic, Medium Italic, Thin Italic, UltraLight Italic, Bold, Bold Italic, Condensed Bold, Condensed Black.
(Assignee)

Comment 7

5 years ago
The site actually calls for font-family "Helvetica Neue" with font-weight 300.

(Yes, the CSS also mentions "HelveticaNeue-Light" and "Helvetica Neue Light", but these are not actually valid font FAMILY names, at least with the set of fonts as shipped on OS X; the former is a PostScript font name, and the latter is a font FACE name.)

So the question is then what values are present in the usWeightClass field in the OS/2 table of the various Helvetica Neue faces. That should determine which face is used for the properties "font-family:Helvetica Neue; font-weight:300" as specified in the CSS. (Unless perhaps Cocoa's NSFontManager is doing something strange with the font attributes...)

Mario, if you're able to check the OS/2 tables in the Thin and Light faces (e.g. using a tool such as FontForge or TTX), I'd be interested to know what weight values they have.
(Reporter)

Comment 8

5 years ago
Helvetica Neue is a system font in *.dfont format located in /System/Library/Fonts/. I'm not sure how I can look at properties of individual faces inside the *.dfont file, but if I inspect the HelveticaNeue.dfont with ttx and dump the OS/2 table it only lists one usWeightClass with value 700 which seems too high.
(Assignee)

Comment 9

5 years ago
Hmm, yes, it looks like ttx doesn't know how to handle the individual faces within the .dfont - sorry for suggesting a tool that apparently isn't up to the job. Sounds like it's showing you the value from the Bold (or Bold Italic) face - presumably it's whichever face happens to be first in the .dfont collection.

If you're willing to try fontforge[1], you can open the .dfont with it and it will prompt you to choose a specific face, so you can look at each in turn. The "weight class" value can be found under Element > Font Info > OS/2 > Misc.


[1] http://fontforge.github.io/en-US/downloads/mac/index.html
From a quick test file: it looks like both WebKit & Blink and Gecko use the same face for font-weight 200 and 300. But WebKit/Blink use the 'light' face, whereas Gecko uses the 'Thin' face.
Created attachment 823694 [details]
test, various weights with Helvetica Neue
(Assignee)

Comment 12

5 years ago
Created attachment 824124 [details] [diff] [review]
debug-logging patch

If someone with a Mavericks system and a local debug build on hand would be willing to try the attached debugging patch, and report what it prints about Helvetica Neue (more or less at the end of startup), that would help to confirm what is actually going on here. My suspicion is that the code in gfxMacPlatformFontList to map from appkit to CSS weight values is not handling this case well. Thanks!
(Assignee)

Updated

5 years ago
Assignee: nobody → jfkthame
## HelveticaNeue: appkit weight 5, css weight 400
## HelveticaNeue-Medium: appkit weight 6, css weight 500
## HelveticaNeue-Light: appkit weight 3, css weight 200
## HelveticaNeue-Thin: appkit weight 3, css weight 200
## HelveticaNeue-UltraLight: appkit weight 2, css weight 100
## HelveticaNeue-Italic: appkit weight 5, css weight 400
## HelveticaNeue-MediumItalic: appkit weight 7, css weight 600
## HelveticaNeue-LightItalic: appkit weight 3, css weight 200
## HelveticaNeue-ThinItalic: appkit weight 3, css weight 200
## HelveticaNeue-UltraLightItalic: appkit weight 2, css weight 100
## HelveticaNeue-Bold: appkit weight 9, css weight 700
## HelveticaNeue-BoldItalic: appkit weight 9, css weight 700
## HelveticaNeue-CondensedBold: appkit weight 9, css weight 700
## HelveticaNeue-CondensedBlack: appkit weight 11, css weight 800
(Assignee)

Comment 14

5 years ago
Thanks!

Ugh - note how the HelveticaNeue-Light and -Thin faces are showing the SAME value for "appkit weight", which is the value the Cocoa font manager reports to us. This explains the behavior here: the Light and Thin faces are ending up with identical properties in CSS terms, and so we can't distinguish between them on the basis of CSS font-weight. Which one you get is essentially arbitrary; it probably depends on either the alphabetical order of their names, or the order they happen to get enumerated by NSFontManager, I forget exactly.

(I'd still be curious to know what's in the weight field of the OS/2 table of those fonts, if someone with FontForge would be willing to take a look. On my 10.7 system, at least, HelveticaNeue-Light has an OS/2 weight of 300, but we end up mapping its appkit weight of 3 to 200 for CSS, which seems somewhat broken to me. But given that Light and Thin are apparently indistinguishable even at the appkit level, the problem here is not just with our mapping.)

AFAICT, the best chance of fixing this - provided the OS/2 tables actually have correct, distinct weight values in them! - is probably to scrap our reliance on the appkit descriptors, and instead read the font table directly. Sigh.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Here is the requested info from FontForge:

HelveticaNeue-Thin:
* Weight class: 250


HelveticaNeue-Light:
* Weight class: 300 Light

If you're saying that Apple could fix this on their end, it may be worth filing a ticket in Radar.
(Assignee)

Comment 16

5 years ago
Interesting. And what value does Helvetica-Ultralight have, then?

And visually, what is the actual weight order among the Light, Ultralight and Thin faces?

According to the Microsoft OpenType spec at http://www.microsoft.com/typography/otspec/os2.htm#wtc, we'd expect "Thin" to be the lightest (even lighter than "Ultralight"); is that true for Helvetica Neue, or is the ordering of the faces different?
Created attachment 824262 [details]
Screenshot of Light, Thin, UltraLight on 10.9

(In reply to Jonathan Kew (:jfkthame) from comment #16)
> Interesting. And what value does Helvetica-Ultralight have, then?

Weight class: 100 Thin

> And visually, what is the actual weight order among the Light, UltraLight
> and Thin faces?

I would say Light > Thin > Ultralight (the order of the screenshot)

> According to the Microsoft OpenType spec at
> http://www.microsoft.com/typography/otspec/os2.htm#wtc, we'd expect "Thin"
> to be the lightest (even lighter than "Ultralight"); is that true for
> Helvetica Neue, or is the ordering of the faces different?

It's wrong according to my eyes.
(In reply to Jonathan Kew (:jfkthame) from comment #16)
> Interesting. And what value does Helvetica-Ultralight have, then?
> 
> And visually, what is the actual weight order among the Light, Ultralight
> and Thin faces?


Visually in Font Book: Ultra-light - Thin - Light
That is also how they display if I use @font-face { src: local() } in both WebKit and Gecko (Tested: nightly).

Fwiw, if the weight class for 'Thin' returns 250, it doesn't surprise me that both Gecko and WebKit treat it the same as 'Light' as far as CSS font-weight is concerned. There is another newly installed font on 10.9 that exhibits a similar issue for CSS font-weight: 'Avenir' - it has these 3 weights: Light, Book, Roman, but Light and Book load for font-weight 100, 200, 300. I haven't had a change to play with FontForge yet.

Comment 19

5 years ago
(In reply to Matthew N. [:MattN] (catching up on reviews) from comment #13)
> ## HelveticaNeue: appkit weight 5, css weight 400
> ## HelveticaNeue-Medium: appkit weight 6, css weight 500
> ## HelveticaNeue-Light: appkit weight 3, css weight 200
> ## HelveticaNeue-Thin: appkit weight 3, css weight 200
> ## HelveticaNeue-UltraLight: appkit weight 2, css weight 100
> ## HelveticaNeue-Italic: appkit weight 5, css weight 400
> ## HelveticaNeue-MediumItalic: appkit weight 7, css weight 600
> ## HelveticaNeue-LightItalic: appkit weight 3, css weight 200
> ## HelveticaNeue-ThinItalic: appkit weight 3, css weight 200
> ## HelveticaNeue-UltraLightItalic: appkit weight 2, css weight 100
> ## HelveticaNeue-Bold: appkit weight 9, css weight 700
> ## HelveticaNeue-BoldItalic: appkit weight 9, css weight 700
> ## HelveticaNeue-CondensedBold: appkit weight 9, css weight 700
> ## HelveticaNeue-CondensedBlack: appkit weight 11, css weight 800

This can be done with the built-in logging, no need for extra patches!

Steps:

1. Open Terminal
2. Enter commands:

  cd /Applications/Firefox.app/Contents/MacOS
  export NSPR_LOG_MODULES=fontlist:5
  export NSPR_LOG_FILE=/path/to/fontlist.out
  ./firefox

3. Wait 60 seconds, then quit Firefox

Result: a logfile listing all fonts on the system

On 10.8, the results for Avenir:

(fontlist) added (Avenir-Book) to family (Avenir) with style: normal weight: 400 stretch: 0 (apple-weight: 5 macTraits: 00000000)
(fontlist) added (Avenir-Roman) to family (Avenir) with style: normal weight: 400 stretch: 0 (apple-weight: 5 macTraits: 00000000)
(fontlist) added (Avenir-Medium) to family (Avenir) with style: normal weight: 500 stretch: 0 (apple-weight: 6 macTraits: 00000000)
(fontlist) added (Avenir-Light) to family (Avenir) with style: normal weight: 200 stretch: 0 (apple-weight: 3 macTraits: 00000000)
(fontlist) added (Avenir-BookOblique) to family (Avenir) with style: italic weight: 400 stretch: 0 (apple-weight: 5 macTraits: 00000001)
(fontlist) added (Avenir-Oblique) to family (Avenir) with style: italic weight: 400 stretch: 0 (apple-weight: 5 macTraits: 00000001)
(fontlist) added (Avenir-MediumOblique) to family (Avenir) with style: italic weight: 600 stretch: 0 (apple-weight: 7 macTraits: 00000001)
(fontlist) added (Avenir-LightOblique) to family (Avenir) with style: italic weight: 200 stretch: 0 (apple-weight: 3 macTraits: 00000001)
(fontlist) added (Avenir-Black) to family (Avenir) with style: normal weight: 800 stretch: 0 (apple-weight: 11 macTraits: 00000002)
(fontlist) added (Avenir-Heavy) to family (Avenir) with style: normal weight: 800 stretch: 0 (apple-weight: 11 macTraits: 00000002)
(fontlist) added (Avenir-BlackOblique) to family (Avenir) with style: italic weight: 800 stretch: 0 (apple-weight: 11 macTraits: 00000003)
(fontlist) added (Avenir-HeavyOblique) to family (Avenir) with style: italic weight: 800 stretch: 0 (apple-weight: 11 macTraits: 00000003)
  
Argh, looks like there's some wacky weight mapping within appkit, the
Book and Roman faces have matching weights!  Same for the Black/Heavy
faces.

Comment 20

5 years ago
I think the Thin/Ultralight problem may be related to the scale that Apple uses, which is not precisely the same at the OpenType scale (i.e. the usWeightClass value).


From the Microsoft OS/2 table description for usWeightClass:
http://www.microsoft.com/typography/otspec/os2.htm#wtc

100 	Thin
200 	Extra-light (Ultra-light)
300 	Light
400 	Normal (Regular)
500 	Medium
600 	Semi-bold (Demi-bold)
700 	Bold
800 	Extra-bold (Ultra-bold)
900 	Black (Heavy)

From the NSFontManager docs:
https://developer.apple.com/library/mac/documentation/cocoa/reference/applicationkit/classes/NSFontManager_Class/Reference/Reference.html

Apple Terminology     ISO Equivalent

1. ultralight
2. thin               W1. ultralight
3. light, extralight  W2. extralight
4. book               W3. light
5. regular, plain, display, roman   W4. semilight  
6. medium             W5. medium
7. demi, demibold
8. semi, semibold     W6. semibold
9. bold               W7. bold
10. extra, extrabold  W8. extrabold
11. heavy, heavyface
12. black, super      W9. ultrabold
13. ultra, ultrablack, fat
14. extrablack, obese, nord

As I recall from past bugs, Apple is doing something similar to what DirectWrite does, it's looking at the style name to classify the weight (mainly due to all the crazy remapping that's used to avoid synthetic bold under GDI).  Note above that the relative ordering of Thin/Ultralight is different.
(Assignee)

Comment 21

5 years ago
(In reply to John Daggett (:jtd) from comment #20)
> From the NSFontManager docs:
> https://developer.apple.com/library/mac/documentation/cocoa/reference/
> applicationkit/classes/NSFontManager_Class/Reference/Reference.html
> 
> Apple Terminology     ISO Equivalent
> 
> 1. ultralight
> 2. thin               W1. ultralight
> 3. light, extralight  W2. extralight
> 4. book               W3. light
> 5. regular, plain, display, roman   W4. semilight  
> 6. medium             W5. medium
> 7. demi, demibold
> 8. semi, semibold     W6. semibold
> 9. bold               W7. bold
> 10. extra, extrabold  W8. extrabold
> 11. heavy, heavyface
> 12. black, super      W9. ultrabold
> 13. ultra, ultrablack, fat
> 14. extrablack, obese, nord

Hmmm... The Apple list is "interesting" from a font designer's point of view. Demibold and Semibold are distinct weights with a defined ordering? Not in any family I can think of. And Book is lighter than Regular? I've certainly seen professionally-designed families where Book is a slightly *heavier* weight. And Display is the same as Regular? Usually it's a much lighter face, in my experience. (Look at Adobe families with optical-size variants, where the largest - and lightest - of them is generally called Display.)

The summary at http://en.wikipedia.org/wiki/Font#Weight, for example, does support the idea that Thin is normally lighter than Ultralight. But clearly Apple's Helvetica Neue deviates from this.
(Assignee)

Comment 22

5 years ago
Created attachment 824553 [details] [diff] [review]
read font weight directly from OS/2 table to avoid AppKit remapping of values

Maybe we can avoid the issue by doing something like this. I've pushed a tryserver job to see whether this breaks any existing reftests: https://tbpl.mozilla.org/?tree=Try&rev=1f815ed80219. Also, once the build is ready, if someone with Mavericks could give it a whirl and see if it helps the problem here, that'd be great.
(Assignee)

Comment 23

5 years ago
Created attachment 824603 [details] [diff] [review]
read font weight directly from OS/2 table to avoid AppKit remapping of values

Well, that made reftest unhappy, apparently because OS X ships with some fonts that have invalid weight values in the OS/2 table. (The build should be usable for testing the Helvetica Neue issue on Mavericks, however.) Added some sanity-checking to protect against such bad fonts; new try job: https://tbpl.mozilla.org/?tree=Try&rev=fe0135cfc4ac.
(Assignee)

Updated

5 years ago
Attachment #824553 - Attachment is obsolete: true
(Assignee)

Comment 24

5 years ago
Created attachment 824716 [details] [diff] [review]
read font weight directly from OS/2 table to avoid AppKit remapping of values

That almost worked, except that on 10.6 the old Helvetica Bold font has the wrong weight value in OS/2, which causes us to treat Regular and Bold as indistinguishable there. Added a workaround for that (and any similar cases), and pushed to try once more: https://tbpl.mozilla.org/?tree=Try&rev=4d98605ddbac.
(Assignee)

Updated

5 years ago
Attachment #824603 - Attachment is obsolete: true
(Assignee)

Comment 25

5 years ago
Matt, could you try the build from http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jkew@mozilla.com-fe0135cfc4ac/try-macosx64/ on Mavericks and let me know whether it fixes the Helvetica Neue issue, please?
Flags: needinfo?(mnoorenberghe+bmo)
(In reply to Jonathan Kew (:jfkthame) from comment #25)
> Matt, could you try the build from
> http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jkew@mozilla.com-
> fe0135cfc4ac/try-macosx64/ on Mavericks and let me know whether it fixes the
> Helvetica Neue issue, please?

* Helvetica Neue Light is now used on the article text of appleinsider.com
* In attachment 823694 [details], Helvetica Neue Light is now used for font-weight:300 instead of Helvetica Neue Thin
** All fonts used in the page (from devtools):
*** Before (8):
    Helvetica Neue Thin
    Helvetica Neue Thin Italic
    Helvetica Neue Italic
    Helvetica Neue UltraLight
    Helvetica Neue
    Helvetica Neue Medium
    Helvetica Neue Bold
    Helvetica Neue Bold Italic

*** After (9):
    Helvetica Neue Italic
    Helvetica Neue Bold Italic
    Helvetica Neue Light
    Helvetica Neue
    Helvetica Neue Bold
    Helvetica Neue UltraLight
    Helvetica Neue Light Italic
    Helvetica Neue Medium
    Helvetica Neue Thin

Anything else I should test?
Status: NEW → ASSIGNED
Flags: needinfo?(mnoorenberghe+bmo)
(Assignee)

Comment 27

5 years ago
(In reply to Matthew N. [:MattN] (catching up on reviews) from comment #26)
> Anything else I should test?

No, it sounds like it's behaving as intended - in particular, properly distinguishing between the Light and Thin faces. Thanks!
(Assignee)

Updated

5 years ago
Attachment #824716 - Flags: review?(jdaggett)
(Assignee)

Comment 28

5 years ago
Argh - it turns out this triggered failures on OS X in a couple of a11y tests:

4542 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/a11y/accessible/tests/mochitest/events/test_textattrchange.html | Attribute 'font-weight' has wrong value. Getting default text attributes for input
4551 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/a11y/accessible/tests/mochitest/events/test_textattrchange.html | Attribute 'font-weight' has wrong value for input at offset 0
4562 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/a11y/accessible/tests/mochitest/events/test_textattrchange.html | Attribute 'font-weight' has wrong value for input at offset 11
4571 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/a11y/accessible/tests/mochitest/events/test_textattrchange.html | Attribute 'font-weight' has wrong value for input at offset 17
4582 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/a11y/accessible/tests/mochitest/events/test_textattrchange.html | Attribute 'font-weight' has wrong value for input at offset 18
30556 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/a11y/accessible/tests/mochitest/textattrs/test_general.html | Attribute 'font-weight' has wrong value for area14 at offset 0

The reason is that the Lucida Grande and Lucida Grande Bold fonts on OS X actually have weights (according to their OS/2 tables) of 500 "Medium" and 600 "Demibold" respectively, not the 400 and 700 that we might expect.

So with the code now using the font's "real" weight, the default Lucida Grande font used for elements such as <input> will now report a weight of 500, whereas previously it returned 400.

This doesn't affect rendering; font-weight:normal will still map to the "regular" face, and font-weight:bold to the "bold" face. But the a11y test is checking for "normal" by testing that font-weight <= 400. And that fails with the Mac's Lucida Grande.
(Assignee)

Comment 29

5 years ago
Created attachment 825231 [details] [diff] [review]
fix for a11y mochitest that was making too-strict assumption about font-weight values

This fixes the a11y test breakage triggered by using "true" font weight values on OS X; see comment above.
Attachment #825231 - Flags: review?(dbolter)
Comment on attachment 825231 [details] [diff] [review]
fix for a11y mochitest that was making too-strict assumption about font-weight values

Review of attachment 825231 [details] [diff] [review]:
-----------------------------------------------------------------

Hmmm yeah okay, r=me provided the whole test suite passes thanks.
Attachment #825231 - Flags: review?(dbolter) → review+
(Assignee)

Comment 31

5 years ago
Yep, tests are happy according to https://tbpl.mozilla.org/?tree=Try&rev=4276ba5cc63d. (The bc oranges there are unrelated intermittent failures.)

Comment 32

5 years ago
Created attachment 829113 [details]
testcase, Apple SD Gothic Neo family weights

Not sure if this shipped with 10.7 but 10.8 shipped with Apple SD Neo Gothic which includes 9 weights.  Our existing mappings:

(AppleSDGothicNeo-Thin) weight 100 
(AppleSDGothicNeo-UltraLight) weight 100 
(AppleSDGothicNeo-Light) weight 200 
(AppleSDGothicNeo-Regular) weight 400 
(AppleSDGothicNeo-Medium) weight 500 
(AppleSDGothicNeo-SemiBold) weight 600 
(AppleSDGothicNeo-Bold) weight 700 
(AppleSDGothicNeo-ExtraBold) weight 800 
(AppleSDGothicNeo-Heavy) weight 800 

With patch:

(AppleSDGothicNeo-Thin) weight 300 
(AppleSDGothicNeo-UltraLight) weight 300 
(AppleSDGothicNeo-Light) weight 400 
(AppleSDGothicNeo-Regular) weight 400 
(AppleSDGothicNeo-Medium) weight 500 
(AppleSDGothicNeo-SemiBold) weight 500 
(AppleSDGothicNeo-Bold) weight 600 
(AppleSDGothicNeo-ExtraBold) weight 600 
(AppleSDGothicNeo-Heavy) weight 600 

Yikes!

The patch also introduces weights that aren't multiples of 100 such as 275 for Avenir UltraLight.
(In reply to John Daggett (:jtd) from comment #32)

> Not sure if this shipped with 10.7 but 10.8 shipped with Apple SD Neo Gothic
> which includes 9 weights.

That would be 10.8, as I don't see it listed for 10.7.

10.7 http://support.apple.com/kb/HT5098
10.8 http://support.apple.com/kb/HT5379
10.9 http://support.apple.com/kb/HT5944
(Assignee)

Comment 34

5 years ago
(In reply to John Daggett (:jtd) from comment #32)
> Created attachment 829113 [details]
> testcase, Apple SD Gothic Neo family weights
> 
> Not sure if this shipped with 10.7 but 10.8 shipped with Apple SD Neo Gothic
> which includes 9 weights. 

Not present on 10.7.

> Our existing mappings:
> 
> (AppleSDGothicNeo-Thin) weight 100 
> (AppleSDGothicNeo-UltraLight) weight 100 
> (AppleSDGothicNeo-Light) weight 200 
> (AppleSDGothicNeo-Regular) weight 400 
> (AppleSDGothicNeo-Medium) weight 500 
> (AppleSDGothicNeo-SemiBold) weight 600 
> (AppleSDGothicNeo-Bold) weight 700 
> (AppleSDGothicNeo-ExtraBold) weight 800 
> (AppleSDGothicNeo-Heavy) weight 800 
> 
> With patch:
> 
> (AppleSDGothicNeo-Thin) weight 300 
> (AppleSDGothicNeo-UltraLight) weight 300 
> (AppleSDGothicNeo-Light) weight 400 
> (AppleSDGothicNeo-Regular) weight 400 
> (AppleSDGothicNeo-Medium) weight 500 
> (AppleSDGothicNeo-SemiBold) weight 500 
> (AppleSDGothicNeo-Bold) weight 600 
> (AppleSDGothicNeo-ExtraBold) weight 600 
> (AppleSDGothicNeo-Heavy) weight 600 
> 
> Yikes!

Ugh. But that family is going to be broken (in the sense that there are different faces reporting the same weight, so we can't correctly select among them) under either approach. I'd argue this is a badly-constructed font family and should be reported as a bug to Apple. Those nine faces should have nine different weight values in the OS/2 table.

(Is it the same on 10.9, or has there by any chance been an update?)

> The patch also introduces weights that aren't multiples of 100 such as 275
> for Avenir UltraLight.

True, as that's what the font file says. Currently, it looks like we'll end up truncating the weight value when we divide by 100 in gfxFontFamily::FindWeightsForStyle; perhaps we should round instead.

It's not clear to me that anything bad will happen as a result, anyhow - at least nothing worse than the problem we already have of certain faces not being correctly distinguished by weight.

(Or perhaps we should re-open the question of whether font weight should in fact be an integer - maybe clamped to a range such as 0..1000 - rather than being restricted to a set of 9 keywords that happen to look like integers.)
(Assignee)

Comment 35

5 years ago
Created attachment 829218 [details] [diff] [review]
read font weight directly from OS/2 table to avoid AppKit remapping of values

For now, I've added rounding to multiples of 100, so that we don't get intermediate values in the font entries. It's not clear to me what we should do about the Apple SD Gothic Neo weights for now... I suppose we could look at the overall ordering of the Cocoa weights and try to come up with a better mapping back to CSS from there, but I suggest that should be a separate followup bug.
Attachment #829218 - Flags: review?(jdaggett)
(Assignee)

Updated

5 years ago
Attachment #824716 - Attachment is obsolete: true
Attachment #824716 - Flags: review?(jdaggett)

Comment 36

5 years ago
Created attachment 829970 [details]
comparison of weights within OSX font families (10.8)

List of all fonts on my system (10.8 plus a smattering of fonts in ~/Library/Fonts).  For each there are two weights listed, existing and after the latest patch.

Comment 37

5 years ago
Looking over the changes with fonts on 10.8, I think some families have problems with prioritizing the OS/2 weight while others are better using it:

problematic:
Apple SD Gothic Neo
Avenir (black/heavy)
Frutiger LT Std (black/ultrablack) *
Verlag (light/extra light/book all 300, black is 500!)
Whitney (book/medium faces shift down, black at 500)

better:
Garamond Premier Pro (medium/semi-bold)
M+ font families
Minion Pro (medium/semi-bold)
Univers LT Std

The one thing about the patch I'm not so keen on is that it's reading the OS/2 table for all fonts (via the font loader).   For standard fonts there's typically no variation.  It's only when dealing with larger families that differences arise.  Maybe use this logic as a post-process for large families with many weights or with collisions between weights?

Comment 38

5 years ago
Created attachment 830050 [details]
comparison of weights within OSX font families (10.9)

Ran a quick test with a tryserver build on a coworker's 10.9 machine.  I don't think this patch is really doing the trick, it looks like it still doesn't disambiguate different weights.

Helvetica Neue:
Helvetica Neue: stretch: 0 style: italic weight: 100 100 HelveticaNeue-UltraLightItalic
Helvetica Neue: stretch: 0 style: italic weight: 200 300 HelveticaNeue-LightItalic
Helvetica Neue: stretch: 0 style: italic weight: 200 300 HelveticaNeue-ThinItalic
Helvetica Neue: stretch: 0 style: italic weight: 400 400 HelveticaNeue-Italic
Helvetica Neue: stretch: 0 style: italic weight: 600 500 HelveticaNeue-MediumItalic
Helvetica Neue: stretch: 0 style: italic weight: 700 700 HelveticaNeue-BoldItalic

Helvetica Neue: stretch: 0 style: normal weight: 100 100 HelveticaNeue-UltraLight
Helvetica Neue: stretch: 0 style: normal weight: 200 300 HelveticaNeue-Light
Helvetica Neue: stretch: 0 style: normal weight: 200 300 HelveticaNeue-Thin
Helvetica Neue: stretch: 0 style: normal weight: 400 400 HelveticaNeue
Helvetica Neue: stretch: 0 style: normal weight: 500 500 HelveticaNeue-Medium
Helvetica Neue: stretch: 0 style: normal weight: 700 700 HelveticaNeue-Bold

Argh, what a mess...

Updated

5 years ago
Attachment #829218 - Flags: review?(jdaggett) → review-
(Assignee)

Comment 39

5 years ago
Your dump from comment #36 illustrates another problem, too: AppKit is marking certain fonts as "condensed" (resulting in a font-stretch value of -2) based on their name, rather than respecting the usWidthClass value in the OS/2 table. In particular, any font with "thin" or "cond[ensed]" in its name is reporting stretch = -2.

(I see this locally with Charis SIL Compact, too, indicating that apparently "compact" is also a magic string.)

Your Myriad Pro family, which contains condensed, semicondensed, normal, and semiextended faces (in various weights), is an example of this problem: AppKit's width mapping doesn't recognize the "SemiCn" name fragment, and so the semicondensed faces will be confused with the normal ones, whereas they ought to be separated by font-stretch.

Similarly, the mplus-1*-thin faces get stretch = -2, which means they won't form part of the weight series for font-stretch: normal, as they should.

It's a mess indeed.
(Assignee)

Comment 40

5 years ago
So the font-stretch trouble comes from relying on the Cocoa font manager's "font traits" to set the NS_FONT_STRETCH_CONDENSED or NS_FONT_STRETCH_EXPANDED values in mStretch. We'd probably do better to use the OS/2 table for this, where available.
(Assignee)

Comment 41

5 years ago
I did another tryserver build using a somewhat different approach. John, could you give this a try on your 10.8 and 10.9 systems and see whether it does a better job with those problem families?

https://tbpl.mozilla.org/?tree=Try&rev=25f2ec96878e
Flags: needinfo?(jdaggett)
(Assignee)

Comment 42

5 years ago
Created attachment 8336752 [details] [diff] [review]
improve handling of font-weight values for families with many faces on OS X.

Here's an alternative approach, which aims to use the font's OS/2 weight as the primary source of weight values, but for cases where multiple faces (incorrectly) have the same OS/2 value, it also uses the AppKit weight (which may be based on the face name, or other mysterious heuristics) to distinguish them. I've experimented with instrumenting the code to see how expensive this is, and found that reading all the OS/2 tables and then calling the new RemapFontWeights() method ends up adding about 2% to the total time spent in FindStyleVariations(). Given that this (mostly) happens during the background font-loader process, I don't think that's an unacceptable cost if this substantially improves behavior for these tricky families.
Attachment #8336752 - Flags: review?(jdaggett)
(Assignee)

Updated

5 years ago
Attachment #829218 - Attachment is obsolete: true
John and Jonathan:

Do you guys think it might be worthwhile to reverse engineer how Apple determines the "AppKit weight"?  If you do, and if you can give me one call (or a small number of calls), presumably in the AppKit framework, that return "AppKit weights", I could spend a couple of hours looking at these calls in a disassembler.  To get comparisons, I'd probably need to do it on at least two different versions of OS X (say 10.9 and 10.8).

I could probably find time for this next week.
(Assignee)

Comment 44

5 years ago
Thanks, but I don't think there's much value in that really, given that we already know that the results it comes up with are not always useful anyway (e.g. see comment 19).

Comment 45

5 years ago
Looks much better.  Still seeing problems with Apple SD Gothic Neo (default font for ko):

Before:
(fontlist) added (AppleSDGothicNeo-Bold) to family (Apple SD Gothic Neo) with style: normal weight: 700 stretch: 0 (apple-weight: 9 macTraits: 00000002)
(fontlist) added (AppleSDGothicNeo-ExtraBold) to family (Apple SD Gothic Neo) with style: normal weight: 800 stretch: 0 (apple-weight: 11 macTraits: 00000002)
(fontlist) added (AppleSDGothicNeo-Heavy) to family (Apple SD Gothic Neo) with style: normal weight: 800 stretch: 0 (apple-weight: 11 macTraits: 00000002)
(fontlist) added (AppleSDGothicNeo-Light) to family (Apple SD Gothic Neo) with style: normal weight: 200 stretch: 0 (apple-weight: 3 macTraits: 00000000)
(fontlist) added (AppleSDGothicNeo-Medium) to family (Apple SD Gothic Neo) with style: normal weight: 500 stretch: 0 (apple-weight: 6 macTraits: 00000000)
(fontlist) added (AppleSDGothicNeo-Regular) to family (Apple SD Gothic Neo) with style: normal weight: 400 stretch: 0 (apple-weight: 5 macTraits: 00000000)
(fontlist) added (AppleSDGothicNeo-SemiBold) to family (Apple SD Gothic Neo) with style: normal weight: 600 stretch: 0 (apple-weight: 8 macTraits: 00000002)
(fontlist) added (AppleSDGothicNeo-Thin) to family (Apple SD Gothic Neo) with style: normal weight: 100 stretch: 0 (apple-weight: 2 macTraits: 00000000)
(fontlist) added (AppleSDGothicNeo-UltraLight) to family (Apple SD Gothic Neo) with style: normal weight: 100 stretch: 0 (apple-weight: 2 macTraits: 00000000)

After:
(Apple SD Gothic Neo):   face (AppleSDGothicNeo-Bold) wt:800 st:0 it:N valid:Y
(Apple SD Gothic Neo):   face (AppleSDGothicNeo-ExtraBold) wt:900 st:0 it:N valid:Y
(Apple SD Gothic Neo):   face (AppleSDGothicNeo-Heavy) wt:900 st:0 it:N valid:Y
(Apple SD Gothic Neo):   face (AppleSDGothicNeo-Light) wt:400 st:0 it:N valid:Y
(Apple SD Gothic Neo):   face (AppleSDGothicNeo-Medium) wt:600 st:0 it:N valid:Y
(Apple SD Gothic Neo):   face (AppleSDGothicNeo-Regular) wt:500 st:0 it:N valid:Y
(Apple SD Gothic Neo):   face (AppleSDGothicNeo-SemiBold) wt:700 st:0 it:N valid:Y
(Apple SD Gothic Neo):   face (AppleSDGothicNeo-Thin) wt:300 st:0 it:N valid:Y
(Apple SD Gothic Neo):   face (AppleSDGothicNeo-UltraLight) wt:200 st:0 it:N valid:Y

Note how the Light face is getting pushed to be the default font and heavy/extra bold are still use the same weight.

What's the effect on the running time of the startup/font loader?
Flags: needinfo?(jdaggett)

Comment 46

5 years ago
Created attachment 8340914 [details]
OSX 10.8 fonts with 22-nov patch applied

Comment 47

5 years ago
Created attachment 8340917 [details]
OSX 10.9 fonts with 22-nov patch applied

Updated

5 years ago
Attachment #8336752 - Flags: review?(jdaggett)
(Assignee)

Comment 48

5 years ago
(In reply to John Daggett (:jtd) from comment #45)
> Looks much better.  Still seeing problems with Apple SD Gothic Neo (default
> font for ko):

> Note how the Light face is getting pushed to be the default font and
> heavy/extra bold are still use the same weight.

What does the OS/2 table from the Heavy and ExtraBold faces actually contain? It's not clear to me from the logs above whether we have -any- usable basis for distinguishing them.

> What's the effect on the running time of the startup/font loader?

Pretty marginal; it regresses FindStyleVariations() by about 2% (see comment 42), which mostly happens in the post-startup loader task, so very little of it will show as part of startup. I'll post some numbers once we have an algorithm that gives results we're happy with.

I also think we'll be able to improve this by avoiding the OS/2 table access for "simple" families (for a still-to-be-determined definition of "simple").

Comment 49

5 years ago
I think for the Apple SD Gothic Neo family, the OS/2 weight is clearly *not* what we should be using:

Existing weighting using appkit weighting:
(AppleSDGothicNeo-Thin) style: normal weight: 100 stretch: 0 (apple-weight: 2 macTraits: 00000000)
(AppleSDGothicNeo-UltraLight) style: normal weight: 100 stretch: 0 (apple-weight: 2 macTraits: 00000000)
(AppleSDGothicNeo-Light) style: normal weight: 200 stretch: 0 (apple-weight: 3 macTraits: 00000000)
(AppleSDGothicNeo-Regular) style: normal weight: 400 stretch: 0 (apple-weight: 5 macTraits: 00000000)
(AppleSDGothicNeo-Medium) style: normal weight: 500 stretch: 0 (apple-weight: 6 macTraits: 00000000)
(AppleSDGothicNeo-SemiBold) style: normal weight: 600 stretch: 0 (apple-weight: 8 macTraits: 00000002)
(AppleSDGothicNeo-Bold) style: normal weight: 700 stretch: 0 (apple-weight: 9 macTraits: 00000002)
(AppleSDGothicNeo-ExtraBold) style: normal weight: 800 stretch: 0 (apple-weight: 11 macTraits: 00000002)
(AppleSDGothicNeo-Heavy) style: normal weight: 800 stretch: 0 (apple-weight: 11 macTraits: 00000002)

OS/2 usWeight (oy vey, what a mess!):
AppleSDGothicNeo-Thin.ttx:    <usWeightClass value="300"/>
AppleSDGothicNeo-UltraLight.ttx:    <usWeightClass value="300"/>
AppleSDGothicNeo-Light.ttx:    <usWeightClass value="400"/>
AppleSDGothicNeo-Regular.ttx:    <usWeightClass value="400"/>
AppleSDGothicNeo-Medium.ttx:    <usWeightClass value="500"/>
AppleSDGothicNeo-SemiBold.ttx:    <usWeightClass value="500"/>
AppleSDGothicNeo-Bold.ttx:    <usWeightClass value="600"/>
AppleSDGothicNeo-ExtraBold.ttx:    <usWeightClass value="600"/>
AppleSDGothicNeo-Heavy.ttx:    <usWeightClass value="600"/>

with patch:
(AppleSDGothicNeo-Thin) wt:300 st:0 it:N valid:Y
(AppleSDGothicNeo-UltraLight) wt:200 st:0 it:N valid:Y
(AppleSDGothicNeo-Light) wt:400 st:0 it:N valid:Y
(AppleSDGothicNeo-Regular) wt:500 st:0 it:N valid:Y
(AppleSDGothicNeo-Medium) wt:600 st:0 it:N valid:Y
(AppleSDGothicNeo-SemiBold) wt:700 st:0 it:N valid:Y
(AppleSDGothicNeo-Bold) wt:800 st:0 it:N valid:Y
(AppleSDGothicNeo-ExtraBold) wt:900 st:0 it:N valid:Y
(AppleSDGothicNeo-Heavy) wt:900 st:0 it:N valid:Y

The Thin face is lighter than the UltraLight face, so the weightings of 300 and 200 are problematic.  I really hate to say it but it seems like we have to dig into the style name (akin to what DirectWrite does) to distinguish weights in these cases. ick.
(Assignee)

Comment 50

5 years ago
I've pushed another try build with a slightly revised patch; please test this and let me know if it gives a better result with the Apple SD Gothic Neo family.

https://tbpl.mozilla.org/?tree=Try&rev=a324633599e2
Flags: needinfo?(jdaggett)
(Assignee)

Comment 51

4 years ago
It looks like that try build has expired, so I pushed a new one based on current m-c tip; should be ready in a couple of hours, if all goes well:

https://tbpl.mozilla.org/?tree=Try&rev=91ed77136b1c

Testing with the problem font families on 10.8/10.9 would be much appreciated!
(Assignee)

Comment 52

4 years ago
jdaggett: ping re the tryserver build above.
(Assignee)

Comment 53

4 years ago
Created attachment 8378878 [details] [diff] [review]
improve handling of font-weight values for families with many faces on OS X.

Un-bitrotted and pushed yet another try build, as the previous one has expired: https://tbpl.mozilla.org/?tree=Try&rev=dc316d2edeab. Still looking for feedback re the behavior this gives on 10.8/10.9 with the SD Gothic Neo family, etc. Flagging for r? in the hope we can move forward here soon...
Attachment #8378878 - Flags: review?(jdaggett)
(Assignee)

Updated

4 years ago
Attachment #8336752 - Attachment is obsolete: true

Comment 54

4 years ago
Created attachment 8381979 [details]
weight/stretch data for trunk vs. patch on OSX 10.8


Google docs spreadsheet with 10.8 data:
https://docs.google.com/spreadsheet/ccc?key=0ArCKGq7OfNmMdHo2WkhVQTJQdXFhLTFCRUlxRF9jOVE
Flags: needinfo?(jdaggett)

Comment 55

4 years ago
Comment on attachment 8378878 [details] [diff] [review]
improve handling of font-weight values for families with many faces on OS X.

*sigh*

I realize the time and effort you've put into this patch and I hate to
keep putting r- here.  But I have to say that I think for the most
part we shouldn't be doing this sort of swizzling.

Some of the values the Apple API's are returning are incorrect but I
just don't see that all this *really* complicated swizzling that has
perf impact on all font families is really worth the effort.  There
are definitely places it seems to improve but it also introduces
definite bugs.  I really think we should limit the scope of fixup to a
small, limited set of fonts or use some criteria that explicitly
excludes most normal families.  I don't think we should be reading the
OS/2 table for *all* fonts!  It seems like it would be far simpler to
simply fix up the weights in font families containing conflicts or
light weights (e.g. <300) using the name (thin/ultralight/light).

At this point, I think it would make sense to contact Apple directly
(Julio Gonzalez?) and try and figure out how to rectify the situation
with these Apple API's.  Did you test the CoreText attribute value
API's to see what the font stretch numbers would be? It would be good
to hear from someone at Apple about the issue of conflicts and where
the values in system fonts don't make any sense at all.

Problematic cases within the OSX 10.8 data:

  Avenir Heavy ==> 900
  Avenir Black ==> 800  <== this is the heavier face!

  Minion Semibold 600 ==> 700  <== this will now be the "bold" face for this family
  Minion Bold     700 ==> 800 

  Nanum Myeongjo Bold       700 ==> 600
  Nanum Myeongjo Extra Bold 800 ==> 700  <== this will now be the "bold" face for this family

  Papyrus Condensed st:-2 ==> st:0  <== these faces now look the "same" in terms of stretch
  Papyrus           st:0  ==> st:0

  Segoe UI           400 ==> 500
  Segoe UI Semilight 300 ==> 400  <== this will now be the "normal" face for this family

I think the ideal here is that we *never* have to do this sort of
thing, that Apple fixes the bug and we have no need to do this sort of
swizzling.

I'll run some tests on my coworkers OSX 10.9 system tomorrow and
attach the data.

I think I'd like roc to also take a look at this.  If he seems to feel okay with this sort of swizzling, I'll withdraw my r-, as the patch does make improvements for some families even as it introduces problems.  But right now, I'm relatively uncomfortable with the changes here.
Attachment #8378878 - Flags: review?(roc)
Attachment #8378878 - Flags: review?(jdaggett)
Attachment #8378878 - Flags: review-
My initial reaction is the same as yours. Maybe we're better off just packaging a big table of weight corrections for Apple-shipped fonts.

What do Webkit and Blink do?
(Assignee)

Comment 57

4 years ago
Sure, I realize all this is horrible and ugly and hackish. Ideally, Apple would fix the fonts they're shipping (and the font-attribute APIs they provide!); but I can't see them making changes like that for OS X 10.8 or 10.9, so at best we might see an improvement in a future system version. But if we want to fix our behavior on the existing OS version - i.e. this bug as filed - then we need to work around the broken fonts somehow.

I've been trying to come up with heuristics here that could result in more reasonable behavior (which is tricky, given the lack of any reliable "source of truth", either in the fonts or the available APIs). If we could find behavior that we're happier with, I'd then want to consider how to avoid the extra cost for simple families.

Part of the problem here, too, is the brokenness of the CSS-Fonts spec, where font-weight allows just 9 discrete values instead of actually being an integer. Consider a family like Frutiger LT Std, which has not only Black, but also weights that it calls ExtraBlack and UltraBlack. With font-weight being limited to 900, we have to use that for the UltraBlack; this then pushes ExtraBlack down to 800, and Black to 700. Which is the standard value for Bold, but as Black has claimed it, we have to push Bold down to 600. And that means <b> in this family will get the Black face instead of the (desired) Bold. If font-weight didn't have an artificial ceiling at 900, or wasn't limited to multiples of 100, all this could have worked better.

If we'd prefer to just fix up the known problem cases in Apple's fonts via a table of remappings or something like that, then I think we should simplify this code by abandoning the use of the AppKit APIs for font weight altogether, and just read the font's usWeight value. That's our best option to get consistent behavior where the user has the same font families on multiple platforms - which is a reasonable thing for users to expect. It won't resolve the problem where multiple faces within a family have the same OS/2 data.

(The CTFontDescriptor attributes aren't going to be much help, as weight is exposed there as a "normalized" floating-point value from -1.0 to 1.0. Mapping that to the limited set of allowed values for CSS is at least as problematic as mapping the 15 AppKit weights.)
(Assignee)

Comment 58

4 years ago
Created attachment 8382304 [details] [diff] [review]
revise handling of font-weight for fonts on OS X.

Here's an alternative, greatly simplified approach: just use default 'normal' and 'bold' values for faces with standard names, and otherwise use the OS/2 usWeightClass value. In addition, allow per-face overrides that we can define for the families where we know OS X ships with problematic faces. Otherwise, no attempt is made to heuristically 'fix' arbitrary families where there may be conflicting usWeightClass values.
Attachment #8382304 - Flags: review?(jdaggett)
(Assignee)

Comment 59

4 years ago
Created attachment 8382308 [details] [diff] [review]
overrides for Mac families with bad font-weight values.

And some overrides for problematic families. There are probably a few more we need, but I don't have a 10.9 system on hand to survey all the font data... anyhow, as a general approach, is this how we want to do it?
Attachment #8382308 - Flags: review?(jdaggett)

Comment 60

4 years ago
Tryserver build of Jonathan's Feb. 20 patch with the logging tweaked to be more grep friendly:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jdaggett@mozilla.com-d4e87a6d8964/try-macosx64/

1. Download and install
2. Open Terminal and navigate to /path/to/trybuild/FirefoxNightly.app/Contents/MacOS
3. Enter these commands:

  NSPR_LOG_MODULE=fontlist:5 NSPR_LOG_FILE=fontlist.out ./firefox -P <profile>

4. After browser starts, wait 30 secs and quit

Result: full dump of font weight/width/style data can be found in fontlist.out

Comment 61

4 years ago
Created attachment 8382827 [details]
OSX 10.8 fontlist data with weight/width/style attributes

Data from my system running OSX 10.8

Columns are:
  family name
  postscript name
  face name
  stretch
  style
  appKitWeight
  CSS weight based on appKitWeight
  OS/2 stretch (-5 to 5 scaled)
  appKitWeight (again, oops)
  OS/2 weight
  macTraits
  CTFont weight attribute (-1.0 to 1.0 scale)
  CTFont width attribute (-1.0 to 1.0 scale)

Comment 62

4 years ago
Created attachment 8382828 [details]
OSX 10.9 fontlist data with weight/width/style attributes

Columns same as last comment. From a coworker's machine running OSX 10.9.
Summary: Font rendering in OS X 10.9 Maverics is much lighter and less legible with some sites → Font rendering in OS X 10.9 Mavericks is much lighter and less legible with some sites

Comment 63

4 years ago
Created attachment 8382854 [details]
font weight data, OSX 10.8

Font weight data for all 10.8 fonts, sorted by ascending appKit weight.

Columns
  appKit wt
  css wt (based on appKit wt)
  os2 wt
  ctfont wt
  postscript name
  family name

Comment 64

4 years ago
Created attachment 8382856 [details]
font weight data, OSX 10.9

Font weight data for all fonts on 10.9, same format as the last comment.

Comment 65

4 years ago
Created attachment 8382857 [details]
font families with weight mapping problems
(Assignee)

Comment 66

4 years ago
Looking at AppleSDGothicNeo, the ExtraBold and Heavy weights are indistinguishable even on 10.9; on 10.8, the same applies to Thin and UltraLight. Similarly, neither AppKit nor CoreText APIs give us a distinction between Avenir Black and Heavy, or between Book and Roman (even though, judging by the samples at http://www.fonts.com/font/linotype/avenir?isRatingExpanded=False#product_43906, the Book face is visibly lighter).

And reading the OS/2 table doesn't always work either, as some families have clashing values there. Especially once we have to round to multiples of 100.

So perhaps the font.weight-override.<name>=<value> mechanism, as per the most recent patches here, is our best option. Try run: https://tbpl.mozilla.org/?tree=Try&rev=ac87bef376cd.

Comment 67

4 years ago
(In reply to Jonathan Kew (:jfkthame) from comment #66)

> Looking at AppleSDGothicNeo, the ExtraBold and Heavy weights are
> indistinguishable even on 10.9; on 10.8, the same applies to Thin
> and UltraLight. Similarly, neither AppKit nor CoreText APIs give us
> a distinction between Avenir Black and Heavy, or between Book and
> Roman (even though, judging by the samples at
> http://www.fonts.com/font/linotype/
> avenir?isRatingExpanded=False#product_43906, the Book face is
> visibly lighter).

It's actually even worse than this.  For Avenir, AppKit/CoreText say
the Black and Heavy faces are the same weight. The OS/2 weight
indicates that the Black face is 800 while the Heavy face is 900. But
in fact, looking in Font Book, the Black face is the heavier of the
two!!

> And reading the OS/2 table doesn't always work either, as some
> families have clashing values there. Especially once we have to
> round to multiples of 100.
> 
> So perhaps the font.weight-override.<name>=<value> mechanism, as per
> the most recent patches here, is our best option.

I think we should use a combination of changes here.

First, I think we should adjust the logic for lighter weights:

- map appKitWeight 3 to css weight 300 instead of 200
- explicitly check the CTFont weight attribute for fonts with appKitWeight == 3
- if CTFont weight < -0.40, set the css weight to 200

These changes will fix the mapping problems for lighter fonts in the
Helvetica Neue and AppleSDGothicNeo families, leaving only the problem
of the bold faces.

Second, I think using the explicit override mechanism is fine.  Rather
than define override values for all faces including the regular, bold
faces, restrict the override values to only the problem faces (e.g. Heavy,
Black faces of Avenir).

The relative number of overrides I think will end up being very small,
on the order of 10-20 or so. So rather than going through the work of
putting together a pref-like string and calling GetIntPref for *all*
fonts, it would be simpler I think to iterate over the branch children
and construct a small hashtable with the direct postscript name ==>
weight value overrides.

Example code that does something like this:
http://mxr.mozilla.org/mozilla-central/source/intl/hyphenation/src/nsHyphenationManager.cpp#285

Comment 68

4 years ago
(In reply to Jonathan Kew (:jfkthame) from comment #57)
> Part of the problem here, too, is the brokenness of the CSS-Fonts
> spec, where font-weight allows just 9 discrete values instead of
> actually being an integer. 

It's not broken, it's simply limited to those 9 discrete values. Yes,
that makes the mapping tricky or impossible for large families that
use a larger number of weights or try to have more weights on one end
of the scale, but in practice those sorts of families are rare. 

Allowing any integer value between 100 and 900 makes implementations
more complex and authoring more confusing, especially given that few
authors are even aware of values other than "normal" and "bold". It
might make sense in the future but that's not really the problem here.
In this case, platform API's and/or the font data don't give us a
clear way to determine the font weight accurately. The AppKit weight,
the CTFont weight attribute, or the OS/2 usWeightClass values all
provide maddening variations on the weight of a given font.

Comment 69

4 years ago
Comment on attachment 8382308 [details] [diff] [review]
overrides for Mac families with bad font-weight values.

As noted in comment 67, I think these should be limited to the specific problematic overrides. For Avenir, just the Heavy/Black faces.
Attachment #8382308 - Flags: review?(jdaggett) → review-

Updated

4 years ago
Attachment #8378878 - Flags: review?(roc)

Comment 70

4 years ago
Comment on attachment 8382304 [details] [diff] [review]
revise handling of font-weight for fonts on OS X.


> +static inline int
> +GetWeightOverride(nsAString& aPSName)
> +{
> +    nsAutoCString prefName("font.weight-override.");
> +    // The PostScript name is required to be ASCII; if it's not, the font is
> +    // broken anyway, so we really don't care that this is lossy.
> +    LossyAppendUTF16toASCII(aPSName, prefName);
> +    return Preferences::GetInt(prefName.get(), 0);
> +}

As noted in comment 67, rather than calling Preferences::GetInt for
every font on the system, use GetChildList to construct a small
hashtable once and lookup each font in that hashtable.

Clearing the review? for now as we're converging on an approach that doesn't involve reading the OS/2 table.
Attachment #8382304 - Flags: review?(jdaggett)
(Assignee)

Comment 71

4 years ago
(In reply to John Daggett (:jtd) from comment #70)
> Comment on attachment 8382304 [details] [diff] [review]
> revise handling of font-weight for fonts on OS X.
> 
> 
> > +static inline int
> > +GetWeightOverride(nsAString& aPSName)
> > +{
> > +    nsAutoCString prefName("font.weight-override.");
> > +    // The PostScript name is required to be ASCII; if it's not, the font is
> > +    // broken anyway, so we really don't care that this is lossy.
> > +    LossyAppendUTF16toASCII(aPSName, prefName);
> > +    return Preferences::GetInt(prefName.get(), 0);
> > +}
> 
> As noted in comment 67, rather than calling Preferences::GetInt for
> every font on the system, use GetChildList to construct a small
> hashtable once and lookup each font in that hashtable.

I'm not sure it's worth it. By the time we get here, Preferences has loaded everything into its hashtable, so calling GetInt simply does a single hash lookup. I timed this in my (debug) build, and it typically takes around 1-2 microseconds; an opt build should be even faster.

For comparison, the code in nsHyphenationManager::LoadAliases - which I originally wrote in bug 253317, before we had the new Preferences APIs - takes over 200µs just to call GetChildList, and a further 300µs-plus to read them into its local hashtable (again, in a debug build). It may be worthwhile there, as it does make the subsequent lookups a little faster - no need to prefix the string with the relevant pref.branch, and so it's also quicker to hash - given that looking up the hyphenator for a locale will be done repeatedly whenever hyphenation is enabled.

In this case, however, we're only going to look this up once per font entry, so even with 1000 fonts on the system, the *total* cost will be a couple of milliseconds (based on debug timing - less for opt). I don't think it's worth building the extra hashtable (and keeping it around until all families have been populated) for this; Preferences::GetInt() is fast enough.
I agree, let's keep it simple and use Preferences directly.
(Assignee)

Comment 73

4 years ago
(In reply to John Daggett (:jtd) from comment #67)

> First, I think we should adjust the logic for lighter weights:
> 
> - map appKitWeight 3 to css weight 300 instead of 200

I don't see any real gain in that change; all it does is to wipe out any distinction between AppKit weights 3 and 4.

> - explicitly check the CTFont weight attribute for fonts with appKitWeight
> == 3
> - if CTFont weight < -0.40, set the css weight to 200
> 
> These changes will fix the mapping problems for lighter fonts in the
> Helvetica Neue and AppleSDGothicNeo families, leaving only the problem
> of the bold faces.
> 
> Second, I think using the explicit override mechanism is fine.  Rather
> than define override values for all faces including the regular, bold
> faces, restrict the override values to only the problem faces (e.g. Heavy,
> Black faces of Avenir).

If we're creating the override mechanism at all, I suggest we use it to fix up the light Helvetica Neue and SD Gothic Neo faces as well. No point in adding a hack to check the CTFont weight just at that specific AppKit weight in order to work around a couple of cases that we can equally well specify directly in prefs.

(Note that looking up weight via the CTFont traits is *much* more expensive than checking for the override pref: I'm seeing timings of several hundred µs, compared to just a couple of µs for the pref.)
(Assignee)

Comment 74

4 years ago
Created attachment 8383722 [details] [diff] [review]
fix up font-weight values for some problematic faces on OS X.

Here's a patch that just overrides the weight for known problem faces; otherwise, everything is left as-is. This should be about the simplest fix for the current bug, and allows tweaking for additional faces if necessary. Try run: https://tbpl.mozilla.org/?tree=Try&rev=1f486018d9e4.
Attachment #8383722 - Flags: review?(jdaggett)
(Assignee)

Comment 75

4 years ago
Created attachment 8383802 [details] [diff] [review]
fix up font-weight values for some problematic faces on OS X.

Ugh, that had a silly error. Fixed; also ensure that override values are actually legal for CSS. New try run: https://tbpl.mozilla.org/?tree=Try&rev=22f435bdea4c.
Attachment #8383802 - Flags: review?(jdaggett)
(Assignee)

Updated

4 years ago
Attachment #8383722 - Attachment is obsolete: true
Attachment #8383722 - Flags: review?(jdaggett)

Comment 76

4 years ago
(In reply to Jonathan Kew (:jfkthame) from comment #73)
(In reply to Jonathan Kew (:jfkthame) from comment #73)

>> First, I think we should adjust the logic for lighter weights:
>> 
>> - map appKitWeight 3 to css weight 300 instead of 200
> 
> I don't see any real gain in that change; all it does is to wipe out
> any distinction between AppKit weights 3 and 4.

Hmmm. The data indicates that the mapping of AppKit weight 3 to CSS
weight 200 is incorrect for the large majority of the faces
categorized as appKit weight '3'.  Look at the style names and OS/2
weights, the clear majority are marked as a Light face. There's no
"distinction" that needs to be maintained here, simply an error that
we should fix in our mapping table.

appKit weight == 3

10.8 data:
fonts marked as light: 67 (style name or OS/2 weight)
fonts marked as thin/extralight/ultralight: 34
others: 2

10.9 data:
fonts marked as light: 45 (style name or OS/2 weight)
fonts marked as thin/extralight/ultralight: 16
others: 2

>> - explicitly check the CTFont weight attribute for fonts with appKitWeight
>> == 3
>> - if CTFont weight < -0.40, set the css weight to 200
>> 
>> These changes will fix the mapping problems for lighter fonts in the
>> Helvetica Neue and AppleSDGothicNeo families, leaving only the problem
>> of the bold faces.
>> 
>> Second, I think using the explicit override mechanism is fine.  Rather
>> than define override values for all faces including the regular, bold
>> faces, restrict the override values to only the problem faces (e.g. Heavy,
>> Black faces of Avenir).
> 
> If we're creating the override mechanism at all, I suggest we use it to fix
> up the light Helvetica Neue and SD Gothic Neo faces as well. No point in
> adding a hack to check the CTFont weight just at that specific AppKit weight
> in order to work around a couple of cases that we can equally well specify
> directly in prefs.

I'm not arguing against the override prefs, I'm arguing for a better
mechanism to avoid conflating weights. Overrides work fine but only
for those families which we explicitly add overrides for.  I'm
suggesting a minor workaround that obviates the need to apply override
prefs for this one specific problematic case.

Comment 77

4 years ago
Comment on attachment 8383802 [details] [diff] [review]
fix up font-weight values for some problematic faces on OS X.

> +static inline int
> +GetWeightOverride(nsAString& aPSName)
> +{
> +    nsAutoCString prefName("font.weight-override.");
> +    // The PostScript name is required to be ASCII; if it's not, the font is
> +    // broken anyway, so we really don't care that this is lossy.
> +    LossyAppendUTF16toASCII(aPSName, prefName);
> +    return Preferences::GetInt(prefName.get(), 0);
> +}

Please include a comment with this noting that a return value of '0'
indicates no weight override.

> +// Faces where AppKit gives us reasonable weights are commented out,
> +// but mentioned here to provide context for the override values.
> +pref("font.weight-override.AppleSDGothicNeo-Thin", 100);
> +pref("font.weight-override.AppleSDGothicNeo-UltraLight", 200);
> +pref("font.weight-override.AppleSDGothicNeo-Light", 300);
> +//pref("font.weight-override.AppleSDGothicNeo-Regular", 400);
> +//pref("font.weight-override.AppleSDGothicNeo-Medium", 500);
> +//pref("font.weight-override.AppleSDGothicNeo-SemiBold", 600);
> +//pref("font.weight-override.AppleSDGothicNeo-Bold", 700);
> +//pref("font.weight-override.AppleSDGothicNeo-ExtraBold", 800);
> +pref("font.weight-override.AppleSDGothicNeo-Heavy", 900);

This is hard to read and unnecessary I think. Only include actual
overrides, not every font for these families.

> +//pref("font.weight-override.Avenir-Light", 200);
> +//pref("font.weight-override.Avenir-LightOblique", 200);
> +pref("font.weight-override.Avenir-Book", 300);
> +pref("font.weight-override.Avenir-BookOblique", 300);
> +//pref("font.weight-override.Avenir-Roman", 400);
> +//pref("font.weight-override.Avenir-Oblique", 400);
> +pref("font.weight-override.Avenir-Medium", 500);
> +pref("font.weight-override.Avenir-MediumOblique", 500);
> +//pref("font.weight-override.Avenir-Heavy", 800);
> +//pref("font.weight-override.Avenir-HeavyOblique", 800);
> +pref("font.weight-override.Avenir-Black", 900);
> +pref("font.weight-override.Avenir-BlackOblique", 900);

For Avenir, I think the only thing we should be adjusting here is the
Black face. Apple is shipping two faces (Book and Roman) with
precisely the same weight attributes:

3 200 300 -0.40 Avenir-Light [Avenir]
3 200 300 -0.40 Avenir-LightOblique [Avenir]
5 400 400 0.00 Avenir-Book [Avenir]
5 400 400 0.00 Avenir-BookOblique [Avenir]
5 400 400 0.00 Avenir-Oblique [Avenir]
5 400 400 0.00 Avenir-Roman [Avenir]
6 500 500 0.23 Avenir-Medium [Avenir]
7 600 500 0.23 Avenir-MediumOblique [Avenir]
11 800 800 0.62 Avenir-Black [Avenir]
11 800 800 0.62 Avenir-BlackOblique [Avenir]
11 800 900 0.62 Avenir-Heavy [Avenir]
11 800 900 0.62 Avenir-HeavyOblique [Avenir]

In this case I don't think we should be adhoc reworking the family
layout to move the Book face into the Light slot.  And why the
override for the Medium face?  I don't think that or the similar
mapping for Helvetica Neue Medium are needed.
(Assignee)

Comment 78

4 years ago
(In reply to John Daggett (:jtd) from comment #77)
> Comment on attachment 8383802 [details] [diff] [review]
> fix up font-weight values for some problematic faces on OS X.
> 

> Please include a comment with this noting that a return value of '0'
> indicates no weight override.

OK.

> > +// Faces where AppKit gives us reasonable weights are commented out,
> > +// but mentioned here to provide context for the override values.
> > +pref("font.weight-override.AppleSDGothicNeo-Thin", 100);
> > +pref("font.weight-override.AppleSDGothicNeo-UltraLight", 200);
> > +pref("font.weight-override.AppleSDGothicNeo-Light", 300);
> > +//pref("font.weight-override.AppleSDGothicNeo-Regular", 400);
> > +//pref("font.weight-override.AppleSDGothicNeo-Medium", 500);
> > +//pref("font.weight-override.AppleSDGothicNeo-SemiBold", 600);
> > +//pref("font.weight-override.AppleSDGothicNeo-Bold", 700);
> > +//pref("font.weight-override.AppleSDGothicNeo-ExtraBold", 800);
> > +pref("font.weight-override.AppleSDGothicNeo-Heavy", 900);
> 
> This is hard to read and unnecessary I think. Only include actual
> overrides, not every font for these families.

OK, fine.

> > +//pref("font.weight-override.Avenir-Light", 200);
> > +//pref("font.weight-override.Avenir-LightOblique", 200);
> > +pref("font.weight-override.Avenir-Book", 300);
> > +pref("font.weight-override.Avenir-BookOblique", 300);
> > +//pref("font.weight-override.Avenir-Roman", 400);
> > +//pref("font.weight-override.Avenir-Oblique", 400);
> > +pref("font.weight-override.Avenir-Medium", 500);
> > +pref("font.weight-override.Avenir-MediumOblique", 500);
> > +//pref("font.weight-override.Avenir-Heavy", 800);
> > +//pref("font.weight-override.Avenir-HeavyOblique", 800);
> > +pref("font.weight-override.Avenir-Black", 900);
> > +pref("font.weight-override.Avenir-BlackOblique", 900);
> 
> For Avenir, I think the only thing we should be adjusting here is the
> Black face. Apple is shipping two faces (Book and Roman) with
> precisely the same weight attributes:
> 
> 3 200 300 -0.40 Avenir-Light [Avenir]
> 3 200 300 -0.40 Avenir-LightOblique [Avenir]
> 5 400 400 0.00 Avenir-Book [Avenir]
> 5 400 400 0.00 Avenir-BookOblique [Avenir]
> 5 400 400 0.00 Avenir-Oblique [Avenir]
> 5 400 400 0.00 Avenir-Roman [Avenir]
> 6 500 500 0.23 Avenir-Medium [Avenir]
> 7 600 500 0.23 Avenir-MediumOblique [Avenir]
> 11 800 800 0.62 Avenir-Black [Avenir]
> 11 800 800 0.62 Avenir-BlackOblique [Avenir]
> 11 800 900 0.62 Avenir-Heavy [Avenir]
> 11 800 900 0.62 Avenir-HeavyOblique [Avenir]
> 
> In this case I don't think we should be adhoc reworking the family
> layout to move the Book face into the Light slot.

It's not ad hoc. The Book face is lighter than the Roman. So it should have a lower CSS font-weight. If we leave them identical, as shipped by Apple, then which one people get becomes either "random" or an accidental artifact of how our code happens to enumerate them. Which is exactly what's happening with the Light vs Thin faces of Helvetica Neue right now, resulting in this bug.

We need to either distinguish the two weights - which ARE different - so that font-weight can reliably select between them, or else we need to explicitly choose which one we're going to support and somehow exclude the other. ISTM that mapping Book to 300 (given that Light is already being mapped to 200) is the simplest solution.

(Though with your suggested change of AppKit 3 -> CSS 300, that becomes more awkward, as there'd no longer be a gap between Light and Roman. I suppose we could leave Book at 400, map Roman to 500 and Medium to 600, but I don't see how that's any better. The weight data in the font family as shipped is simply buggy, but to ensure predictable behavior we should override it one way or the other.)

>  And why the
> override for the Medium face?  I don't think that or the similar
> mapping for Helvetica Neue Medium are needed.

In both these families, the AppKit weight for the Medium faces (upright and oblique) is inconsistent, so the overrides are to fix this.

Comment 79

4 years ago
(In reply to Jonathan Kew (:jfkthame) from comment #78)

> It's not ad hoc. The Book face is lighter than the Roman. So it
> should have a lower CSS font-weight. If we leave them identical, as
> shipped by Apple, then which one people get becomes either "random"
> or an accidental artifact of how our code happens to enumerate them.
> Which is exactly what's happening with the Light vs Thin faces of
> Helvetica Neue right now, resulting in this bug.

I realize the Book face is lighter but I don't think you solve
anything by shifting it down to be the Light face when a Light face
already exists. In this case I think we should do whatever is
necessary to mimic Safari/Chrome, there's no benefit to authors to
come up with our own custom arrangement of weights within the family.

I think we should log a bug with Apple and let them fix it when/if
they see fit to do so.

> We need to either distinguish the two weights - which ARE different
> - so that font-weight can reliably select between them, or else we
> need to explicitly choose which one we're going to support and
> somehow exclude the other. ISTM that mapping Book to 300 (given that
> Light is already being mapped to 200) is the simplest solution.

I think it would be simpler to mimic Safari/Chrome here so that
authors see the same result across browsers. We should simply favor
the "standard" face, using the logic present in existing code.

>> And why the override for the Medium face?  I don't think that or
>> the similar mapping for Helvetica Neue Medium are needed.
> 
> In both these families, the AppKit weight for the Medium faces
> (upright and oblique) is inconsistent, so the overrides are to fix
> this.

Ok, but the override only needs to exist for the Medium Oblique case.

Comment 80

4 years ago
Comment on attachment 8383802 [details] [diff] [review]
fix up font-weight values for some problematic faces on OS X.

As stated in previous comments, basically this is fine but needs some cleanup.
Attachment #8383802 - Flags: review?(jdaggett) → review-
(Assignee)

Comment 81

4 years ago
Created attachment 8393792 [details] [diff] [review]
fix up font-weight values for some problematic faces on OS X.
Attachment #8393792 - Flags: review?(jdaggett)
(Assignee)

Updated

4 years ago
Attachment #8383802 - Attachment is obsolete: true
(Assignee)

Comment 82

4 years ago
I've cleaned this up, removed commented entries, etc. One thing I did -not- remove is the override for Avenir-Book, as we don't have any other code, AFAICS, that will determine whether the user sees Book or Roman for "font-family:Avenir; font-weight:400". This override will give us clear, logical and deterministic behavior.

Note that it's not really meaningful to think of font-weight:300 as necessarily being "the Light slot" (comment 77). The naming of weights is inconsistent between families, foundries, etc., and there's no fixed association between face names and CSS font-weight values. font-weight:300 is simply a face that's lighter than the family's default weight, but heavier than any faces with font-weight:200 or 100. Whether it's named "Light" or "Book" is immaterial.

(For an example of the inconsistency of weight *names*, and hence the futility of trying to associate weight names with particular CSS font-weight values in a consistent way, compare AppleSDGothicNeo, where Thin < UltraLight, with HelveticaNeue, where UltraLight < Thin.)

Comment 83

4 years ago
Comment on attachment 8393792 [details] [diff] [review]
fix up font-weight values for some problematic faces on OS X.

Looks good. Two minor additions, we need mappings for the Medium Italic faces for Avenir Next, Avenir Next Condensed:

AvenirNext-MediumItalic 500
AvenirNextCondensed-MediumItalic 500

r+ with those added.
Attachment #8393792 - Flags: review?(jdaggett) → review+
https://hg.mozilla.org/mozilla-central/rev/f26d12bf4252
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
(Assignee)

Updated

4 years ago
Duplicate of this bug: 992559
You need to log in before you can comment on or make changes to this bug.