Bug 29689

Summary: [Layout tests] [Gtk] Gtk DumpRenderTree should use WebKit test fonts
Product: WebKit Reporter: Zan Dobersek <zan>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, hamaji
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Final patch, first attempt
gustavo: review-
Final patch, second attempt
none
Build fix jmalonzo: review+, hamaji: commit-queue+

Zan Dobersek
Reported 2009-09-23 12:15:15 PDT
DumpRenderTree should load test fonts through fontconfig at the start and before each test to insure proper fonts are used for testing. This insures a more proper rendering results.
Attachments
Final patch, first attempt (3.93 KB, patch)
2009-09-23 12:38 PDT, Zan Dobersek
gustavo: review-
Final patch, second attempt (12.91 KB, patch)
2009-09-26 07:44 PDT, Zan Dobersek
no flags
Build fix (948 bytes, patch)
2009-10-06 22:20 PDT, Shinichiro Hamaji
jmalonzo: review+
hamaji: commit-queue+
Zan Dobersek
Comment 1 2009-09-23 12:38:41 PDT
Created attachment 40012 [details] Final patch, first attempt First attempt at final patch. Mimics Qt's way of doing it.
Gustavo Noronha (kov)
Comment 2 2009-09-23 15:42:46 PDT
Comment on attachment 40012 [details] Final patch, first attempt > + FcFontSet* appFontSet = FcConfigGetFonts(0, FcSetApplication); > + if (appFontSet && numFonts >= 0 && appFontSet->nfont == numFonts) > + return; So, before reading the configuration this will return NULL or something? What if the user has a configuration already at their home? > dumprendertree_cppflags += \ > - -DTEST_PLUGIN_DIR=\"${shell pwd}/${top_builddir}/TestNetscapePlugin/.libs\" > + -DTEST_PLUGIN_DIR=\"${shell pwd}/${top_builddir}/TestNetscapePlugin/.libs\" \ > + -DFONTS_CONF_FILE=\"${shell pwd}/${srcdir}/WebKitTools/DumpRenderTree/qt/fonts.conf\" I think we should have a copy of that configuration file in gtk/, instead of refering from the qt one, to avoid being hit by any changes they need to do. Did you test if render tree dumps are matching or more closely matching the expected ones in Mac? =) I'll say r- till we get these two issues sorted out.
Zan Dobersek
Comment 3 2009-09-26 04:33:58 PDT
(In reply to comment #2) > (From update of attachment 40012 [details]) > > + FcFontSet* appFontSet = FcConfigGetFonts(0, FcSetApplication); > > + if (appFontSet && numFonts >= 0 && appFontSet->nfont == numFonts) > > + return; > > So, before reading the configuration this will return NULL or something? What > if the user has a configuration already at their home? This sees if any fonts were added or removed via CSS @font-face rule. It checks if the number of fonts that the current config (which was created and made default, 'current', when initializeFonts() was called for the first time) contains the exact same number as when it was created. Should've added a comment explaining why that part of code is there. > Did you test if render tree dumps are matching or more closely matching the > expected ones in Mac? =) Ignoring all the fonts problems, I'll start creating bugs for each of the directories that contain tests with no generated results. Patches with these results and diff between mac's and gtk's expected results will be uploaded to clarify things. If there are any large differences between expected results, the generated results will not be added to the patch, a bug will be created about the problem and the latter will be examined thoroughly to see what's wrong. I'm also planning to add support for pixel tests, which will bring a better overview to changes and help out with examining rendering differences. Regards, Zan Dobersek
Zan Dobersek
Comment 4 2009-09-26 07:44:23 PDT
Created attachment 40172 [details] Final patch, second attempt Adds a comment about why we check the number of fonts, adds our own copy of fonts.conf.
WebKit Commit Bot
Comment 5 2009-09-26 10:51:24 PDT
Comment on attachment 40172 [details] Final patch, second attempt Clearing flags on attachment: 40172 Committed r48791: <http://trac.webkit.org/changeset/48791>
WebKit Commit Bot
Comment 6 2009-09-26 10:51:31 PDT
All reviewed patches have been landed. Closing bug.
Shinichiro Hamaji
Comment 7 2009-10-06 22:20:44 PDT
Created attachment 40763 [details] Build fix
Shinichiro Hamaji
Comment 8 2009-10-06 22:21:07 PDT
(In reply to comment #7) > Created an attachment (id=40763) [details] > Build fix It seems we need to add -lfontconfig ?
Jan Alonzo
Comment 9 2009-10-07 04:09:09 PDT
Comment on attachment 40763 [details] Build fix r=me.
Shinichiro Hamaji
Comment 10 2009-10-07 09:24:18 PDT
Note You need to log in before you can comment on or make changes to this bug.