Fill incomplete images in SkCodec parent class

src/codec/SkCodec_libgif.cpp

Index: src/codec/SkCodec_libgif.cpp
diff --git a/src/codec/SkCodec_libgif.cpp b/src/codec/SkCodec_libgif.cpp
index 03980b586a1df9a6b3c2d41b213614b7e8f93465..106894b61b9dbf2b583d946d048d6dce9dea5598 100644
--- a/src/codec/SkCodec_libgif.cpp
+++ b/src/codec/SkCodec_libgif.cpp
@@ -530,8 +530,8 @@ SkCodec::Result SkGifCodec::onGetPixels(const SkImageInfo& dstInfo,
 
         // Fill the background
         const SkPMColor* colorPtr = get_color_ptr(fColorTable.get());
-        SkSwizzler::Fill(dst, dstInfo, dstRowBytes, this->getInfo().height(),
-                fFillIndex, colorPtr, opts.fZeroInitialized);
+        SkSwizzler::Fill(dst, dstInfo, dstRowBytes, this->getFillValue(dstInfo),
+                opts.fZeroInitialized);
 
         // Modify the dst pointer
         const int32_t dstBytesPerPixel = SkColorTypeBytesPerPixel(dstInfo.colorType());
@@ -552,12 +552,16 @@ SkCodec::Result SkGifCodec::onGetPixels(const SkImageInfo& dstInfo,
         // rearrange the decoded rows.
         for (uint32_t y = 0; y < height; y++) {
             if (kSuccess != this->readRow()) {
-                // Recover from error by filling remainder of image
-                memset(fSrcBuffer.get(), fFillIndex, width);
+                // Incomplete interlaced gifs are a unique case.  Rather than,
+                // relying on the base class, we will fill the remaining lines
+                // here.
+                const SkImageInfo fillInfo = dstInfo.makeWH(dstInfo.width(), 1);
+                const uint32_t fillValue = this->getFillValue(fillInfo);
                 for (; y < height; y++) {
                     void* dstRow = SkTAddOffset<void>(dst,
                             dstRowBytes * get_output_row_interlaced(y, height));
-                    fSwizzler->swizzle(dstRow, fSrcBuffer.get());
+                    SkSwizzler::Fill(dstRow, fillInfo, dstRowBytes, fillValue,
+                            opts.fZeroInitialized);
                 }
                 return gif_error("Could not decode line.\n", kIncompleteInput);
             }
@@ -570,9 +574,7 @@ SkCodec::Result SkGifCodec::onGetPixels(const SkImageInfo& dstInfo,
         void* dstRow = dst;
         for (uint32_t y = 0; y < height; y++) {
             if (kSuccess != this->readRow()) {
-                const SkPMColor* colorPtr = get_color_ptr(fColorTable.get());
-                SkSwizzler::Fill(dstRow, dstInfo, dstRowBytes,
-                        height - y, fFillIndex, colorPtr, opts.fZeroInitialized);
+                this->setIncompleteScanlines(height - y);
                 return gif_error("Could not decode line\n", kIncompleteInput);
             }
             fSwizzler->swizzle(dstRow, fSrcBuffer.get());
@@ -582,14 +584,19 @@ SkCodec::Result SkGifCodec::onGetPixels(const SkImageInfo& dstInfo,
     return kSuccess;
 }
 
+uint32_t SkGifCodec::onGetFillValue(const SkImageInfo& dstInfo) const {
+    const SkPMColor* colorPtr = get_color_ptr(fColorTable.get());
+    return get_color_table_fill_value(dstInfo.colorType(), colorPtr, fFillIndex);
+}
+
 // TODO (msarett): skbug.com/3582
 //                 Should we implement reallyHasAlpha?  Or should we read extension blocks in the
 //                 header?  Or should we do both?
 
 class SkGifScanlineDecoder : public SkScanlineDecoder {
 public:
-    SkGifScanlineDecoder(const SkImageInfo& srcInfo, SkGifCodec* codec)
-        : INHERITED(srcInfo)
+    SkGifScanlineDecoder(SkGifCodec* codec)
+        : INHERITED(codec, codec->getInfo())
         , fCodec(codec)
     {}
 
@@ -632,12 +639,11 @@ public:
         return SkCodec::kSuccess;
     }
 
-    SkCodec::Result onGetScanlines(void* dst, int count, size_t rowBytes) override {
+    uint32_t onGetScanlines(void* dst, int count, size_t rowBytes) override {
         if (fCodec->fFrameIsSubset) {
             // Fill the requested rows
-            const SkPMColor* colorPtr = get_color_ptr(fCodec->fColorTable.get());
-            SkSwizzler::Fill(dst, this->dstInfo(), rowBytes, count, fCodec->fFillIndex,
-                    colorPtr, this->options().fZeroInitialized);
+            SkSwizzler::Fill(dst, this->dstInfo().makeWH(this->dstInfo().width(), count), rowBytes,
+                    fCodec->onGetFillValue(dstInfo()), this->options().fZeroInitialized);
 
             // Do nothing for rows before the image frame
             int rowsBeforeFrame = fCodec->fFrameDims.top() - INHERITED::getY();
@@ -657,18 +663,15 @@ public:
                     this->dstInfo().colorType()) * fCodec->fFrameDims.left());
         }
 
+        void* dstRow = dst;
         for (int i = 0; i < count; i++) {
             if (SkCodec::kSuccess != fCodec->readRow()) {

[scroggo, 2015/09/22 18:02:48]

Should readRow just return a boolean?

[msarett, 2015/09/23 13:22:40]

Yes!

-                const SkPMColor* colorPtr = get_color_ptr(fCodec->fColorTable.get());
-                SkSwizzler::Fill(dst, this->dstInfo(), rowBytes,
-                        count - i, fCodec->fFillIndex, colorPtr,
-                        this->options().fZeroInitialized);
-                return gif_error("Could not decode line\n", SkCodec::kIncompleteInput);
+                return i;
             }
-            fCodec->fSwizzler->swizzle(dst, fCodec->fSrcBuffer.get());
-            dst = SkTAddOffset<void>(dst, rowBytes);
+            fCodec->fSwizzler->swizzle(dstRow, fCodec->fSrcBuffer.get());
+            dstRow = SkTAddOffset<void>(dstRow, rowBytes);
         }
-        return SkCodec::kSuccess;
+        return count;
     }
 
     SkScanlineOrder onGetScanlineOrder() const override {
@@ -679,16 +682,16 @@ public:
         }
     }
 
-    int onGetY() const override {
+    int onGetY(int encodedY) const override {
         if (fCodec->fGif->Image.Interlace) {
-            return get_output_row_interlaced(INHERITED::onGetY(), this->dstInfo().height());
+            return get_output_row_interlaced(encodedY, this->dstInfo().height());
         } else {
-            return INHERITED::onGetY();
+            return encodedY;
         }
     }
 
 private:
-    SkAutoTDelete<SkGifCodec>   fCodec;
+    SkGifCodec* fCodec; // Owned by parent class
 
     typedef SkScanlineDecoder INHERITED;
 };
@@ -699,7 +702,5 @@ SkScanlineDecoder* SkGifCodec::NewSDFromStream(SkStream* stream) {
         return NULL;
     }
 
-    const SkImageInfo& srcInfo = codec->getInfo();
-
-    return SkNEW_ARGS(SkGifScanlineDecoder, (srcInfo, codec.detach()));
+    return SkNEW_ARGS(SkGifScanlineDecoder, (codec.detach()));
 }