JPEGImageDecoder RGB565 and downsample support and related Skia imagegenerator changes

third_party/WebKit/Source/platform/image-decoders/jpeg/JPEGImageDecoder.cpp

Index: third_party/WebKit/Source/platform/image-decoders/jpeg/JPEGImageDecoder.cpp
diff --git a/third_party/WebKit/Source/platform/image-decoders/jpeg/JPEGImageDecoder.cpp b/third_party/WebKit/Source/platform/image-decoders/jpeg/JPEGImageDecoder.cpp
index a77a3f4bff35338403be10a21d71786f4ac15189..083a5499e6d90909e0eb54ee0520639c856c81c6 100644
--- a/third_party/WebKit/Source/platform/image-decoders/jpeg/JPEGImageDecoder.cpp
+++ b/third_party/WebKit/Source/platform/image-decoders/jpeg/JPEGImageDecoder.cpp
@@ -65,7 +65,7 @@ inline J_COLOR_SPACE rgbOutputColorSpace() { return JCS_EXT_RGBA; }
 #else // Output little-endian BGRA pixels.
 inline J_COLOR_SPACE rgbOutputColorSpace() { return JCS_EXT_BGRA; }
 #endif
-inline bool turboSwizzled(J_COLOR_SPACE colorSpace) { return colorSpace == JCS_EXT_RGBA || colorSpace == JCS_EXT_BGRA; }
+inline bool turboSwizzled(J_COLOR_SPACE colorSpace) { return colorSpace == JCS_EXT_RGBA || colorSpace == JCS_EXT_BGRA || colorSpace == JCS_RGB565; }
 inline bool colorSpaceHasAlpha(J_COLOR_SPACE colorSpace) { return turboSwizzled(colorSpace); }
 #else
 inline J_COLOR_SPACE rgbOutputColorSpace() { return JCS_RGB; }
@@ -472,6 +472,23 @@ public:
             default:
                 return m_decoder->setFailed();
             }
+            if (m_decoder->decodeToRGB565Enabled()) {
+                if (m_info.out_color_space == rgbOutputColorSpace()) {
+                    // the rest would need to be converted manually on write
+#if defined (JCS_ALPHA_EXTENSIONS)
+                    m_info.out_color_space = JCS_RGB565;
+#else
+                    m_info.out_color_space = JCS_RGB;

[scroggo_chromium, 2015/10/19 20:41:36]

Isn't that the same as rgbOutputColorSpace when JCS_ALPHA_EXTENSIONS is not
defined? So isn't this line unnecessary?

[aleksandar.stojiljkovic, 2015/10/20 09:51:12]

Correct, not needed to set to same.

+#endif
+#if USE(QCMSLIB)
+                    // if there is colorprofile, use JCS_RGB as QCMS is not supporting RGB565
+                    // conversion, RGB->RGB565 happens in outputRows<>()
+                    if (!m_decoder->ignoresGammaAndColorProfile())
+                        m_info.out_color_space = JCS_RGB;
+#endif
+                }
+            }
+            // fell through

[scroggo_chromium, 2015/10/19 20:41:36]

nit: Other places in this file say "FALL THROUGH". I think this should be
consistent.

[aleksandar.stojiljkovic, 2015/10/20 09:51:12]

Done.

 
             m_state = JPEG_START_DECOMPRESS;
 
