reading in font names at startup takes too long

RESOLVED FIXED

Status

()

Core
Graphics
P2
normal
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: jtd, Assigned: jtd)

Tracking

({perf})

Trunk
x86
Mac OS X
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments, 5 obsolete attachments)

(Assignee)

Description

11 years ago
Created attachment 289281 [details] [diff] [review]
patch to dump out time spent in InitFontList

We currently read through the list of all font families and font names at startup.  Depending on the state of the font cache and the number of fonts on the system, this can take a *very* long time.

With fonts in the Adobe Font Folio installed, the gfxQuartzFontCache::InitFontList iterates over 650 font families, 2642 fonts and a total of 73,025 name table entries.  This takes:

Total startup time*: 66 secs
Reading font info:   41 secs

*startup == time from command-line start until 'Done' appears in status bar with Minefield default startup page.

Subsequent runs take much less time:

Run 2:
Startup:  10s
Font info: 4.8s

Run 3:
Startup:   7s
Font info: 4.3s

after many runs:
Startup:   6s
Font info: 2.0s

This is clearly a worst-case scenario but one that is not necessarily unusual among web developers, one of our key target audiences.

650 families, 2642 fonts, 73025 names

Suggested priority: P3
Flags: blocking1.9?

Comment 1

11 years ago
I don't think we should block on having a ridiculous number of fonts installed.  We should try to get an idea of how many fonts are typical on someones system.  If you have a good idea on how to fix it already, having some time estimate might help making a decision on blocking.
(Assignee)

Comment 2

11 years ago
Without Adobe fonts installed:

222 families, 421 fonts, 18544 name table entries

Startup after reboot: 29s (4.5s reading font info)
2nd run: 6s (0.21s reading font info)
3rd run: 4s (0.28s reading font info)

I see two potential solutions:

1. Read in font info when font misses happen instead
2. Read in font info on a separate thread so that startup time isn't impacted

I think the first is the better option.  Specifically I think we should read in the font family names within InitFontList time, then set up other localized family names when a font miss occurs.  I don't think this would take more than a couple days.

The reason I think this is a blocker is that reading in all the font names doesn't scale, it's not too bad on systems with a default install but on systems with larger amounts of fonts, like those of web designers, we start to spend a lot of time reading in font names that isn't absolutely necessary.
Some fonts are read when we cannot find the name in our list. See gfxQuartzFontCache::ResolveFontName. It might help you.
On the Mac I'm sitting in front of right now, I have 118 font *files* in /System/Library/Fonts and /Library/Fonts (these are the standard OS fonts, so they have regular/bold/italic/bolditalic. all in one font file).  Everyone's going to at least have this number.

I then have 142 font files in my ~/Library/Fonts folder, but 1) only about half of them are enabled and 2) many of these are xp TTFs, with one style per file.

Users with AppleWorks or MS Office installed are going to have a number of fonts dumped into their /Library/Fonts folder (about a couple dozen font *files* with the 4 styles, IIRC).

See also bug 364785 comment 15; this is a "known" Ts regression that hasn't actually been fixed, and it's particularly noticeable on older Macs (startup became basically "go take a break and come back" on my PowerBook after bug 362665 landed).  If there's a way we can create a correct list without iterating every font at startup, we really should at least investigate it IMO.
Note that we should read suitable amount font/family names at startup. Because if gfxQuartzFontCache::ResolveFontName makes too slower, the Tp value will be damaged.

Comment 6

11 years ago
My $.02 is that preventing really bad startup times is worth looking into.   can we do this in delayed startup (e.g. after the main window is displayed)?

|mFamilies| is not used in ResolveFontName, so, it might be able to be delayed, but I think that the improvement is too little...

There may be another way. If most i18n fonts can be found by ATSUFindFontFromName, we can delay the third for loop in InitFontList. However, maybe, it will make a heavy delay at first unknown font being found. And note that many sites specify the fonts only for Windows.
(Assignee)

Updated

11 years ago
Keywords: perf

Updated

11 years ago
Flags: blocking1.9? → blocking1.9+
Priority: -- → P3
(Assignee)

Updated

11 years ago
Status: NEW → ASSIGNED
(Assignee)

