Fill incomplete images in SkCodec parent class

src/codec/SkCodec_libgif.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_libgif.h"
 #include "SkCodecPriv.h"
 #include "SkColorPriv.h"
@@ skipping 77 matching lines @@
             // There should only be one graphics control extension for the image frame
             break;
         }
     }
 
     // Use maximum unsigned int (surely an invalid index) to indicate that a valid
     // index was not found.
     return SK_MaxU32;
 }
 
-static inline uint32_t ceil_div(uint32_t a, uint32_t b) {
-    return (a + b - 1) / b;
-}
-
-/*
- * Gets the output row corresponding to the encoded row for interlaced gifs
- */
-static uint32_t get_output_row_interlaced(uint32_t encodedRow, uint32_t height) {
-    SkASSERT(encodedRow < height);
-    // First pass
-    if (encodedRow * 8 < height) {
-        return encodedRow * 8;
-    }
-    // Second pass
-    if (encodedRow * 4 < height) {
-        return 4 + 8 * (encodedRow - ceil_div(height, 8));
-    }
-    // Third pass
-    if (encodedRow * 2 < height) {
-        return 2 + 4 * (encodedRow - ceil_div(height, 4));
-    }
-    // Fourth pass
-    return 1 + 2 * (encodedRow - ceil_div(height, 2));
-}
-
 /*
  * This function cleans up the gif object after the decode completes
  * It is used in a SkAutoTCallIProc template
  */
 void SkGifCodec::CloseGif(GifFileType* gif) {
     DGifCloseFile(gif, NULL);
 }
 
 /*
  * This function free extension data that has been saved to assist the image
@@ skipping 320 matching lines @@
         ZeroInitialized zeroInit) {
     const SkPMColor* colorPtr = get_color_ptr(fColorTable.get());
     fSwizzler.reset(SkSwizzler::CreateSwizzler(SkSwizzler::kIndex,
             colorPtr, dstInfo, zeroInit));
     if (nullptr != fSwizzler.get()) {
         return kSuccess;
     }
     return kUnimplemented;
 }
 
-SkCodec::Result SkGifCodec::readRow() {
-    if (GIF_ERROR == DGifGetLine(fGif, fSrcBuffer.get(), fFrameRect.width())) {
-        return kIncompleteInput;
-    }
-    return kSuccess;
+bool SkGifCodec::readRow() {
+    return GIF_ERROR != DGifGetLine(fGif, fSrcBuffer.get(), fFrameRect.width());
 }
 
 /*
  * Initiates the gif decode
  */
 SkCodec::Result SkGifCodec::onGetPixels(const SkImageInfo& dstInfo,
                                         void* dst, size_t dstRowBytes,
                                         const Options& opts,
                                         SkPMColor* inputColorPtr,
-                                        int* inputColorCount) {
+                                        int* inputColorCount,
+                                        int* rowsDecoded) {
     Result result = this->prepareToDecode(dstInfo, inputColorPtr, inputColorCount, opts);
     if (kSuccess != result) {
         return result;
     }
 
     if (dstInfo.dimensions() != this->getInfo().dimensions()) {
         return gif_error("Scaling not supported.\n", kInvalidScale);
     }
 
     // Initialize the swizzler
     if (fFrameIsSubset) {
         const SkImageInfo subsetDstInfo = dstInfo.makeWH(fFrameRect.width(), fFrameRect.height());
         if (kSuccess != this->initializeSwizzler(subsetDstInfo, opts.fZeroInitialized)) {
             return gif_error("Could not initialize swizzler.\n", kUnimplemented);
         }
 
         // Fill the background
-        const SkPMColor* colorPtr = get_color_ptr(fColorTable.get());
-        SkSwizzler::Fill(dst, dstInfo, dstRowBytes, this->getInfo().height(),
-                fFillIndex, colorPtr, opts.fZeroInitialized);
+        SkSampler::Fill(dst, dstInfo, dstRowBytes,
+                this->getFillValue(dstInfo.colorType(), dstInfo.alphaType()),
+                opts.fZeroInitialized);
 
         // Modify the dst pointer
         const int32_t dstBytesPerPixel = SkColorTypeBytesPerPixel(dstInfo.colorType());
         dst = SkTAddOffset<void*>(dst, dstRowBytes * fFrameRect.top() +
                 dstBytesPerPixel * fFrameRect.left());
     } else {
         if (kSuccess != this->initializeSwizzler(dstInfo, opts.fZeroInitialized)) {
             return gif_error("Could not initialize swizzler.\n", kUnimplemented);
         }
     }
 
-    // Check the interlace flag and iterate over rows of the input
+    // Iterate over rows of the input
     uint32_t width = fFrameRect.width();
     uint32_t height = fFrameRect.height();
-    if (fGif->Image.Interlace) {
-        // In interlace mode, the rows of input are rearranged in
-        // the output image.  We a helper function to help us
-        // 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);
-                for (; y < height; y++) {
-                    void* dstRow = SkTAddOffset<void>(dst,
-                            dstRowBytes * get_output_row_interlaced(y, height));
-                    fSwizzler->swizzle(dstRow, fSrcBuffer.get());
-                }
-                return gif_error("Could not decode line.\n", kIncompleteInput);
-            }
-            void* dstRow = SkTAddOffset<void>(dst,
-                    dstRowBytes * get_output_row_interlaced(y, height));
-            fSwizzler->swizzle(dstRow, fSrcBuffer.get());
+    for (uint32_t y = 0; y < height; y++) {
+        if (!this->readRow()) {
+            *rowsDecoded = y;
+            return gif_error("Could not decode line.\n", kIncompleteInput);
         }
-    } else {
-        // Standard mode
-        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);
-                return gif_error("Could not decode line\n", kIncompleteInput);
-            }
-            fSwizzler->swizzle(dstRow, fSrcBuffer.get());
-            dstRow = SkTAddOffset<void>(dstRow, dstRowBytes);
-        }
+        void* dstRow = SkTAddOffset<void>(dst, dstRowBytes * this->outputScanline(y));
+        fSwizzler->swizzle(dstRow, fSrcBuffer.get());
     }
     return kSuccess;
 }
 
