33624 2010-01-13 13:56:12 -0800 Open-source image decoders have decode bugs when scaling 2010-01-14 17:30:29 -0800 1 1 1 Unclassified WebKit Images 528+ (Nightly build) PC All RESOLVED FIXED P2 Normal --- 1 pkasting pkasting yong.li.webkit oldest_to_newest 180391 0 pkasting 2010-01-13 13:56:12 -0800 When image scaling is on, the PNGImageDecoder wrongly assumes that all images are 4 bytes per pixel, when some images may have no alpha channel. This leads to reading/displaying garbage. Simple patch to fix coming soon. 180399 1 46504 pkasting 2010-01-13 14:05:37 -0800 Created attachment 46504 patch v1 180405 2 pkasting 2010-01-13 14:22:14 -0800 Yong, I think there's a similar (though inverted) bug in JPEG decoding for CMYK JPEGs: for (int x = 0; x < numColumns; ++x) { JSAMPLE* jsample = src + scaledColumns[x] * 3; unsigned c = jsample[0]; unsigned m = jsample[1]; unsigned y = jsample[2]; unsigned k = jsample[3]; ... Note how we access four bytes, but only step by three. I'm going to update my patch to fix this too. 180407 3 46508 pkasting 2010-01-13 14:24:48 -0800 Created attachment 46508 patch v2 180412 4 46508 levin 2010-01-13 14:40:59 -0800 Comment on attachment 46508 patch v2 The change looks great, but there is no layout test. Please either add one or explain why that isn't possible (in the changelog). 180413 5 yong.li.webkit 2010-01-13 14:42:06 -0800 (In reply to comment #2) > Yong, I think there's a similar (though inverted) bug in JPEG decoding for CMYK > JPEGs: > for (int x = 0; x < numColumns; ++x) { > JSAMPLE* jsample = src + scaledColumns[x] * 3; > unsigned c = jsample[0]; > unsigned m = jsample[1]; > unsigned y = jsample[2]; > unsigned k = jsample[3]; > ... > Note how we access four bytes, but only step by three. > I'm going to update my patch to fix this too. ? I don't think so. 180415 6 yong.li.webkit 2010-01-13 14:43:49 -0800 (In reply to comment #5) > (In reply to comment #2) > > Yong, I think there's a similar (though inverted) bug in JPEG decoding for CMYK > > JPEGs: > > for (int x = 0; x < numColumns; ++x) { > > JSAMPLE* jsample = src + scaledColumns[x] * 3; > > unsigned c = jsample[0]; > > unsigned m = jsample[1]; > > unsigned y = jsample[2]; > > unsigned k = jsample[3]; > > ... > > Note how we access four bytes, but only step by three. > > I'm going to update my patch to fix this too. > ? I don't think so. oops. never mind. 180416 7 46512 pkasting 2010-01-13 14:48:11 -0800 Created attachment 46512 patch v2.1 Added a sentence about why there is no layout test. If Yong wants to create a pair of tests for this and add them to the patch here, he's welcome to do so. I don't know whether the platforms that support scaling are running the pixel tests, though. 180903 8 yong.li.webkit 2010-01-14 13:49:04 -0800 (In reply to comment #7) > Created an attachment (id=46512) [details] > patch v2.1 > > Added a sentence about why there is no layout test. > > If Yong wants to create a pair of tests for this and add them to the patch > here, he's welcome to do so. I don't know whether the platforms that support > scaling are running the pixel tests, though. I also have no idea whether there's any buildbot that turns this image decoder down-sampling on. 180921 9 yong.li.webkit 2010-01-14 14:16:47 -0800 Peter, I'll do a test with your patch. 180958 10 yong.li.webkit 2010-01-14 15:17:19 -0800 I just tested the patch agains a cmyk jpeg and a 24bit png. I've seen problems and the patch fixes them. (I modified the threshold to force scaling) 181029 11 46512 abarth 2010-01-14 17:25:45 -0800 Comment on attachment 46512 patch v2.1 I wish this had tests, but I understand why it doesn't. 181034 12 pkasting 2010-01-14 17:30:29 -0800 Fixed in r53305. 46504 2010-01-13 14:05:37 -0800 2010-01-13 14:24:48 -0800 patch v1 patch text/plain 1481 pkasting SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA1MzIwMykKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTMgQEAKKzIwMTAtMDEtMTMgIFBldGVyIEthc3RpbmcgIDxwa2FzdGluZ0Bnb29n bGUuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg IERlY29kaW5nIG5vLWFscGhhIFBOR3MgdG8gc2NhbGVkIG91dHB1dCBidWZmZXJzIHJlc3VsdGVk IGluIGdhcmJhZ2UuCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNn aT9pZD0zMzYyNAorCisgICAgICAgICogcGxhdGZvcm0vaW1hZ2UtZGVjb2RlcnMvcG5nL1BOR0lt YWdlRGVjb2Rlci5jcHA6CisgICAgICAgIChXZWJDb3JlOjpQTkdJbWFnZURlY29kZXI6OnJvd0F2 YWlsYWJsZSk6CisKIDIwMTAtMDEtMTMgIEJyaWFuIFdlaW5zdGVpbiAgPGJ3ZWluc3RlaW5AYXBw bGUuY29tPgogCiAgICAgICAgIFJldmlld2VkIGJ5IEFkYW0gUm9iZW4uCkluZGV4OiBXZWJDb3Jl L3BsYXRmb3JtL2ltYWdlLWRlY29kZXJzL3BuZy9QTkdJbWFnZURlY29kZXIuY3BwCj09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT0KLS0tIFdlYkNvcmUvcGxhdGZvcm0vaW1hZ2UtZGVjb2RlcnMvcG5nL1BOR0ltYWdlRGVjb2Rl ci5jcHAJKHJldmlzaW9uIDUzMjAzKQorKysgV2ViQ29yZS9wbGF0Zm9ybS9pbWFnZS1kZWNvZGVy cy9wbmcvUE5HSW1hZ2VEZWNvZGVyLmNwcAkod29ya2luZyBjb3B5KQpAQCAtNDA0LDggKzQwNCw4 IEBAIHZvaWQgUE5HSW1hZ2VEZWNvZGVyOjpyb3dBdmFpbGFibGUodW5zaWcKICAgICAgICAgaW50 IGNvbHVtbnMgPSBtX3NjYWxlZENvbHVtbnMuc2l6ZSgpOwogICAgICAgICBib29sIHNhd0FscGhh ID0gYnVmZmVyLmhhc0FscGhhKCk7CiAgICAgICAgIGZvciAoaW50IHggPSAwOyB4IDwgY29sdW1u czsgKyt4KSB7Ci0gICAgICAgICAgICBwbmdfYnl0ZXAgcGl4ZWwgPSByb3cgKyBtX3NjYWxlZENv bHVtbnNbeF0gKiA0OwotICAgICAgICAgICAgdW5zaWduZWQgYWxwaGEgPSBwaXhlbFszXTsKKyAg ICAgICAgICAgIHBuZ19ieXRlcCBwaXhlbCA9IHJvdyArIG1fc2NhbGVkQ29sdW1uc1t4XSAqIGNv bG9yQ2hhbm5lbHM7CisgICAgICAgICAgICB1bnNpZ25lZCBhbHBoYSA9IChoYXNBbHBoYSA/IHBp eGVsWzNdIDogMjU1KTsKICAgICAgICAgICAgIGJ1ZmZlci5zZXRSR0JBKHgsIGRlc3RZLCBwaXhl bFswXSwgcGl4ZWxbMV0sIHBpeGVsWzJdLCBhbHBoYSk7CiAgICAgICAgICAgICBpZiAoIXNhd0Fs cGhhICYmIGFscGhhIDwgMjU1KSB7CiAgICAgICAgICAgICAgICAgc2F3QWxwaGEgPSB0cnVlOwo= 46508 2010-01-13 14:24:48 -0800 2010-01-13 14:48:11 -0800 patch v2 patch text/plain 2320 pkasting SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA1MzIwNSkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTYgQEAKKzIwMTAtMDEtMTMgIFBldGVyIEthc3RpbmcgIDxwa2FzdGluZ0Bnb29n bGUuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg IERlY29kaW5nIGltYWdlcyB0byBzY2FsZWQgb3V0cHV0IGJ1ZmZlcnMgcmVzdWx0ZWQgaW4gZ2Fy YmFnZSBmb3IKKyAgICAgICAgbm8tYWxwaGEgUE5HcyBhbmQgQ01ZSyBKUEVHcy4KKyAgICAgICAg aHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTMzNjI0CisKKyAgICAgICAg KiBwbGF0Zm9ybS9pbWFnZS1kZWNvZGVycy9qcGVnL0pQRUdJbWFnZURlY29kZXIuY3BwOgorICAg ICAgICAoV2ViQ29yZTo6Y29udmVydENNWUtUb1JHQkEpOgorICAgICAgICAqIHBsYXRmb3JtL2lt YWdlLWRlY29kZXJzL3BuZy9QTkdJbWFnZURlY29kZXIuY3BwOgorICAgICAgICAoV2ViQ29yZTo6 UE5HSW1hZ2VEZWNvZGVyOjpyb3dBdmFpbGFibGUpOgorCiAyMDEwLTAxLTEzICBQYXZlbCBGZWxk bWFuICA8cGZlbGRtYW5AY2hyb21pdW0ub3JnPgogCiAgICAgICAgIFJldmlld2VkIGJ5IFRpbW90 aHkgSGF0Y2hlci4KSW5kZXg6IFdlYkNvcmUvcGxhdGZvcm0vaW1hZ2UtZGVjb2RlcnMvanBlZy9K UEVHSW1hZ2VEZWNvZGVyLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBXZWJDb3JlL3BsYXRmb3JtL2ltYWdl LWRlY29kZXJzL2pwZWcvSlBFR0ltYWdlRGVjb2Rlci5jcHAJKHJldmlzaW9uIDUzMjAzKQorKysg V2ViQ29yZS9wbGF0Zm9ybS9pbWFnZS1kZWNvZGVycy9qcGVnL0pQRUdJbWFnZURlY29kZXIuY3Bw CSh3b3JraW5nIGNvcHkpCkBAIC00NzAsNyArNDcwLDcgQEAgc3RhdGljIHZvaWQgY29udmVydENN WUtUb1JHQkEoUkdCQTMyQnVmZgogICAgIGlmIChzY2FsZWQpIHsKICAgICAgICAgaW50IG51bUNv bHVtbnMgPSBzY2FsZWRDb2x1bW5zLnNpemUoKTsKICAgICAgICAgZm9yIChpbnQgeCA9IDA7IHgg PCBudW1Db2x1bW5zOyArK3gpIHsKLSAgICAgICAgICAgIEpTQU1QTEUqIGpzYW1wbGUgPSBzcmMg KyBzY2FsZWRDb2x1bW5zW3hdICogMzsKKyAgICAgICAgICAgIEpTQU1QTEUqIGpzYW1wbGUgPSBz cmMgKyBzY2FsZWRDb2x1bW5zW3hdICogNDsKICAgICAgICAgICAgIHVuc2lnbmVkIGMgPSBqc2Ft cGxlWzBdOwogICAgICAgICAgICAgdW5zaWduZWQgbSA9IGpzYW1wbGVbMV07CiAgICAgICAgICAg ICB1bnNpZ25lZCB5ID0ganNhbXBsZVsyXTsKSW5kZXg6IFdlYkNvcmUvcGxhdGZvcm0vaW1hZ2Ut ZGVjb2RlcnMvcG5nL1BOR0ltYWdlRGVjb2Rlci5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gV2ViQ29yZS9w bGF0Zm9ybS9pbWFnZS1kZWNvZGVycy9wbmcvUE5HSW1hZ2VEZWNvZGVyLmNwcAkocmV2aXNpb24g NTMyMDMpCisrKyBXZWJDb3JlL3BsYXRmb3JtL2ltYWdlLWRlY29kZXJzL3BuZy9QTkdJbWFnZURl Y29kZXIuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC00MDQsOCArNDA0LDggQEAgdm9pZCBQTkdJbWFn ZURlY29kZXI6OnJvd0F2YWlsYWJsZSh1bnNpZwogICAgICAgICBpbnQgY29sdW1ucyA9IG1fc2Nh bGVkQ29sdW1ucy5zaXplKCk7CiAgICAgICAgIGJvb2wgc2F3QWxwaGEgPSBidWZmZXIuaGFzQWxw aGEoKTsKICAgICAgICAgZm9yIChpbnQgeCA9IDA7IHggPCBjb2x1bW5zOyArK3gpIHsKLSAgICAg ICAgICAgIHBuZ19ieXRlcCBwaXhlbCA9IHJvdyArIG1fc2NhbGVkQ29sdW1uc1t4XSAqIDQ7Ci0g ICAgICAgICAgICB1bnNpZ25lZCBhbHBoYSA9IHBpeGVsWzNdOworICAgICAgICAgICAgcG5nX2J5 dGVwIHBpeGVsID0gcm93ICsgbV9zY2FsZWRDb2x1bW5zW3hdICogY29sb3JDaGFubmVsczsKKyAg ICAgICAgICAgIHVuc2lnbmVkIGFscGhhID0gKGhhc0FscGhhID8gcGl4ZWxbM10gOiAyNTUpOwog ICAgICAgICAgICAgYnVmZmVyLnNldFJHQkEoeCwgZGVzdFksIHBpeGVsWzBdLCBwaXhlbFsxXSwg cGl4ZWxbMl0sIGFscGhhKTsKICAgICAgICAgICAgIGlmICghc2F3QWxwaGEgJiYgYWxwaGEgPCAy NTUpIHsKICAgICAgICAgICAgICAgICBzYXdBbHBoYSA9IHRydWU7Cg== 46512 2010-01-13 14:48:11 -0800 2010-01-14 17:25:45 -0800 patch v2.1 patch text/plain 2435 pkasting SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA1MzIwNSkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTkgQEAKKzIwMTAtMDEtMTMgIFBldGVyIEthc3RpbmcgIDxwa2FzdGluZ0Bnb29n bGUuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg IERlY29kaW5nIGltYWdlcyB0byBzY2FsZWQgb3V0cHV0IGJ1ZmZlcnMgcmVzdWx0ZWQgaW4gZ2Fy YmFnZSBmb3IKKyAgICAgICAgbm8tYWxwaGEgUE5HcyBhbmQgQ01ZSyBKUEVHcy4KKyAgICAgICAg aHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTMzNjI0CisgICAgICAgIAor ICAgICAgICBObyBsYXlvdXQgdGVzdCBzaW5jZSBJIGRvbid0IGhhdmUgYWNjZXNzIHRvIGEgcGxh dGZvcm0gdGhhdCBzY2FsZXMgdGhlCisgICAgICAgIG91dHB1dCBidWZmZXJzLgorCisgICAgICAg ICogcGxhdGZvcm0vaW1hZ2UtZGVjb2RlcnMvanBlZy9KUEVHSW1hZ2VEZWNvZGVyLmNwcDoKKyAg ICAgICAgKFdlYkNvcmU6OmNvbnZlcnRDTVlLVG9SR0JBKToKKyAgICAgICAgKiBwbGF0Zm9ybS9p bWFnZS1kZWNvZGVycy9wbmcvUE5HSW1hZ2VEZWNvZGVyLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6 OlBOR0ltYWdlRGVjb2Rlcjo6cm93QXZhaWxhYmxlKToKKwogMjAxMC0wMS0xMyAgUGF2ZWwgRmVs ZG1hbiAgPHBmZWxkbWFuQGNocm9taXVtLm9yZz4KIAogICAgICAgICBSZXZpZXdlZCBieSBUaW1v dGh5IEhhdGNoZXIuCkluZGV4OiBXZWJDb3JlL3BsYXRmb3JtL2ltYWdlLWRlY29kZXJzL2pwZWcv SlBFR0ltYWdlRGVjb2Rlci5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gV2ViQ29yZS9wbGF0Zm9ybS9pbWFn ZS1kZWNvZGVycy9qcGVnL0pQRUdJbWFnZURlY29kZXIuY3BwCShyZXZpc2lvbiA1MzIwMykKKysr IFdlYkNvcmUvcGxhdGZvcm0vaW1hZ2UtZGVjb2RlcnMvanBlZy9KUEVHSW1hZ2VEZWNvZGVyLmNw cAkod29ya2luZyBjb3B5KQpAQCAtNDcwLDcgKzQ3MCw3IEBAIHN0YXRpYyB2b2lkIGNvbnZlcnRD TVlLVG9SR0JBKFJHQkEzMkJ1ZmYKICAgICBpZiAoc2NhbGVkKSB7CiAgICAgICAgIGludCBudW1D b2x1bW5zID0gc2NhbGVkQ29sdW1ucy5zaXplKCk7CiAgICAgICAgIGZvciAoaW50IHggPSAwOyB4 IDwgbnVtQ29sdW1uczsgKyt4KSB7Ci0gICAgICAgICAgICBKU0FNUExFKiBqc2FtcGxlID0gc3Jj ICsgc2NhbGVkQ29sdW1uc1t4XSAqIDM7CisgICAgICAgICAgICBKU0FNUExFKiBqc2FtcGxlID0g c3JjICsgc2NhbGVkQ29sdW1uc1t4XSAqIDQ7CiAgICAgICAgICAgICB1bnNpZ25lZCBjID0ganNh bXBsZVswXTsKICAgICAgICAgICAgIHVuc2lnbmVkIG0gPSBqc2FtcGxlWzFdOwogICAgICAgICAg ICAgdW5zaWduZWQgeSA9IGpzYW1wbGVbMl07CkluZGV4OiBXZWJDb3JlL3BsYXRmb3JtL2ltYWdl LWRlY29kZXJzL3BuZy9QTkdJbWFnZURlY29kZXIuY3BwCj09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUv cGxhdGZvcm0vaW1hZ2UtZGVjb2RlcnMvcG5nL1BOR0ltYWdlRGVjb2Rlci5jcHAJKHJldmlzaW9u IDUzMjAzKQorKysgV2ViQ29yZS9wbGF0Zm9ybS9pbWFnZS1kZWNvZGVycy9wbmcvUE5HSW1hZ2VE ZWNvZGVyLmNwcAkod29ya2luZyBjb3B5KQpAQCAtNDA0LDggKzQwNCw4IEBAIHZvaWQgUE5HSW1h Z2VEZWNvZGVyOjpyb3dBdmFpbGFibGUodW5zaWcKICAgICAgICAgaW50IGNvbHVtbnMgPSBtX3Nj YWxlZENvbHVtbnMuc2l6ZSgpOwogICAgICAgICBib29sIHNhd0FscGhhID0gYnVmZmVyLmhhc0Fs cGhhKCk7CiAgICAgICAgIGZvciAoaW50IHggPSAwOyB4IDwgY29sdW1uczsgKyt4KSB7Ci0gICAg ICAgICAgICBwbmdfYnl0ZXAgcGl4ZWwgPSByb3cgKyBtX3NjYWxlZENvbHVtbnNbeF0gKiA0Owot ICAgICAgICAgICAgdW5zaWduZWQgYWxwaGEgPSBwaXhlbFszXTsKKyAgICAgICAgICAgIHBuZ19i eXRlcCBwaXhlbCA9IHJvdyArIG1fc2NhbGVkQ29sdW1uc1t4XSAqIGNvbG9yQ2hhbm5lbHM7Cisg ICAgICAgICAgICB1bnNpZ25lZCBhbHBoYSA9IChoYXNBbHBoYSA/IHBpeGVsWzNdIDogMjU1KTsK ICAgICAgICAgICAgIGJ1ZmZlci5zZXRSR0JBKHgsIGRlc3RZLCBwaXhlbFswXSwgcGl4ZWxbMV0s IHBpeGVsWzJdLCBhbHBoYSk7CiAgICAgICAgICAgICBpZiAoIXNhd0FscGhhICYmIGFscGhhIDwg MjU1KSB7CiAgICAgICAgICAgICAgICAgc2F3QWxwaGEgPSB0cnVlOwo=