Bug 42052 - [Cairo] Font fallback determination is very ineffecient
Summary: [Cairo] Font fallback determination is very ineffecient
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Gtk (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 36548
Blocks:
  Show dependency treegraph
 
Reported: 2010-07-11 22:57 PDT by keqiao zhuo
Modified: 2010-10-27 14:05 PDT (History)
7 users (show)

See Also:


Attachments
Profile graph from Alex (519.27 KB, image/svg+xml)
2010-09-22 13:10 PDT, Martin Robinson
no flags Details
Patch for this issue (4.67 KB, patch)
2010-09-22 16:58 PDT, Martin Robinson
no flags Details | Formatted Diff | Diff
Patch for this issue also fixing custom fonts (17.55 KB, patch)
2010-09-27 17:07 PDT, Martin Robinson
gns: review+
Details | Formatted Diff | Diff
Profile of loading the page with the patch applied (1.21 MB, image/svg+xml)
2010-10-22 05:57 PDT, Alejandro G. Castro
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description keqiao zhuo 2010-07-11 22:57:54 PDT
dear:
   I get webkit-r62241 source form Archived Nightly Builds of Trunk, build GTK Port, then open (http://www.sina.com.cn、http://www.sohu.com.cn)(Simplified Chinese) very slow.
   Then i get latest Nightly Builds,also very slow.I guess because of font process,add i open (http://www.bbc.com) is normal.
 
   please tell me how to do this, tks!
Comment 1 Sergio Villar Senin 2010-09-07 07:22:20 PDT
Seems that libz and libfreetype show on top of the profile when loading that page. Could be a font rendering issue.
Comment 2 Martin Robinson 2010-09-22 13:00:53 PDT
After an initial glance at this, it looks like this is due to the implementation of FontCache::getFontDataForCharacters for the FreeType backend. It seems be checking if all FontConfig fallbacks contain the characters needed. This requires locking and unlocking each FreeType face for these fallback fonts. Really, we should probably be asking FontConfig directly if it has a good font for these characters.

Likely, once we fix this issue there will be other performance issues with CJK characters we need to tackle as well.
Comment 3 Martin Robinson 2010-09-22 13:10:07 PDT
Created attachment 68426 [details]
Profile graph from Alex
Comment 4 Martin Robinson 2010-09-22 16:58:41 PDT
Created attachment 68472 [details]
Patch for this issue
Comment 5 WebKit Review Bot 2010-09-22 18:26:40 PDT
Attachment 68472 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/4061091
Comment 6 Martin Robinson 2010-09-22 18:44:34 PDT
This fix doesn't build because the patch depends on 36548.
Comment 7 Gustavo Noronha (kov) 2010-09-24 11:15:19 PDT
Comment on attachment 68472 [details]
Patch for this issue

View in context: https://bugs.webkit.org/attachment.cgi?id=68472&action=review

Makes sense to me.

> WebCore/platform/graphics/cairo/FontCacheFreeType.cpp:-50
> -
> -    // FIXME: This should not happen, apparently. We are null-checking
> -    // for now just to avoid crashing.

I'd keep this FIXME comment for now, till it's accurate to remove it, as discussed on IRC.
Comment 8 keqiao zhuo 2010-09-24 23:06:41 PDT
this problem is because of font-config, i modify font-config, and leave only one font characters, browser show page normal.

(In reply to comment #1)
> Seems that libz and libfreetype show on top of the profile when loading that page. Could be a font rendering issue.
Comment 9 Martin Robinson 2010-09-25 10:36:35 PDT
(In reply to comment #8)
> this problem is because of font-config, i modify font-config, and leave only one font characters, browser show page normal.

Can you confirm that when you have your original FontConfig setup (the slow one) that you saw the same issue in FireFox and/or Chromium.
Comment 10 keqiao zhuo 2010-09-25 17:59:32 PDT
yes,I can confirm that I use the same fontconfig , firefox is normal, but webkit open page very slow. My linux system have not install chromium.

(In reply to comment #9)
> (In reply to comment #8)
> > this problem is because of font-config, i modify font-config, and leave only one font characters, browser show page normal.
> 
> Can you confirm that when you have your original FontConfig setup (the slow one) that you saw the same issue in FireFox and/or Chromium.
Comment 11 Martin Robinson 2010-09-27 07:29:50 PDT
Okay. There's definitely a bug here, so we should keep this open until a fix lands.
Comment 12 Martin Robinson 2010-09-27 07:33:14 PDT
Comment on attachment 68472 [details]
Patch for this issue

Clearing the review flag, because I have another version of this patch which fixes some issues as well as fixing this method for custom fonts.
Comment 13 Martin Robinson 2010-09-27 12:28:19 PDT
Incorrect fallback determination also seems to expose issues with this test:   editing/selection/extend-selection.html
Comment 14 Martin Robinson 2010-09-27 17:07:06 PDT
Created attachment 68992 [details]
Patch for this issue also fixing custom fonts
Comment 15 Alejandro G. Castro 2010-10-22 05:57:04 PDT
Created attachment 71555 [details]
Profile of loading the page with the patch applied

Apparently the patch fixes the problem, you can check the new profile of loading the same page.
Comment 16 Gustavo Noronha (kov) 2010-10-27 05:33:11 PDT
Comment on attachment 68992 [details]
Patch for this issue also fixing custom fonts

Looks right to me, but where did you get the mostly empty font? It's important to clearly identify that, and be sure the licensing is OK
Comment 17 Martin Robinson 2010-10-27 08:46:56 PDT
(In reply to comment #16)
> (From update of attachment 68992 [details])
> Looks right to me, but where did you get the mostly empty font? It's important to clearly identify that, and be sure the licensing is OK

Good point. :) I created the font myself in fontforge by selecting "New" and drawing a polyhedron for capital A. All other glyphs are empty and use whatever defaults fontforge selected. I'll make a note of that in the ChangeLog.
Comment 18 Martin Robinson 2010-10-27 12:32:17 PDT
Committed r70688: <http://trac.webkit.org/changeset/70688>
Comment 19 WebKit Review Bot 2010-10-27 14:05:41 PDT
http://trac.webkit.org/changeset/70688 might have broken GTK Linux 64-bit Debug