Updated

11 years ago
Blocks: 401989
(Assignee)

Updated

11 years ago
Blocks: 400717
(Assignee)

Updated

11 years ago
Blocks: 409342
(Assignee)

Comment 8

11 years ago
List of hashtables used in the font family name lookup process (FF3/Mac) along with a count of the number of entries on my system (10.4, all system fonts + small set of additional fonts):

mCache // appended on the fly, name can be family name, psname, or full name(!!!)
  <name, style> ==> ATSUFontID
  
mFamilies // initialized at startup, [fontManager availableFontFamilies] + special-case fixed-width fonts (e.g. Osaka-Mono)
  <cocoa family name> ==> <FamilyEntry refptr>
  my system: 104
  
mPostscriptFonts // initialized at startup
  <psname> ==> <FontEntry refptr>
  my system: 314
  
mFontIDTable // initialized at startup
  <ATSUFontID> ==> <FontEntry refptr>
  my system: 314
  
mAppleFamilyNames // initialized at startup, ATSUGetIndFontName (contains all localized family names)
  <family name> ==> <cocoa family name>
  my system: 356
  
mAllFamilyNames // appended on the fly, caches plain-vanilla face lookups for a given family
  <family name> ==> <psname>
  
mAllFontNames // initialized at startup, ATSUGetIndFontName (contains names like Times Italic, Arial Negrita Cursiva, etc.)
  <full face name> ==> <psname>
  my system: 1030
  
mNonExistingFonts // initialized at startup with ignored fonts, appended on the fly with font name misses
  array of non-matching font family names


(Assignee)

Comment 9

11 years ago
Psuedo-code synopsis of the process used to look up font family names.  This occurs within the gfxAtsuiFontGroup constructor and transforms font-family strings into an array of gfxAtsuiFont objects.

input: "Tahoma, Futura, serif"
output: array of objects for <Tahoma>, <Futura-Medium>, <Times-Roman> faces (assuming no special style settings)

font-family ==> font face specifier ==> use in gfxAtsuiFont

gfxAtsuiFontGroup::gfxAtsuiFontGroup

  for each font name in font families string
    gfxFontGroup::ForEachFontInternal
      resolve generic ==> pref font list ==> <psname>
      
    gfxAtsuiFontGroup::FindATSUFont

      gfxQuartzFontCache::FindATSUFontIDForFamilyAndStyle (familyName, fontStyle)
        lookup in mCache ==> return <ATSUFontID>
        gfxQuartzFontCache::FindFromSystem (familyName, fontStyle)
        
          resolve font name:
          
            lookup in mPostscriptFonts ==> <FontEntry obj>
            else
              gfxQuartzFontCache::ResolveFontName (familyName) ==> <psname>
              
                check in mNonExistingFonts array ==> if found, return not found
                check in mPostscriptFonts ==> if found, return <psname> [Note: Tahoma, serif will resolve here]
                check in mAllFontNames ==> if found, return <psname>
                call ATSUFindFontFromName (familyName) with kFontFullName set
                if found
                  lookup in mFontIDTable ==> if found, return <psname>
                check in mAllFamilyNames ==> if found, return <psname>
                check in mAppleFamilyNames ==> <cocoa family name>   [Note: Futura will resolve here the first time]
                if found
                  [fontManager fontWithFamily:cocoaFamilyName]
                  lookup in mFontIDTable ==> if found, return <psname>
                call ATSUFindFontFromName (familyName) with kFontFamilyName set
                if found
                  lookup in mFontIDTable ==> if found, return <psname>
                return not found
                
              lookup in mPostscriptFonts ==> <FontEntry obj> (argh, why???)
            
          convert to a face to match the style:
          
            [NSFont fontWithName:name size:aSize] ==> [NSFont*]
            if italic or fixed, [fontManager convertFont:font toHaveTrait:desiredTraits]
            gfxQuartzFontCache::FindFontWeight
            if weight different, [fontManager fontWithFamily:[currentFont familyName] weight:desiredWeight]
            return [newFont _atsFontID]            

      GetOrMakeFont (ATSUFontID, fontStyle)            
        lookup in mFontIDTable ==> <psname>
        check in gfxFontCache::Lookup (psname, fontStyle)
        if not found
          new gfxAtsuiFont
            lookup in mFontIDTable ==> <FontEntry refptr>


