[Bug 458169] implement downloadable font support on Linux

[bugzilla-daemon at mozilla.org]

Do not reply to this email.  You can add comments to this bug at
https://bugzilla.mozilla.org/show_bug.cgi?id=458169





--- Comment #6 from Robert O'Callahan (:roc) (Mozilla Corporation) <roc at ocallahan.org>  2008-11-24 13:40:16 PST ---
(In reply to comment #5)
> > Should these functions be changed (in a separate patch) to return
> > already_AddRefed?
> 
> I think so.  I find it confusing to pass around references to objects that
> have a reference count of zero.

Please file a bug on that.

> I'd like to leave these conditional.
> 
> For FC_WEIGHT, the only reason is to avoid the reallocation of memory for the
> property value and the memmove back and forth of all the trailing properties
> and pointers to their values.  The weight is expected to often (maybe usually)
> already have the right value.
> 
> For FC_SLANT, there is the additional benefit of retaining the distinction
> between italic and oblique where possible.
> 
> For FC_FULLNAME, if there is an existing value, then that is the best value as
> it comes from the OpenType name table.  Appending style to family should only
> be a fallback.

OK.

> > +        PRUint8 savedStyle = aStyle.style;
> > +        aStyle.style = FONT_STYLE_NORMAL;
> > +        fontEntry = static_cast<gfxFcFontEntry*>
> > +            (mUserFontSet->FindFontEntry(utf16Family, aStyle, needsBold));
> > +        aStyle.style = savedStyle;
> > 
> > This is yuck. Can we make aStyle a const reference and just use a temporary
> > copy here?
> 
> Yes, this is yuck.
> 
> Constructing a gfxFontStyle always requires a memory allocation because it has
> an nsCString member, which is always forced to be non-empty, even though it
> doesn't get used here.
> 
> What I think would look nicest here would be to change the FindFontPattern()
> gfxFontStyle argument to thebes style and weight arguments.  That would avoid
> the new gfxFontStyle allocation in SortPreferredFonts, and confine all the
> gfxFontStyle yuck to within FindFontPattern.
> 
> Then modifying gfxUserFontSet::FindFontEntry arguments so that only the
> information actually used needs to be provided, and/or modifying gfxFontStyle
> so that the nsCString member can be empty, can be considered as future
> improvements.

OK

> > +            // User fonts are already filtered by slant (but not size) in
> > +            // mUserFontSet->FindFontEntry().
> > 
> > Aren't you working around that by retrying FindFontEntry with
> > FONT_STYLE_NORMAL, in FindFontPattern?
> 
> SlantIsAcceptable() also accepts faces with FONT_STYLE_NORMAL/FC_SLANT_ROMAN
> when the requested style is not normal/roman (as an oblique can be synthesized
> from normal/roman).

OK

> This code was copied from the code for Mac and Windows, so I suggest
> considering making that change to all platforms in a separate bug, probably
> bug 465452.

OK

-- 
Configure bugmail: https://bugzilla.mozilla.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.