[Bug 458169] implement downloadable font support on Linux
bugzilla-daemon at mozilla.org
bugzilla-daemon at mozilla.org
Mon Nov 24 21:40:25 UTC 2008
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.
More information about the fonts-bugs
mailing list