36737 2010-03-28 19:34:21 -0700 Eliminate use of GL_BGRA in GraphicsContext3DSkia.cpp 2010-03-29 11:28:35 -0700 1 1 1 Unclassified WebKit WebGL 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 kbr kbr dglazkov fishd webkit.review.bot zmo oldest_to_newest 205281 0 kbr 2010-03-28 19:34:21 -0700 GraphicsContext3DSkia.cpp currently assumes it is running on desktop GL and uses the GL_BGRA format, which is not part of OpenGL ES 2.0. This code should swizzle the red and blue channels manually and use the GL_RGBA format. 205282 1 51874 kbr 2010-03-28 19:39:04 -0700 Created attachment 51874 Patch 205306 2 webkit.review.bot 2010-03-29 00:02:12 -0700 Attachment 51874 did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebCore/platform/graphics/skia/GraphicsContext3DSkia.cpp:31: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style. 205455 3 51874 fishd 2010-03-29 08:36:52 -0700 Comment on attachment 51874 Patch > Index: WebCore/platform/graphics/skia/GraphicsContext3DSkia.cpp > + // FIXME: add GL_BGRA extension support > + for (int i = 0; i < size; i += 4) > + std::swap(outputVector[i], outputVector[i + 2]); > + *format = RGBA; > return true; > } ^^^ it would be nice to add a comment above the loop explaining that you are converting from BGRA to RGBA. r=me with such a comment 205525 4 51928 kbr 2010-03-29 10:41:00 -0700 Created attachment 51928 Revised patch Revised patch addressing review feedback and style issue. 205565 5 51928 kbr 2010-03-29 11:28:30 -0700 Comment on attachment 51928 Revised patch Clearing flags on attachment: 51928 Committed r56735: <http://trac.webkit.org/changeset/56735> 205566 6 kbr 2010-03-29 11:28:35 -0700 All reviewed patches have been landed. Closing bug. 51874 2010-03-28 19:39:04 -0700 2010-03-29 10:41:00 -0700 Patch bgra.patch text/plain 1913 kbr SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA1NjcwMykKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTUgQEAKKzIwMTAtMDMtMjggIEtlbm5ldGggUnVzc2VsbCAgPGtickBnb29nbGUu Y29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIEVs aW1pbmF0ZSB1c2Ugb2YgR0xfQkdSQSBpbiBHcmFwaGljc0NvbnRleHQzRFNraWEuY3BwCisgICAg ICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0zNjczNworCisgICAg ICAgIE5vIG5ldyB0ZXN0czsgcmFuIFdlYkdMIGRlbW9zIGluIENocm9taXVtIG9uIFdpbmRvd3Mg dG8gdmVyaWZ5IGZpeC4KKworICAgICAgICAqIHBsYXRmb3JtL2dyYXBoaWNzL3NraWEvR3JhcGhp Y3NDb250ZXh0M0RTa2lhLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OkdyYXBoaWNzQ29udGV4dDNE OjpnZXRJbWFnZURhdGEpOgorCiAyMDEwLTAzLTI4ICBMYXN6bG8gR29tYm9zICA8bGFzemxvLjEu Z29tYm9zQG5va2lhLmNvbT4KIAogICAgICAgICBSZXZpZXdlZCBieSBTaW1vbiBIYXVzbWFubi4K SW5kZXg6IFdlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3Mvc2tpYS9HcmFwaGljc0NvbnRleHQzRFNr aWEuY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3Mvc2tpYS9HcmFw aGljc0NvbnRleHQzRFNraWEuY3BwCShyZXZpc2lvbiA1NjcwMykKKysrIFdlYkNvcmUvcGxhdGZv cm0vZ3JhcGhpY3Mvc2tpYS9HcmFwaGljc0NvbnRleHQzRFNraWEuY3BwCSh3b3JraW5nIGNvcHkp CkBAIC0yOCw2ICsyOCw4IEBACiAKICNpZiBFTkFCTEUoM0RfQ0FOVkFTKQogCisjaW5jbHVkZSA8 YWxnb3JpdGhtPgorCiAjaW5jbHVkZSAiR3JhcGhpY3NDb250ZXh0M0QuaCIKIAogI2luY2x1ZGUg IkltYWdlLmgiCkBAIC01OSwxMyArNjEsMTYgQEAgYm9vbCBHcmFwaGljc0NvbnRleHQzRDo6Z2V0 SW1hZ2VEYXRhKEltYQogICAgIEFTU0VSVChyb3dCeXRlcyA9PSB3aWR0aCAqIDQpOwogICAgIHVp bnQ4X3QqIHBpeGVscyA9IHJlaW50ZXJwcmV0X2Nhc3Q8dWludDhfdCo+KHNraWFJbWFnZS0+Z2V0 UGl4ZWxzKCkpOwogICAgIG91dHB1dFZlY3Rvci5yZXNpemUocm93Qnl0ZXMgKiBoZWlnaHQpOwot ICAgIG1lbWNweShvdXRwdXRWZWN0b3IuZGF0YSgpLCBwaXhlbHMsIHJvd0J5dGVzICogaGVpZ2h0 KTsKKyAgICBpbnQgc2l6ZSA9IHJvd0J5dGVzICogaGVpZ2h0OworICAgIG1lbWNweShvdXRwdXRW ZWN0b3IuZGF0YSgpLCBwaXhlbHMsIHNpemUpOwogICAgICpoYXNBbHBoYUNoYW5uZWwgPSB0cnVl OwogICAgIGlmICghcHJlbXVsdGlwbHlBbHBoYSkKICAgICAgICAgLy8gRklYTUU6IG11c3QgZmV0 Y2ggdGhlIGltYWdlIGRhdGEgYmVmb3JlIHRoZSBwcmVtdWx0aXBsaWNhdGlvbiBzdGVwCiAgICAg ICAgICpuZWVkZWRBbHBoYU9wID0ga0FscGhhRG9Vbm11bHRpcGx5OwotICAgIC8vIEZJWE1FOiBy ZW1vdmUgdGhpcyBkZXBlbmRlbmN5IG9uIGRlc2t0b3AgT3BlbkdMCi0gICAgKmZvcm1hdCA9IDB4 ODBFMTsgLy8gR0xfQkdSQQorICAgIC8vIEZJWE1FOiBhZGQgR0xfQkdSQSBleHRlbnNpb24gc3Vw cG9ydAorICAgIGZvciAoaW50IGkgPSAwOyBpIDwgc2l6ZTsgaSArPSA0KQorICAgICAgICBzdGQ6 OnN3YXAob3V0cHV0VmVjdG9yW2ldLCBvdXRwdXRWZWN0b3JbaSArIDJdKTsKKyAgICAqZm9ybWF0 ID0gUkdCQTsKICAgICByZXR1cm4gdHJ1ZTsKIH0KIAo= 51928 2010-03-29 10:41:00 -0700 2010-03-29 11:28:30 -0700 Revised patch bgra.patch text/plain 2033 kbr SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA1NjcyOSkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTUgQEAKKzIwMTAtMDMtMjkgIEtlbm5ldGggUnVzc2VsbCAgPGtickBnb29nbGUu Y29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIEVs aW1pbmF0ZSB1c2Ugb2YgR0xfQkdSQSBpbiBHcmFwaGljc0NvbnRleHQzRFNraWEuY3BwCisgICAg ICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0zNjczNworCisgICAg ICAgIE5vIG5ldyB0ZXN0czsgcmFuIFdlYkdMIGRlbW9zIGluIENocm9taXVtIG9uIFdpbmRvd3Mg dG8gdmVyaWZ5IGZpeC4KKworICAgICAgICAqIHBsYXRmb3JtL2dyYXBoaWNzL3NraWEvR3JhcGhp Y3NDb250ZXh0M0RTa2lhLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OkdyYXBoaWNzQ29udGV4dDNE OjpnZXRJbWFnZURhdGEpOgorCiAyMDEwLTAzLTI5ICBBbGV4YW5kZXIgUGF2bG92ICA8YXBhdmxv dkBjaHJvbWl1bS5vcmc+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgUGF2ZWwgRmVsZG1hbi4KSW5k ZXg6IFdlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3Mvc2tpYS9HcmFwaGljc0NvbnRleHQzRFNraWEu Y3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3Mvc2tpYS9HcmFwaGlj c0NvbnRleHQzRFNraWEuY3BwCShyZXZpc2lvbiA1NjcyOSkKKysrIFdlYkNvcmUvcGxhdGZvcm0v Z3JhcGhpY3Mvc2tpYS9HcmFwaGljc0NvbnRleHQzRFNraWEuY3BwCSh3b3JraW5nIGNvcHkpCkBA IC0zMyw2ICszMyw4IEBACiAjaW5jbHVkZSAiSW1hZ2UuaCIKICNpbmNsdWRlICJOYXRpdmVJbWFn ZVNraWEuaCIKIAorI2luY2x1ZGUgPGFsZ29yaXRobT4KKwogbmFtZXNwYWNlIFdlYkNvcmUgewog CiBib29sIEdyYXBoaWNzQ29udGV4dDNEOjpnZXRJbWFnZURhdGEoSW1hZ2UqIGltYWdlLApAQCAt NTksMTMgKzYxLDE3IEBAIGJvb2wgR3JhcGhpY3NDb250ZXh0M0Q6OmdldEltYWdlRGF0YShJbWEK ICAgICBBU1NFUlQocm93Qnl0ZXMgPT0gd2lkdGggKiA0KTsKICAgICB1aW50OF90KiBwaXhlbHMg PSByZWludGVycHJldF9jYXN0PHVpbnQ4X3QqPihza2lhSW1hZ2UtPmdldFBpeGVscygpKTsKICAg ICBvdXRwdXRWZWN0b3IucmVzaXplKHJvd0J5dGVzICogaGVpZ2h0KTsKLSAgICBtZW1jcHkob3V0 cHV0VmVjdG9yLmRhdGEoKSwgcGl4ZWxzLCByb3dCeXRlcyAqIGhlaWdodCk7CisgICAgaW50IHNp emUgPSByb3dCeXRlcyAqIGhlaWdodDsKKyAgICBtZW1jcHkob3V0cHV0VmVjdG9yLmRhdGEoKSwg cGl4ZWxzLCBzaXplKTsKICAgICAqaGFzQWxwaGFDaGFubmVsID0gdHJ1ZTsKICAgICBpZiAoIXBy ZW11bHRpcGx5QWxwaGEpCiAgICAgICAgIC8vIEZJWE1FOiBtdXN0IGZldGNoIHRoZSBpbWFnZSBk YXRhIGJlZm9yZSB0aGUgcHJlbXVsdGlwbGljYXRpb24gc3RlcAogICAgICAgICAqbmVlZGVkQWxw aGFPcCA9IGtBbHBoYURvVW5tdWx0aXBseTsKLSAgICAvLyBGSVhNRTogcmVtb3ZlIHRoaXMgZGVw ZW5kZW5jeSBvbiBkZXNrdG9wIE9wZW5HTAotICAgICpmb3JtYXQgPSAweDgwRTE7IC8vIEdMX0JH UkEKKyAgICAvLyBDb252ZXJ0IGZyb20gQkdSQSB0byBSR0JBLiBGSVhNRTogYWRkIEdMX0JHUkEg ZXh0ZW5zaW9uIHN1cHBvcnQKKyAgICAvLyB0byBhbGwgdW5kZXJseWluZyBPcGVuR0wgaW1wbGVt ZW50YXRpb25zLgorICAgIGZvciAoaW50IGkgPSAwOyBpIDwgc2l6ZTsgaSArPSA0KQorICAgICAg ICBzdGQ6OnN3YXAob3V0cHV0VmVjdG9yW2ldLCBvdXRwdXRWZWN0b3JbaSArIDJdKTsKKyAgICAq Zm9ybWF0ID0gUkdCQTsKICAgICByZXR1cm4gdHJ1ZTsKIH0KIAo=