[webkit-reviews] review denied: [Bug 103856] ENABLE(IMAGE_DECODER_DOWN_SAMPLING): Don't swizzle decode down sampled images : [Attachment 177256] Patch

[bugzilla-daemon at webkit.org]

Yong Li <yoli at rim.com> has denied noel gordon <noel.gordon at gmail.com>'s request
for review:
Bug 103856: ENABLE(IMAGE_DECODER_DOWN_SAMPLING): Don't swizzle decode down
sampled images
https://bugs.webkit.org/show_bug.cgi?id=103856

Attachment 177256: Patch
https://bugs.webkit.org/attachment.cgi?id=177256&action=review

------- Additional Comments from Yong Li <yoli at rim.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=177256&action=review


r- until we get perf numbers showing this helps.

> Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.cpp:397
> +#if ENABLE(IMAGE_DECODER_DOWN_SAMPLING) && defined(TURBO_JPEG_RGB_SWIZZLE)
> +	       // There's no point swizzle decoding if image down sampling will

> +	       // be applied. Revert to using JSC_RGB in that case.
> +	       if (m_decoder->willDownSample() &&
turboSwizzled(m_info.out_color_space))
> +		   m_info.out_color_space = JCS_RGB;
> +#endif

What will happen if jpeg_color_space is JCS_CMYK? I would move this code into:

359359 switch (m_info.jpeg_color_space) {
360360 case JCS_GRAYSCALE:
361361 case JCS_RGB:
362362 case JCS_YCbCr:
363363 // libjpeg can convert GRAYSCALE and YCbCr image pixels to RGB.
364364 m_info.out_color_space = rgbOutputColorSpace();
365365#if defined(TURBO_JPEG_RGB_SWIZZLE)
366366 if (m_info.saw_JFIF_marker)
367367 break;
368368 // FIXME: Swizzle decoding does not support Adobe transform=0
369369 // images (yet), so revert to using JSC_RGB in that case.
370370 if (m_info.saw_Adobe_marker && !m_info.Adobe_transform)
371371 m_info.out_color_space = JCS_RGB;
372372#endif
373373 break;

And move the switch block after setSize(). What do you think?