85268 2012-05-01 00:26:47 -0700 PNGImageDecoder: Add ENABLE(IMAGE_DECODER_DOWN_SAMPLING) guards to rowAvailable 2012-05-01 19:51:52 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 noel.gordon noel.gordon eric webkit.review.bot oldest_to_newest 613283 0 noel.gordon 2012-05-01 00:26:47 -0700 PNGImageDecoder: Add ENABLE(IMAGE_DECODER_DOWN_SAMPLING) guards to rowAvailable 613285 1 139598 noel.gordon 2012-05-01 00:28:55 -0700 Created attachment 139598 Patch 613411 2 139598 eric 2012-05-01 08:24:02 -0700 Comment on attachment 139598 Patch I would have reversed that if I think, to avoid the !, but this is fine. 613413 3 139598 eric 2012-05-01 08:24:58 -0700 Comment on attachment 139598 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=139598&action=review > Source/WebCore/platform/image-decoders/png/PNGImageDecoder.cpp:441 > png_bytep pixel = row + (m_scaled ? m_scaledColumns[x] : x) * colorChannels; Couldn't we just put the #if around this line? 613752 4 noel.gordon 2012-05-01 16:20:02 -0700 (In reply to comment #2) > (From update of attachment 139598 [details]) > I would have reversed that if I think, to avoid the !, but this is fine. Reversed is better, so will do. (In reply to comment #3) > > Source/WebCore/platform/image-decoders/png/PNGImageDecoder.cpp:441 > > png_bytep pixel = row + (m_scaled ? m_scaledColumns[x] : x) * colorChannels; > > Couldn't we just put the #if around this line? Yes, but created more #if's nearby when I fiddled with it :( 613756 5 139702 noel.gordon 2012-05-01 16:22:33 -0700 Created attachment 139702 Patch for landing 613760 6 139702 eric 2012-05-01 16:25:10 -0700 Comment on attachment 139702 Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=139702&action=review > Source/WebCore/platform/image-decoders/png/PNGImageDecoder.cpp:438 > + ASSERT(!m_scaled); Why don't we just branch the whole for here, instead of having this ENABLE? Why would someone want to disable this support? 613794 7 noel.gordon 2012-05-01 16:52:45 -0700 (In reply to comment #6) > (From update of attachment 139702 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=139702&action=review > > > Source/WebCore/platform/image-decoders/png/PNGImageDecoder.cpp:438 > > + ASSERT(!m_scaled); > > Why don't we just branch the whole for here, instead of having this ENABLE? Why would someone want to disable this support? Support is only enabled for Touch Mobile folks (WinCE port). Could branch yes. I'm really thinking of someone new to this code (tpayne) working here to add color correction support (this is the place to do it) and preempting their questions about image down scaling ... 613904 8 139702 webkit.review.bot 2012-05-01 19:51:47 -0700 Comment on attachment 139702 Patch for landing Clearing flags on attachment: 139702 Committed r115782: <http://trac.webkit.org/changeset/115782> 613905 9 webkit.review.bot 2012-05-01 19:51:52 -0700 All reviewed patches have been landed. Closing bug. 139598 2012-05-01 00:28:55 -0700 2012-05-01 16:22:18 -0700 Patch bug-85268-20120501172853.patch text/plain 3438 noel.gordon U3VidmVyc2lvbiBSZXZpc2lvbjogMTE1NzExCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMjUwZGIwM2UyNGE0YTQ1 ZGJhYjkxN2MxZmUyZGM3MzRkMDM0MGE5ZC4uN2ZkNzM3MjEyZjlkZjRlYTI5Y2RkMmE3NWM1NTYx M2I4YjY5YTY2ZCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIwIEBACisyMDEyLTA1LTAxICBOb2Vs IEdvcmRvbiAgPG5vZWwuZ29yZG9uQGdtYWlsLmNvbT4KKworICAgICAgICBQTkdJbWFnZURlY29k ZXI6IEFkZCBFTkFCTEUoSU1BR0VfREVDT0RFUl9ET1dOX1NBTVBMSU5HKSBndWFyZHMgdG8gcm93 QXZhaWxhYmxlCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9p ZD04NTI2OAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg IFBOR0ltYWdlRGVjb2RlciBzdXBwb3J0cyBpbWFnZSBkb3duc2FtcGxpbmcuIEFkZCBFTkFCTEUg Z3VhcmRzIHRvIHNob3cgd2hlcmUKKyAgICAgICAgZG93bnNhbXBsaW5nIGlzIGFwcGxpZWQgd2hl biBvdXRwdXR0aW5nIGRlY29kZWQgcm93cyB0byB0aGUgZnJhbWUgYnVmZmVyLiBNb3N0CisgICAg ICAgIHBvcnRzIGRvbid0IGVuYWJsZSB0aGUgZmxhZzogZG9uJ3QgcGVuYWxpemUgdGhlbSBpbiB0 ZXJtcyBzcGVlZCBpbiB0aGlzIHRpZ2h0CisgICAgICAgIHJvdyBwaXhlbCB3cml0ZSBsb29wLiBz L3kvZGVzdFkvIHRvIG1hdGNoIHNldFJHQkEoKSBhbmQgYW1lbmQgc29tZSBjb21tZW50cy4KKwor ICAgICAgICBObyBuZXcgdGVzdHMuIE5vIGNoYW5nZSBpbiBiZWhhdmlvci4KKworICAgICAgICAq IHBsYXRmb3JtL2ltYWdlLWRlY29kZXJzL3BuZy9QTkdJbWFnZURlY29kZXIuY3BwOgorICAgICAg ICAoV2ViQ29yZTo6UE5HSW1hZ2VEZWNvZGVyOjpyb3dBdmFpbGFibGUpOgorCiAyMDEyLTA0LTMw ICBLZW50IFRhbXVyYSAgPHRrZW50QGNocm9taXVtLm9yZz4KIAogICAgICAgICBSRUdSRVNTSU9O KHIxMTU2MDApOiBwYXJzZUxvY2FsaXplZERhdGUoKSBzaG91bGQgZmFpbCBmb3IgaW52YWxpZCBp bnB1dHMKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2ltYWdlLWRlY29kZXJz L3BuZy9QTkdJbWFnZURlY29kZXIuY3BwIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vaW1hZ2Ut ZGVjb2RlcnMvcG5nL1BOR0ltYWdlRGVjb2Rlci5jcHAKaW5kZXggYmNjYTQ3NDAxNDVkMzU2NzBj ZjFjYzJiZjQ0ZGEwZDdmODgxYjcxZS4uZmY0OWUwNDA0YTY0MmNjNmZjMDk1NDZkZDY1OGRiMGU5 MDc4MThhOCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vaW1hZ2UtZGVjb2Rl cnMvcG5nL1BOR0ltYWdlRGVjb2Rlci5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0v aW1hZ2UtZGVjb2RlcnMvcG5nL1BOR0ltYWdlRGVjb2Rlci5jcHAKQEAgLTQxMSw2ICs0MTEsNyBA QCB2b2lkIFBOR0ltYWdlRGVjb2Rlcjo6cm93QXZhaWxhYmxlKHVuc2lnbmVkIGNoYXIqIHJvd0J1 ZmZlciwgdW5zaWduZWQgcm93SW5kZXgsCiAgICAgYm9vbCBoYXNBbHBoYSA9IG1fcmVhZGVyLT5o YXNBbHBoYSgpOwogICAgIHVuc2lnbmVkIGNvbG9yQ2hhbm5lbHMgPSBoYXNBbHBoYSA/IDQgOiAz OwogICAgIHBuZ19ieXRlcCByb3c7CisKICAgICBwbmdfYnl0ZXAgaW50ZXJsYWNlQnVmZmVyID0g bV9yZWFkZXItPmludGVybGFjZUJ1ZmZlcigpOwogICAgIGlmIChpbnRlcmxhY2VCdWZmZXIpIHsK ICAgICAgICAgcm93ID0gaW50ZXJsYWNlQnVmZmVyICsgKHJvd0luZGV4ICogY29sb3JDaGFubmVs cyAqIHNpemUoKS53aWR0aCgpKTsKQEAgLTQxOCwyMCArNDE5LDMyIEBAIHZvaWQgUE5HSW1hZ2VE ZWNvZGVyOjpyb3dBdmFpbGFibGUodW5zaWduZWQgY2hhciogcm93QnVmZmVyLCB1bnNpZ25lZCBy b3dJbmRleCwKICAgICB9IGVsc2UKICAgICAgICAgcm93ID0gcm93QnVmZmVyOwogCi0gICAgLy8g Q29weSB0aGUgZGF0YSBpbnRvIG91ciBidWZmZXIuCi0gICAgaW50IHdpZHRoID0gc2NhbGVkU2l6 ZSgpLndpZHRoKCk7Ci0gICAgaW50IGRlc3RZID0gc2NhbGVkWShyb3dJbmRleCk7Ci0KLSAgICAv LyBDaGVjayB0aGF0IHRoZSByb3cgaXMgd2l0aGluIHRoZSBpbWFnZSBib3VuZHMuIExpYlBORyBt YXkgc3VwcGx5IGFuIGV4dHJhIHJvdy4KLSAgICBpZiAoZGVzdFkgPCAwIHx8IGRlc3RZID49IHNj YWxlZFNpemUoKS5oZWlnaHQoKSkKKyAgICAvLyBDaGVjayB0aGUgcm93IGlzIHdpdGhpbiB0aGUg aW1hZ2UgYm91bmRzOiBsaWJwbmcgbWF5IHN1cHBseSBhbiBleHRyYSByb3cuCisgICAgaW50IHkg PSAhbV9zY2FsZWQgPyByb3dJbmRleCA6IHNjYWxlZFkocm93SW5kZXgpOworICAgIGlmICh5IDwg MCB8fCB5ID49IHNjYWxlZFNpemUoKS5oZWlnaHQoKSkKICAgICAgICAgcmV0dXJuOworCisgICAg Ly8gV3JpdGUgdGhlIGRlY29kZWQgcm93IGludG8gdGhlIGZyYW1lIGJ1ZmZlci4KKyAgICBpbnQg d2lkdGggPSBzY2FsZWRTaXplKCkud2lkdGgoKTsKICAgICBib29sIG5vblRyaXZpYWxBbHBoYSA9 IGZhbHNlOworCisjaWYgIUVOQUJMRShJTUFHRV9ERUNPREVSX0RPV05fU0FNUExJTkcpCisgICAg QVNTRVJUKCFtX3NjYWxlZCk7CisgICAgcG5nX2J5dGVwIHBpeGVsID0gcm93OworICAgIGZvciAo aW50IHggPSAwOyB4IDwgd2lkdGg7ICsreCwgcGl4ZWwgKz0gY29sb3JDaGFubmVscykgeworICAg ICAgICB1bnNpZ25lZCBhbHBoYSA9IGhhc0FscGhhID8gcGl4ZWxbM10gOiAyNTU7CisgICAgICAg IGJ1ZmZlci5zZXRSR0JBKHgsIHksIHBpeGVsWzBdLCBwaXhlbFsxXSwgcGl4ZWxbMl0sIGFscGhh KTsKKyAgICAgICAgbm9uVHJpdmlhbEFscGhhIHw9IGFscGhhIDwgMjU1OworICAgIH0KKyNlbHNl CiAgICAgZm9yIChpbnQgeCA9IDA7IHggPCB3aWR0aDsgKyt4KSB7CiAgICAgICAgIHBuZ19ieXRl cCBwaXhlbCA9IHJvdyArIChtX3NjYWxlZCA/IG1fc2NhbGVkQ29sdW1uc1t4XSA6IHgpICogY29s b3JDaGFubmVsczsKICAgICAgICAgdW5zaWduZWQgYWxwaGEgPSBoYXNBbHBoYSA/IHBpeGVsWzNd IDogMjU1OwotICAgICAgICBidWZmZXIuc2V0UkdCQSh4LCBkZXN0WSwgcGl4ZWxbMF0sIHBpeGVs WzFdLCBwaXhlbFsyXSwgYWxwaGEpOworICAgICAgICBidWZmZXIuc2V0UkdCQSh4LCB5LCBwaXhl bFswXSwgcGl4ZWxbMV0sIHBpeGVsWzJdLCBhbHBoYSk7CiAgICAgICAgIG5vblRyaXZpYWxBbHBo YSB8PSBhbHBoYSA8IDI1NTsKICAgICB9CisjZW5kaWYKKwogICAgIGlmIChub25Ucml2aWFsQWxw aGEgJiYgIWJ1ZmZlci5oYXNBbHBoYSgpKQogICAgICAgICBidWZmZXIuc2V0SGFzQWxwaGEobm9u VHJpdmlhbEFscGhhKTsKIH0K 139702 2012-05-01 16:22:33 -0700 2012-05-01 19:51:47 -0700 Patch for landing bug-85268-20120502092229.patch text/plain 3056 noel.gordon U3VidmVyc2lvbiBSZXZpc2lvbjogMTE1NzU1CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggZWNmZWYyMjU0MWQ5MDQ4 YTRiYTIyZjZiYjdiMDU3YjUxMzI0YjMwNC4uYzNmOWUyYzkxOWQyY2M1MTU0YTE4ODhiYjYxNDAy MzljY2VhOWUwYSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIwIEBACisyMDEyLTA1LTAxICBOb2Vs IEdvcmRvbiAgPG5vZWwuZ29yZG9uQGdtYWlsLmNvbT4KKworICAgICAgICBQTkdJbWFnZURlY29k ZXI6IEFkZCBFTkFCTEUoSU1BR0VfREVDT0RFUl9ET1dOX1NBTVBMSU5HKSBndWFyZHMgdG8gcm93 QXZhaWxhYmxlCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9p ZD04NTI2OAorCisgICAgICAgIFJldmlld2VkIGJ5IEVyaWMgU2VpZGVsLgorCisgICAgICAgIFBO R0ltYWdlRGVjb2RlciBzdXBwb3J0cyBpbWFnZSBkb3duc2FtcGxpbmcuIEFkZCBFTkFCTEUgZ3Vh cmRzIHRvIHNob3cgd2hlcmUKKyAgICAgICAgZG93bnNhbXBsaW5nIGlzIGFwcGxpZWQgd2hlbiBv dXRwdXR0aW5nIGRlY29kZWQgcm93cyB0byB0aGUgZnJhbWUgYnVmZmVyLiBNb3N0CisgICAgICAg IHBvcnRzIGRvbid0IGVuYWJsZSB0aGUgZmxhZzogZG9uJ3QgcGVuYWxpemUgdGhlbSBpbiB0ZXJt cyBzcGVlZCBpbiB0aGlzIHRpZ2h0CisgICAgICAgIHJvdyBwaXhlbCB3cml0ZSBsb29wLiBzL3kv ZGVzdFkvIHRvIG1hdGNoIHNldFJHQkEoKSBhbmQgYW1lbmQgc29tZSBjb21tZW50cy4KKworICAg ICAgICBObyBuZXcgdGVzdHMuIE5vIGNoYW5nZSBpbiBiZWhhdmlvci4KKworICAgICAgICAqIHBs YXRmb3JtL2ltYWdlLWRlY29kZXJzL3BuZy9QTkdJbWFnZURlY29kZXIuY3BwOgorICAgICAgICAo V2ViQ29yZTo6UE5HSW1hZ2VEZWNvZGVyOjpyb3dBdmFpbGFibGUpOgorCiAyMDEyLTA1LTAxICBK dWxpZW4gQ2hhZmZyYWl4ICA8amNoYWZmcmFpeEB3ZWJraXQub3JnPgogCiAgICAgICAgIFJlbW92 ZSBvbmUgYml0IGZyb20gbV9jb2x1bW4gdG8gcGFjayBSZW5kZXJUYWJsZUNlbGwgYml0cyBtb3Jl CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9pbWFnZS1kZWNvZGVycy9wbmcv UE5HSW1hZ2VEZWNvZGVyLmNwcCBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2ltYWdlLWRlY29k ZXJzL3BuZy9QTkdJbWFnZURlY29kZXIuY3BwCmluZGV4IGJjY2E0NzQwMTQ1ZDM1NjcwY2YxY2My YmY0NGRhMGQ3Zjg4MWI3MWUuLjQ5MjE2ZDBhNWM1YTQzNmE2OGNlZGE5MTg3NTAxZjhhNTA4ZmM3 NWYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2ltYWdlLWRlY29kZXJzL3Bu Zy9QTkdJbWFnZURlY29kZXIuY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2ltYWdl LWRlY29kZXJzL3BuZy9QTkdJbWFnZURlY29kZXIuY3BwCkBAIC00MTgsMjAgKzQxOCwzMiBAQCB2 b2lkIFBOR0ltYWdlRGVjb2Rlcjo6cm93QXZhaWxhYmxlKHVuc2lnbmVkIGNoYXIqIHJvd0J1ZmZl ciwgdW5zaWduZWQgcm93SW5kZXgsCiAgICAgfSBlbHNlCiAgICAgICAgIHJvdyA9IHJvd0J1ZmZl cjsKIAotICAgIC8vIENvcHkgdGhlIGRhdGEgaW50byBvdXIgYnVmZmVyLgotICAgIGludCB3aWR0 aCA9IHNjYWxlZFNpemUoKS53aWR0aCgpOwotICAgIGludCBkZXN0WSA9IHNjYWxlZFkocm93SW5k ZXgpOwotCi0gICAgLy8gQ2hlY2sgdGhhdCB0aGUgcm93IGlzIHdpdGhpbiB0aGUgaW1hZ2UgYm91 bmRzLiBMaWJQTkcgbWF5IHN1cHBseSBhbiBleHRyYSByb3cuCi0gICAgaWYgKGRlc3RZIDwgMCB8 fCBkZXN0WSA+PSBzY2FsZWRTaXplKCkuaGVpZ2h0KCkpCisgICAgLy8gQ2hlY2sgdGhlIHJvdyBp cyB3aXRoaW4gdGhlIGltYWdlIGJvdW5kczogbGlicG5nIG1heSBzdXBwbHkgYW4gZXh0cmEgcm93 LgorICAgIGludCB5ID0gIW1fc2NhbGVkID8gcm93SW5kZXggOiBzY2FsZWRZKHJvd0luZGV4KTsK KyAgICBpZiAoeSA8IDAgfHwgeSA+PSBzY2FsZWRTaXplKCkuaGVpZ2h0KCkpCiAgICAgICAgIHJl dHVybjsKKworICAgIC8vIFdyaXRlIHRoZSBkZWNvZGVkIHJvdyBwaXhlbHMgdG8gdGhlIGZyYW1l IGJ1ZmZlci4KKyAgICBpbnQgd2lkdGggPSBzY2FsZWRTaXplKCkud2lkdGgoKTsKICAgICBib29s IG5vblRyaXZpYWxBbHBoYSA9IGZhbHNlOworCisjaWYgRU5BQkxFKElNQUdFX0RFQ09ERVJfRE9X Tl9TQU1QTElORykKICAgICBmb3IgKGludCB4ID0gMDsgeCA8IHdpZHRoOyArK3gpIHsKICAgICAg ICAgcG5nX2J5dGVwIHBpeGVsID0gcm93ICsgKG1fc2NhbGVkID8gbV9zY2FsZWRDb2x1bW5zW3hd IDogeCkgKiBjb2xvckNoYW5uZWxzOwogICAgICAgICB1bnNpZ25lZCBhbHBoYSA9IGhhc0FscGhh ID8gcGl4ZWxbM10gOiAyNTU7Ci0gICAgICAgIGJ1ZmZlci5zZXRSR0JBKHgsIGRlc3RZLCBwaXhl bFswXSwgcGl4ZWxbMV0sIHBpeGVsWzJdLCBhbHBoYSk7CisgICAgICAgIGJ1ZmZlci5zZXRSR0JB KHgsIHksIHBpeGVsWzBdLCBwaXhlbFsxXSwgcGl4ZWxbMl0sIGFscGhhKTsKICAgICAgICAgbm9u VHJpdmlhbEFscGhhIHw9IGFscGhhIDwgMjU1OwogICAgIH0KKyNlbHNlCisgICAgQVNTRVJUKCFt X3NjYWxlZCk7CisgICAgcG5nX2J5dGVwIHBpeGVsID0gcm93OworICAgIGZvciAoaW50IHggPSAw OyB4IDwgd2lkdGg7ICsreCwgcGl4ZWwgKz0gY29sb3JDaGFubmVscykgeworICAgICAgICB1bnNp Z25lZCBhbHBoYSA9IGhhc0FscGhhID8gcGl4ZWxbM10gOiAyNTU7CisgICAgICAgIGJ1ZmZlci5z ZXRSR0JBKHgsIHksIHBpeGVsWzBdLCBwaXhlbFsxXSwgcGl4ZWxbMl0sIGFscGhhKTsKKyAgICAg ICAgbm9uVHJpdmlhbEFscGhhIHw9IGFscGhhIDwgMjU1OworICAgIH0KKyNlbmRpZgorCiAgICAg aWYgKG5vblRyaXZpYWxBbHBoYSAmJiAhYnVmZmVyLmhhc0FscGhhKCkpCiAgICAgICAgIGJ1ZmZl ci5zZXRIYXNBbHBoYShub25Ucml2aWFsQWxwaGEpOwogfQo=