@@ -588,6 +605,7 @@ public:
 
         case JPEG_DECOMPRESS_PROGRESSIVE:
             if (m_state == JPEG_DECOMPRESS_PROGRESSIVE) {
+

[scroggo_chromium, 2015/10/19 20:41:36]

Why the extra line here?

                 int status = 0;
                 do {
                     decoder_error_mgr* err = reinterpret_cast_ptr<decoder_error_mgr*>(m_info.err);
@@ -780,6 +798,7 @@ void term_source(j_decompress_ptr jd)
 JPEGImageDecoder::JPEGImageDecoder(AlphaOption alphaOption, GammaAndColorProfileOption colorOptions, size_t maxDecodedBytes)
     : ImageDecoder(alphaOption, colorOptions, maxDecodedBytes)
     , m_hasColorProfile(false)
+    , m_decodeToRGB565(false)
 {
 }
 
@@ -841,6 +860,30 @@ bool JPEGImageDecoder::canDecodeToYUV()
     return isSizeAvailable() && m_reader->info()->out_color_space == JCS_YCbCr;
 }
 
+bool JPEGImageDecoder::canDecodeToRGB565()
+{
+    // Calling isSizeAvailable() ensures the reader is created and the output

[scroggo_chromium, 2015/10/19 20:41:36]

I do not follow how this comment applies.

[aleksandar.stojiljkovic, 2015/10/20 09:51:12]

Didn't do cleanup after copy paste.
Also decoding works without #if defined(JCS_ALPHA_EXTENSIONS). I'm
over-complicating it, here and bellow. 
Done.

+    // color space is set.
+#if defined(JCS_ALPHA_EXTENSIONS)
+    return true;
+#else
+    return false;
+#endif
+}
+
+bool JPEGImageDecoder::setDecodeRGB565Enabled(const bool enable)
+{
+#if defined(JCS_ALPHA_EXTENSIONS)
+    m_decodeToRGB565 = enable;

[scroggo_chromium, 2015/10/19 20:41:36]

Should m_decodeToRGB565 only be defined if JCS_ALPHA_EXTENSIONS is defined?
Otherwise it seems like we do not need it.

[aleksandar.stojiljkovic, 2015/10/20 09:51:12]

Done.

+    return true;
+#else
+    if (!enable)

[scroggo_chromium, 2015/10/19 20:41:36]

Can this just be:

  return !enable

and then drop the extra return at the end?

[aleksandar.stojiljkovic, 2015/10/20 09:51:12]

Done.

+        return true;
+#endif
+    return false;
+}
+
+
 bool JPEGImageDecoder::decodeToYUV()
 {
     if (!hasImagePlanes())
@@ -856,18 +899,18 @@ void JPEGImageDecoder::setImagePlanes(PassOwnPtr<ImagePlanes> imagePlanes)
     m_imagePlanes = imagePlanes;
 }
 
-template <J_COLOR_SPACE colorSpace> void setPixel(ImageFrame& buffer, ImageFrame::PixelData* pixel, JSAMPARRAY samples, int column)
+template <J_COLOR_SPACE colorSpace, class T> void setPixel(ImageFrame& buffer, T* pixel, JSAMPARRAY samples, int column)
 {
     ASSERT_NOT_REACHED();
 }
 
-template <> void setPixel<JCS_RGB>(ImageFrame& buffer, ImageFrame::PixelData* pixel, JSAMPARRAY samples, int column)
+template <> void setPixel<JCS_RGB, ImageFrame::PixelData>(ImageFrame& buffer, ImageFrame::PixelData* pixel, JSAMPARRAY samples, int column)
 {
     JSAMPLE* jsample = *samples + column * 3;
     buffer.setRGBARaw(pixel, jsample[0], jsample[1], jsample[2], 255);
 }
 
-template <> void setPixel<JCS_CMYK>(ImageFrame& buffer, ImageFrame::PixelData* pixel, JSAMPARRAY samples, int column)
+template <> void setPixel<JCS_CMYK, ImageFrame::PixelData>(ImageFrame& buffer, ImageFrame::PixelData* pixel, JSAMPARRAY samples, int column)
 {
     JSAMPLE* jsample = *samples + column * 4;
 
@@ -884,7 +927,30 @@ template <> void setPixel<JCS_CMYK>(ImageFrame& buffer, ImageFrame::PixelData* p
     buffer.setRGBARaw(pixel, jsample[0] * k / 255, jsample[1] * k / 255, jsample[2] * k / 255, 255);
 }
 
-template <J_COLOR_SPACE colorSpace> bool outputRows(JPEGImageReader* reader, ImageFrame& buffer)
+template <> void setPixel<JCS_RGB, ImageFrame::PixelData16>(ImageFrame& buffer, ImageFrame::PixelData16* pixel, JSAMPARRAY samples, int column)
+{
+    JSAMPLE* jsample = *samples + column * 3;
+    buffer.setRGB565(pixel, jsample[0], jsample[1], jsample[2]);
+}
+
+template <> void setPixel<JCS_CMYK, ImageFrame::PixelData16>(ImageFrame& buffer, ImageFrame::PixelData16* pixel, JSAMPARRAY samples, int column)
+{
+    JSAMPLE* jsample = *samples + column * 4;
+
+    // Source is 'Inverted CMYK', output is RGB.
+    // See: http://www.easyrgb.com/math.php?MATH=M12#text12
+    // Or: http://www.ilkeratalay.com/colorspacesfaq.php#rgb
+    // From CMYK to CMY:
+    // X =   X    * (1 -   K   ) +   K  [for X = C, M, or Y]
+    // Thus, from Inverted CMYK to CMY is:
+    // X = (1-iX) * (1 - (1-iK)) + (1-iK) => 1 - iX*iK
+    // From CMY (0..1) to RGB (0..1):
+    // R = 1 - C => 1 - (1 - iC*iK) => iC*iK  [G and B similar]
+    unsigned k = jsample[3];
+    buffer.setRGB565(pixel, jsample[0] * k / 255, jsample[1] * k / 255, jsample[2] * k / 255);
+}
+
+template <J_COLOR_SPACE colorSpace, class T> bool outputRows(JPEGImageReader* reader, ImageFrame& buffer)
 {
     JSAMPARRAY samples = reader->samples();
     jpeg_decompress_struct* info = reader->info();
@@ -901,9 +967,9 @@ template <J_COLOR_SPACE colorSpace> bool outputRows(JPEGImageReader* reader, Ima
         if (reader->colorTransform() && colorSpace == JCS_RGB)
             qcms_transform_data(reader->colorTransform(), *samples, *samples, width);
 #endif
-        ImageFrame::PixelData* pixel = buffer.getAddr(0, y);
+        T* pixel = buffer.getAddrT<T>(0, y);
         for (int x = 0; x < width; ++pixel, ++x)
-            setPixel<colorSpace>(buffer, pixel, samples, x);
+            setPixel<colorSpace, T>(buffer, pixel, samples, x);
     }
 
     buffer.setPixelsChanged(true);
@@ -1005,11 +1071,13 @@ bool JPEGImageDecoder::outputScanlines()
         ASSERT(info->output_width == static_cast<JDIMENSION>(m_decodedSize.width()));
         ASSERT(info->output_height == static_cast<JDIMENSION>(m_decodedSize.height()));
 
-        if (!buffer.setSize(info->output_width, info->output_height))
+        if (!buffer.setSize(info->output_width, info->output_height, decodeToRGB565Enabled()
+            ? ImageFrame::RGB565 : ImageFrame::RGBA8888))
             return setFailed();
         buffer.setStatus(ImageFrame::FramePartial);
         // The buffer is transparent outside the decoded area while the image is
         // loading. The image will be marked fully opaque in complete().
+        // for RGB565 this would just fail, having bitmap opaque during decoding

[scroggo_chromium, 2015/10/19 20:41:36]

So doesn't this need to say something like if (!565)?

[aleksandar.stojiljkovic, 2015/10/20 09:51:12]

Done.

         buffer.setHasAlpha(true);
 
         // For JPEGs, the frame always fills the entire image.
@@ -1019,9 +1087,12 @@ bool JPEGImageDecoder::outputScanlines()
 #if defined(TURBO_JPEG_RGB_SWIZZLE)
     if (turboSwizzled(info->out_color_space)) {
         while (info->output_scanline < info->output_height) {
-            unsigned char* row = reinterpret_cast_ptr<unsigned char*>(buffer.getAddr(0, info->output_scanline));
-            if (jpeg_read_scanlines(info, &row, 1) != 1)
+            unsigned char* row = decodeToRGB565Enabled()
+                ? reinterpret_cast_ptr<unsigned char*>(buffer.getAddrT<ImageFrame::PixelData16>(0, info->output_scanline))
+                : reinterpret_cast_ptr<unsigned char*>(buffer.getAddr(0, info->output_scanline));
+            if (jpeg_read_scanlines(info, &row, 1) != 1) {
                 return false;
+            }
 #if USE(QCMSLIB)
             if (qcms_transform* transform = m_reader->colorTransform())
                 qcms_transform_data_type(transform, row, row, info->output_width, rgbOutputColorSpace() == JCS_EXT_BGRA ? QCMS_OUTPUT_BGRX : QCMS_OUTPUT_RGBX);
@@ -1034,9 +1105,13 @@ bool JPEGImageDecoder::outputScanlines()
 
     switch (info->out_color_space) {
     case JCS_RGB:
-        return outputRows<JCS_RGB>(m_reader.get(), buffer);
+        if (decodeToRGB565Enabled())
+            return outputRows<JCS_RGB, ImageFrame::PixelData16>(m_reader.get(), buffer);
+        return outputRows<JCS_RGB, ImageFrame::PixelData>(m_reader.get(), buffer);
     case JCS_CMYK:
-        return outputRows<JCS_CMYK>(m_reader.get(), buffer);
+        if (decodeToRGB565Enabled())
+            return outputRows<JCS_CMYK, ImageFrame::PixelData16>(m_reader.get(), buffer);
+        return outputRows<JCS_CMYK, ImageFrame::PixelData>(m_reader.get(), buffer);
     default:
         ASSERT_NOT_REACHED();
     }