Fill incomplete images in SkCodec parent class

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"
@@ skipping 448 matching lines @@
         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,
-                                        SkPMColor ctable[], int* ctableCount) {
+                                        SkPMColor ctable[], int* ctableCount,
+                                        int* rowsDecoded) {
     if (!conversion_possible(requestedInfo, this->getInfo())) {
         return kInvalidConversion;
     }
     if (options.fSubset) {
         // Subsets are not supported.
         return kUnimplemented;
     }
 
     // Note that ctable and ctableCount may be modified if there is a color table
     const Result result = this->initializeSwizzler(requestedInfo, options,
                                                    ctable, ctableCount);
     if (result != kSuccess) {
         return result;
     }
     // FIXME: Could we use the return value of setjmp to specify the type of
     // error?
+    int row = 0;
+    // This must be declared above the call to setjmp to avoid memory leaks on incomplete images.
+    SkAutoMalloc storage;
     if (setjmp(png_jmpbuf(fPng_ptr))) {
-        SkCodecPrintf("setjmp long jump!\n");
-        return kInvalidInput;
+        // Assume that any error that occurs while reading rows is caused by an incomplete input.
+        if (fNumberPasses > 1) {
+            // FIXME (msarett): Handle incomplete interlaced pngs.
+            return kInvalidInput;
+        }
+        // FIXME: We do a poor job on incomplete pngs compared to other decoders (ex: Chromium,
+        // Ubuntu Image Viewer).  This is because we use the default buffer size in libpng (8192
+        // bytes), and if we can't fill the buffer, we immediately fail.
+        // For example, if we try to read 8192 bytes, and the image (incorrectly) only contains
+        // half that, which may have been enough to contain a non-zero number of lines, we fail
+        // when we could have decoded a few more lines and then failed.
+        // The read function that we provide for libpng has no way of indicating that we have
+        // made a partial read.
+        // Making our buffer size smaller improves our incomplete decodes, but what impact does
+        // it have on regular decode performance?  Should we investigate using a different API
+        // instead of png_read_row(s)?  Chromium uses png_process_data.
+        *rowsDecoded = row;
+        return kIncompleteInput;
     }
 
     bool hasAlpha = false;
     // FIXME: We could split these out based on subclass.
-    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++) {
             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++) {
+        for (; row < requestedInfo.height(); row++) {
             png_read_rows(fPng_ptr, &srcRow, png_bytepp_NULL, 1);
             // FIXME: Only call IsOpaque once, outside the loop. Same for onGetScanlines.
             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;
 }
 
+uint32_t SkPngCodec::onGetFillValue(SkColorType colorType, SkAlphaType alphaType) const {
+    const SkPMColor* colorPtr = get_color_ptr(fColorTable.get());
+    if (colorPtr) {
+        return get_color_table_fill_value(colorType, colorPtr, 0);
+    }
+    return INHERITED::onGetFillValue(colorType, alphaType);
+}
+
 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:
@@ skipping 34 matching lines @@
             return result;
         }
 
         fAlphaState = kUnknown_AlphaState;
         fStorage.reset(this->getInfo().width() * SkSwizzler::BytesPerPixel(this->srcConfig()));
         fSrcRow = static_cast<uint8_t*>(fStorage.get());
 
         return kSuccess;
     }
 