+uint32_t SkGifCodec::onGetFillValue(SkColorType colorType, SkAlphaType alphaType) const {

[scroggo, 2015/10/08 13:50:30]

It looks like there are three classes with similar overrides for this function.
Not sure if we can combine them better...

We could have a virtual accessor for the color table, and then make getFillValue
non-virtual:

getFillValue() {
  SkColorTable* ct = this->getColorTable(); // calls a virtual
  if (ct) {
    return get_color_table_fill_value();
  }
  return // base implementation from onGetFillValue
}

Think that is better?

[msarett, 2015/10/08 15:33:25]

It won't quite work because gif may have an fFillIndex, while bmp and png will
just use 0 as the index.

I'll add a FIXME to see about sharing code better, but I don't think it's too
bad right now.

+    const SkPMColor* colorPtr = get_color_ptr(fColorTable.get());
+    return get_color_table_fill_value(colorType, colorPtr, fFillIndex);
+}
+
 SkCodec::Result SkGifCodec::onStartScanlineDecode(const SkImageInfo& dstInfo,
         const SkCodec::Options& opts, SkPMColor inputColorPtr[], int* inputColorCount) {
-    Result result = this->prepareToDecode(dstInfo, inputColorPtr, inputColorCount,
-                                          this->options());
+
+    Result result = this->prepareToDecode(dstInfo, inputColorPtr, inputColorCount, this->options());
     if (kSuccess != result) {
         return result;
     }
 
     // Initialize the swizzler
     if (fFrameIsSubset) {
         const SkImageInfo subsetDstInfo = dstInfo.makeWH(fFrameRect.width(), fFrameRect.height());
         if (kSuccess != this->initializeSwizzler(subsetDstInfo, opts.fZeroInitialized)) {
             return gif_error("Could not initialize swizzler.\n", kUnimplemented);
         }
     } else {
         if (kSuccess != this->initializeSwizzler(dstInfo, opts.fZeroInitialized)) {
             return gif_error("Could not initialize swizzler.\n", kUnimplemented);
         }
     }
 
     return kSuccess;
 }
 
