Bug 55411 – [chromium] premultipliedAlpha WebGL context attribute is ignored.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebGL

[John Bauman]

The premultipliedAlpha context attribute is ignored, and all webgl contexts are essentially treated as having premultiplied alpha. This affects Chromium's layer compositor, drawImage, and toDataURL. I have a patch that should fix this.

[John Bauman]

Created attachment 84121 [details]
Patch

[John Bauman]

Created attachment 84126 [details]
Patch

[Kenneth Russell]

I've added this information to the associated Chromium bug, but please take a look at Gregg's new premultiply-alpha test in the WebGL conformance suite. We should enhance it if necessary to cover the cases you're working on fixing, and pull a copy into the WebKit layout tests. We can comment out sections that aren't working yet and incrementally fix them with multiple bugs if needed.

[John Bauman]

Created attachment 84143 [details]
Patch

[John Bauman]

That change removes the drawImage part of the LayoutTest, as it could easily be tested in a Khronos test. All of the layer-compositing changes remain.

[Kenneth Russell]

Comment on attachment 84143 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=84143&action=review

The code changes look good. Some more work is needed on the test case.

> LayoutTests/ChangeLog:10
> +        * compositing/webgl/webgl-nonpremultiplied-blend.html: Added.

You'll need to incorporate expected results for this new test. Chromium's results for the existing compositing/webgl/ tests seem to be under LayoutTests in platform/chromium-gpu-{win,mac,linux}/compositing/webgl/ . Despite the fact that the existing tests in compositing/webgl/ have text on the web page, that makes the pixel results non-portable, so I would recommend putting the text in an HTML comment instead. You may then be able to share the pixel results across platforms and put them under platform/chromium-gpu/ .

Because you aren't fixing the Mac code path in this patch, you'll need to add your new test to the Skipped file in LayoutTests/platform/mac/ . I don't think the other ports are running WebGL layout tests by default right now. You'll also need to add it to that in LayoutTests/platform/mac-wk2/ to keep the bots green at http://build.webkit.org/console .

See http://www.chromium.org/developers/testing/webkit-layout-tests for instructions on building DumpRenderTree out of your Chromium tree and running layout tests. You will likely need to pass the arguments "--debug --platform chromium-gpu" to run_webkit_tests.sh.

> LayoutTests/compositing/webgl/webgl-nonpremultiplied-blend.html:20
> +      .compare {
> +        margin: 20px;
> +        width: 200px;
> +        height: 200px;
> +        background-color: rgba(0, 0, 128, 0.5);
> +      }

It looks like the "compare" class is left over and should be taken out.

> LayoutTests/compositing/webgl/webgl-nonpremultiplied-blend.html:62
> +    <div class="compare"></div>

This should be taken out.

[Kenneth Russell]

Also, when adding your test to the Skipped list for the mac platform, you should file a new bug to track proper implementation of the premultipliedAlpha flag on that code path.

[John Bauman]

Created attachment 84292 [details]
Patch

[John Bauman]

Note: This needs the chromium patch in order for the layout test to work properly.

[James Robinson]

What does this need on the chromium side?  Can the chromium patch land first?

Preferably we wouldn't land a WebKit patch until it would pass.

[Kenneth Russell]

Comment on attachment 84292 [details]
Patch

Please file a bug about the fact that premultipliedAlpha=false isn't implemented on Mac WebKit yet, and reference it in LayoutTests/platform/mac/Skipped. Looks perfect otherwise.

[John Bauman]

It needs a patch to make sure that DumpRenderTree's GL implementation doesn't disable the premultipliedAlpha flag. The patch to do that has been reviewed, but not committed yet.

[John Bauman]

Created attachment 84514 [details]
Patch

[John Bauman]

Ok, the fix for DumpRenderTree is in chromium, so this should be ready.

[Kenneth Russell]

Comment on attachment 84514 [details]
Patch

Looks good. r=me

[WebKit Commit Bot]

Comment on attachment 84514 [details]
Patch

Clearing flags on attachment: 84514

Committed r80404: <http://trac.webkit.org/changeset/80404>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Mihai Parparita]

(In reply to comment #11)
> (From update of attachment 84292 [details])
> Please file a bug about the fact that premultipliedAlpha=false isn't implemented on Mac WebKit yet, and reference it in LayoutTests/platform/mac/Skipped. Looks perfect otherwise.

Does this apply to chromium-mac too? The test is failing there:

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showExpectations=true&tests=compositing%2Fwebgl%2Fwebgl-nonpremultiplied-blend.html&group=%40ToT%20GPU%20Mesa%20-%20chromium.org

(chromium-mac does not use the mac Skipped file)

Also, there's a failure on on the Windows debug bot only that you may want to look into.

[John Bauman]

I'm not sure why the render-tree is different; some pixel-precision issues, I guess. Also, I'm curious why a win debug CPU machine is running this test. 

As for the pixel results do you know whether they're using accelerated compositing on those macs? I'm pretty sure that non-accelerated compositing is still broken for the moment.

[Mihai Parparita]

(In reply to comment #19)
> I'm not sure why the render-tree is different; some pixel-precision issues, I guess. Also, I'm curious why a win debug CPU machine is running this test. 

That's a Windows GPU bot (running with Mesa).
 
> As for the pixel results do you know whether they're using accelerated compositing on those macs? I'm pretty sure that non-accelerated compositing is still broken for the moment.

It should be accelerated compositing (also with Mesa).

[Kenneth Russell]

(In reply to comment #18)
> (In reply to comment #11)
> > (From update of attachment 84292 [details] [details])
> > Please file a bug about the fact that premultipliedAlpha=false isn't implemented on Mac WebKit yet, and reference it in LayoutTests/platform/mac/Skipped. Looks perfect otherwise.
> 
> Does this apply to chromium-mac too? The test is failing there:
> 
> http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showExpectations=true&tests=compositing%2Fwebgl%2Fwebgl-nonpremultiplied-blend.html&group=%40ToT%20GPU%20Mesa%20-%20chromium.org
> 
> (chromium-mac does not use the mac Skipped file)

The exclusion doesn't apply to Chromium's Mac port; it's Core Animation-specific and so only applies to Safari.

The various failures need to be investigated. John, could you please file a new bug tracking them?