754215 - implement lazy font downloads and instantiation of platform font objects (gfxFont subclasses) from gfxFontGroup

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect)

Description

[Jonathan Kew [:jfkthame]]

Attachment: sample showing unnecessary webfont downloads

Currently, when gfxFontGroup resolves the font family names (from CSS, prefs, etc) to available fonts, it instantiates (or finds in cache) a gfxFont object for each font in the list.

This means that with CSS that lists a number of families (to provide fallbacks depending on what is available) we may instantiate numerous fonts that we never actually need, because they're later in the list and we actually use the first font.

This is somewhat inefficient for locally-installed fonts - we may be creating gfxFont and gfxFontShaper objects that we never use - but is much more of an issue for downloadable fonts. If an author does something like

@font-face {
  font-family: MyDownloadedFont;
  src: url(foo/bar.ttf);
}

body {
  MyLocalFont, MyDownloadedFont, serif;
}

with the intention of using the local font if installed, and the downloaded one as a fallback, the resource download will be triggered when gfxFontGroup calls BuildFontList *even if MyLocalFont is available*.

What I'm proposing to try is deferring gfxFont instantiation until gfxFontGroup::FindFontForChar, so that we avoid creating font objects at all for the fonts later in the list unless the earlier ones did not cover the characters actually present in the text. So gfxFont::BuildFontList will only resolve the CSS font names to platform gfxFontEntry objects (which may be proxies, if they're downloadable fonts that haven't actually been loaded yet). The instantiation of gfxFont objects, or the downloading of @font-face resources, will be triggered when FindFontForChar decides to actually use the font.

The attached example illustrates the issue: on OS X, where Zapfino (the first font listed in font-family) is available, none of the webfonts will be used - yet all of them are downloaded, just because they're in the fontGroup's list. (Loading with the Web Console open shows that all the resource downloads are triggered.)

Comment 2

[Jonathan Kew [:jfkthame]]

Attachment: sample showing unnecessary webfont downloads [updated]

Updated the example because mixed-content blocking broke the behavior of the original version.

Comment 3

[Jonathan Kew [:jfkthame]]

Attachment: pt 1 - refactor user-font loading code in preparation for lazier downloading.

This is some preparatory refactoring; the idea here is basically to move the font-loading code from gfxUserFontSet to gfxProxyFontEntry, which IMO is more logically where it belongs anyway.

Comment 4

[Jonathan Kew [:jfkthame]]

Attachment: pt 2 - don't initiate font downloads until the font is actually needed.

This makes our font loading properly lazy (except on linux/fontconfig, which is such a different beast...). The idea is that instead of kicking off font loads when we're resolving the fonts from the font-family list - which may not all be used - we have a proxy font object that can exist in the gfxFontGroup's list of fonts, and only initiate an actual load when FindFontForChar encounters such a proxy and wants to try using it. Tryserver run: https://tbpl.mozilla.org/?tree=Try&rev=ebd071ffd314.

Comment 5

[Jonathan Kew [:jfkthame]]

Attachment: log of network activity using current trunk build

Comment 6

[Jonathan Kew [:jfkthame]]

Attachment: log of network activity using tryserver build with these patches

Comparison of the network activity triggered by force-reloading the testcase here. With current Nightly, we download all 4 of the .woff fonts, even though none of them are actually used.

The patched build still loads the 4 google-fonts CSS files, of course, as these are unconditionally linked, but it does not load any of the .woff resources because all text is handled by a locally-installed font that is earlier in the font-family list.

Comment 7

[Jonathan Kew [:jfkthame]]

Note that the patches here - so far, at least - give us lazy font downloads on gfxPlatformFontList-based systems; on Linux with fontconfig, where font handling and lookup is very different, I haven't attempted to make it lazy. So a desktop Linux build will still download all the resources here, even though they're redundant.

It'd be nice to handle that too, but I think we could move forward with fixing this for the other platforms and consider Linux desktop as a followup.

Comment 8

[John Daggett (:jtd)]

Just to be clear, what are the exact semantics we're trying to implement here?

It seems like the two goals should be:

- only load a font when falling back from other fonts in the fontlist
- while one font is loading, never load another font later in the fontlist

Examples:

For the examples below, assume px is a platform font, dx is a
downloadable font and ldx is a downloadable font that includes local()
as one of the elements in the src descriptor array.

  font-family: p1, d1;

If p1 covers all codepoints for all text nodes, d1 is never loaded if
it's in the unloaded state.  I think this is probably the majority of
possible use cases for mixed downloadable/platform fontlists.

  font-family: d1, d2, p1;

While d1 is loading, d2 is not loaded if it's currently in the
unloaded state. Until d1 loads, default metrics are those of p1. 
After d1 loads, metrics are those of d1.

  font-family: d1, ld1, p1;

While d1 is loading, if ld1 is available locally its default metrics are used.

Comment 9

[John Daggett (:jtd)]

This looks like it does the right thing in terms of functionality,
however I'm not really comfortable with the direction the code is
taking.  I realized reading over this patch that we really should take
care of all the overhead and inefficiency associated with doing font
matching on gfxFont objects rather than the underlying gfxFontEntry
objects which contain the cmaps.  I filed a new bug for this, bug
998869, and I'll try to take a crack at coming up with a patch for
that this week.  As noted in that bug, a small sample of random pages
indicated that we're only shaping text with ~40% of the gfxFont
objects we create!

For this bug, I'd prefer not to create gfxProxyFont objects but rather
do matching on the underlying gfxProxyFontEntry objects.  I think that
way we'll end up with simpler and more efficient code, both for lazy
loading of downloadable fonts and for font families using
unicode-range.

Let me see if I can make progress on bug 998869 and then we can
reconsider what needs to happen for this bug.

Comment 10

[Cameron McCormack (:heycam)]

Attachment: part 1 rebased

I'll want to base my bug 1028497 patches on top of part 1.  I've rebased it up to today's mozilla-central.  The one meaningful change I made was to make sure gfxUserFontSet::mLocalRulesUsed gets set properly.

Comment 11

[John Daggett (:jtd)]

Comment on attachment 8407464 [details] [diff] [review]
pt 2 - don't initiate font downloads until the font is actually needed.

r- for the reasons described in comment 9. We should be doing font matching on gfxFontEntry objects rather than gfxFont objects, then implement lazy loading within the fontlist. And we should tame gfxPangoFontGroup once and for all... ;P

Comment 12

[John Daggett (:jtd)]

So I think we should move the lazy loading patch to another bug and make this a simple refactoring of the userfont set code. This will allow the font loading API stuff to move forward.

Comment 13

[John Daggett (:jtd)]

Jonathan, okay to break out and land part 1 in order to move forward on font loading API patches?

Comment 14

[Jonathan Kew [:jfkthame]]

Sure, no reason why not AFAICS.

Comment 15

[Cameron McCormack (:heycam)]

I landed the rebased part 1:

https://hg.mozilla.org/integration/mozilla-inbound/rev/0af811dfc164

Comment 16

[Carsten Book [:Tomcat]]

sorry had to backout this changes in https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=0294fdaef9be since one of this changes caused test failures like https://tbpl.mozilla.org/php/getParsedLog.php?id=43424728&tree=Mozilla-Inbound

Comment 17

[Cameron McCormack (:heycam)]

Relanded: https://hg.mozilla.org/integration/mozilla-inbound/rev/6131c6ac70cb

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6131c6ac70cb

Comment 19

[John Daggett (:jtd)]

Rather than create a new bug, I think we should tackle lazy loading behavior for the fontlist as part of the work on bug 998869.