Bug 29689 – [Layout tests] [Gtk] Gtk DumpRenderTree should use WebKit test fonts

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Zan Dobersek]

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.

[Zan Dobersek]

Created attachment 40012 [details]
Final patch, first attempt

First attempt at final patch.

Mimics Qt's way of doing it.

[Gustavo Noronha (kov)]

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]

(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]

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 on attachment 40172 [details]
Final patch, second attempt

Clearing flags on attachment: 40172

Committed r48791: <http://trac.webkit.org/changeset/48791>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Shinichiro Hamaji]

Created attachment 40763 [details]
Build fix

[Shinichiro Hamaji]

(In reply to comment #7)
> Created an attachment (id=40763) [details]
> Build fix

It seems we need to add -lfontconfig ?

[Jan Alonzo]

Comment on attachment 40763 [details]
Build fix

r=me.

[Shinichiro Hamaji]

Committed r49246: <http://trac.webkit.org/changeset/49246>