-SkCodec::Result SkGifCodec::onGetScanlines(void* dst, int count, size_t rowBytes) {
+int SkGifCodec::onGetScanlines(void* dst, int count, size_t rowBytes) {
+    int rowsBeforeFrame = 0;
+    int rowsAfterFrame = 0;
+    int rowsInFrame = count;
     if (fFrameIsSubset) {
         // Fill the requested rows
-        const SkPMColor* colorPtr = get_color_ptr(fColorTable.get());
-        SkSwizzler::Fill(dst, this->dstInfo(), rowBytes, count, fFillIndex,
-                         colorPtr, this->options().fZeroInitialized);
+        SkSampler::Fill(dst, this->dstInfo().makeWH(this->dstInfo().width(), count), rowBytes,

[scroggo, 2015/10/08 13:50:30]

nit: some local variables would probably make this easier to read.

[msarett, 2015/10/08 15:33:25]

Done.

+                this->onGetFillValue(this->dstInfo().colorType(), this->dstInfo().alphaType()),
+                this->options().fZeroInitialized);
 
         // Do nothing for rows before the image frame
-        int rowsBeforeFrame = SkTMax(0, fFrameRect.top() - this->INHERITED::onNextScanline());
-        count = SkTMax(0, count - rowsBeforeFrame);
+        rowsBeforeFrame = SkTMax(0, fFrameRect.top() - this->INHERITED::nextScanline());
+        rowsInFrame = SkTMax(0, rowsInFrame - rowsBeforeFrame);
         dst = SkTAddOffset<void>(dst, rowBytes * rowsBeforeFrame);
 
         // Do nothing for rows after the image frame
-        int rowsAfterFrame = SkTMax(0,
-                this->INHERITED::onNextScanline() + count - fFrameRect.bottom());
-        count = SkTMax(0, count - rowsAfterFrame);
+        rowsAfterFrame = SkTMax(0,
+                this->INHERITED::nextScanline() + rowsInFrame - fFrameRect.bottom());
+        rowsInFrame = SkTMax(0, rowsInFrame - rowsAfterFrame);
 
         // Adjust dst pointer for left offset
         int offset = SkColorTypeBytesPerPixel(this->dstInfo().colorType()) * fFrameRect.left();
         dst = SkTAddOffset<void>(dst, offset);
     }
 
-    for (int i = 0; i < count; i++) {
-        if (kSuccess != this->readRow()) {
-            const SkPMColor* colorPtr = get_color_ptr(fColorTable.get());
-            SkSwizzler::Fill(dst, this->dstInfo().makeWH(fFrameRect.width(),
-                    this->dstInfo().height()), rowBytes, count - i, fFillIndex, colorPtr,
-                    this->options().fZeroInitialized);
-            return kIncompleteInput;
+    for (int i = 0; i < rowsInFrame; i++) {
+        if (!this->readRow()) {
+            return i + rowsBeforeFrame;
         }
         fSwizzler->swizzle(dst, fSrcBuffer.get());
         dst = SkTAddOffset<void>(dst, rowBytes);
     }
-    return kSuccess;
+
+    return count;
 }
 
 SkCodec::SkScanlineOrder SkGifCodec::onGetScanlineOrder() const {
     if (fGif->Image.Interlace) {
         return kOutOfOrder_SkScanlineOrder;
-    } else {
-        return kTopDown_SkScanlineOrder;
     }
+    return kTopDown_SkScanlineOrder;
 }
 
-int SkGifCodec::onNextScanline() const {
-    int nextScanline = this->INHERITED::onNextScanline();
+int SkGifCodec::onOutputScanline(int inputScanline) const {
     if (fGif->Image.Interlace) {
-        if (nextScanline < fFrameRect.top() || nextScanline >= fFrameRect.bottom()) {
-            return nextScanline;
+        if (inputScanline < fFrameRect.top() || inputScanline >= fFrameRect.bottom()) {
+            return inputScanline;
         }
-        return get_output_row_interlaced(nextScanline - fFrameRect.top(), fFrameRect.height());
+        return get_output_row_interlaced(inputScanline - fFrameRect.top(), fFrameRect.height());
     }
-    return nextScanline;
+    return inputScanline;
 }
-