-    Result onGetScanlines(void* dst, int count, size_t rowBytes) override {
+    int onGetScanlines(void* dst, int count, size_t rowBytes) override {
+        // Assume that an error in libpng indicates an incomplete input.
+        int row = 0;
         if (setjmp(png_jmpbuf(this->png_ptr()))) {
             SkCodecPrintf("setjmp long jump!\n");
-            return kInvalidInput;
+            return row;
         }
 
         void* dstRow = dst;
         bool hasAlpha = false;
-        for (int i = 0; i < count; i++) {
+        for (; row < count; row++) {
             png_read_rows(this->png_ptr(), &fSrcRow, png_bytepp_NULL, 1);
             hasAlpha |= !SkSwizzler::IsOpaque(this->swizzler()->swizzle(dstRow, fSrcRow));
             dstRow = SkTAddOffset<void>(dstRow, rowBytes);
         }
 
         if (hasAlpha) {
             fAlphaState = kHasAlpha_AlphaState;
         } else {
             if (kUnknown_AlphaState == fAlphaState) {
                 fAlphaState = kOpaque_AlphaState;
             }
             // Otherwise, the AlphaState is unchanged.
         }
 
-        return kSuccess;
+        return row;
     }
 
-    Result onSkipScanlines(int count) override {
-        // FIXME: Could we use the return value of setjmp to specify the type of
-        // error?
+    bool onSkipScanlines(int count) override {
+        // Assume that an error in libpng indicates an incomplete input.
         if (setjmp(png_jmpbuf(this->png_ptr()))) {
             SkCodecPrintf("setjmp long jump!\n");
-            return kInvalidInput;
+            return false;
         }
         //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++) {
+        for (int row = 0; row < count; row++) {
             png_read_rows(this->png_ptr(), &fSrcRow, png_bytepp_NULL, 1);
         }
-        return SkCodec::kSuccess;
+        return true;
     }
 
     AlphaState alphaInScanlineDecode() const override {
         return fAlphaState;
     }
 
 private:
     AlphaState                  fAlphaState;
     SkAutoMalloc                fStorage;
     uint8_t*                    fSrcRow;
@@ skipping 31 matching lines @@
         fHeight = dstInfo.height();
         // FIXME: This need not be called on a second call to onStartScanlineDecode.
         fSrcRowBytes = this->getInfo().width() * SkSwizzler::BytesPerPixel(this->srcConfig());
         fGarbageRow.reset(fSrcRowBytes);
         fGarbageRowPtr = static_cast<uint8_t*>(fGarbageRow.get());
         fCanSkipRewind = true;
 
         return SkCodec::kSuccess;
     }
 
-    Result onGetScanlines(void* dst, int count, size_t dstRowBytes) override {
+    int 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 onStartScanlineDecode, so there is no reason to rewind.
             // Next time onGetScanlines is called, we will need to rewind.
             fCanSkipRewind = false;
         } 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->onNextScanline();
+            const int currScanline = this->nextScanline();
             // This method would never be called if currScanline is -1
             SkASSERT(currScanline != -1);
 
             if (!this->rewindIfNeeded()) {
                 return kCouldNotRewind;
             }
             this->updateNextScanline(currScanline);
         }
 
         if (setjmp(png_jmpbuf(this->png_ptr()))) {
             SkCodecPrintf("setjmp long jump!\n");
-            return kInvalidInput;
+            // FIXME (msarett): Returning 0 is pessimistic.  If we can complete a single pass,
+            // we may be able to report that all of the memory has been initialized.  Even if we
+            // fail on the first pass, we can still report than some scanlines are initialized.
+            return 0;
         }
         SkAutoMalloc storage(count * fSrcRowBytes);
         uint8_t* storagePtr = static_cast<uint8_t*>(storage.get());
         uint8_t* srcRow;
-        const int startRow = this->onNextScanline();
+        const int startRow = this->nextScanline();
         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
             srcRow = storagePtr;
             for (int y = 0; y < count; y++) {
                 png_read_rows(this->png_ptr(), &srcRow, png_bytepp_NULL, 1);
                 srcRow += fSrcRowBytes;
@@ skipping 15 matching lines @@
 
         if (hasAlpha) {
             fAlphaState = kHasAlpha_AlphaState;
         } else {
             if (kUnknown_AlphaState == fAlphaState) {
                 fAlphaState = kOpaque_AlphaState;
             }
             // Otherwise, the AlphaState is unchanged.
         }
 
-        return kSuccess;
+        return count;
     }
 
-    SkCodec::Result onSkipScanlines(int count) override {
+    bool onSkipScanlines(int count) override {
         // The non-virtual version will update fCurrScanline.
-        return SkCodec::kSuccess;
+        return true;
     }
 
     AlphaState alphaInScanlineDecode() const override {
         return fAlphaState;
     }
 
     SkScanlineOrder onGetScanlineOrder() const override {
         return kNone_SkScanlineOrder;
     }
 
@@ skipping 30 matching lines @@
 
     if (1 == numberPasses) {
         return new SkPngScanlineDecoder(imageInfo, streamDeleter.detach(), png_ptr, info_ptr,
                                         bitDepth);
     }
 
     return new SkPngInterlacedScanlineDecoder(imageInfo, streamDeleter.detach(), png_ptr,
                                               info_ptr, bitDepth, numberPasses);
 }