Use possible read_partial_scanlines() API in SkJpegCodec

src/codec/SkJpegCodec.cpp

Index: src/codec/SkJpegCodec.cpp
diff --git a/src/codec/SkJpegCodec.cpp b/src/codec/SkJpegCodec.cpp
index 7db772da6386ad9a3b0002d5334598a625fd6fa2..ff6bce2b96039eb8254e23a447a5e9c8607a5f53 100644
--- a/src/codec/SkJpegCodec.cpp
+++ b/src/codec/SkJpegCodec.cpp
@@ -387,9 +387,35 @@ SkCodec::Result SkJpegCodec::onStartScanlineDecode(const SkImageInfo& dstInfo,
         return kInvalidInput;
     }
 
-    // We will need a swizzler if we are performing a subset decode or
-    // converting from CMYK.
-    if (options.fSubset || JCS_CMYK == fDecoderMgr->dinfo()->out_color_space) {
+    Options subsetOptions = options;

[scroggo, 2015/12/16 18:17:01]

I almost suggested "partialOptions", but it is actually used even when we are
not doing partial scanlines.

Maybe the parameter should be renamed to originalOptions? (This one could also
be named actualOptions.)

+    SkIRect subsetRect = *options.fSubset;
+    if (options.fSubset) {
+        uint32_t startX = options.fSubset->x();
+        uint32_t width = options.fSubset->width();
+        // libjpeg-turbo may need to align startX to a multiple of the IDCT

[scroggo, 2015/12/16 18:17:01]

nit: blank line before comments

+        // block size.  If this is the case, it will decrease the value of
+        // startX to the appropriate alignment and also increase the value
+        // of width so that the right edge of the requested subset remains
+        // the same.
+        jpeg_set_partial_scanline(fDecoderMgr->dinfo(), &startX, &width);

[scroggo, 2015/12/16 18:17:01]

Should we have a fallback if jpeg_set_partial_scanline is not available?

+
+        // We will need a swizzler if libjpeg-turbo cannot provide the exact subset
+        // that we request.  Or if we are converting from CMYK.
+        if (startX != (uint32_t) options.fSubset->x() ||
+                width != (uint32_t) options.fSubset->width() ||
+                JCS_CMYK == fDecoderMgr->dinfo()->out_color_space) {

[scroggo, 2015/12/16 18:17:01]

I don't understand why we need to check for this here. It only needs you to call
initializeSwizzler, which should be covered by the if statement below?

+            SkASSERT(startX <= (uint32_t) options.fSubset->x());
+            SkASSERT(width >= (uint32_t) options.fSubset->width());
+            SkASSERT(startX + width >= (uint32_t) options.fSubset->right());

[scroggo, 2015/12/16 18:17:01]

nit: blank line between asserts and code

+            subsetRect.fLeft = options.fSubset->x() - startX;

[scroggo, 2015/12/16 18:17:00]

IIUC, this means that the subset is now the subset of the partial scanline?
Maybe worth a comment?

+            subsetRect.fRight = subsetRect.fLeft + options.fSubset->width();
+            subsetOptions.fSubset = &subsetRect;
+            this->initializeSwizzler(dstInfo, subsetOptions);
+        }
+    }
+
+    // Make sure we have a swizzler if we are converting from CMYK.
+    if (!fSwizzler && JCS_CMYK == fDecoderMgr->dinfo()->out_color_space) {
         this->initializeSwizzler(dstInfo, options);
     }