Make SkPngCodec decode progressively.

src/codec/SkSampledCodec.cpp

Index: src/codec/SkSampledCodec.cpp
diff --git a/src/codec/SkSampledCodec.cpp b/src/codec/SkSampledCodec.cpp
index 49c939c1f887c500f8795173a031dc2c996b36eb..a3c2b01dc84d2c4b1e934514d606c53d8e224548 100644
--- a/src/codec/SkSampledCodec.cpp
+++ b/src/codec/SkSampledCodec.cpp
@@ -101,37 +101,68 @@ SkCodec::Result SkSampledCodec::onGetAndroidPixels(const SkImageInfo& info, void
     int scaledSubsetWidth = info.width();
     int scaledSubsetHeight = info.height();
 
+    const SkImageInfo scaledInfo = info.makeWH(scaledSize.width(), scaledSize.height());
+
+    {
+        // Although startScanlineDecode expects the bottom and top to match the
+        // SkImageInfo, startIncrementalDecode uses them to determine when to start
+        // and end calling the callback.
+        SkIRect incrementalSubset = SkIRect::MakeXYWH(scaledSubsetX, scaledSubsetY,

[msarett, 2016/05/20 15:04:41]

Here's a case where the new API makes more sense :)

[scroggo_chromium, 2016/05/20 16:52:00]

Agreed. I felt good about the new API when some of my call sites became much
simpler.

+                                                      scaledSubsetWidth, scaledSubsetHeight);
+        codecOptions.fSubset = &incrementalSubset;
+        const SkCodec::Result startResult = this->codec()->startIncrementalDecode(
+                scaledInfo, &codecOptions, options.fColorPtr, options.fColorCount);
+        if (SkCodec::kSuccess == startResult) {
+            int rowsDecoded;
+            const SkCodec::Result incrementalResult = this->codec()->incrementalDecode(
+                [pixels, scaledSubsetY, rowBytes](int rowNum) -> void* {
+                    // No sampling, so this calculation is simple.
+                    return SkTAddOffset<void>(pixels, (rowNum - scaledSubsetY) * rowBytes);
+                }, &rowsDecoded);
+            if (incrementalResult == SkCodec::kSuccess) {
+                return SkCodec::kSuccess;
+            }
+            SkASSERT(SkCodec::kIncompleteInput == incrementalResult);
+
+            // FIXME: Can zero initialized be read from SkCodec::fOptions?
+            this->codec()->fillIncompleteImage(scaledInfo, pixels, rowBytes, options.fZeroInitialized,

[msarett, 2016/05/20 15:04:41]

nit: 100 chars

[scroggo_chromium, 2016/05/20 16:51:59]

Done.

+                    scaledSubsetHeight, rowsDecoded);
+            return SkCodec::kIncompleteInput;
+        } else if (startResult != SkCodec::kUnimplemented) {
+            return startResult;
+        }
+        // Otherwise fall down to use the old scanline decoder.
+        // codecOptions.fSubset will be reset below, so it will not continue to
+        // point to the object that is no longer on the stack.
+    }
+
     // Start the scanline decode.
     SkIRect scanlineSubset = SkIRect::MakeXYWH(scaledSubsetX, 0, scaledSubsetWidth,
             scaledSize.height());
     codecOptions.fSubset = &scanlineSubset;
-    SkCodec::Result result = this->codec()->startScanlineDecode(info.makeWH(scaledSize.width(),
-            scaledSize.height()), &codecOptions, options.fColorPtr, options.fColorCount);
+
+    SkCodec::Result result = this->codec()->startScanlineDecode(scaledInfo,
+            &codecOptions, options.fColorPtr, options.fColorCount);
     if (SkCodec::kSuccess != result) {
         return result;
     }
 
     // At this point, we are only concerned with subsetting.  Either no scale was
     // requested, or the this->codec() is handling the scale.
-    switch (this->codec()->getScanlineOrder()) {
-        case SkCodec::kTopDown_SkScanlineOrder:
-        case SkCodec::kNone_SkScanlineOrder: {
-            if (!this->codec()->skipScanlines(scaledSubsetY)) {
-                this->codec()->fillIncompleteImage(info, pixels, rowBytes, options.fZeroInitialized,
-                        scaledSubsetHeight, 0);
-                return SkCodec::kIncompleteInput;
-            }
+    // Note that subsetting is only supported for kTopDown, so this code will not be
+    // reached for other orders.
+    SkASSERT(this->codec()->getScanlineOrder() == SkCodec::kTopDown_SkScanlineOrder);
+    if (!this->codec()->skipScanlines(scaledSubsetY)) {
+        this->codec()->fillIncompleteImage(info, pixels, rowBytes, options.fZeroInitialized,
+                scaledSubsetHeight, 0);
+        return SkCodec::kIncompleteInput;
+    }
 
-            int decodedLines = this->codec()->getScanlines(pixels, scaledSubsetHeight, rowBytes);
-            if (decodedLines != scaledSubsetHeight) {
-                return SkCodec::kIncompleteInput;
-            }
-            return SkCodec::kSuccess;
-        }
-        default:
-            SkASSERT(false);
-            return SkCodec::kUnimplemented;
+    int decodedLines = this->codec()->getScanlines(pixels, scaledSubsetHeight, rowBytes);
+    if (decodedLines != scaledSubsetHeight) {
+        return SkCodec::kIncompleteInput;
     }
+    return SkCodec::kSuccess;
 }
 
 
@@ -174,10 +205,74 @@ SkCodec::Result SkSampledCodec::sampledDecode(const SkImageInfo& info, void* pix
         sampledOptions.fSubset = ⊂
     }
 
+    // Since we guarantee that output dimensions are always at least one (even if the sampleSize
+    // is greater than a given dimension), the input sampleSize is not always the sampleSize that
+    // we use in practice.
+    const int sampleX = subsetWidth / info.width();
+    const int sampleY = subsetHeight / info.height();
+
+    const int samplingOffsetY = get_start_coord(sampleY);
+    const int startY = samplingOffsetY + subsetY;
+    int dstHeight = info.height();
+
+    const SkImageInfo nativeInfo = info.makeWH(nativeSize.width(), nativeSize.height());
+
+    {
+        // Although startScanlineDecode expects the bottom and top to match the
+        // SkImageInfo, startIncrementalDecode uses them to determine when to start
+        // and end calling the callback.
+        SkIRect incrementalSubset;
+        incrementalSubset.fTop = startY;
+        incrementalSubset.fBottom = startY + (dstHeight - 1) * sampleY + 1;
+        if (sampledOptions.fSubset) {
+            incrementalSubset.fLeft = sampledOptions.fSubset->fLeft;
+            incrementalSubset.fRight = sampledOptions.fSubset->fRight;
+        } else {
+            incrementalSubset.fLeft = 0;
+            incrementalSubset.fRight = nativeSize.width();
+        }
+        SkCodec::Options incrementalOptions = sampledOptions;
+        incrementalOptions.fSubset = &incrementalSubset;
+        const SkCodec::Result startResult = this->codec()->startIncrementalDecode(nativeInfo,
+                &incrementalOptions, options.fColorPtr, options.fColorCount);
+        if (SkCodec::kSuccess == startResult) {
+            SkSampler* sampler = this->codec()->getSampler(true);
+            if (!sampler) {
+                return SkCodec::kUnimplemented;
+            }
+
+            if (sampler->setSampleX(sampleX) != info.width()) {
+                return SkCodec::kInvalidScale;
+            }
+            int rowsDecoded;
+            const SkCodec::Result incrementalResult = this->codec()->incrementalDecode(
+                    [pixels, rowBytes, startY, dstHeight, sampleY](int rowNum) -> void* {
+                        SkASSERT(rowNum >= startY && rowNum <= startY + (dstHeight - 1) * sampleY);
+                        if ((rowNum - startY) % sampleY != 0) {
+                            return nullptr;
+                        }
+                        const int dstRow = (rowNum - startY) / sampleY;
+                        return SkTAddOffset<void>(pixels, dstRow * rowBytes);
+                    }, &rowsDecoded);
+            if (incrementalResult == SkCodec::kSuccess) {
+                return SkCodec::kSuccess;
+            }
+            SkASSERT(incrementalResult == SkCodec::kIncompleteInput);
+            // This uses the same formula as the callback uses to compute dstRow, but
+            // it counts on the divide to truncate.
+            const int lastRowInOutput = (rowsDecoded - startY) / sampleY;
+            // FIXME: Should this be info or nativeInfo? Does it make a difference?

[msarett, 2016/05/20 15:04:41]

I think it's irrelevant, though the fill() code has become confusing (I might
try to refactor/simplify after this lands).  Certainly the height is irrelevant,
that will be determined by info.height() and lastRowInOutput.

As far as width, we want to fill the "actual width of the memory". 
nativeInfo.width() is not equal to the "actual width" if we are sampling...  But
SkSwizzler corrects the width if we are sampling anyway.

[scroggo_chromium, 2016/05/20 16:51:59]

That was my interpretation as well. 
sgtm

+            this->codec()->fillIncompleteImage(info, pixels, rowBytes, options.fZeroInitialized,
+                                               info.height(), lastRowInOutput);
+            return SkCodec::kIncompleteInput;
+        } else if (startResult != SkCodec::kUnimplemented) {
+            return startResult;
+        } // kUnimplemented means use the old method.
+    }
+
     // Start the scanline decode.
