Closed
Bug 931426
Opened 11 years ago
Closed 10 years ago
Font rendering in OS X 10.9 Mavericks is much lighter and less legible with some sites
Categories
(Core :: Layout: Text and Fonts, defect)
Tracking
()
RESOLVED
FIXED
mozilla31
People
(Reporter: mario_grgic, Assigned: jfkthame)
References
(Blocks 1 open bug)
Details
Attachments
(22 files, 7 obsolete files)
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 |
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•11 years ago
|
||
Reporter | ||
Comment 2•11 years ago
|
||
Assignee | ||
Comment 3•11 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.
Blocks: mavericks-compat
Reporter | ||
Comment 4•11 years ago
|
||
Reporter | ||
Comment 5•11 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•11 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•11 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•11 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•11 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
Comment 10•11 years ago
|
||
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.
Comment 11•11 years ago
|
||
Assignee | ||
Comment 12•11 years ago
|
||
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•11 years ago
|
Assignee: nobody → jfkthame
Comment 13•11 years ago
|
||
## 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•11 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
Comment 15•11 years ago
|
||
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•11 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?
Comment 17•11 years ago
|
||
(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.
Comment 18•11 years ago
|
||
(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•11 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•11 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•11 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•11 years ago
|
||
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•11 years ago
|
||
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•11 years ago
|
Attachment #824553 -
Attachment is obsolete: true
Assignee | ||
Comment 24•11 years ago
|
||
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•11 years ago
|
Attachment #824603 -
Attachment is obsolete: true
Assignee | ||
Comment 25•11 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)
Comment 26•11 years ago
|
||
(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•11 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•11 years ago
|
Attachment #824716 -
Flags: review?(jdaggett)
Assignee | ||
Comment 28•11 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•11 years ago
|
||
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 30•11 years ago
|
||
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•11 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•11 years ago
|
||
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.
Comment 33•11 years ago
|
||
(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•11 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•11 years ago
|
||
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•11 years ago
|
Attachment #824716 -
Attachment is obsolete: true
Attachment #824716 -
Flags: review?(jdaggett)
Comment 36•11 years ago
|
||
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•11 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•11 years ago
|
||
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•11 years ago
|
Attachment #829218 -
Flags: review?(jdaggett) → review-
Assignee | ||
Comment 39•11 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•11 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•11 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•11 years ago
|
||
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•11 years ago
|
Attachment #829218 -
Attachment is obsolete: true
Comment 43•11 years ago
|
||
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•11 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•11 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•11 years ago
|
||
Comment 47•11 years ago
|
||
Updated•11 years ago
|
Attachment #8336752 -
Flags: review?(jdaggett)
Assignee | ||
Comment 48•11 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•11 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•11 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•10 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•10 years ago
|
||
jdaggett: ping re the tryserver build above.
Assignee | ||
Comment 53•10 years ago
|
||
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•10 years ago
|
Attachment #8336752 -
Attachment is obsolete: true
Comment 54•10 years ago
|
||
Google docs spreadsheet with 10.8 data: https://docs.google.com/spreadsheet/ccc?key=0ArCKGq7OfNmMdHo2WkhVQTJQdXFhLTFCRUlxRF9jOVE
Flags: needinfo?(jdaggett)
Comment 55•10 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•10 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•10 years ago
|
||
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•10 years ago
|
||
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•10 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•10 years ago
|
||
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•10 years ago
|
||
Columns same as last comment. From a coworker's machine running OSX 10.9.
Updated•10 years ago
|
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•10 years ago
|
||
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•10 years ago
|
||
Font weight data for all fonts on 10.9, same format as the last comment.
Comment 65•10 years ago
|
||
Assignee | ||
Comment 66•10 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•10 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•10 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•10 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•10 years ago
|
Attachment #8378878 -
Flags: review?(roc)
Comment 70•10 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•10 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•10 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•10 years ago
|
||
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•10 years ago
|
||
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•10 years ago
|
Attachment #8383722 -
Attachment is obsolete: true
Attachment #8383722 -
Flags: review?(jdaggett)
Comment 76•10 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•10 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•10 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•10 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•10 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•10 years ago
|
||
Attachment #8393792 -
Flags: review?(jdaggett)
Assignee | ||
Updated•10 years ago
|
Attachment #8383802 -
Attachment is obsolete: true
Assignee | ||
Comment 82•10 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•10 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+
Assignee | ||
Comment 84•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/f26d12bf4252
Target Milestone: --- → mozilla31
Comment 85•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/f26d12bf4252
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•