Chrome on Android is failing a few WebGL conformance tests (canvas/canvas-test.html, context/context-attributes-alpha-depth-stencil-antialias.html, context/premultiplyalpha-test.html, textures/gl-pixelstorei.html) because of incorrect swizzling of color channels. The basic problem is that the contract of WebGraphicsContext3D::readBackFrameBuffer is not well enough defined, causing problems when drawing a WebGL-rendered canvas into another, and when constructing an ImageData from a WebGL-rendered canvas. One half of the fix can be done in WebKit, but the other half must be done in Chromium.
The associated Chromium bug is https://code.google.com/p/chromium/issues/detail?id=176029 and that patch will be landed separately. While the bug won't be completely fixed until both land, the intermediate state is no worse than the current one.
Created attachment 191888 [details] Patch
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Comment on attachment 191888 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=191888&action=review > Source/WebCore/platform/chromium/support/GraphicsContext3DChromium.cpp:533 > +#if (SK_R32_SHIFT == 16) && !SK_B32_SHIFT this is the same check as https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/WebKit/chromium/src/WebViewImpl.cpp&q=WebViewImpl.cpp&sq=package:chromium&type=cs&l=1885, although it's written the other way 'round. Any chance of getting something clearer?
(In reply to comment #4) > (From update of attachment 191888 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=191888&action=review > > > Source/WebCore/platform/chromium/support/GraphicsContext3DChromium.cpp:533 > > +#if (SK_R32_SHIFT == 16) && !SK_B32_SHIFT > > this is the same check as https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/WebKit/chromium/src/WebViewImpl.cpp&q=WebViewImpl.cpp&sq=package:chromium&type=cs&l=1885, although it's written the other way 'round. Any chance of getting something clearer? Argh, sorry, I would have used the same #if test at least. I could define a macro like SKIA_USING_RGBA_BYTE_ORDERING in one of the headers in Source/Platform/chromium/public/ which would do this check. It would (currently) be used in WebViewImpl.cpp, webgraphicscontext3d_command_buffer_impl.cc, and GraphicsContext3DChromium.cpp. Where would you say would be a good place to put that macro and what would you suggest it be called? Note that the header file would have to include SkTypes.h.
Comment on attachment 191888 [details] Patch Clearing flags on attachment: 191888 Committed r145027: <http://trac.webkit.org/changeset/145027>
All reviewed patches have been landed. Closing bug.
(In reply to comment #5) > (In reply to comment #4) > > (From update of attachment 191888 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=191888&action=review > > > > > Source/WebCore/platform/chromium/support/GraphicsContext3DChromium.cpp:533 > > > +#if (SK_R32_SHIFT == 16) && !SK_B32_SHIFT > > > > this is the same check as https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/WebKit/chromium/src/WebViewImpl.cpp&q=WebViewImpl.cpp&sq=package:chromium&type=cs&l=1885, although it's written the other way 'round. Any chance of getting something clearer? > > Argh, sorry, I would have used the same #if test at least. It's no biggie. > > I could define a macro like SKIA_USING_RGBA_BYTE_ORDERING in one of the headers in Source/Platform/chromium/public/ which would do this check. It would (currently) be used in WebViewImpl.cpp, webgraphicscontext3d_command_buffer_impl.cc, and GraphicsContext3DChromium.cpp. Where would you say would be a good place to put that macro and what would you suggest it be called? Note that the header file would have to include SkTypes.h. Could we get a nicer-named macro in skia, perhaps?
(In reply to comment #8) > Could we get a nicer-named macro in skia, perhaps? I'll ask the Skia team about this.