Implement SkGifCodec::onSkipScanlines()

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 488 matching lines @@
 uint32_t SkGifCodec::onGetFillValue(SkColorType colorType, SkAlphaType alphaType) const {
     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) {
     return this->prepareToDecode(dstInfo, inputColorPtr, inputColorCount, this->options());
 }
 
+void SkGifCodec::handleScanlineFrame(int count, int* rowsBeforeFrame, int* rowsInFrame) {

[scroggo, 2015/11/19 22:23:06]

SkASSERT(fFrameIsSubset);

[msarett, 2015/11/19 22:39:22]

Acknowledged.

+    // The number of rows that remain to be skipped before reaching rows that we
+    // actually must decode into.
+    // This must be at least zero.  We also make sure that it is less than or
+    // equal to count, since we will skip at most count rows.
+    *rowsBeforeFrame = SkTMin(count, SkTMax(0, fFrameRect.top() - INHERITED::nextScanline()));

[scroggo, 2015/11/19 22:23:06]

Would it make sense to cache the value of nextScanline locally?

[msarett, 2015/11/19 22:39:22]

Yeah I think so.

+
+    // Rows left to decode once we reach the start of the frame.
+    int rowsLeft = count - *rowsBeforeFrame;

[scroggo, 2015/11/19 22:23:06]

could be const?

[msarett, 2015/11/19 22:39:22]

Done.

+
+    // Count the number of that extend beyond the bottom of the frame.  We do not
+    // need to decode into these rows.
+    int rowsAfterFrame = SkTMax(0, INHERITED::nextScanline() + rowsLeft - fFrameRect.bottom());
+
+    // Set the actual number of source rows that we need to decode.
+    *rowsInFrame = rowsLeft - rowsAfterFrame;
+}
+
 int SkGifCodec::onGetScanlines(void* dst, int count, size_t rowBytes) {
     int rowsBeforeFrame = 0;
-    int rowsAfterFrame = 0;
     int rowsInFrame = count;
     if (fFrameIsSubset) {
+        this->handleScanlineFrame(count, &rowsBeforeFrame, &rowsInFrame);

[msarett, 2015/11/19 21:37:23]

We could do without the count parameter since we set rowsInFrame to count before
calling.  But I'm not sure if that's too confusing.

[scroggo, 2015/11/19 22:23:06]

Either way. We sometimes use the same variable as input/output, but it might be
clearer to use separate variables.

As an alternative, if you always called handleScanlineFrame, and it checked
fFrameIsSubset, then you could let handleScanlineFrame always initialize it. I
suppose a downside is that you'll then check fFrameIsSubset twice. 

[msarett, 2015/11/19 22:39:22]

Yeah I thought about doing it that way...

I think you're right, I prefer that strategy.

+
         // Fill the requested rows
         SkImageInfo fillInfo = this->dstInfo().makeWH(this->dstInfo().width(), count);
         uint32_t fillValue = this->onGetFillValue(this->dstInfo().colorType(),
                 this->dstInfo().alphaType());
         fSwizzler->fill(fillInfo, dst, rowBytes, fillValue, this->options().fZeroInitialized);
 
-        // Do nothing for rows before the image frame
-        rowsBeforeFrame = SkTMax(0, fFrameRect.top() - this->INHERITED::nextScanline());
-        rowsInFrame = SkTMax(0, rowsInFrame - rowsBeforeFrame);
+        // Start to write pixels at the start of the image frame
         dst = SkTAddOffset<void>(dst, rowBytes * rowsBeforeFrame);
-
-        // Do nothing for rows after the image frame
-        rowsAfterFrame = SkTMax(0,
-                this->INHERITED::nextScanline() + rowsInFrame - fFrameRect.bottom());
-        rowsInFrame = SkTMax(0, rowsInFrame - rowsAfterFrame);
     }
 
     for (int i = 0; i < rowsInFrame; i++) {
         if (!this->readRow()) {
             return i + rowsBeforeFrame;
         }
         fSwizzler->swizzle(dst, fSrcBuffer.get());
         dst = SkTAddOffset<void>(dst, rowBytes);
     }
 
     return count;
 }
 
+bool SkGifCodec::onSkipScanlines(int count) {
+    int rowsBeforeFrame = 0;
+    int rowsInFrame = count;
+    if (fFrameIsSubset) {
+        this->handleScanlineFrame(count, &rowsBeforeFrame, &rowsInFrame);
+    }
+
+    for (int i = 0; i < rowsInFrame; i++) {
+        if (!this->readRow()) {
+            return i + rowsBeforeFrame;
+        }
+    }
+
+    return count;
+}
+
 SkCodec::SkScanlineOrder SkGifCodec::onGetScanlineOrder() const {
     if (fGif->Image.Interlace) {
         return kOutOfOrder_SkScanlineOrder;
     }
     return kTopDown_SkScanlineOrder;
 }
 
 int SkGifCodec::onOutputScanline(int inputScanline) const {
     if (fGif->Image.Interlace) {
         if (inputScanline < fFrameRect.top() || inputScanline >= fFrameRect.bottom()) {
             return inputScanline;
         }
         return get_output_row_interlaced(inputScanline - fFrameRect.top(), fFrameRect.height()) +
                 fFrameRect.top();
     }
     return inputScanline;
 }