-    SkCodec::Result result = this->codec()->startScanlineDecode(
-            info.makeWH(nativeSize.width(), nativeSize.height()), &sampledOptions,
-            options.fColorPtr, options.fColorCount);
+    SkCodec::Result result = this->codec()->startScanlineDecode(nativeInfo,
+            &sampledOptions, options.fColorPtr, options.fColorCount);
     if (SkCodec::kSuccess != result) {
         return result;
     }
@@ -187,11 +282,6 @@ SkCodec::Result SkSampledCodec::sampledDecode(const SkImageInfo& info, void* pix
         return SkCodec::kUnimplemented;
     }
 
-    // Since we guarantee that output dimensions are always at least one (even if the sampleSize
-    // is greater than a given dimension), the input sampleSize is not always the sampleSize that
-    // we use in practice.
-    const int sampleX = subsetWidth / info.width();
-    const int sampleY = subsetHeight / info.height();
     if (sampler->setSampleX(sampleX) != info.width()) {
         return SkCodec::kInvalidScale;
     }
@@ -199,9 +289,6 @@ SkCodec::Result SkSampledCodec::sampledDecode(const SkImageInfo& info, void* pix
         return SkCodec::kInvalidScale;
     }
 
-    const int samplingOffsetY = get_start_coord(sampleY);
-    const int startY = samplingOffsetY + subsetY;
-    int dstHeight = info.height();
     switch(this->codec()->getScanlineOrder()) {
         case SkCodec::kTopDown_SkScanlineOrder: {
             if (!this->codec()->skipScanlines(startY)) {
@@ -266,30 +353,6 @@ SkCodec::Result SkSampledCodec::sampledDecode(const SkImageInfo& info, void* pix
             }
             return SkCodec::kIncompleteInput;
         }
-        case SkCodec::kNone_SkScanlineOrder: {
-            const int linesNeeded = subsetHeight - samplingOffsetY;
-            SkAutoTMalloc<uint8_t> storage(linesNeeded * rowBytes);
-            uint8_t* storagePtr = storage.get();
-
-            if (!this->codec()->skipScanlines(startY)) {
-                this->codec()->fillIncompleteImage(info, pixels, rowBytes, options.fZeroInitialized,
-                        dstHeight, 0);
-                return SkCodec::kIncompleteInput;
-            }
-            int scanlines = this->codec()->getScanlines(storagePtr, linesNeeded, rowBytes);
-
-            for (int y = 0; y < dstHeight; y++) {
-                memcpy(pixels, storagePtr, info.minRowBytes());
-                storagePtr += sampleY * rowBytes;
-                pixels = SkTAddOffset<void>(pixels, rowBytes);
-            }
-
-            if (scanlines < dstHeight) {
-                // this->codec() has already handled filling uninitialized memory.
-                return SkCodec::kIncompleteInput;
-            }
-            return SkCodec::kSuccess;
-        }
         default:
             SkASSERT(false);
             return SkCodec::kUnimplemented;