Use possible read_partial_scanlines() API in SkJpegCodec

src/codec/SkJpegCodec.cpp

 /*
  * Copyright 2015 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkCodec.h"
 #include "SkJpegCodec.h"
 #include "SkJpegDecoderMgr.h"
@@ skipping 369 matching lines @@
     fSwizzler.reset(nullptr);
     fSrcRow = nullptr;
     fStorage.free();
 
     // Now, given valid output dimensions, we can start the decompress
     if (!jpeg_start_decompress(fDecoderMgr->dinfo())) {
         SkCodecPrintf("start decompress failed\n");
         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);
     }
 
     return kSuccess;
 }
 
 int SkJpegCodec::onGetScanlines(void* dst, int count, size_t rowBytes) {
     // Set the jump location for libjpeg errors
     if (setjmp(fDecoderMgr->getJmpBuf())) {
         return fDecoderMgr->returnFailure("setjmp", kInvalidInput);
@@ skipping 42 matching lines @@
 #endif
 
 bool SkJpegCodec::onSkipScanlines(int count) {
     // Set the jump location for libjpeg errors
     if (setjmp(fDecoderMgr->getJmpBuf())) {
         return fDecoderMgr->returnFalse("setjmp");
     }
 
     return (uint32_t) count == jpeg_skip_scanlines(fDecoderMgr->dinfo(), count);
 }