49330 2010-11-10 11:12:24 -0800 [chromium] Canvas getImageData/putImageData cycle should not change values for valid premultiplied colours. 2010-11-10 13:14:27 -0800 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Other OS X 10.5 RESOLVED FIXED P2 Normal --- 1 senorblanco senorblanco abarth eric fishd jamesr webkit.review.bot oldest_to_newest 307446 0 senorblanco 2010-11-10 11:12:24 -0800 [chromium] Canvas getImageData/putImageData cycle should not change values for valid premultiplied colours. 307452 1 73516 senorblanco 2010-11-10 11:17:23 -0800 Created attachment 73516 Patch 307460 2 73516 jamesr 2010-11-10 11:44:25 -0800 Comment on attachment 73516 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=73516&action=review R=me, please consider comment. > WebCore/platform/graphics/skia/ImageBufferSkia.cpp:241 > +// This function does the equivalent of (a * b + 254) / 255, without an integer divide. Is there any reason to avoid an integer divide? I think it'd be a lot easier to read that way and the compiler can figure out the best code to generate for a given platform. 307465 3 senorblanco 2010-11-10 11:52:08 -0800 (In reply to comment #2) > (From update of attachment 73516 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=73516&action=review > > R=me, please consider comment. > > > WebCore/platform/graphics/skia/ImageBufferSkia.cpp:241 > > +// This function does the equivalent of (a * b + 254) / 255, without an integer divide. > > Is there any reason to avoid an integer divide? I think it'd be a lot easier to read that way and the compiler can figure out the best code to generate for a given platform. Sadly, divides are still slow on modern CPUs. And since this is integer, one can't prerecompute the reciprocal and do a multiply, as you can in FP. This trick is used a lot in skia (see SkMulDiv255Round). The compiler can't do this optimization, since it's only valid for a restricted set of inputs, which the compiler doesn't know. 307475 4 senorblanco 2010-11-10 12:16:16 -0800 Committed r71760: <http://trac.webkit.org/changeset/71760> 307511 5 webkit.review.bot 2010-11-10 13:14:27 -0800 http://trac.webkit.org/changeset/71760 might have broken GTK Linux 32-bit Release 73516 2010-11-10 11:17:23 -0800 2010-11-10 11:44:25 -0800 Patch bug-49330-20101110141721.patch text/plain 4324 senorblanco SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA3MTc1NikKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMjEgQEAKKzIwMTAtMTEtMTAgIFN0ZXBoZW4gV2hpdGUgIDxzZW5vcmJsYW5jb0Bj aHJvbWl1bS5vcmc+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAg ICAgICAgRml4IGNhbnZhcy5wdXRJbWFnZURhdGEoY2FudmFzLmdldEltYWdlRGF0YSguLi4pKSB0 byBiZSBsb3NzbGVzcy4KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcu Y2dpP2lkPTQ5MzMwCisKKyAgICAgICAgVGhlc2UgY3ljbGVzIHNob3VsZCBiZSBsb3NzbGVzcyBm b3IgdmFsaWQgY29sb3Vycy4gIEEgc2ltaWxhciBmaXggd2VudAorICAgICAgICBpbnRvIHRoZSBD RyBwb3J0IGF0IHIzMjg3OC4KKworICAgICAgICBDb3ZlcmVkIGJ5IGNhbnZhcy9waGlsaXAvdGVz dHMvMmQuaW1hZ2VEYXRhLnB1dC51bmNoYW5nZWQuaHRtbC4KKworICAgICAgICAqIHBsYXRmb3Jt L2dyYXBoaWNzL3NraWEvSW1hZ2VCdWZmZXJTa2lhLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6Om11 bERpdjI1NUNlaWwpOgorICAgICAgICBBIGhlbHBlciBmdW5jdGlvbiB0byBkbyAoYSArIGIgKyAy NTQpIC8gMjU1IHdpdGhvdXQgYSBkaXZpZGUuCisgICAgICAgIChXZWJDb3JlOjpwdXRJbWFnZURh dGEpOgorICAgICAgICBVc2UgdGhlIGFib3ZlIGhlbHBlciB0byByb3VuZCB1cCB3aGVuIHByZW11 bHRpcGx5aW5nIGFscGhhLgorCiAyMDEwLTExLTEwICBDc2FiYSBPc3p0cm9nb27DoWMgIDxvc3N5 QHdlYmtpdC5vcmc+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgRGF2aWQgSHlhdHQuCkluZGV4OiBX ZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL3NraWEvSW1hZ2VCdWZmZXJTa2lhLmNwcAo9PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09Ci0tLSBXZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL3NraWEvSW1hZ2VCdWZmZXJTa2lhLmNw cAkocmV2aXNpb24gNzE3NTUpCisrKyBXZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL3NraWEvSW1h Z2VCdWZmZXJTa2lhLmNwcAkod29ya2luZyBjb3B5KQpAQCAtMjM4LDYgKzIzOCwxNCBAQCBQYXNz UmVmUHRyPEltYWdlRGF0YT4gSW1hZ2VCdWZmZXI6OmdldFByCiAgICAgcmV0dXJuIGdldEltYWdl RGF0YTxQcmVtdWx0aXBsaWVkPihyZWN0LCAqY29udGV4dCgpLT5wbGF0Zm9ybUNvbnRleHQoKS0+ Yml0bWFwKCksIG1fc2l6ZSk7CiB9CiAKKy8vIFRoaXMgZnVuY3Rpb24gZG9lcyB0aGUgZXF1aXZh bGVudCBvZiAoYSAqIGIgKyAyNTQpIC8gMjU1LCB3aXRob3V0IGFuIGludGVnZXIgZGl2aWRlLgor Ly8gVmFsaWQgZm9yIGEsIGIgaW4gdGhlIHJhbmdlIFswLi4yNTVdLgordW5zaWduZWQgbXVsRGl2 MjU1Q2VpbCh1bnNpZ25lZCBhLCB1bnNpZ25lZCBiKQoreworICAgIHVuc2lnbmVkIHZhbHVlID0g YSAqIGIgKyAyNTU7CisgICAgcmV0dXJuICh2YWx1ZSArICh2YWx1ZSA+PiA4KSkgPj4gODsKK30K KwogdGVtcGxhdGUgPE11bHRpcGx5IG11bHRpcGxpZWQ+CiB2b2lkIHB1dEltYWdlRGF0YShJbWFn ZURhdGEqJiBzb3VyY2UsIGNvbnN0IEludFJlY3QmIHNvdXJjZVJlY3QsIGNvbnN0IEludFBvaW50 JiBkZXN0UG9pbnQsIAogICAgICAgICAgICAgICAgICAgY29uc3QgU2tCaXRtYXAmIGJpdG1hcCwg Y29uc3QgSW50U2l6ZSYgc2l6ZSkKQEAgLTI3OSwxMCArMjg3LDEzIEBAIHZvaWQgcHV0SW1hZ2VE YXRhKEltYWdlRGF0YSomIHNvdXJjZSwgY28KICAgICAgICAgdWludDMyX3QqIGRlc3RSb3cgPSBi aXRtYXAuZ2V0QWRkcjMyKGRlc3RYLCBkZXN0WSArIHkpOwogICAgICAgICBmb3IgKGludCB4ID0g MDsgeCA8IG51bUNvbHVtbnM7ICsreCkgewogICAgICAgICAgICAgY29uc3QgdW5zaWduZWQgY2hh ciogc3JjUGl4ZWwgPSAmc3JjUm93W3ggKiA0XTsKLSAgICAgICAgICAgIGlmIChtdWx0aXBsaWVk ID09IFVubXVsdGlwbGllZCkKLSAgICAgICAgICAgICAgICBkZXN0Um93W3hdID0gU2tQcmVNdWx0 aXBseUFSR0Ioc3JjUGl4ZWxbM10sIHNyY1BpeGVsWzBdLAotICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICBzcmNQaXhlbFsxXSwgc3JjUGl4ZWxbMl0pOwotICAg ICAgICAgICAgZWxzZQorICAgICAgICAgICAgdW5zaWduZWQgY2hhciBhbHBoYSA9IHNyY1BpeGVs WzNdOworICAgICAgICAgICAgaWYgKG11bHRpcGxpZWQgPT0gVW5tdWx0aXBsaWVkKSB7CisgICAg ICAgICAgICAgICAgdW5zaWduZWQgY2hhciByID0gbXVsRGl2MjU1Q2VpbChzcmNQaXhlbFswXSwg YWxwaGEpOworICAgICAgICAgICAgICAgIHVuc2lnbmVkIGNoYXIgZyA9IG11bERpdjI1NUNlaWwo c3JjUGl4ZWxbMV0sIGFscGhhKTsKKyAgICAgICAgICAgICAgICB1bnNpZ25lZCBjaGFyIGIgPSBt dWxEaXYyNTVDZWlsKHNyY1BpeGVsWzJdLCBhbHBoYSk7CisgICAgICAgICAgICAgICAgZGVzdFJv d1t4XSA9IFNrUGFja0FSR0IzMihhbHBoYSwgciwgZywgYik7CisgICAgICAgICAgICB9IGVsc2UK ICAgICAgICAgICAgICAgICBkZXN0Um93W3hdID0gU2tQYWNrQVJHQjMyKHNyY1BpeGVsWzNdLCBz cmNQaXhlbFswXSwKICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIHNy Y1BpeGVsWzFdLCBzcmNQaXhlbFsyXSk7CiAgICAgICAgIH0KSW5kZXg6IExheW91dFRlc3RzL0No YW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09Ci0tLSBMYXlvdXRUZXN0cy9DaGFuZ2VMb2cJKHJldmlzaW9uIDcx NzU2KQorKysgTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEs MTIgQEAKKzIwMTAtMTEtMTAgIFN0ZXBoZW4gV2hpdGUgIDxzZW5vcmJsYW5jb0BjaHJvbWl1bS5v cmc+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgTWFy ayAyZC5pbWFnZURhdGEucHV0LnVuY2hhbmdlZC5odG1sIGFzIHBhc3Npbmcgb24gV2luL0xpbnV4 LgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NDkzMzAK KworICAgICAgICAqIHBsYXRmb3JtL2Nocm9taXVtL3Rlc3RfZXhwZWN0YXRpb25zLnR4dDoKKwog MjAxMC0xMS0xMCAgTWFydGluIFJvYmluc29uICA8bXJvYmluc29uQGlnYWxpYS5jb20+CiAKICAg ICAgICAgUmViYXNlbGluZSBhIG5ldyB0ZXN0LiBUaGUgcGF0Y2ggd2FzIHBvc3RlZCBiZWZvcmUg Zm9udApJbmRleDogTGF5b3V0VGVzdHMvcGxhdGZvcm0vY2hyb21pdW0vdGVzdF9leHBlY3RhdGlv bnMudHh0Cj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT0KLS0tIExheW91dFRlc3RzL3BsYXRmb3JtL2Nocm9taXVtL3Rlc3Rf ZXhwZWN0YXRpb25zLnR4dAkocmV2aXNpb24gNzE3NTUpCisrKyBMYXlvdXRUZXN0cy9wbGF0Zm9y bS9jaHJvbWl1bS90ZXN0X2V4cGVjdGF0aW9ucy50eHQJKHdvcmtpbmcgY29weSkKQEAgLTI0NDYs NyArMjQ0Niw2IEBAIEJVR1dLNDU5OTEgTElOVVggV0lOIDogY2FudmFzL3BoaWxpcC90ZXMKIEJV R1dLNDU5OTEgTElOVVggV0lOIDogY2FudmFzL3BoaWxpcC90ZXN0cy8yZC5ncmFkaWVudC5yYWRp YWwub3V0c2lkZTMuaHRtbCA9IFRFWFQKIEJVR1dLNDU5OTEgTElOVVggV0lOIDogY2FudmFzL3Bo aWxpcC90ZXN0cy8yZC5ncmFkaWVudC5yYWRpYWwudG91Y2gxLmh0bWwgPSBURVhUCiBCVUdXSzQ1 OTkxIExJTlVYIFdJTiA6IGNhbnZhcy9waGlsaXAvdGVzdHMvMmQuZ3JhZGllbnQucmFkaWFsLnRv dWNoMy5odG1sID0gVEVYVAotQlVHV0s0NTk5MSBMSU5VWCBXSU4gOiBjYW52YXMvcGhpbGlwL3Rl c3RzLzJkLmltYWdlRGF0YS5wdXQudW5jaGFuZ2VkLmh0bWwgPSBURVhUCiBCVUdXSzQ1OTkxIExJ TlVYIFdJTiA6IGNhbnZhcy9waGlsaXAvdGVzdHMvMmQubGluZS53aWR0aC5iYXNpYy5odG1sID0g VEVYVAogQlVHV0s0NTk5MSBMSU5VWCBXSU4gOiBjYW52YXMvcGhpbGlwL3Rlc3RzLzJkLmxpbmUu d2lkdGgudHJhbnNmb3JtZWQuaHRtbCA9IFRFWFQKIEJVR1dLNDU5OTEgTElOVVggV0lOIDogY2Fu dmFzL3BoaWxpcC90ZXN0cy8yZC5wYXRoLmFyYy5hbmdsZS4zLmh0bWwgPSBURVhUCg==