(Assignee)

Updated

11 years ago
Blocks: 390901
(In reply to comment #9)
>               lookup in mPostscriptFonts ==> <FontEntry obj> (argh, why???)

for getting the FontEntry for the resolved postscript name.

Comment 11

11 years ago
Assuming font's don't change on the system often can we cache the font list in the profile and double-check it during idle or something?
(Assignee)

Comment 12

11 years ago
(In reply to comment #11)
> Assuming font's don't change on the system often can we cache the font list in
> the profile and double-check it during idle or something?

We need a list of fonts for a few reasons, to lookup a font family, to iterate over all fonts when fallback occurs and to populate the list of fonts in the prefs panel.  Font lookup will happen first.  Originally we just used a system api, something like [NSFontManager fontWithFamily], but this has one small gotcha - it only works with "canonical" font family names, it can't be used to lookup localized font family names like ヒラギノ角ゴ Pro, only Hiragino Kaku Gothic Pro will work in this case.  That's why the code now spends so much time searching through the name tables of all the fonts.  Caching a list that flags fonts containing localized font family names would greatly reduce the work that needs to be done the next time the user starts the browser.

The list of fonts is also rebuilt from scratch(!) when a font system update occurs, either when a user adds a new font or disables one via FontBook.  The JavaVM also forces this to occur, if you run through the Talos page set you'll see gfxQuartzFontCache::InitFontList fire twice, first at startup and soon after when the JavaVM loads.  I'm guessing it's adding a font programatically.  It would be better to walk the current list and rebuild more judiciously.

Any pointers for good example code that stores data into a profile?

BTW, there's an example list of all font name info attached to the cmap font matching bug:

https://bugzilla.mozilla.org/show_bug.cgi?id=396137

(Assignee)

Comment 13

11 years ago
Created attachment 297130 [details] [diff] [review]
patch, v.0.1, beginnings of simplified font lists

Patch to set up new font lists.  Code doesn't do anything right now but if logging is enabled, it will output font family name resolution using both the old and new schemes to a logfile.

To enable logfile output:

export NSPR_LOG_MODULES=resolveFontName:5
export NSPR_LOG_FILE=resolveFontName.log

The idea is to make Mac font family name resolution work more like Windows, so that gfxAtsuiFontGroup transforms a list of font family names into a family entries which combined with a style are used to create gfxAtsuiFont objects.  These are cached in the gfxFontCache.  This cuts down on a lot of unnecessary hashtable lookups to go between font family name ==> postscript name ==> ATSUI font id, yadda yadda.

Next step is to integrate this into gfxAtsuiFontGroup.
Comment on attachment 297130 [details] [diff] [review]
patch, v.0.1, beginnings of simplified font lists


>+    CFStringRef outName = 0;

use NULL please.

>+        if ([name compare:familyName] != NSOrderedSame) {

You might want to use isEqualToString: instead of compare:, if you end up continuing to use this approach.

>+    NSString *availableFamily;

Initialize this to nil, please.

>+        [localizedNames removeAllObjects];

Is this really faster than creating a new array each time?


Your memory management looks OK, but keep an eye on the autorelease pools -- you may want to do some manual management of them so you don't put hundreds of thousands of objects into the main one.
(Assignee)

Comment 15

11 years ago
Created attachment 297328 [details] [diff] [review]
patch,v.0.2,weight resolution set up

Code to lookup up a font face given a style set up.  This version just logs the font looked up, the old lookup tables are still in use.  Weight handling of relative weights (i.e. bolder, lighter) not correct yet.
Attachment #297130 - Attachment is obsolete: true
(Assignee)

Comment 16

11 years ago
Created attachment 297510 [details] [diff] [review]
patch, v.0.3, new font family lookup hooked in

This is still in a hacky form, I swizzled the lookup that occurs in ResolveFontName and in FindATSUFontIDForFamilyAndStyle to use the new lookup routines which do (1) font family name resolution, including localized family names and (2) handles style/weight matching of individual faces.

Next step is to rework the cmap lookup process in gfxQuartzFontCache::FindFontForChar, this will now iterate over families and then individual fonts within each family.  Then the old lookup process needs to be untangled in gfxAtsuiFontGroup.  After that we can begin more extensive testing and measure performance speedup.

The new lookup tables should cut down on the time to lookup and create gfxAtsuiFont objects, so this may end up improving Tp along with Ts.

The changes here will also probably fix bug 411891 and bug 400717.
Attachment #297328 - Attachment is obsolete: true
(Assignee)

Updated

11 years ago
Blocks: 411891
(Assignee)

Comment 17

11 years ago
Created attachment 298187 [details] [diff] [review]
patch, v.0.4, new and old code cleaned up, conditionalized

This version uses the conditional USE_OLD_FONT_LOOKUP in gfxAtsuiFonts.h to control whether to use the old font resolution methods or not.  The constructor for gfxAtsuiFont now takes a MacOSFontEntry pointer instead of an ATSUFontID and in general ATSUFontID's are only used in places that involve direct interaction with ATSUI api's.  The system font fallback search now iterates over all families and calls a method of MacOSFamilyEntry to do the lookup for the set of families for a particular family.  This should allow us to optimize the fallback process further by quickly skipping over font families that don't contain any fonts supporting a particular unicode range.

Next step is more extensive testing, especially of the effect on startup time and Tp.
Attachment #297510 - Attachment is obsolete: true
(Assignee)

Comment 18

11 years ago
First set of timings for the worst 10 pages in the talos page set, each run 5 times, mean Tp number with existing code and patch v.0.4:

www.aftonbladet.se/www.aftonbladet.se/index.html   1520 ==> 1012
www.265.com/www.265.com/index.html                 1522 ==>  779
www.chinacars.com/www.chinacars.com/index.html     1595 ==>  772
www.qihoo.com/www.qihoo.com/index.html             1652 ==> 1482
www.phoenixtv.com/www.phoenixtv.com/index.html     1997 ==>  993
www.dailymotion.com/www.dailymotion.com/index.html 2191 ==>  278
www.yesky.com/www.yesky.com/index.html             2551 ==>  682
www.pcpop.com/www.pcpop.com/index.html             2853 ==>  866
www.verycd.com/www.verycd.com/index.html           4327 ==> 1403
www.it.com.cn/www.it.com.cn/index.html            16705 ==>  826

Wonder if anyone is paying attention to this bug, tee hee...

Caution: I still need to verify that these speedups are legit, I'm going to run through the font matching that occurs on each page and make sure there's nothing funky going on.

(Assignee)

Comment 19

11 years ago
Need to figure out what other infrastructure needs to be in place to deal with pref fonts and the way they are displayed in different localized versions.  This is bug 390901.

With the new lookup scheme, there are two lookup tables:

    // canonical family name ==> family entry (unique, one name per family entry)
    nsDataHashtable<nsStringHashKey, nsRefPtr<MacOSFamilyEntry> > mFontFamilies;    

    // localized family name ==> family entry (not unique, can have multiple names per 
    // family entry, only names *other* than the canonical names are stored here)
    nsDataHashtable<nsStringHashKey, nsRefPtr<MacOSFamilyEntry> > mLocalizedFamilies;    

The question is whether these are sufficient to map old, crufty QD font family names squirreled away in profiles (probably not).  We should be able to map any font family name, canonical or localized, to a canonical family name that is consistent across locales.  The problem is that QD family names don't follow simple font family rules, they include the full name in some cases (e.g. Osaka-Mono).  If at all possible, I think we should keep code to handle this sort of cruftiness out of the structure of generalized font lookup.  The only exception would be in cases where we're forced to add "special" proxy families that don't exist to support monospace fonts for some locales (e.g. Osaka-Mono).
(Assignee)

Comment 20

11 years ago
Running startup time tests, time spent in InitFontList, in microseconds.

Procedure: run FontFinagler to clear out font caches.  This requires a reboot.  Then start and quit the browser repeatedly.

http://homepage.mac.com/mdouma46/fontfinagler/

Existing code:

7,160,104 usec
  163,238
  211,946
  124,402
  164,425
  204,300
  
With patch v.0.4:

3,691,342 usec
   66,290
   78,296
   65,167
  127,053
   66,744

The first run is the absolute worst-case scenario, the ATS caches are not completely cleared out, even across reboots so most users won't see times this bad.  This is on a relatively fast MacPro, I'm going to do some more testing on an older PPC machine.

Comment 21

11 years ago
(In reply to comment #18)
> First set of timings for the worst 10 pages in the talos page set, each run 5
> times, mean Tp number with existing code and patch v.0.4:
> 
> www.aftonbladet.se/www.aftonbladet.se/index.html   1520 ==> 1012
> www.265.com/www.265.com/index.html                 1522 ==>  779
> www.chinacars.com/www.chinacars.com/index.html     1595 ==>  772
> www.qihoo.com/www.qihoo.com/index.html             1652 ==> 1482
> www.phoenixtv.com/www.phoenixtv.com/index.html     1997 ==>  993
> www.dailymotion.com/www.dailymotion.com/index.html 2191 ==>  278
> www.yesky.com/www.yesky.com/index.html             2551 ==>  682
> www.pcpop.com/www.pcpop.com/index.html             2853 ==>  866
> www.verycd.com/www.verycd.com/index.html           4327 ==> 1403
> www.it.com.cn/www.it.com.cn/index.html            16705 ==>  826
> 
> Wonder if anyone is paying attention to this bug, tee hee...
> 
> Caution: I still need to verify that these speedups are legit, I'm going to run
> through the font matching that occurs on each page and make sure there's
> nothing funky going on.
> 

Yea - um this plus the startup wins are pretty amazing.  Hope they are legit, we can get this landed, and do similar work for windows :-).
(Assignee)

Comment 22

11 years ago
Created attachment 298679 [details] [diff] [review]
patch, v.0.5, old code removed
Attachment #298187 - Attachment is obsolete: true
(Assignee)

Comment 23

11 years ago
Created attachment 298854 [details]
performance comparison of existing trunk to v.0.5 patch with speedup

More performance testing, using patch v.0.5.  The results in comment #18 appear to be the effect of "cold" font caches, I ran the tests with the old code, then with the new.  I did repeated testing and it shows a general speedup but not as dramatic as in comment #18.

With the existing trunk code and the code in patch v.0.5, I ran through the Talos page sets 10 times for each, then averaged the mean times over three runs.  Of the 356 page sets, 151 were faster, 183 were roughly the same, and 22 were slower (faster means perf > 1.05x, slower < 0.95x, same otherwise).

Ranked by slower to faster, the third column reflects Tp for existing trunk code and the fourth column is with patch v.0.5 code.  Speedup ( execution old / execution new ) is in the last numeric column.
(Assignee)

Comment 24

11 years ago
Comment on attachment 298679 [details] [diff] [review]
patch, v.0.5, old code removed

Vlad, could I get your initial review of this?  I'm going to ask Stuart for the final review but your comments would be helpful.

Summarized simply, the changes are:

- at startup, use up a single hashtable for all font families and populate the list of fonts for each

- at startup, read through the name table for localized names of *one* font in the family; if a localized name exists, check the other fonts in the family

- eliminated the caching of font lookups since we now are storing most of the information already, no Cocoa calls are made as part of the font selection process

- gfxAtsuiFont objects are now initialized directly from the MacOSFontEntry object and ATSUFontID's are only used in conjunction with ATSUI api calls, they are not used as hashtable indices anywhere.

With these changes it will be much simpler to implement the caching of pref fonts lookup, these can now be cached as <lang group> ==> <list of MacOSFamilyEntry objects>, lists that will only change when prefs are modified.  Since profiling indicates that pref list construction is relatively expensive, this should help make significant improvements in Tp performance.

One note about Mac vs. Windows performance, these changes actually make the structure of font lookup on the Mac much closer to the structure on Windows, so unfortunately there aren't many gains here that will help us on Windows.  The one optimization I can imagine is the ability to quickly eliminate whole families of fonts for given Unicode ranges when system fallback occurs by unioning the cmap bit vectors together for each family lazily.
Attachment #298679 - Flags: review?(vladimir)

Comment 25

11 years ago
(In reply to comment #23)
> Created an attachment (id=298854) [details]
> performance comparison of existing trunk to v.0.5 patch with speedup
> Ranked by slower to faster, the third column reflects Tp for existing trunk
> code and the fourth column is with patch v.0.5 code.  Speedup ( execution old /
> execution new ) is in the last numeric column.
> 

You've got a handful there that are 50% slower - any ideas why?  Overall looks like a win - just not sure if the 2 3x slower cases show anything interesting.
(Assignee)

Comment 26

11 years ago
(In reply to comment #25)

> You've got a handful there that are 50% slower - any ideas why?  Overall looks
> like a win - just not sure if the 2 3x slower cases show anything interesting.

Looks like several of these are Taiwanese sites.  Our Taiwanese (zh-TW) pref settings are incorrect, they use full font names and not family names, like "Apple LiGothic Medium" instead of "Apple LiGothic".  The existing code allowed either full font names, Postscript names or family names to be used but the new code only allows family names.  So these sites were not hitting the fallback pref fonts but were instead hitting system font fallback and that probably explains for the big performance difference.  I'll fix up the names and run my tests again to confirm this tomorrow.

Mac OS X font prefs in modules/libpref/src/init/all.js:

pref("font.name.serif.zh-TW", "Apple LiSung Light"); 
pref("font.name.sans-serif.zh-TW", "Apple LiGothic Medium");  
pref("font.name.monospace.zh-TW", "Apple LiGothic Medium");  
pref("font.name-list.serif.zh-TW", "Apple LiSung Light"); 
pref("font.name-list.sans-serif.zh-TW", "Apple LiGothic Medium");  
pref("font.name-list.monospace.zh-TW", "Apple LiGothic Medium");  

(In reply to comment #26)
> (In reply to comment #25)
> 
> > You've got a handful there that are 50% slower - any ideas why?  Overall looks
> > like a win - just not sure if the 2 3x slower cases show anything interesting.
> 
> Looks like several of these are Taiwanese sites.  Our Taiwanese (zh-TW) pref
> settings are incorrect, they use full font names and not family names, like
> "Apple LiGothic Medium" instead of "Apple LiGothic".  The existing code allowed
> either full font names, Postscript names or family names to be used but the new
> code only allows family names.  So these sites were not hitting the fallback
> pref fonts but were instead hitting system font fallback and that probably
> explains for the big performance difference.  I'll fix up the names and run my
> tests again to confirm this tomorrow.
> 
> Mac OS X font prefs in modules/libpref/src/init/all.js:
> 
> pref("font.name.serif.zh-TW", "Apple LiSung Light"); 
> pref("font.name.sans-serif.zh-TW", "Apple LiGothic Medium");  
> pref("font.name.monospace.zh-TW", "Apple LiGothic Medium");  
> pref("font.name-list.serif.zh-TW", "Apple LiSung Light"); 
> pref("font.name-list.sans-serif.zh-TW", "Apple LiGothic Medium");  
> pref("font.name-list.monospace.zh-TW", "Apple LiGothic Medium");  

Don't we support any font names on Mac? I think that it is really big compatibility issue for web designers... (Of course it's only for the existing pages, new pages should have both style if they want to support Fx2.) And note that current Fx2 users might set the font names to prefs, so, they will get the slow Fx3. (And probably, they cannot know they can get the faster Fx3 with new profile.) It's really worse.

And I have an worry. The major web page authoring tools use which names? (postscript? family? or font?) If they are using font names, we should not kill the font name support now.
(Assignee)

Comment 28

11 years ago
(In reply to comment #27)
> Don't we support any font names on Mac? I think that it is really big
> compatibility issue for web designers... (Of course it's only for the existing
> pages, new pages should have both style if they want to support Fx2.) And note
> that current Fx2 users might set the font names to prefs, so, they will get the
> slow Fx3. (And probably, they cannot know they can get the faster Fx3 with new
> profile.) It's really worse.
> 
> And I have an worry. The major web page authoring tools use which names?
> (postscript? family? or font?) If they are using font names, we should not kill
> the font name support now.

Just to be clear, I'm referring to the *full* font names, i.e. names for individual faces like Arial Bold, Arial Italic.  We should only support the family name, i.e. Arial, just as FF2, Safari, Opera, IE and FF3 Windows do, we should be consistent.  Otherwise a web author designing a page on the Mac will see things work one way in FF3 but differently in other browsers.

That's why I logged bug 401989 "inconsistency with font name matching across platforms and compared to safari/opera".  Font family names are what are specified in the CSS specs and they are what are supported across all platforms and browsers.  If you allow full names you get into confusing situations determining how to render styles like:

   font-family: "Arial Bold";
   font-weight: normal;

Is what is rendered bold or not?

I think part of the confusion here are the names the Quickdraw api's return, they are a bit of a jumble (see Prefs > Content tab > Default font), a font like Osaka-Mono shows up even though it is part of the Osaka family.  I think we should do what we can to ameliorate the situation here but we should move to a naming scheme that's consistent and more in line with the specs.


O.K. Thanks. However, what's the zh-TW pref values? Didn't work the prefs on Fx2?? If so, please ignore my previous comment, otherwise, we should support these names I think.
Comment on attachment 298679 [details] [diff] [review]
patch, v.0.5, old code removed

Patch looks fine to me, but we do need to figure out what to do with existing prefs that have incorrect font names.

However, these are prefs that users generally don't modify -- so if we were to change ours, they would get the updated defaults, right?
Attachment #298679 - Flags: review?(vladimir) → review+
(In reply to comment #27)
> Don't we support any font names on Mac? I think that it is really big
> compatibility issue for web designers... (Of course it's only for the existing
> pages, new pages should have both style if they want to support Fx2.) And 

If a designer has been trying to support both Gecko and WebKit already, he's probably already been specifying both font names all along, right? (philippe?)

(In reply to comment #29)
> O.K. Thanks. However, what's the zh-TW pref values? Didn't work the prefs on
> Fx2?? If so, please ignore my previous comment, otherwise, we should support
> these names I think.

Those were the font names Gecko < 1.9 required for those fonts; go figure.

Comment 33

11 years ago
(In reply to comment #32)
> (In reply to comment #27)
> > Don't we support any font names on Mac? I think that it is really big
> > compatibility issue for web designers... (Of course it's only for the existing
> > pages, new pages should have both style if they want to support Fx2.) And 
> 
> If a designer has been trying to support both Gecko and WebKit already, he's
> probably already been specifying both font names all along, right? (philippe?)

Yes, the few page/stylesheet authors that do care about Mac (Gecko/Safari) set both. In practice, I see few stylesheets that actually do set something. Most of the time, they just set some Windows-only fonts (fall back then goes to the users prefs), or they just set something generic.
(Assignee)

Comment 34

11 years ago
(In reply to comment #30)
> (From update of attachment 298679 [details] [diff] [review])
> Patch looks fine to me, but we do need to figure out what to do with existing
> prefs that have incorrect font names.

I looked into this a little more deeply, it's actually a subset of bug 390901.  In Gecko 1.8 code we set up the prefs font list by iterating over ATSFontFamily objects and extracting the Quickdraw name:

http://mxr.mozilla.org/seamonkey/source/gfx/src/mac/nsDeviceContextMac.cpp#790

These are old FMFontFamily style font families and the way fonts are unified into these families I can't begin to fathom; you end up with "Futura" and "Futura Condensed" as separate families.  Unfortunately there's no Cocoa/ATSUI api that can match these names to actual fonts or font families, we need to use the ATSFontFamily/FMFontManager api's to pull out the psname of a default font for a family for use with Cocoa.

As long as we clean up our pref font settings in all.js we will only need to worry about how to update FF2 user profiles with user-specified weirdo font family settings like "Futura Condensed".  If we use a mixture of old font api's, we should be able to extract a canonical family name and map it to the correct family name in the new scheme.  We should avoid adding these old-style names into tables used to lookup font families when rendering.

See comment 28 of bug 390901 for more details.
The only way users could have those weirdo font families in their prefs is with manual editing of user.js, right? There was never any UI for that? If so, I think this is not worth supporting.
Well, I guess there was UI, but would it have stored the weirdo font family names?
(Assignee)

Comment 37

11 years ago
(In reply to comment #35)
> The only way users could have those weirdo font families in their prefs is with
> manual editing of user.js, right? There was never any UI for that? If so, I
> think this is not worth supporting.

If the user changes one of the font settings to be something other than the default, it gets tucked away in prefs.js within the profile.  So to support profiles from FF2, we need this I think.
(Assignee)

Comment 38

11 years ago
Created attachment 299133 [details] [diff] [review]
patch, adjust zh-TW font defaults to use canonical family names

After adjusting the zh-TW font settings in all.js, reran the Tp tests for the slower pages.  All but 4 now perform the same or slightly faster under the new scheme.
Attachment #299133 - Flags: superreview?(pavlov)
Attachment #299133 - Flags: review?(pavlov)
(Assignee)

Updated

11 years ago
Attachment #298679 - Flags: superreview?(pavlov)

Updated

11 years ago
Attachment #299133 - Flags: superreview?(pavlov)
Attachment #299133 - Flags: superreview+
Attachment #299133 - Flags: review?(pavlov)
Attachment #299133 - Flags: review+

Updated

11 years ago
Priority: P3 → P2

Comment 39

11 years ago
Comment on attachment 298679 [details] [diff] [review]
patch, v.0.5, old code removed

go for it
Attachment #298679 - Flags: superreview?(pavlov) → superreview+

Comment 40

11 years ago
This breaks the build on 10.5. kFontPreferredFamilyName is defined on 10.5 so you redefine it.

gfxQuartzFontCache.mm:448: error: conflicting declaration ‘kFontPreferredFamilyName’
/System/Library/Frameworks/ApplicationServices.framework/Frameworks/ATS.framework/Headers/SFNTTypes.h:312: error: ‘kFontPreferredFamilyName’ has a previous declaration as ‘<anonymous enum> kFontPreferredFamilyName’

(Assignee)

Comment 41

11 years ago
Created attachment 299949 [details] [diff] [review]
patch, conditionalize definition of kFontPreferredFamilyName
(Assignee)

Updated

11 years ago
Attachment #299949 - Flags: superreview?(pavlov)
Attachment #299949 - Flags: review?(joshmoz)

Updated

11 years ago
Attachment #299949 - Flags: superreview?(pavlov) → superreview+
(Assignee)

Comment 42

11 years ago
Created attachment 299952 [details] [diff] [review]
patch, avoid enum conflict entirely (checked in)

Just avoid the whole issue, use a different name for the enum value.
Attachment #299949 - Attachment is obsolete: true
Attachment #299952 - Flags: superreview?(pavlov)
Attachment #299952 - Flags: review?(joshmoz)
Attachment #299949 - Flags: review?(joshmoz)

Updated

11 years ago
Attachment #299952 - Flags: review?(joshmoz) → review+

Comment 43

11 years ago
It would be nice to get this patch landed as soon as possible.
Comment on attachment 299952 [details] [diff] [review]
patch, avoid enum conflict entirely (checked in)

I've checked this in after checking with stuart and Mossop on IRC.

mozilla/gfx/thebes/src/gfxQuartzFontCache.mm 	1.19
Attachment #299952 - Attachment description: patch, avoid enum conflict entirely → patch, avoid enum conflict entirely (checked in)
Attachment #299952 - Flags: superreview?(pavlov)
(Assignee)

Comment 45

11 years ago
All checked in!
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Looking at some numbers (old-style Ts):

boxset (CmTrunk): 1640 -> 1500ms (9% win)
cb-xserve01 (CmTrunk): 540 -> 470ms (13% win)
bm-xserve08 (FxTrunk): 810 -> 780ms (4% win)

The boxen are mostly no longer the same as when the old font name resolver landed and killed Ts (bug 362665 comment 29), but perf improvement is almost the inverse (%wise) of the perf loss from 362665 :)
Depends on: 416232
(Assignee)

Updated

11 years ago
No longer blocks: 390901
You need to log in before you can comment on or make changes to this bug.