Bug 107338

Summary: [chromium] support the lucid version of freetype on precise in DRT
Product: WebKit Reporter: Dirk Pranke <dpranke>
Component: Tools / TestsAssignee: Dirk Pranke <dpranke>
Status: RESOLVED FIXED    
Severity: Normal CC: phajdan.jr, tony
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
s/TODO(dpranke)/FIXME
none
s/unix/linux, add comments
none
update reference to freetype2.gyp
none
Patch tony: review+

Dirk Pranke
Reported 2013-01-18 16:24:33 PST
Changes in FreeType between Ubuntu Lucid and Ubuntu Precise change how the text in pretty much every pixel test is rendered, causing ~7000 failures when running Chromium on Precise. The currently recommended workaround is to use a Lucid chroot, but it would be nice to run precise natively. One option is to use two sets of baselines (like we do on the mac), but we'd like to avoid this. Soon (April if not earlier) we'll be dropping support for Lucid and need to update the baselines, but in the meantime it looks like we can simply preload the lucid version of freetype when running DRT and get the same results, so this seems like a good workaround (or we could compile a static version into DRT, or adjust the rpath of the binary). Note that we don't want to change how Chrome behaves; that should continue using the system version of freetype.
Attachments
Patch (4.19 KB, patch)
2013-02-13 17:49 PST, Dirk Pranke
no flags
s/TODO(dpranke)/FIXME (4.18 KB, patch)
2013-02-13 17:52 PST, Dirk Pranke
no flags
s/unix/linux, add comments (4.27 KB, patch)
2013-02-14 12:29 PST, Dirk Pranke
no flags
update reference to freetype2.gyp (4.27 KB, patch)
2013-02-14 12:51 PST, Dirk Pranke
no flags
Patch (3.94 KB, patch)
2013-02-21 15:30 PST, Dirk Pranke
tony: review+
Tony Chang
Comment 1 2013-01-18 17:02:42 PST
This is fine with me. I think it's more work than just picking a day to upgrade the bots and rebaseline the Linux results, but I'm not the one volunteering to do the extra work.
Dirk Pranke
Comment 2 2013-02-13 17:49:16 PST
Dirk Pranke
Comment 3 2013-02-13 17:52:08 PST
Created attachment 188238 [details] s/TODO(dpranke)/FIXME
Dirk Pranke
Comment 4 2013-02-13 17:53:26 PST
Note that for this to be enabled and functional by default we need to actually pull the code from a git repo mirrored on googlesource.com (i.e., the FIXME goes away). Posting this for review in the meantime. I plan to land this patch and the patch in chromium to update the deps, and then, in a third patch, flip the default variable setting in DRT.gyp so that we start building by default. That should make it easy to revert anything if this doesn't work right.
Tony Chang
Comment 5 2013-02-14 09:43:35 PST
Comment on attachment 188238 [details] s/TODO(dpranke)/FIXME View in context: https://bugs.webkit.org/attachment.cgi?id=188238&action=review > Tools/DumpRenderTree/DumpRenderTree.gyp/DumpRenderTree.gyp:47 > + ['OS=="unix"', { How does this work? I thought OS was linux/win/mac/freebsd/openbsd/solaris/ios/android. > Tools/DumpRenderTree/DumpRenderTree.gyp/DumpRenderTree.gyp:48 > + 'use_custom_freetype%': 0, # FIXME: This should be on by default. Bug link?
Dirk Pranke
Comment 6 2013-02-14 10:00:50 PST
(In reply to comment #5) > (From update of attachment 188238 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=188238&action=review > > > Tools/DumpRenderTree/DumpRenderTree.gyp/DumpRenderTree.gyp:47 > > + ['OS=="unix"', { > > How does this work? I thought OS was linux/win/mac/freebsd/openbsd/solaris/ios/android. > I will double check, but all the examples I saw seemed to be using "unix" as a synonym for "linux". > > Tools/DumpRenderTree/DumpRenderTree.gyp/DumpRenderTree.gyp:48 > > + 'use_custom_freetype%': 0, # FIXME: This should be on by default. > > Bug link? Will fix.
Dirk Pranke
Comment 7 2013-02-14 12:29:44 PST
Created attachment 188403 [details] s/unix/linux, add comments
Dirk Pranke
Comment 8 2013-02-14 12:30:11 PST
apparently I got the syntax in DEPS confused w/ the syntax in gyp :(.
Dirk Pranke
Comment 9 2013-02-14 12:51:25 PST
Created attachment 188408 [details] update reference to freetype2.gyp
Tony Chang
Comment 10 2013-02-14 13:36:11 PST
Comment on attachment 188408 [details] update reference to freetype2.gyp It's a little silly to add use_custom_freetype when it's only used in one place, but I guess it makes it more clear until we flip the flag.
Dirk Pranke
Comment 11 2013-02-14 14:06:57 PST
(In reply to comment #10) > (From update of attachment 188408 [details]) > It's a little silly to add use_custom_freetype when it's only used in one place, but I guess it makes it more clear until we flip the flag. I anticipate using this setting in more places as I work on getting us to use freetype everywhere, otherwise I probably wouldn't have bothered.
Dirk Pranke
Comment 12 2013-02-21 15:30:20 PST
Dirk Pranke
Comment 13 2013-02-21 15:31:37 PST
Tony, can you take a look at this? I think this is ready to go. (Note that it won't actually flip the switch to using the lucid FT by default, we'll still be using the system lib, so this should basically be a no-op).
Tony Chang
Comment 14 2013-02-21 15:54:53 PST
Comment on attachment 189618 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=189618&action=review r=me with the third_party/freetype2 lines removed. > Source/WebKit/chromium/DEPS:189 > + 'third_party/freetype2': > + Var('chromium_svn')+'/third_party/freetype2@'+Var('chromium_rev'), I don't think you need this. We pull in all of third_party in the last rule before deps_os =.
Dirk Pranke
Comment 15 2013-02-21 17:24:34 PST
(In reply to comment #14) > (From update of attachment 189618 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=189618&action=review > > r=me with the third_party/freetype2 lines removed. > > > Source/WebKit/chromium/DEPS:189 > > + 'third_party/freetype2': > > + Var('chromium_svn')+'/third_party/freetype2@'+Var('chromium_rev'), > > I don't think you need this. We pull in all of third_party in the last rule before deps_os =. Ah, I missed that. Thanks!
Dirk Pranke
Comment 16 2013-02-21 17:28:02 PST
Note You need to log in before you can comment on or make changes to this bug.