I've been working on this and I've realized that after the 2.8 upgrade we have some failures that don't look correct. The problem is not new, though see bug #102374. That bug was closed because a patch was added for freetype 2.4.11, it was a cherry-pick of a freetype commit (http://git.savannah.gnu.org/cgit/freetype/freetype2.git/commit/?id=e0469372be3870a5ad60b2c4586e9c281357bd28), but that was reverted in freetype just a few revisions later (http://git.savannah.gnu.org/cgit/freetype/freetype2.git/commit/?id=87dc86d68c93c2b5b836501ece57583d1d6d1a2e), so when upgrading to 2.8 I decided not to include the patch. So, with 2.8 now we are getting: ascent: 15.000000, descent: 4.000000, height: 18.000000 while with previous patched version we got: ascent: 14.000000, descent: 3.000000, height: 17.000000 this means that now ascent + descent > height and we are getting a lineGap of -1. I don't want to keep using a patch that has been reverted upstream. This patch is also the reason why we get different results in many layout tests when not using the internal jhbuild. Also, I don't know why, but I tried to backport the patch to 2.8 just to check what happened and I was still getting -1, so that's no longer a solution. I'm not a fonts expert but the problem seems to be the rounding done by cairo when using hint metrics. When not using them what we get is: ascent: 14.257812, descent: 3.460938, height: 18.398438 where ascent + descent <= height and we get a line gap of 0.679688 I think we have several options here: a) Try to clamp the line gap to be 0 when we get a negative value b) Create a temporary cairo font with hint metrics disable only to get the metrics (this is the patch proposed by Dominik in bug #102374). c) Use freetype api directly to get the metrics instead of cairo. I don't know if these are valid solutions or if there's a better one. Thoughts?

Created attachment 320008 [details] Patch This patch upgrades to 2.8 and removes the patch we were using. Instead it uses Dominiks's patch (slightly modified just ot use smart pointers and a fe other cosmetic changes). With this I'm getting 5886 failures. The differences are changes in the text height in 1px. I'm preparing the rebaseline in blocks.

I have the rebaselines ready in 7 patches: $ ls -lh 000*.patch -rw-r--r-- 1 cgarcia cgarcia 36M sep 6 14:55 0001-rebaseline-1.patch -rw-r--r-- 1 cgarcia cgarcia 20M sep 6 14:55 0002-rebaseline-2.patch -rw-r--r-- 1 cgarcia cgarcia 27M sep 6 14:55 0003-rebaseline-3.patch -rw-r--r-- 1 cgarcia cgarcia 42M sep 6 14:55 0004-rebaseline-4.patch -rw-r--r-- 1 cgarcia cgarcia 32M sep 6 14:55 0005-rebaseline-5.patch -rw-r--r-- 1 cgarcia cgarcia 39M sep 6 14:56 0006-rebaseline-6.patch -rw-r--r-- 1 cgarcia cgarcia 42M sep 6 14:56 0007-rebaseline-7.patch Hopefully future freetype upgrades will not be so painful now that we don't patch it anymore.

Comment on attachment 320008 [details] Patch Thanks! getting rid of patches on our jhbuild is always a good thing.

Committed r221670: <http://trac.webkit.org/changeset/221670>

This killed the WPE bot :(

(In reply to Michael Catanzaro from comment #6) > This killed the WPE bot :( hmm, right, they are also using the freetype 2.4.11 patch. As a workaround we could ifdef the code introduced in this patch. Then I suggests someone bumps freetype to 2.8 in wpe as well, removing the patch and ifdefs and then rebaselining. Not fun at all :-(

(In reply to Carlos Garcia Campos from comment #7) > (In reply to Michael Catanzaro from comment #6) > > This killed the WPE bot :( > > hmm, right, they are also using the freetype 2.4.11 patch. As a workaround > we could ifdef the code introduced in this patch. Then I suggests someone > bumps freetype to 2.8 in wpe as well, removing the patch and ifdefs and then > rebaselining. Not fun at all :-( I think we can do this now, not need to ifdef the code.