Merge SkCodec with SkScanlineDecoder

src/codec/SkCodec_libpng.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_libpng.h"
 #include "SkCodecPriv.h"
 #include "SkColorPriv.h"
 #include "SkColorTable.h"
 #include "SkBitmap.h"
 #include "SkMath.h"
 #include "SkScaledCodec.h"
-#include "SkScanlineDecoder.h"
 #include "SkSize.h"
 #include "SkStream.h"
 #include "SkSwizzler.h"
 
 ///////////////////////////////////////////////////////////////////////////////
 // Helper macros
 ///////////////////////////////////////////////////////////////////////////////
 
 #ifndef png_jmpbuf
 #  define png_jmpbuf(png_ptr) ((png_ptr)->jmpbuf)
@@ skipping 130 matching lines @@
         proc = &SkPreMultiplyARGB;
     } else {
         proc = &SkPackARGB32NoCheck;
     }
     for (; index < numTrans; index++) {
         transLessThanFF |= (int)*trans - 0xFF;
         *colorPtr++ = proc(*trans++, palette->red, palette->green, palette->blue);
         palette++;
     }
 
-    fReallyHasAlpha = transLessThanFF < 0;
+    if (transLessThanFF >= 0) {

[scroggo, 2015/09/25 16:07:49]

If I understand this code correctly, the old code here decided that there was
really alpha if one of the colors in the palette has alpha, regardless of
whether those colors are actually used. This defeats the purpose of the swizzler
function which returns whether a single line had alpha (for the indexed png
case).

Maybe it's unlikely that the client will attempt to decode a non-transparent
piece of a transparent image. We have outstanding questions about whether we
should bother doing the check at all (skbug.com/3582). So long as it's here,
though, I think it makes sense to only modify the alpha if we found no colors
with alpha, to the new state which is known to be opaque.

[msarett, 2015/09/28 14:48:50]

I agree with this interpretation.

In this case, will we still check all the alpha values as we set the pixels or
will we skip the check since we know already that the image is opaque?

EDIT: Reading below, it looks like we will still check all of the alpha values. 
Maybe add a FIXME?

[scroggo, 2015/09/28 16:01:52]

Well, swizzle() returns the alpha value, and I don't want to create a separate,
mostly identical loop to ignore it (unless we find it makes a difference), but I
do think there's room for improvement here. Rather than calling IsOpaque in each
iteration, and or-ing that with hasAlpha, I think I can just store a ResultAlpha
and check it at the end. Added a FIXME.

+        // No transparent colors were found.
+        fAlphaState = kOpaque_AlphaState;
+    }
 
     for (; index < numPalette; index++) {
         *colorPtr++ = SkPackARGB32(0xFF, palette->red, palette->green, palette->blue);
         palette++;
     }
 
     /*  BUGGY IMAGE WORKAROUND
         Invalid images could contain pixel values that are greater than the number of palette
         entries. Since we use pixel values as indices into the palette this could result in reading
         beyond the end of the palette which could leak the contents of uninitialized memory. To
@@ skipping 32 matching lines @@
     }
     return true;
 }
 
 // Reads the header, and initializes the passed in fields, if not nullptr (except
 // stream, which is passed to the read function).
 // Returns true on success, in which case the caller is responsible for calling
 // png_destroy_read_struct. If it returns false, the passed in fields (except
 // stream) are unchanged.
 static bool read_header(SkStream* stream, png_structp* png_ptrp,
-                        png_infop* info_ptrp, SkImageInfo* imageInfo, int* bitDepthPtr) {
+                        png_infop* info_ptrp, SkImageInfo* imageInfo,
+                        int* bitDepthPtr, int* numberPassesPtr) {
     // The image is known to be a PNG. Decode enough to know the SkImageInfo.
     png_structp png_ptr = png_create_read_struct(PNG_LIBPNG_VER_STRING, nullptr,
                                                  sk_error_fn, sk_warning_fn);
     if (!png_ptr) {
         return false;
     }
 
     AutoCleanPng autoClean(png_ptr);
 
     png_infop info_ptr = png_create_info_struct(png_ptr);
@@ skipping 96 matching lines @@
             break;
         case PNG_COLOR_TYPE_RGBA:
             skColorType = kN32_SkColorType;
             skAlphaType = kUnpremul_SkAlphaType;
             break;
         default:
             //all the color types have been covered above
             SkASSERT(false);
     }
 
+    int numberPasses = png_set_interlace_handling(png_ptr);
+    if (numberPassesPtr) {
+        *numberPassesPtr = numberPasses;
+    }
+
     // FIXME: Also need to check for sRGB (skbug.com/3471).
 
     if (imageInfo) {
         *imageInfo = SkImageInfo::Make(origWidth, origHeight, skColorType, skAlphaType);
     }
     autoClean.detach();
     if (png_ptrp) {
         *png_ptrp = png_ptr;
     }
     if (info_ptrp) {
         *info_ptrp = info_ptr;
     }
 
     return true;
 }
 
-SkCodec* SkPngCodec::NewFromStream(SkStream* stream) {
-    SkAutoTDelete<SkStream> streamDeleter(stream);
-    png_structp png_ptr;
-    png_infop info_ptr;
-    SkImageInfo imageInfo;
-    int bitDepth;
-    if (read_header(stream, &png_ptr, &info_ptr, &imageInfo, &bitDepth)) {
-        return new SkPngCodec(imageInfo, streamDeleter.detach(), png_ptr, info_ptr, bitDepth);
-    }
-    return nullptr;
-}
-
-#define INVALID_NUMBER_PASSES -1
 SkPngCodec::SkPngCodec(const SkImageInfo& info, SkStream* stream,
-                       png_structp png_ptr, png_infop info_ptr, int bitDepth)
+                       png_structp png_ptr, png_infop info_ptr, int bitDepth, int numberPasses)
     : INHERITED(info, stream)
     , fPng_ptr(png_ptr)
     , fInfo_ptr(info_ptr)
     , fSrcConfig(SkSwizzler::kUnknown)
-    , fNumberPasses(INVALID_NUMBER_PASSES)
-    , fReallyHasAlpha(false)
+    , fNumberPasses(numberPasses)
     , fBitDepth(bitDepth)
-{}
+{
+    if (info.alphaType() == kOpaque_SkAlphaType) {
+        fAlphaState = kOpaque_AlphaState;
+    } else {
+        fAlphaState = kUnknown_AlphaState;
+    }
+}
 
 SkPngCodec::~SkPngCodec() {
     this->destroyReadStruct();
 }
 
 void SkPngCodec::destroyReadStruct() {
     if (fPng_ptr) {
         // We will never have a nullptr fInfo_ptr with a non-nullptr fPng_ptr
         SkASSERT(fInfo_ptr);
         png_destroy_read_struct(&fPng_ptr, &fInfo_ptr, png_infopp_NULL);
         fPng_ptr = nullptr;
         fInfo_ptr = nullptr;
     }
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // Getting the pixels
 ///////////////////////////////////////////////////////////////////////////////
 
 SkCodec::Result SkPngCodec::initializeSwizzler(const SkImageInfo& requestedInfo,
                                                const Options& options,
                                                SkPMColor ctable[],
                                                int* ctableCount) {
     // FIXME: Could we use the return value of setjmp to specify the type of
     // error?
     if (setjmp(png_jmpbuf(fPng_ptr))) {
         SkCodecPrintf("setjmp long jump!\n");
         return kInvalidInput;
     }
-    fNumberPasses = png_set_interlace_handling(fPng_ptr);
     png_read_update_info(fPng_ptr, fInfo_ptr);  
 
-    // Set to the default before calling decodePalette, which may change it.
-    fReallyHasAlpha = false;
-
-    //srcColorType was determined in readHeader() which determined png color type
+    //srcColorType was determined in read_header() which determined png color type
     const SkColorType srcColorType = this->getInfo().colorType();
 
     switch (srcColorType) {
         case kIndex_8_SkColorType:
             //decode palette to Skia format
             fSrcConfig = SkSwizzler::kIndex;
             if (!this->decodePalette(kPremul_SkAlphaType == requestedInfo.alphaType(),
                     ctableCount)) {
                 return kInvalidInput;
             }
@@ skipping 31 matching lines @@
 bool SkPngCodec::onRewind() {
     // This sets fPng_ptr and fInfo_ptr to nullptr. If read_header
     // succeeds, they will be repopulated, and if it fails, they will
     // remain nullptr. Any future accesses to fPng_ptr and fInfo_ptr will
     // come through this function which will rewind and again attempt
     // to reinitialize them.
     this->destroyReadStruct();
 
     png_structp png_ptr;
     png_infop info_ptr;
-    if (!read_header(this->stream(), &png_ptr, &info_ptr, nullptr, nullptr)) {
+    if (!read_header(this->stream(), &png_ptr, &info_ptr, nullptr, nullptr, nullptr)) {
         return false;
     }
 
     fPng_ptr = png_ptr;
     fInfo_ptr = info_ptr;
     return true;
 }
 
 SkCodec::Result SkPngCodec::onGetPixels(const SkImageInfo& requestedInfo, void* dst,
                                         size_t dstRowBytes, const Options& options,
@@ skipping 18 matching lines @@
     if (result != kSuccess) {
         return result;
     }
     // FIXME: Could we use the return value of setjmp to specify the type of
     // error?
     if (setjmp(png_jmpbuf(fPng_ptr))) {
         SkCodecPrintf("setjmp long jump!\n");
         return kInvalidInput;
     }
 
-    SkASSERT(fNumberPasses != INVALID_NUMBER_PASSES);
+    bool hasAlpha = false;
+    // FIXME: We could split these out based on subclass.

[msarett, 2015/09/28 14:48:50]

Yeah it really feels like these should be separate classes, given that we decode
differently and the scanline decoders are already separate.  The shared code at
the top of this function could be in refactored into the base class.

     SkAutoMalloc storage;
     void* dstRow = dst;
     if (fNumberPasses > 1) {
         const int width = requestedInfo.width();
         const int height = requestedInfo.height();
         const int bpp = SkSwizzler::BytesPerPixel(fSrcConfig);
         const size_t srcRowBytes = width * bpp;
 
         storage.reset(width * height * bpp);
         uint8_t* const base = static_cast<uint8_t*>(storage.get());
 
         for (int i = 0; i < fNumberPasses; i++) {
             uint8_t* srcRow = base;
             for (int y = 0; y < height; y++) {
                 uint8_t* bmRow = srcRow;
                 png_read_rows(fPng_ptr, &bmRow, png_bytepp_NULL, 1);
                 srcRow += srcRowBytes;
             }
         }
 
         // Now swizzle it.
         uint8_t* srcRow = base;
         for (int y = 0; y < height; y++) {
-            fReallyHasAlpha |= !SkSwizzler::IsOpaque(fSwizzler->swizzle(dstRow, srcRow));
+            hasAlpha |= !SkSwizzler::IsOpaque(fSwizzler->swizzle(dstRow, srcRow));
             dstRow = SkTAddOffset<void>(dstRow, dstRowBytes);
             srcRow += srcRowBytes;
         }
     } else {
         storage.reset(requestedInfo.width() * SkSwizzler::BytesPerPixel(fSrcConfig));
         uint8_t* srcRow = static_cast<uint8_t*>(storage.get());
         for (int y = 0; y < requestedInfo.height(); y++) {
             png_read_rows(fPng_ptr, &srcRow, png_bytepp_NULL, 1);
-            fReallyHasAlpha |= !SkSwizzler::IsOpaque(fSwizzler->swizzle(dstRow, srcRow));
+            hasAlpha |= !SkSwizzler::IsOpaque(fSwizzler->swizzle(dstRow, srcRow));
             dstRow = SkTAddOffset<void>(dstRow, dstRowBytes);
         }
     }
 
+    if (hasAlpha) {
+        fAlphaState = kHasAlpha_AlphaState;
+    } else {
+        fAlphaState = kOpaque_AlphaState;
+    }
+
     // FIXME: do we need substituteTranspColor? Note that we cannot do it for
     // scanline decoding, but we could do it here. Alternatively, we could do
     // it as we go, instead of in post-processing like SkPNGImageDecoder.
 
     if (setjmp(png_jmpbuf(fPng_ptr))) {
         // We've already read all the scanlines. This is a success.
         return kSuccess;
     }
 
     // read rest of file, and get additional comment and time chunks in info_ptr
     png_read_end(fPng_ptr, fInfo_ptr);
+
     return kSuccess;
 }
 
-class SkPngScanlineDecoder : public SkScanlineDecoder {
+bool SkPngCodec::onReallyHasAlpha() const {
+    switch (fAlphaState) {
+        case kOpaque_AlphaState:
+            return false;
+        case kUnknown_AlphaState:
+            // Maybe the subclass knows?
+            return this->alphaInScanlineDecode() == kHasAlpha_AlphaState;
+        case kHasAlpha_AlphaState:
+            switch (this->alphaInScanlineDecode()) {
+                case kUnknown_AlphaState:
+                    // Scanline decoder must not have been used. Return our knowledge.
+                    return true;
+                case kOpaque_AlphaState:
+                    // Scanline decoder was used, and did not find alpha in its subset.
+                    return false;
+                case kHasAlpha_AlphaState:
+                    return true;
+            }
+    }
+}
+
+// Subclass of SkPngCodec which supports scanline decoding
+class SkPngScanlineDecoder : public SkPngCodec {
 public:
-    SkPngScanlineDecoder(const SkImageInfo& srcInfo, SkPngCodec* codec)
-        : INHERITED(srcInfo)
-        , fCodec(codec)
-        , fHasAlpha(false)
+    SkPngScanlineDecoder(const SkImageInfo& srcInfo, SkStream* stream,
+            png_structp png_ptr, png_infop info_ptr, int bitDepth)
+        : INHERITED(srcInfo, stream, png_ptr, info_ptr, bitDepth, 1)
+        , fSrcRow(nullptr)
+        , fAlphaState(kUnknown_AlphaState)
     {}
 
-    SkCodec::Result onStart(const SkImageInfo& dstInfo,
-                            const SkCodec::Options& options,
-                            SkPMColor ctable[], int* ctableCount) override {
-        if (!fCodec->rewindIfNeeded()) {
-            return SkCodec::kCouldNotRewind;
+    Result onStart(const SkImageInfo& dstInfo, const Options& options,
+                   SkPMColor ctable[], int* ctableCount) override {
+        if (!this->rewindIfNeeded()) {
+            return kCouldNotRewind;
         }
 
         if (!conversion_possible(dstInfo, this->getInfo())) {
-            return SkCodec::kInvalidConversion;
+            return kInvalidConversion;
         }
 
         // Check to see if scaling was requested.
         if (dstInfo.dimensions() != this->getInfo().dimensions()) {
             if (!SkScaledCodec::DimensionsSupportedForSampling(this->getInfo(), dstInfo)) {
-                return SkCodec::kInvalidScale;   
+                return kInvalidScale;   
             }
         }
 
-        const SkCodec::Result result = fCodec->initializeSwizzler(dstInfo, options, ctable,
-                                                                  ctableCount);
-        if (result != SkCodec::kSuccess) {
+        const Result result = this->initializeSwizzler(dstInfo, options, ctable,
+                                                       ctableCount);
+        if (result != kSuccess) {
             return result;
         }
 
-        fHasAlpha = false;
-        fStorage.reset(this->getInfo().width() * SkSwizzler::BytesPerPixel(fCodec->fSrcConfig));
+        fAlphaState = kUnknown_AlphaState;
+        fStorage.reset(this->getInfo().width() * SkSwizzler::BytesPerPixel(this->srcConfig()));
         fSrcRow = static_cast<uint8_t*>(fStorage.get());
 
-        return SkCodec::kSuccess;
+        return kSuccess;
     }
 
-    SkCodec::Result onGetScanlines(void* dst, int count, size_t rowBytes) override {
-        if (setjmp(png_jmpbuf(fCodec->fPng_ptr))) {
+    Result onGetScanlines(void* dst, int count, size_t rowBytes) override {
+        if (setjmp(png_jmpbuf(this->png_ptr()))) {
             SkCodecPrintf("setjmp long jump!\n");
-            return SkCodec::kInvalidInput;
+            return kInvalidInput;
         }
 
         void* dstRow = dst;
+        bool hasAlpha = false;
         for (int i = 0; i < count; i++) {
-            png_read_rows(fCodec->fPng_ptr, &fSrcRow, png_bytepp_NULL, 1);
-            fHasAlpha |= !SkSwizzler::IsOpaque(fCodec->fSwizzler->swizzle(dstRow, fSrcRow));
+            png_read_rows(this->png_ptr(), &fSrcRow, png_bytepp_NULL, 1);
+            hasAlpha |= !SkSwizzler::IsOpaque(this->swizzler()->swizzle(dstRow, fSrcRow));
             dstRow = SkTAddOffset<void>(dstRow, rowBytes);
         }
-        return SkCodec::kSuccess;
+
+        if (hasAlpha) {
+            fAlphaState = kHasAlpha_AlphaState;
+        } else {
+            if (kUnknown_AlphaState == fAlphaState) {
+                fAlphaState = kOpaque_AlphaState;
+            }
+            // Otherwise, the AlphaState is unchanged.
+        }
+
+        return kSuccess;
     }
 
-    SkCodec::Result onSkipScanlines(int count) override {
+    Result onSkipScanlines(int count) override {
         // FIXME: Could we use the return value of setjmp to specify the type of
         // error?
-        if (setjmp(png_jmpbuf(fCodec->fPng_ptr))) {
+        if (setjmp(png_jmpbuf(this->png_ptr()))) {
             SkCodecPrintf("setjmp long jump!\n");
-            return SkCodec::kInvalidInput;
+            return kInvalidInput;
         }
         //there is a potential tradeoff of memory vs speed created by putting this in a loop. 
         //calling png_read_rows in a loop is insignificantly slower than calling it once with count 
         //as png_read_rows has it's own loop which calls png_read_row count times.
         for (int i = 0; i < count; i++) {
-            png_read_rows(fCodec->fPng_ptr, &fSrcRow, png_bytepp_NULL, 1);
+            png_read_rows(this->png_ptr(), &fSrcRow, png_bytepp_NULL, 1);
         }
         return SkCodec::kSuccess;
     }
 
-    bool onReallyHasAlpha() const override { return fHasAlpha; }
-
-    SkEncodedFormat onGetEncodedFormat() const override { 
-        return kPNG_SkEncodedFormat;
+    AlphaState alphaInScanlineDecode() const override {
+        return fAlphaState;
     }
 
-
 private:
-    SkAutoTDelete<SkPngCodec>   fCodec;
-    bool                        fHasAlpha;
+    AlphaState                  fAlphaState;
     SkAutoMalloc                fStorage;
     uint8_t*                    fSrcRow;
 
-    typedef SkScanlineDecoder INHERITED;
+    typedef SkPngCodec INHERITED;
 };
 
 
-class SkPngInterlacedScanlineDecoder : public SkScanlineDecoder {
+class SkPngInterlacedScanlineDecoder : public SkPngCodec {
 public:
-    SkPngInterlacedScanlineDecoder(const SkImageInfo& srcInfo, SkPngCodec* codec)
-        : INHERITED(srcInfo)
-        , fCodec(codec)
-        , fHasAlpha(false)
-        , fCurrentRow(0)
-        , fHeight(srcInfo.height())
+    SkPngInterlacedScanlineDecoder(const SkImageInfo& srcInfo, SkStream* stream,
+            png_structp png_ptr, png_infop info_ptr, int bitDepth, int numberPasses)
+        : INHERITED(srcInfo, stream, png_ptr, info_ptr, bitDepth, numberPasses)
+        , fAlphaState(kUnknown_AlphaState)
+        , fHeight(-1)
         , fCanSkipRewind(false)
-    {}
+    {
+        SkASSERT(numberPasses != 1);
+    }
 
-    SkCodec::Result onStart(const SkImageInfo& dstInfo,
-                            const SkCodec::Options& options,
-                            SkPMColor ctable[], int* ctableCount) override
+    Result onStart(const SkImageInfo& dstInfo, const Options& options,
+                   SkPMColor ctable[], int* ctableCount) override
     {
-        if (!fCodec->rewindIfNeeded()) {
-            return SkCodec::kCouldNotRewind;
+        if (!this->rewindIfNeeded()) {
+            return kCouldNotRewind;
         }
 
         if (!conversion_possible(dstInfo, this->getInfo())) {
-            return SkCodec::kInvalidConversion;    
+            return kInvalidConversion;    
         }
 
         // Check to see if scaling was requested.
         if (dstInfo.dimensions() != this->getInfo().dimensions()) {
             if (!SkScaledCodec::DimensionsSupportedForSampling(this->getInfo(), dstInfo)) {
-                return SkCodec::kInvalidScale;
+                return kInvalidScale;
             }
         }
 
-        const SkCodec::Result result = fCodec->initializeSwizzler(dstInfo, options, ctable,
-                                                                  ctableCount);
-        if (result != SkCodec::kSuccess) {
+        const Result result = this->initializeSwizzler(dstInfo, options, ctable,
+                                                       ctableCount);
+        if (result != kSuccess) {
             return result;
         }
 
-        fHasAlpha = false;
-        fCurrentRow = 0;
+        fAlphaState = kUnknown_AlphaState;
         fHeight = dstInfo.height();
-        fSrcRowBytes = this->getInfo().width() * SkSwizzler::BytesPerPixel(fCodec->fSrcConfig);
+        // FIXME: This need not be called on a second call to onStart.
+        fSrcRowBytes = this->getInfo().width() * SkSwizzler::BytesPerPixel(this->srcConfig());
         fGarbageRow.reset(fSrcRowBytes);
         fGarbageRowPtr = static_cast<uint8_t*>(fGarbageRow.get());
         fCanSkipRewind = true;
 
         return SkCodec::kSuccess;
     }
 
-    SkCodec::Result onGetScanlines(void* dst, int count, size_t dstRowBytes) override {
+    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
         if (fCanSkipRewind) {
             // We already rewound in onStart, so there is no reason to rewind.
             // Next time onGetScanlines is called, we will need to rewind.
             fCanSkipRewind = false;
-        } else if (!fCodec->rewindIfNeeded()) {
-            return SkCodec::kCouldNotRewind;
+        } else {
+            // rewindIfNeeded resets fCurrScanline, since it assumes that start
+            // needs to be called again before scanline decoding. PNG scanline
+            // decoding is the exception, since it needs to rewind between
+            // calls to getScanlines. Keep track of fCurrScanline, to undo the
+            // reset.
+            const int currScanline = this->onGetY();
+            // This method would never be called if currScanline is -1
+            SkASSERT(currScanline != -1);
+
+            if (!this->rewindIfNeeded()) {
+                return kCouldNotRewind;
+            }
+            this->updateY(currScanline);
         }
 
-        if (setjmp(png_jmpbuf(fCodec->fPng_ptr))) {
+        if (setjmp(png_jmpbuf(this->png_ptr()))) {
             SkCodecPrintf("setjmp long jump!\n");
-            return SkCodec::kInvalidInput;
+            return kInvalidInput;
         }
-        const int number_passes = png_set_interlace_handling(fCodec->fPng_ptr);
         SkAutoMalloc storage(count * fSrcRowBytes);
         uint8_t* storagePtr = static_cast<uint8_t*>(storage.get());
         uint8_t* srcRow;
-        for (int i = 0; i < number_passes; i++) {
-            //read rows we planned to skip into garbage row
-            for (int y = 0; y < fCurrentRow; y++){
-                png_read_rows(fCodec->fPng_ptr, &fGarbageRowPtr, png_bytepp_NULL, 1);
+        const int startRow = this->onGetY();
+        for (int i = 0; i < this->numberPasses(); i++) {
+            // read rows we planned to skip into garbage row
+            for (int y = 0; y < startRow; y++){
+                png_read_rows(this->png_ptr(), &fGarbageRowPtr, png_bytepp_NULL, 1);
             }
-            //read rows we care about into buffer
+            // read rows we care about into buffer
             srcRow = storagePtr;
             for (int y = 0; y < count; y++) {
-                png_read_rows(fCodec->fPng_ptr, &srcRow, png_bytepp_NULL, 1);
+                png_read_rows(this->png_ptr(), &srcRow, png_bytepp_NULL, 1);
                 srcRow += fSrcRowBytes;
             }
-            //read rows we don't want into garbage buffer
-            for (int y = 0; y < fHeight - fCurrentRow - count; y++) {
-                png_read_rows(fCodec->fPng_ptr, &fGarbageRowPtr, png_bytepp_NULL, 1);
+            // read rows we don't want into garbage buffer
+            for (int y = 0; y < fHeight - startRow - count; y++) {
+                png_read_rows(this->png_ptr(), &fGarbageRowPtr, png_bytepp_NULL, 1);
             }
         }
         //swizzle the rows we care about
         srcRow = storagePtr;
         void* dstRow = dst;
+        bool hasAlpha = false;
         for (int y = 0; y < count; y++) {
-            fHasAlpha |= !SkSwizzler::IsOpaque(fCodec->fSwizzler->swizzle(dstRow, srcRow));
+            hasAlpha |= !SkSwizzler::IsOpaque(this->swizzler()->swizzle(dstRow, srcRow));
             dstRow = SkTAddOffset<void>(dstRow, dstRowBytes);
             srcRow += fSrcRowBytes;
         }
-        fCurrentRow += count;
+
+        if (hasAlpha) {
+            fAlphaState = kHasAlpha_AlphaState;
+        } else {
+            if (kUnknown_AlphaState == fAlphaState) {
+                fAlphaState = kOpaque_AlphaState;
+            }
+            // Otherwise, the AlphaState is unchanged.
+        }
+
+        return kSuccess;
+    }
+
+    SkCodec::Result onSkipScanlines(int count) override {
+        // The non-virtual version will update fCurrScanline.
         return SkCodec::kSuccess;
     }
 
-    SkCodec::Result onSkipScanlines(int count) override {
-        //when ongetScanlines is called it will skip to fCurrentRow
-        fCurrentRow += count;
-        return SkCodec::kSuccess;
+    AlphaState alphaInScanlineDecode() const override {
+        return fAlphaState;
     }
 
-    bool onReallyHasAlpha() const override { return fHasAlpha; }
-
     SkScanlineOrder onGetScanlineOrder() const override {
         return kNone_SkScanlineOrder;
     }
 
-    SkEncodedFormat onGetEncodedFormat() const override { 
-        return kPNG_SkEncodedFormat;
-    }
-
 private:
-    SkAutoTDelete<SkPngCodec>   fCodec;
-    bool                        fHasAlpha;
-    int                         fCurrentRow;
+    AlphaState                  fAlphaState;
     int                         fHeight;
     size_t                      fSrcRowBytes;
     SkAutoMalloc                fGarbageRow;
     uint8_t*                    fGarbageRowPtr;
     // FIXME: This imitates behavior in SkCodec::rewindIfNeeded. That function
     // is called whenever some action is taken that reads the stream and
     // therefore the next call will require a rewind. So it modifies a boolean
     // to note that the *next* time it is called a rewind is needed.
     // SkPngInterlacedScanlineDecoder has an extra wrinkle - calling onStart
     // followed by onGetScanlines does *not* require a rewind. Since
     // rewindIfNeeded does not have this flexibility, we need to add another
     // layer.
     bool                        fCanSkipRewind;
 
-    typedef SkScanlineDecoder INHERITED;
+    typedef SkPngCodec INHERITED;
 };
 
-SkScanlineDecoder* SkPngCodec::NewSDFromStream(SkStream* stream) {
-    SkAutoTDelete<SkPngCodec> codec (static_cast<SkPngCodec*>(SkPngCodec::NewFromStream(stream)));
-    if (!codec) {
+SkCodec* SkPngCodec::NewFromStream(SkStream* stream) {
+    SkAutoTDelete<SkStream> streamDeleter(stream);
+    png_structp png_ptr;
+    png_infop info_ptr;
+    SkImageInfo imageInfo;
+    int bitDepth;
+    int numberPasses;
+
+    if (!read_header(stream, &png_ptr, &info_ptr, &imageInfo, &bitDepth, &numberPasses)) {
         return nullptr;
     }
 
-    codec->fNumberPasses = png_set_interlace_handling(codec->fPng_ptr);
-    SkASSERT(codec->fNumberPasses != INVALID_NUMBER_PASSES);
-
-    const SkImageInfo& srcInfo = codec->getInfo();
-    if (codec->fNumberPasses > 1) {
-        // interlaced image
-        return new SkPngInterlacedScanlineDecoder(srcInfo, codec.detach());
+    if (1 == numberPasses) {
+        return new SkPngScanlineDecoder(imageInfo, streamDeleter.detach(), png_ptr, info_ptr,
+                                        bitDepth);
     }
 
-    return new SkPngScanlineDecoder(srcInfo, codec.detach());
+    return new SkPngInterlacedScanlineDecoder(imageInfo, streamDeleter.detach(), png_ptr,
+                                              info_ptr, bitDepth, numberPasses);
 }