Bug 42052

Summary: [Cairo] Font fallback determination is very ineffecient
Product: WebKit Reporter: keqiao zhuo <zhuokeqiao>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, eric, gustavo, mrobinson, svillar, webkit.review.bot, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Bug Depends on: 36548    
Bug Blocks:    
Attachments:
Description Flags
Profile graph from Alex
none
Patch for this issue
none
Patch for this issue also fixing custom fonts
gustavo: review+
Profile of loading the page with the patch applied none

keqiao zhuo
Reported 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!
Attachments
Profile graph from Alex (519.27 KB, image/svg+xml)
2010-09-22 13:10 PDT, Martin Robinson
no flags
Patch for this issue (4.67 KB, patch)
2010-09-22 16:58 PDT, Martin Robinson
no flags
Patch for this issue also fixing custom fonts (17.55 KB, patch)
2010-09-27 17:07 PDT, Martin Robinson
gustavo: review+
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
Sergio Villar Senin
Comment 1 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.
Martin Robinson
Comment 2 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.
Martin Robinson
Comment 3 2010-09-22 13:10:07 PDT
Created attachment 68426 [details] Profile graph from Alex
Martin Robinson
Comment 4 2010-09-22 16:58:41 PDT
Created attachment 68472 [details] Patch for this issue
WebKit Review Bot
Comment 5 2010-09-22 18:26:40 PDT
Martin Robinson
Comment 6 2010-09-22 18:44:34 PDT
This fix doesn't build because the patch depends on 36548.
Gustavo Noronha (kov)
Comment 7 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.
keqiao zhuo
Comment 8 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.
Martin Robinson
Comment 9 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.
keqiao zhuo
Comment 10 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.
Martin Robinson
Comment 11 2010-09-27 07:29:50 PDT
Okay. There's definitely a bug here, so we should keep this open until a fix lands.
Martin Robinson
Comment 12 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.
Martin Robinson
Comment 13 2010-09-27 12:28:19 PDT
Incorrect fallback determination also seems to expose issues with this test: editing/selection/extend-selection.html
Martin Robinson
Comment 14 2010-09-27 17:07:06 PDT
Created attachment 68992 [details] Patch for this issue also fixing custom fonts
Alejandro G. Castro
Comment 15 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.
Gustavo Noronha (kov)
Comment 16 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
Martin Robinson
Comment 17 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.
Martin Robinson
Comment 18 2010-10-27 12:32:17 PDT
WebKit Review Bot
Comment 19 2010-10-27 14:05:41 PDT
http://trac.webkit.org/changeset/70688 might have broken GTK Linux 64-bit Debug
Note You need to log in before you can comment on or make changes to this bug.