1163491 - simplify local fontname lookup within gfxFcPlatformFontList code

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect)

Description

[John Daggett (:jtd)]

The code in for looking up fonts by fullname/psname within the gfxFcPlatformFontList code uses a lookup list similar to other platforms that maps these names to a font family, enumerating all font entries in that family and then looking the name up within a family.

This is somewhat convoluted. Karl suggests in bug 1056479, comment 78, thinks it would be similar to map names ==> font pattern and simply use that to create a new font entry:

> However, this still leaves the lookup by family in LookupLocalFont(). 
> It works but is a bit convoluted.
> 
> Here face names are linked to family names in mLocalNames, and that is
> used to look up the gfxFontFamily, to make all gfxFontEntrys in the
> family, to look up the gfxFontEntry by face name from another hash
> table, to get the pattern from the font entry to make a new
> gfxFontEntry.
> 
> Instead, mLocalNames can link the face names to patterns, which can be
> used directly to make the gfxFontEntry, like the existing code.  The
> single hash table is sufficient.

Comment 2

[John Daggett (:jtd)]

Attachment: patch, map local fontnames directly to the underlying patterns

Since we are iterating over all patterns at startup, simply maintain a hashtable of local fontnames to patterns. For local @font-face usage, create the font entry with the pattern directly.

Comment 3

[Karl Tomlinson (:karlt)]

Comment on attachment 8605695 [details] [diff] [review]
patch, map local fontnames directly to the underlying patterns

Thanks.
r+ with the unnecessary extra ref-counting removed as described below.
nsDataHashtable is not helpful for reference counted objects.

Can AddPostscriptName() and AddFullname() in FindStyleVariations() be removed
now?

>+        nsCountedRef<FcPattern> fontPat(font);

Remove this.

>-            mLocalNames.Put(psname, fontFamily);
>+            mLocalNames.Put(psname, fontPat);

mLocalNames.Put(psname, font);

>+    nsCountedRef<FcPattern> fontPattern;

Remove this.

>+    // if name is not in the global list, done
>+    if (!mLocalNames.Get(keyName, &fontPattern)) {
>+        return nullptr;

FcPattern* fontPattern = mLocalNames.Get(keyName);
if (!fontPattern) {
    return nullptr;
}

>-    nsRefPtrHashtable<nsStringHashKey, gfxFontFamily> mLocalNames;
>+    nsDataHashtable<nsStringHashKey, nsCountedRef<FcPattern> > mLocalNames;

nsBaseHashtable<nsStringHashKey,
                nsCountedRef<FcPattern>, FcPattern*> mLocalNames;

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a727cfdf0133

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/a727cfdf0133