Bug 85268 – PNGImageDecoder: Add ENABLE(IMAGE_DECODER_DOWN_SAMPLING) guards to rowAvailable

• Component: New Bugs
• Status: RESOLVED FIXED

[noel gordon]

PNGImageDecoder: Add ENABLE(IMAGE_DECODER_DOWN_SAMPLING) guards to rowAvailable

[noel gordon]

Created attachment 139598 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 139598 [details]
Patch

I would have reversed that if I think, to avoid the !, but this is fine.

[Eric Seidel (no email)]

Comment on attachment 139598 [details]
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?

[noel gordon]

(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 :(

[noel gordon]

Created attachment 139702 [details]
Patch for landing

[Eric Seidel (no email)]

Comment on attachment 139702 [details]
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?

[noel gordon]

(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 ...

[WebKit Review Bot]

Comment on attachment 139702 [details]
Patch for landing

Clearing flags on attachment: 139702

Committed r115782: <http://trac.webkit.org/changeset/115782>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.