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.
Created attachment 46504 [details] patch v1
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.
Created attachment 46508 [details] patch v2
Comment on attachment 46508 [details] 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).
(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.
(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.
Created attachment 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.
(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.
Peter, I'll do a test with your patch.
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)
Comment on attachment 46512 [details] patch v2.1 I wish this had tests, but I understand why it doesn't.
Fixed in r53305.