Allow creating multiple scanline decoders.

src/codec/SkCodec_libpng.cpp

Index: src/codec/SkCodec_libpng.cpp
diff --git a/src/codec/SkCodec_libpng.cpp b/src/codec/SkCodec_libpng.cpp
index a14a67c3497856a908a406b76700cc642bff2599..351a95c81aadcb21d666f4acf2d23ca60ed5edaa 100644
--- a/src/codec/SkCodec_libpng.cpp
+++ b/src/codec/SkCodec_libpng.cpp
@@ -119,7 +119,7 @@ typedef uint32_t (*PackColorProc)(U8CPU a, U8CPU r, U8CPU g, U8CPU b);
 
 // Note: SkColorTable claims to store SkPMColors, which is not necessarily
 // the case here.
-bool SkPngCodec::decodePalette(bool premultiply, int bitDepth, int* ctableCount) {
+bool SkPngCodec::decodePalette(bool premultiply, int* ctableCount) {
     int numPalette;
     png_colorp palette;
     png_bytep trans;
@@ -176,7 +176,7 @@ bool SkPngCodec::decodePalette(bool premultiply, int bitDepth, int* ctableCount)
         addressed by the bitdepth of the image and fill it with the last palette color or black if
         the palette is empty (really broken image).
     */
-    int colorCount = SkTMax(numPalette, 1 << SkTMin(bitDepth, 8));
+    int colorCount = SkTMax(numPalette, 1 << SkTMin(fBitDepth, 8));
     SkPMColor lastColor = index > 0 ? colorPtr[-1] : SkPackARGB32(0xFF, 0, 0, 0);
     for (; index < colorCount; index++) {
         *colorPtr++ = lastColor;
@@ -373,12 +373,6 @@ SkPngCodec::SkPngCodec(const SkImageInfo& info, SkStream* stream,
 {}
 
 SkPngCodec::~SkPngCodec() {
-    // First, ensure that the scanline decoder is left in a finished state.
-    SkAutoTDelete<SkScanlineDecoder> decoder(this->detachScanlineDecoder());
-    if (NULL != decoder) {
-        this->finish();
-    }
-
     this->destroyReadStruct();
 }
 
@@ -453,7 +447,7 @@ SkCodec::Result SkPngCodec::initializeSwizzler(const SkImageInfo& requestedInfo,
         case kIndex_8_SkColorType:
             //decode palette to Skia format
             fSrcConfig = SkSwizzler::kIndex;
-            if (!this->decodePalette(kPremul_SkAlphaType == requestedInfo.alphaType(), fBitDepth,
+            if (!this->decodePalette(kPremul_SkAlphaType == requestedInfo.alphaType(),
                     ctableCount)) {
                 return kInvalidInput;
             }
@@ -522,11 +516,6 @@ SkCodec::Result SkPngCodec::onGetPixels(const SkImageInfo& requestedInfo, void*
     if (!conversion_possible(requestedInfo, this->getInfo())) {
         return kInvalidConversion;
     }
-    // Do not allow a regular decode if the caller has asked for a scanline decoder
-    if (NULL != this->scanlineDecoder()) {
-        SkCodecPrintf("cannot getPixels() if a scanline decoder has been created\n");
-        return kInvalidParameters;
-    }
     if (requestedInfo.dimensions() != this->getInfo().dimensions()) {
         return kInvalidScale;
     }
@@ -595,6 +584,7 @@ void SkPngCodec::finish() {
         // We've already read all the scanlines. This is a success.
         return;
     }
+    // FIXME: Is this necessary?

[scroggo, 2015/07/09 20:52:31]

I copied the below comment from SkImageDecoder_libpng.cpp, but as I made this
change I started to wonder whether it was true. It states that it reads the rest
of the file, which made me wonder what would happen in the case of a scanline
decoder, where we potentially have not read the rest of the image. Does it work
in that case?

Looking at the libpng documentation, it sounds like we only need to call it if
we want to find out information stored in the end of the stream, or if we want
to continue reading the stream after the PNG itself - i.e. the PNG is embedded
in some larger stream we want to keep reading from where the PNG left off. This
may be the case where we did a single decode (perhaps we're reading from some
kind of Asset stream on Android that we are expected to leave at the end of the
PNG so something else can continue reading the stream), but if we've already
copied into an SkMemoryStream (as is the case for region decoding), this call
seems unnecessary. Not sure how expensive it is though.

Emmalee, any thoughts?

     /* read rest of file, and get additional chunks in info_ptr - REQUIRED */
     png_read_end(fPng_ptr, fInfo_ptr);
 }
@@ -610,6 +600,10 @@ public:
         fSrcRow = static_cast<uint8_t*>(fStorage.get());
     }
 
+    ~SkPngScanlineDecoder() {
+        fCodec->finish();
+    }
+
     SkCodec::Result onGetScanlines(void* dst, int count, size_t rowBytes) override {
         if (setjmp(png_jmpbuf(fCodec->fPng_ptr))) {
             SkCodecPrintf("setjmp long jump!\n");
@@ -644,10 +638,10 @@ public:
     bool onReallyHasAlpha() const override { return fHasAlpha; }
 
 private:
-    SkPngCodec*         fCodec;     // Unowned.
-    bool                fHasAlpha;
-    SkAutoMalloc        fStorage;
-    uint8_t*            fSrcRow;
+    SkAutoTDelete<SkPngCodec>   fCodec;
+    bool                        fHasAlpha;
+    SkAutoMalloc                fStorage;
+    uint8_t*                    fSrcRow;
 
     typedef SkScanlineDecoder INHERITED;
 };
@@ -668,6 +662,10 @@ public:
         fGarbageRowPtr = static_cast<uint8_t*>(fGarbageRow.get());
     }
 
+    ~SkPngInterlacedScanlineDecoder() {
+        fCodec->finish();
+    }
+
     SkCodec::Result onGetScanlines(void* dst, int count, size_t dstRowBytes) override {
         //rewind stream if have previously called onGetScanlines, 
         //since we need entire progressive image to get scanlines
@@ -723,14 +721,14 @@ public:
     bool onReallyHasAlpha() const override { return fHasAlpha; }
 
 private:
-    SkPngCodec*         fCodec;     // Unowned.
-    bool                fHasAlpha;
-    int                 fCurrentRow;
-    int                 fHeight;
-    size_t              fSrcRowBytes;
-    bool                fRewindNeeded;
-    SkAutoMalloc        fGarbageRow;
-    uint8_t*            fGarbageRowPtr;
+    SkAutoTDelete<SkPngCodec>   fCodec;
+    bool                        fHasAlpha;
+    int                         fCurrentRow;
+    int                         fHeight;
+    size_t                      fSrcRowBytes;
+    bool                        fRewindNeeded;
+    SkAutoMalloc                fGarbageRow;
+    uint8_t*                    fGarbageRowPtr;
 
     typedef SkScanlineDecoder INHERITED;
 };
@@ -746,14 +744,20 @@ SkScanlineDecoder* SkPngCodec::onGetScanlineDecoder(const SkImageInfo& dstInfo,
     if (dstInfo.dimensions() != this->getInfo().dimensions()) {
         return NULL;
     }
-    if (!this->handleRewind()) {
+    // Create a new SkPngCodec, to be owned by the scanline decoder.
+    SkStream* stream = this->stream()->duplicate();
+    if (!stream) {
+        return NULL;
+    }
+    SkAutoTDelete<SkPngCodec> codec (static_cast<SkPngCodec*>(SkPngCodec::NewFromStream(stream)));
+    if (!codec) {
         return NULL;
     }
 
     // Note: We set dst to NULL since we do not know it yet. rowBytes is not needed,
     // since we'll be manually updating the dstRow, but the SkSwizzler requires it to
     // be at least dstInfo.minRowBytes.
-    if (this->initializeSwizzler(dstInfo, NULL, dstInfo.minRowBytes(), options, ctable,
+    if (codec->initializeSwizzler(dstInfo, NULL, dstInfo.minRowBytes(), options, ctable,
             ctableCount) != kSuccess) {
         SkCodecPrintf("failed to initialize the swizzler.\n");
         return NULL;
@@ -762,9 +766,9 @@ SkScanlineDecoder* SkPngCodec::onGetScanlineDecoder(const SkImageInfo& dstInfo,
     SkASSERT(fNumberPasses != INVALID_NUMBER_PASSES);
     if (fNumberPasses > 1) {
         // interlaced image
-        return SkNEW_ARGS(SkPngInterlacedScanlineDecoder, (dstInfo, this));
+        return SkNEW_ARGS(SkPngInterlacedScanlineDecoder, (dstInfo, codec.detach()));
     }
 
-    return SkNEW_ARGS(SkPngScanlineDecoder, (dstInfo, this));
+    return SkNEW_ARGS(SkPngScanlineDecoder, (dstInfo, codec.detach()));
 }