Bug 78529 – [chromium] fast/canvas/canvas-text-alignment.html fails in Linux debug GPU

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Tony Chang]

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.

[Tony Chang]

Disabled in http://trac.webkit.org/changeset/107602

[Tony Chang]

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 .

[Ami Fischman]

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?

[James Robinson]

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

[Tony Chang]

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

[Ami Fischman]

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

[James Robinson]

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

[Ami Fischman]

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.

[Adrienne Walker]

Committed r110648: <http://trac.webkit.org/changeset/110648>

[Adrienne Walker]

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.