111661 2013-03-06 18:49:42 -0800 [Chromium] Fix byte ordering bugs reading back WebGL canvases' content on Android 2013-03-07 14:51:04 -0800 1 1 1 Unclassified WebKit WebGL 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 kbr kbr abarth dglazkov fishd jamesr skyostil tkent+wkapi webkit.review.bot oldest_to_newest 849599 0 kbr 2013-03-06 18:49:42 -0800 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. 849602 1 kbr 2013-03-06 18:56:44 -0800 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. 849605 2 191888 kbr 2013-03-06 18:59:33 -0800 Created attachment 191888 Patch 849607 3 webkit.review.bot 2013-03-06 19:00:42 -0800 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. 849633 4 191888 jamesr 2013-03-06 19:26:15 -0800 Comment on attachment 191888 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? 849657 5 kbr 2013-03-06 19:40:25 -0800 (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. 849665 6 191888 webkit.review.bot 2013-03-06 19:48:35 -0800 Comment on attachment 191888 Patch Clearing flags on attachment: 191888 Committed r145027: <http://trac.webkit.org/changeset/145027> 849666 7 webkit.review.bot 2013-03-06 19:48:38 -0800 All reviewed patches have been landed. Closing bug. 849681 8 jamesr 2013-03-06 20:10:26 -0800 (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? 850503 9 kbr 2013-03-07 14:51:04 -0800 (In reply to comment #8) > Could we get a nicer-named macro in skia, perhaps? I'll ask the Skia team about this. 191888 2013-03-06 18:59:33 -0800 2013-03-06 19:48:35 -0800 Patch bug-111661-20130306185538.patch text/plain 3849 kbr U3VidmVyc2lvbiBSZXZpc2lvbjogMTQ1MDIxCmRpZmYgLS1naXQgYS9Tb3VyY2UvUGxhdGZvcm0v Q2hhbmdlTG9nIGIvU291cmNlL1BsYXRmb3JtL0NoYW5nZUxvZwppbmRleCA5NDUyNjZiYmJjOGQ4 ZTQ0MzczZTY5MmJkYjQ0YTI2ZWIxNTc1NmY2Li41MDMxZDJjYTU1NWE1YWQxZWNmOTYwNDE4NWEx MWFlMmIxZTNkNmI0IDEwMDY0NAotLS0gYS9Tb3VyY2UvUGxhdGZvcm0vQ2hhbmdlTG9nCisrKyBi L1NvdXJjZS9QbGF0Zm9ybS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNCBAQAorMjAxMy0wMy0wNiAg S2VubmV0aCBSdXNzZWxsICA8a2JyQGdvb2dsZS5jb20+CisKKyAgICAgICAgW0Nocm9taXVtXSBG aXggYnl0ZSBvcmRlcmluZyBidWdzIHJlYWRpbmcgYmFjayBXZWJHTCBjYW52YXNlcycgY29udGVu dCBvbiBBbmRyb2lkCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNn aT9pZD0xMTE2NjEKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAg ICAgICAqIGNocm9taXVtL3B1YmxpYy9XZWJHcmFwaGljc0NvbnRleHQzRC5oOgorICAgICAgICAo V2ViR3JhcGhpY3NDb250ZXh0M0QpOgorICAgICAgICAgICAgQmV0dGVyIGRvY3VtZW50ZWQgY29u dHJhY3Qgb2YgcmVhZEJhY2tGcmFtZUJ1ZmZlci4KKwogMjAxMy0wMy0wNiAgRGFuYSBKYW5zZW5z ICA8ZGFuYWtqQGNocm9taXVtLm9yZz4KIAogICAgICAgICBbY2hyb21pdW1dIFJlbW92ZSBXZWJT aGFyZWRHcmFwaGljc0NvbnRleHQzRCBjbGFzcwpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUv Q2hhbmdlTG9nIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCmluZGV4IGZjOGMyMTc3MzgwNDJl NWVmMzY5MWY3NjcxMDE4MTAyNGUwMzY2MjYuLjQxMDBjNGNiMDg5YmVmNDEwZGM4Y2Q4ZmFlYTJh ODg0Njc4MWM4YTYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZworKysgYi9T b3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxOCBAQAorMjAxMy0wMy0wNiAgS2Vu bmV0aCBSdXNzZWxsICA8a2JyQGdvb2dsZS5jb20+CisKKyAgICAgICAgW0Nocm9taXVtXSBGaXgg Ynl0ZSBvcmRlcmluZyBidWdzIHJlYWRpbmcgYmFjayBXZWJHTCBjYW52YXNlcycgY29udGVudCBv biBBbmRyb2lkCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9p ZD0xMTE2NjEKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAg ICBObyBuZXcgdGVzdHM7IGNvdmVyZWQgYnkgZXhpc3RpbmcgdGVzdHMuIFJhbiBXZWJHTCBjb25m b3JtYW5jZQorICAgICAgICB0ZXN0cyBvbiBBbmRyb2lkIGFuZCBkZXNrdG9wIExpbnV4IHRvIHZl cmlmeSBmaXguCisKKyAgICAgICAgKiBwbGF0Zm9ybS9jaHJvbWl1bS9zdXBwb3J0L0dyYXBoaWNz Q29udGV4dDNEQ2hyb21pdW0uY3BwOgorICAgICAgICAoV2ViQ29yZTo6R3JhcGhpY3NDb250ZXh0 M0Q6OnBhaW50UmVuZGVyaW5nUmVzdWx0c1RvSW1hZ2VEYXRhKToKKyAgICAgICAgICAgIEF2b2lk IGluY29ycmVjdCBieXRlIHN3YXAgb24gQW5kcm9pZCBhbmQgb3RoZXIgT1NzIHdoaWNoIGRvbid0 CisgICAgICAgICAgICBzd2l6emxlIFIgYW5kIEIgY2hhbm5lbHMuCisKIDIwMTMtMDMtMDYgIEFh cm9uIENvbHdlbGwgIDxhY29sd2VsbEBjaHJvbWl1bS5vcmc+CiAKICAgICAgICAgUmVtb3ZlIFdl YkNvcmU6Ok5vZGU6OmlzQWN0aXZlTm9kZSgpIHNpbmNlIGl0IGlzbid0IGNhbGxlZCBhbnl3aGVy ZS4KZGlmZiAtLWdpdCBhL1NvdXJjZS9QbGF0Zm9ybS9jaHJvbWl1bS9wdWJsaWMvV2ViR3JhcGhp Y3NDb250ZXh0M0QuaCBiL1NvdXJjZS9QbGF0Zm9ybS9jaHJvbWl1bS9wdWJsaWMvV2ViR3JhcGhp Y3NDb250ZXh0M0QuaAppbmRleCBjOWM5ZjM5OGI2YjU4MTg2ZmZkN2QxNjAzNWFkYWJlNmJlMzA2 YjRjLi5mNTE1YzI3NjJkMTc4OTczNjg2ODVlY2M1MWE1ZDk2NDIzZDBiMzE0IDEwMDY0NAotLS0g YS9Tb3VyY2UvUGxhdGZvcm0vY2hyb21pdW0vcHVibGljL1dlYkdyYXBoaWNzQ29udGV4dDNELmgK KysrIGIvU291cmNlL1BsYXRmb3JtL2Nocm9taXVtL3B1YmxpYy9XZWJHcmFwaGljc0NvbnRleHQz RC5oCkBAIC0xODUsNyArMTg1LDggQEAgcHVibGljOgogICAgIC8vIEhlbHBlciBmb3Igc29mdHdh cmUgY29tcG9zaXRpbmcgcGF0aC4gUmVhZHMgYmFjayB0aGUgZnJhbWUgYnVmZmVyIGludG8KICAg ICAvLyB0aGUgbWVtb3J5IHJlZ2lvbiBwb2ludGVkIHRvIGJ5ICJwaXhlbHMiIHdpdGggc2l6ZSAi YnVmZmVyU2l6ZSIuIEl0IGlzCiAgICAgLy8gZXhwZWN0ZWQgdGhhdCB0aGUgc3RvcmFnZSBmb3Ig InBpeGVscyIgY292ZXJzICg0ICogd2lkdGggKiBoZWlnaHQpIGJ5dGVzLgotICAgIC8vIFJldHVy bnMgdHJ1ZSBvbiBzdWNjZXNzLgorICAgIC8vIFRoZSBSR0JBIGNoYW5uZWxzIGFyZSBwYWNrZWQg aW50byAicGl4ZWxzIiB1c2luZyBTa0JpdG1hcCdzIGJ5dGUKKyAgICAvLyBvcmRlcmluZy4gUmV0 dXJucyB0cnVlIG9uIHN1Y2Nlc3MuCiAgICAgdmlydHVhbCBib29sIHJlYWRCYWNrRnJhbWVidWZm ZXIodW5zaWduZWQgY2hhciogcGl4ZWxzLCBzaXplX3QgYnVmZmVyU2l6ZSwgV2ViR0xJZCBmcmFt ZWJ1ZmZlciwgaW50IHdpZHRoLCBpbnQgaGVpZ2h0KSA9IDA7CiAKICAgICAvLyBSZXR1cm5zIHRo ZSBpZCBvZiB0aGUgdGV4dHVyZSB3aGljaCBpcyB1c2VkIGZvciBzdG9yaW5nIHRoZSBjb250ZW50 cyBvZgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vY2hyb21pdW0vc3VwcG9y dC9HcmFwaGljc0NvbnRleHQzRENocm9taXVtLmNwcCBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3Jt L2Nocm9taXVtL3N1cHBvcnQvR3JhcGhpY3NDb250ZXh0M0RDaHJvbWl1bS5jcHAKaW5kZXggOTc2 YTgzNDM1NzgyMTI1MDRiMDhiN2VmM2I3MzBiMWQxMWUxNzdjZi4uNmI4ZTFjOGM4MjE4YmYxODRk MzA4NzYwYjhhZTY0OWQ3YjkwMGM3ZiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvcGxhdGZv cm0vY2hyb21pdW0vc3VwcG9ydC9HcmFwaGljc0NvbnRleHQzRENocm9taXVtLmNwcAorKysgYi9T b3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9jaHJvbWl1bS9zdXBwb3J0L0dyYXBoaWNzQ29udGV4dDNE Q2hyb21pdW0uY3BwCkBAIC0zNiw2ICszNiw3IEBACiAjaW5jbHVkZSAiR3JhcGhpY3NDb250ZXh0 M0RQcml2YXRlLmgiCiAjaW5jbHVkZSAiSW1hZ2VCdWZmZXIuaCIKICNpbmNsdWRlICJJbWFnZURh dGEuaCIKKyNpbmNsdWRlICJTa1R5cGVzLmgiCiAjaW5jbHVkZSA8cHVibGljL1BsYXRmb3JtLmg+ CiAjaW5jbHVkZSA8cHVibGljL1dlYkdyYXBoaWNzQ29udGV4dDNELmg+CiAjaW5jbHVkZSA8d3Rm L3RleHQvQ1N0cmluZy5oPgpAQCAtNTI5LDggKzUzMCwxMSBAQCBQYXNzUmVmUHRyPEltYWdlRGF0 YT4gR3JhcGhpY3NDb250ZXh0M0Q6OnBhaW50UmVuZGVyaW5nUmVzdWx0c1RvSW1hZ2VEYXRhKERy YXdpbgogCiAgICAgbV9wcml2YXRlLT53ZWJDb250ZXh0KCktPnJlYWRCYWNrRnJhbWVidWZmZXIo cGl4ZWxzLCBidWZmZXJTaXplLCBmcmFtZWJ1ZmZlcklkLCB3aWR0aCwgaGVpZ2h0KTsKIAorI2lm IChTS19SMzJfU0hJRlQgPT0gMTYpICYmICFTS19CMzJfU0hJRlQKKyAgICAvLyBJZiB0aGUgaW1w bGVtZW50YXRpb24gc3dhcHBlZCB0aGUgcmVkIGFuZCBibHVlIGNoYW5uZWxzLCB1bi1zd2FwIHRo ZW0uCiAgICAgZm9yIChzaXplX3QgaSA9IDA7IGkgPCBidWZmZXJTaXplOyBpICs9IDQpCiAgICAg ICAgIHN0ZDo6c3dhcChwaXhlbHNbaV0sIHBpeGVsc1tpICsgMl0pOworI2VuZGlmCiAKICAgICBy ZXR1cm4gaW1hZ2VEYXRhLnJlbGVhc2UoKTsKIH0K