Fix SkGifCodec to handle gifs where frameSize != imageSize

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 167 matching lines @@
 
     // Read the image descriptor
     if (GIF_ERROR == DGifGetImageDesc(gif)) {
         return false;
     }
     // If reading the image descriptor is successful, the image count will be
     // incremented.
     SkASSERT(gif->ImageCount >= 1);
 
     if (nullptr != codecOut) {
-        // Get fields from header
-        const int32_t width = gif->SWidth;
-        const int32_t height = gif->SHeight;
-        if (width <= 0 || height <= 0) {
-            gif_error("Invalid dimensions.\n");
-            return false;
+        SkISize size;
+        SkIRect frameRect;
+        if (!GetDimensions(gif, &size, &frameRect)) {
+            gif_error("Invalid gif size.\n");
         }
+        bool frameIsSubset = (size != frameRect.size());
 
         // Determine the recommended alpha type.  The transIndex might be valid if it less
         // than 256.  We are not certain that the index is valid until we process the color
         // table, since some gifs have color tables with less than 256 colors.  If
         // there might be a valid transparent index, we must indicate that the image has
         // alpha.
         // In the case where we must support alpha, we have the option to set the
         // suggested alpha type to kPremul or kUnpremul.  Both are valid since the alpha
         // component will always be 0xFF or the entire 32-bit pixel will be set to zero.
         // We prefer kPremul because we support kPremul, and it is more efficient to use
         // kPremul directly even when kUnpremul is supported.
         SkAlphaType alphaType = (transIndex < 256) ? kPremul_SkAlphaType : kOpaque_SkAlphaType;
 
         // Return the codec
         // kIndex is the most natural color type for gifs, so we set this as
         // the default.
-        const SkImageInfo& imageInfo = SkImageInfo::Make(width, height,
-                kIndex_8_SkColorType, alphaType);
-        *codecOut = new SkGifCodec(imageInfo, streamDeleter.detach(), gif.detach(), transIndex);
+        SkImageInfo imageInfo = SkImageInfo::Make(size.width(), size.height(), kIndex_8_SkColorType,
+                alphaType);
+        *codecOut = new SkGifCodec(imageInfo, streamDeleter.detach(), gif.detach(), transIndex,
+                frameRect, frameIsSubset);
     } else {
         SkASSERT(nullptr != gifOut);
         streamDeleter.detach();
         *gifOut = gif.detach();
     }
     return true;
 }
 
 /*
  * Assumes IsGif was called and returned true
  * Creates a gif decoder
  * Reads enough of the stream to determine the image format
  */
 SkCodec* SkGifCodec::NewFromStream(SkStream* stream) {
     SkCodec* codec = nullptr;
     if (ReadHeader(stream, &codec, nullptr)) {
         return codec;
     }
     return nullptr;
 }
 
 SkGifCodec::SkGifCodec(const SkImageInfo& srcInfo, SkStream* stream, GifFileType* gif,
-        uint32_t transIndex)
+        uint32_t transIndex, const SkIRect& frameRect, bool frameIsSubset)
     : INHERITED(srcInfo, stream)
     , fGif(gif)
     , fSrcBuffer(new uint8_t[this->getInfo().width()])
     // If it is valid, fTransIndex will be used to set fFillIndex.  We don't know if
     // fTransIndex is valid until we process the color table, since fTransIndex may
     // be greater than the size of the color table.
     , fTransIndex(transIndex)
     // Default fFillIndex is 0.  We will overwrite this if fTransIndex is valid, or if
     // there is a valid background color.
     , fFillIndex(0)
