78529 2012-02-13 13:53:20 -0800 [chromium] fast/canvas/canvas-text-alignment.html fails in Linux debug GPU 2012-03-13 17:30:42 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 tony webkit-unassigned abarth backer enne fischman jamesr oldest_to_newest 555549 0 tony 2012-02-13 13:53:20 -0800 From 77478: >> Thanks for tracking this down. Debug and Release builds now have slightly different behaviours. The difference in the scrollbars is a most likely an off by one Skia rendering bug. I think that jamesr@ moved to using mock scrollbars for the compositing layout tests to avoid this sort of problem. >> >> @jamesr: Is it possible to use mock scrollbars for these tests as well? > We could enable mock scrollbars for everything in the GPU config, but that means that we can't share results at all for the GPU and CPU > configurations which I think is pretty unfortunate. I'm going to disable the test for now. 555552 1 tony 2012-02-13 13:55:42 -0800 Disabled in http://trac.webkit.org/changeset/107602 556480 2 tony 2012-02-14 12:17:15 -0800 media/video-controls-rendering.html and media/video-zoom.html have the same problem (4 pixels different in scrollbars). I marked them as failing in http://trac.webkit.org/changeset/107723 . 556522 3 fischman 2012-02-14 13:00:06 -0800 James: how do I go about enabling mock scrollbars? For the media tests, at least, I'd rather have unshared GPU/CPU results than ignored-failure tests. Tony: who were you quoting as saying "most likely an off by one Skia rendering bug"? Do you know whether there's a bug filed for that and/or whether anyone's working on it? 556532 4 jamesr 2012-02-14 13:05:04 -0800 (In reply to comment #3) > James: how do I go about enabling mock scrollbars? For the media tests, at least, I'd rather have unshared GPU/CPU results than ignored-failure tests. There are two approaches: 1.) In the test itself, call window.internals.setMockScrollbarsEnabled(true) 2.) Teach the DumpRenderTree binary that tests matching a certain pattern should default to mock scrollbars. http://trac.webkit.org/browser/trunk/Tools/DumpRenderTree/chromium/TestShell.cpp#L240 is where I'm doing this for tests with the string 'compositing'. This would change both the GPU and CPU configurations, but only for Chromium. I'm somewhat partial to (2) as I think we should gradually convert all layout tests over to using mock scrollbars. Doing that would mean you'd have to rebaseline all media tests at least once. You could also make this part of the GPU configuration, which is controlled from python land. Doing this would involve a few more steps. I can help walk you through it if you want to go this route. 556553 5 tony 2012-02-14 13:25:05 -0800 (In reply to comment #3) > Tony: who were you quoting as saying "most likely an off by one Skia rendering bug"? Do you know whether there's a bug filed for that and/or whether anyone's working on it? The quoted text was from bug 77478. That specific quote is by backer. I don't know if there's a bug filed for it other than this. I don't know if anyone is working on it. 556582 6 fischman 2012-02-14 14:08:22 -0800 (In reply to comment #4) > 1.) In the test itself, call window.internals.setMockScrollbarsEnabled(true) If I do this there is no change in the output image, and the text output complains that this method doesn't exist. > 2.) Teach the DumpRenderTree binary that tests matching a certain pattern should default to mock scrollbars. http://trac.webkit.org/browser/trunk/Tools/DumpRenderTree/chromium/TestShell.cpp#L240 is where I'm doing this for tests with the string 'compositing'. This would change both the GPU and CPU configurations, but only for Chromium. Added there: if (testUrl.find("media/") != string::npos || testUrl.find("media\\") != string::npos) { printf("AMI: mock: testUrl==%s\n", testUrl.c_str()); // AMI: NFS m_prefs.mockScrollbarsEnabled = true; } and I see the printf show up in the output, but the scrollbars are still not mocks. > I'm somewhat partial to (2) as I think we should gradually convert all layout tests over to using mock scrollbars. Doing that would mean you'd have to rebaseline all media tests at least once. Is there an SOP for how to do this without accidentally making tests that are legit failures now (b/c of bugs in chromium, say) suddenly turn green? 556598 7 jamesr 2012-02-14 14:26:08 -0800 (In reply to comment #6) > (In reply to comment #4) > > 1.) In the test itself, call window.internals.setMockScrollbarsEnabled(true) > > If I do this there is no change in the output image, and the text output complains that this method doesn't exist. I fail at copy/paste. Should be: window.internals.settings.setMockScrollbarsEnabled(true) see LayoutTests/compositing/resources/mock_scrollbars.js for instance > > > 2.) Teach the DumpRenderTree binary that tests matching a certain pattern should default to mock scrollbars. http://trac.webkit.org/browser/trunk/Tools/DumpRenderTree/chromium/TestShell.cpp#L240 is where I'm doing this for tests with the string 'compositing'. This would change both the GPU and CPU configurations, but only for Chromium. > > Added there: > if (testUrl.find("media/") != string::npos > || testUrl.find("media\\") != string::npos) { > printf("AMI: mock: testUrl==%s\n", testUrl.c_str()); // AMI: NFS > m_prefs.mockScrollbarsEnabled = true; > } > and I see the printf show up in the output, but the scrollbars are still not mocks. You have to call m_prefs.applyTo(m_webView) as well > > > I'm somewhat partial to (2) as I think we should gradually convert all layout tests over to using mock scrollbars. Doing that would mean you'd have to rebaseline all media tests at least once. > > Is there an SOP for how to do this without accidentally making tests that are legit failures now (b/c of bugs in chromium, say) suddenly turn green? Just don't rebaseline tests that are legit failing without your patch. This requires compiling a list of tests that fail before the switchover. I've found it almost necessary to triage the set of exisiting failures to make sure that all the failing pixel tests are really failing before attempting a switchover like this. 556615 8 fischman 2012-02-14 14:40:22 -0800 Ok, the fixed JS works. I'm going with option#1 because the word name "media" is overloaded in LayoutTests to also include the CSS concept (paper vs. monitor, etc) and I don't want to get into the business of whitelisting specific directories for this treatment, at least not right now. Opened bug 78634 to track converting the tests to mock scrollbars & rebaseline. 578034 9 enne 2012-03-13 17:29:36 -0700 Committed r110648: <http://trac.webkit.org/changeset/110648> 578038 10 enne 2012-03-13 17:30:42 -0700 I just rebaselined the original test, after a fix that should make release and debug scrollbars the same. From the comments, it sounds like the additional media/ tests that got tacked onto this bug got handled elsewhere, so this bug is good to close.