Bug 33624 - Open-source image decoders have decode bugs when scaling
: Open-source image decoders have decode bugs when scaling
Status: RESOLVED FIXED
: WebKit
Images
: 528+ (Nightly build)
: PC All
: P2 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2010-01-13 13:56 PST by
Modified: 2010-01-14 17:30 PST (History)


Attachments
patch v1 (1.45 KB, patch)
2010-01-13 14:05 PST, Peter Kasting
no flags Review Patch | Details | Formatted Diff | Diff
patch v2 (2.27 KB, patch)
2010-01-13 14:24 PST, Peter Kasting
levin: review-
Review Patch | Details | Formatted Diff | Diff
patch v2.1 (2.38 KB, patch)
2010-01-13 14:48 PST, Peter Kasting
abarth: review+
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2010-01-13 13:56:12 PST
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.
------- Comment #1 From 2010-01-13 14:05:37 PST -------
Created an attachment (id=46504) [details]
patch v1
------- Comment #2 From 2010-01-13 14:22:14 PST -------
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.
------- Comment #3 From 2010-01-13 14:24:48 PST -------
Created an attachment (id=46508) [details]
patch v2
------- Comment #4 From 2010-01-13 14:40:59 PST -------
(From update of attachment 46508 [details])
The change looks great, but there is no layout test. Please either add one or explain why that isn't possible (in the changelog).
------- Comment #5 From 2010-01-13 14:42:06 PST -------
(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.
------- Comment #6 From 2010-01-13 14:43:49 PST -------
(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.
------- Comment #7 From 2010-01-13 14:48:11 PST -------
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.
------- Comment #8 From 2010-01-14 13:49:04 PST -------
(In reply to comment #7)
> Created an attachment (id=46512) [details] [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.
------- Comment #9 From 2010-01-14 14:16:47 PST -------
Peter, I'll do a test with your patch.
------- Comment #10 From 2010-01-14 15:17:19 PST -------
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 #11 From 2010-01-14 17:25:45 PST -------
(From update of attachment 46512 [details])
I wish this had tests, but I understand why it doesn't.
------- Comment #12 From 2010-01-14 17:30:29 PST -------
Fixed in r53305.