-    , fFrameDims(SkIRect::MakeEmpty())
-    , fFrameIsSubset(false)
+    , fFrameRect(frameRect)
+    , fFrameIsSubset(frameIsSubset)
     , fColorTable(NULL)
     , fSwizzler(NULL)
 {}
 
 bool SkGifCodec::onRewind() {
     GifFileType* gifOut = nullptr;
     if (!ReadHeader(this->stream(), nullptr, &gifOut)) {
         return false;
     }
 
@@ skipping 17 matching lines @@
     // contain more than one image, we will simply decode the first image.
     GifRecordType recordType;
     do {
         // Get the current record type
         if (GIF_ERROR == DGifGetRecordType(gif, &recordType)) {
             return gif_error("DGifGetRecordType failed.\n", kInvalidInput);
         }
         switch (recordType) {
             case IMAGE_DESC_RECORD_TYPE: {
                 *transIndex = find_trans_index(saveExt);
+
                 // FIXME: Gif files may have multiple images stored in a single
                 //        file.  This is most commonly used to enable
                 //        animations.  Since we are leaving animated gifs as a
                 //        TODO, we will return kSuccess after decoding the
                 //        first image in the file.  This is the same behavior
                 //        as SkImageDecoder_libgif.
                 //
                 //        Most times this works pretty well, but sometimes it
                 //        doesn't.  For example, I have an animated test image
                 //        where the first image in the file is 1x1, but the
@@ skipping 43 matching lines @@
                 // not match one of the above cases.  This should not be
                 // reached.
                 SkASSERT(false);
                 break;
         }
     } while (TERMINATE_RECORD_TYPE != recordType);
 
     return gif_error("Could not find any images to decode in gif file.\n", kInvalidInput);
 }
 
-/*
- * A gif may contain many image frames, all of different sizes.
- * This function checks if the frame dimensions are valid and corrects them if
- * necessary.
- */
-bool SkGifCodec::setFrameDimensions(const GifImageDesc& desc) {
-    // Fail on non-positive dimensions
-    int32_t frameLeft = desc.Left;
-    int32_t frameTop = desc.Top;
-    int32_t frameWidth = desc.Width;
-    int32_t frameHeight = desc.Height;
-    int32_t height = this->getInfo().height();
-    int32_t width = this->getInfo().width();
-    if (frameWidth <= 0 || frameHeight <= 0) {
+bool SkGifCodec::GetDimensions(GifFileType* gif, SkISize* size, SkIRect* frameRect) {
+    // Get the encoded dimension values
+    SavedImage* image = &gif->SavedImages[gif->ImageCount - 1];
+    const GifImageDesc& desc = image->ImageDesc;
+    int frameLeft = desc.Left;
+    int frameTop = desc.Top;
+    int frameWidth = desc.Width;
+    int frameHeight = desc.Height;
+    int width = gif->SWidth;
+    int height = gif->SHeight;
+
+    // Ensure that the decode dimensions are large enough to contain the frame
+    width = SkTMax(width, frameWidth + frameLeft);
+    height = SkTMax(height, frameHeight + frameTop);
+
+    // All of these dimensions should be positive, as they are encoded as unsigned 16-bit integers.
+    // It is unclear why giflib casts them to ints.  We will go ahead and check that they are
+    // in fact positive.
+    if (frameLeft < 0 || frameTop < 0 || frameWidth < 0 || frameHeight < 0 || width <= 0 ||
+            height <= 0) {
         return false;
     }
 
-    // Treat the following cases as warnings and try to fix
-    if (frameWidth > width) {
-        gif_warning("Image frame too wide, shrinking.\n");
-        frameWidth = width;
-        frameLeft = 0;
-    } else if (frameLeft + frameWidth > width) {
-        gif_warning("Shifting image frame to left to fit.\n");
-        frameLeft = width - frameWidth;
-    } else if (frameLeft < 0) {
-        gif_warning("Shifting image frame to right to fit\n");
-        frameLeft = 0;
-    }
-    if (frameHeight > height) {
-        gif_warning("Image frame too tall, shrinking.\n");
-        frameHeight = height;
-        frameTop = 0;
-    } else if (frameTop + frameHeight > height) {
-        gif_warning("Shifting image frame up to fit.\n");
-        frameTop = height - frameHeight;
-    } else if (frameTop < 0) {
-        gif_warning("Shifting image frame down to fit\n");
-        frameTop = 0;
-    }
-    fFrameDims.setXYWH(frameLeft, frameTop, frameWidth, frameHeight);
-
-    // Indicate if the frame dimensions do not match the header dimensions
-    if (this->getInfo().dimensions() != fFrameDims.size()) {
-        fFrameIsSubset = true;
-    }
-
+    frameRect->setXYWH(frameLeft, frameTop, frameWidth, frameHeight);
+    size->set(width, height);
     return true;
 }
 
 void SkGifCodec::initializeColorTable(const SkImageInfo& dstInfo, SkPMColor* inputColorPtr,
         int* inputColorCount) {
     // Set up our own color table
     const uint32_t maxColors = 256;
     SkPMColor colorPtr[256];
     if (NULL != inputColorCount) {
         // We set the number of colors to maxColors in order to ensure
@@ skipping 52 matching lines @@
     // Check for valid input parameters
     if (opts.fSubset) {
         // Subsets are not supported.
         return kUnimplemented;
     }
     if (!conversion_possible(dstInfo, this->getInfo())) {
         return gif_error("Cannot convert input type to output type.\n",
                 kInvalidConversion);
     }
 
-
-    // We have asserted that the image count is at least one in ReadHeader().
-    SavedImage* image = &fGif->SavedImages[fGif->ImageCount - 1];
-    const GifImageDesc& desc = image->ImageDesc;
-
-    // Check that the frame dimensions are valid and set them
-    if(!this->setFrameDimensions(desc)) {
-        return gif_error("Invalid dimensions for image frame.\n", kInvalidInput);
-    }
-
     // Initialize color table and copy to the client if necessary
     this->initializeColorTable(dstInfo, inputColorPtr, inputColorCount);
     return kSuccess;
 }
 
 SkCodec::Result SkGifCodec::initializeSwizzler(const SkImageInfo& dstInfo,
         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(), fFrameDims.width())) {
+    if (GIF_ERROR == DGifGetLine(fGif, fSrcBuffer.get(), fFrameRect.width())) {
         return kIncompleteInput;
     }
     return kSuccess;
 }
 
 /*
  * Initiates the gif decode
  */
 SkCodec::Result SkGifCodec::onGetPixels(const SkImageInfo& dstInfo,
                                         void* dst, size_t dstRowBytes,
                                         const Options& opts,
                                         SkPMColor* inputColorPtr,
                                         int* inputColorCount) {
     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(fFrameDims.width(), fFrameDims.height());
+        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);
 
         // Modify the dst pointer
         const int32_t dstBytesPerPixel = SkColorTypeBytesPerPixel(dstInfo.colorType());
-        dst = SkTAddOffset<void*>(dst, dstRowBytes * fFrameDims.top() +
-                dstBytesPerPixel * fFrameDims.left());
+        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
-    uint32_t width = fFrameDims.width();
-    uint32_t height = fFrameDims.height();
+    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,
@@ skipping 26 matching lines @@
 SkCodec::Result SkGifCodec::onStartScanlineDecode(const SkImageInfo& dstInfo,
         const SkCodec::Options& opts, SkPMColor inputColorPtr[], int* inputColorCount) {
     Result result = this->prepareToDecode(dstInfo, inputColorPtr, inputColorCount,
                                           this->options());
     if (kSuccess != result) {
         return result;
     }
 
     // Initialize the swizzler
     if (fFrameIsSubset) {
-        const SkImageInfo subsetDstInfo = dstInfo.makeWH(fFrameDims.width(), fFrameDims.height());
+        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) {
     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);
 
         // Do nothing for rows before the image frame
-        // FIXME: nextScanline is not virtual, so using "INHERITED" does not change
-        // behavior. Was the intent to call this->INHERITED::onNextScanline()? Same
-        // for the next call down below.
-        int rowsBeforeFrame = fFrameDims.top() - this->INHERITED::nextScanline();
-        if (rowsBeforeFrame > 0) {
-            count = SkTMin(0, count - rowsBeforeFrame);
-            dst = SkTAddOffset<void>(dst, rowBytes * rowsBeforeFrame);
-        }
+        int rowsBeforeFrame = SkTMax(0, fFrameRect.top() - this->INHERITED::onNextScanline());
+        count = SkTMax(0, count - rowsBeforeFrame);
+        dst = SkTAddOffset<void>(dst, rowBytes * rowsBeforeFrame);
 
         // Do nothing for rows after the image frame
-        int rowsAfterFrame = this->INHERITED::nextScanline() + count - fFrameDims.bottom();
-        if (rowsAfterFrame > 0) {
-            count = SkTMin(0, count - rowsAfterFrame);
-        }
+        int rowsAfterFrame = SkTMax(0,
+                this->INHERITED::onNextScanline() + count - fFrameRect.bottom());
+        count = SkTMax(0, count - rowsAfterFrame);
 
-            // Adjust dst pointer for left offset
-        int bpp = SkColorTypeBytesPerPixel(this->dstInfo().colorType()) * fFrameDims.left();
-        dst = SkTAddOffset<void>(dst, bpp);
+        // 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(), rowBytes, count - i, fFillIndex, colorPtr,
-                             this->options().fZeroInitialized);
-            return gif_error("Could not decode line\n", SkCodec::kIncompleteInput);
+            SkSwizzler::Fill(dst, this->dstInfo().makeWH(fFrameRect.width(),
+                    this->dstInfo().height()), rowBytes, count - i, fFillIndex, colorPtr,
+                    this->options().fZeroInitialized);
+            return kIncompleteInput;
         }
         fSwizzler->swizzle(dst, fSrcBuffer.get());
         dst = SkTAddOffset<void>(dst, rowBytes);
     }
-    return SkCodec::kSuccess;
+    return kSuccess;
 }
 
 SkCodec::SkScanlineOrder SkGifCodec::onGetScanlineOrder() const {
     if (fGif->Image.Interlace) {
         return kOutOfOrder_SkScanlineOrder;
     } else {
         return kTopDown_SkScanlineOrder;
     }
 }
 
 int SkGifCodec::onNextScanline() const {
+    int nextScanline = this->INHERITED::onNextScanline();
     if (fGif->Image.Interlace) {
-        return get_output_row_interlaced(this->INHERITED::onNextScanline(), this->dstInfo().height());
-    } else {
-        return this->INHERITED::onNextScanline();
+        if (nextScanline < fFrameRect.top() || nextScanline >= fFrameRect.bottom()) {
+            return nextScanline;
+        }
+        return get_output_row_interlaced(nextScanline - fFrameRect.top(), fFrameRect.height());
     }
+    return nextScanline;
 }