Now, we are using font name (nsAString) for creating gfxFont. And Mac need to resolve the name to ASTUI font id, so there are redundant search code for hash tables. And Linux, we should drop the PangoFont for performance, we need to treat fontconfig directly, but I don't want to do it in gfxPangoFont. Becasue the code for fontconfig needs very many lines and goto statement for cleanup the memory. I have a plan for thie issues: 1. Create |gfxFontEntry| in |gfxPlatform.h| that is same as |FontEntry| of Win and Mac. 2. The Font Name Resolver should return the pointer of |gfxFontEntry|, and |gfxFont| should be created from |gfxFontEntry|. 3. The generic family name of |FontCreationCallback| should be number id, not actual string. 4. The gfxFontEntry will be created on Linux/BeOS too. And it will cache the glyph information. (whether the glyph has.) Stuart, Vlad: Do you have some opinions?

I think this is a bad idea at least on windows. There are a number of things already in the Windows FontEntry that are not cross-platform and I'm planning on adding a whole bunch more.

I should also note that I'm working on atomizing all font names which will change a bunch of how this code works.

(In reply to comment #1) > I think this is a bad idea at least on windows. There are a number of things > already in the Windows FontEntry that are not cross-platform and I'm planning > on adding a whole bunch more. I don't think that we should use same font entry in all platforms. My plan: 1. gfxPlatform.h has gfxFontEntry that is almost empty class. 2. gfxWindowsFont.h(should move to gfxPlatformWindows?)/gfxPlatformGtk.h/gfxQuartzFontCache.h have sub class of gfxFontEntry. (gfxWindowsFontEntry, gfxPangoFontEntry, gfxQuartzFontEntry) 3. gfxFont::ForEachFontInternal returns gfxFontEntry by the callback function instead of the string name. And each callback functions cast the gfxFontEntry to gfx*FontEntry. (gfxFont only save the pointer of gfxFontEntry. The name is already haven in gfxFontEntry.) 4. gfxFont should use gfx*FontEntry for creating. If other code needs original font data, it can access to gfx*FontEntry, so, don't need to access the hash table. (In reply to comment #2) > I should also note that I'm working on atomizing all font names which will > change a bunch of how this code works. I cannot understand. What you meant?

And I think that this patch will save the performance in Uniscribe class in gfxWindowsFonts and FontSelector in gfxPangoFonts. They are checking whether the name is already being in the gfxFontGroup. By the pointer compare, it will be little faster. And Mac, I may need this code for bug 364300. And I need the this feature on Linux for boosting up the performance in bug 357637. dbaron said, gfxPangoFont::GetPangoFont() is slow. But we can remove the functio with dynamically accessing to fontconfig. I can do it in current design too. But there are redundant code. Because the name comes from font entry at resolving, and when we need the font entry, we need to find the font entry from hash table. This is not good...

Stuart, this patch is rough sketch of my idea. The new design is very simple, and it will be great for all platforms. The gfx*Font can access to gfx*FontEntry without the hash table searching. In my linux environment(VMware), it needs 100ms per accessing to hash table. If gfx*Font needs to access the cached system fonts, we need the loss. But in my idea, we can cut the loss.

first shot. works fine on Win32.

I'm cc on bug and will be more than happy to test it on linux when in lands :)

This patch works on Win32 and Linux.

(In reply to comment #8) > Created an attachment (id=264099) [details] > Patch rv0.2 (work in progress) > > This patch works on Win32 and Linux. Oops, works on Win32 and Mac, sorry. (not Linux)

cannot build on Linux, I cannot go to next step until bug 357637 being fixed.

fix some nits...

I think that this should work fine on Win/Linux/Mac. But I cannot confirm on Mac, because I cannot build today's code on my environment... :-(

Comment on attachment 264251 [details] [diff] [review] Patch rv1.0 I recreated my environment for Mac build, and I confirmed that the patch works fine on Mac too. This patch does *not* change the actual behavior on all platforms. Would you review the patch, Vlad? # Is this also needed the Stuart's review? If so, we cannot take this patch at 1.9a5...

I'd feel much more comfortable if Stuart were to review this as well, but I'll try to get a hold of him and see what he thinks.

This does similar changes for OS/2. While adapting the code I was wondering what the check for FFRECountHyphens(fe->GetName()) < 3 is about in gfxPangoFontGroup::FontCallback. Fontconfig font names on OS/2 don't normally contain hyphens so I'm not sure if I should just ignore that or have to add something similar.

Updating to latest trunk.

updating to latest trunk. I hope the review in early time.

Comment on attachment 268954 [details] [diff] [review] Patch rv1.2 some of this patch would be nice to take if it were cleaned up a bit -- the resolver part so that we can create gfxWindowsFonts directly from FontEntrys and on mac, but I don't want to have to add lots of baseclasses and inheritance all over the place to get there.

I'm resetting bugs which are assigned to me but I'm not working on them and I don't have plan for fixing them in near future.

This is obsolete because thebes code was redesigned after 1.9.0, so, this bug doesn't fit to current code. I close this bug. If some optimization is needed on current trunk, you should file some new bugs. -> WFM