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) <[EMAIL PROTECTED]> 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. _______________________________________________ Fedora-fonts-bugs-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/fedora-fonts-bugs-list
