Implement SkGifCodec::onSkipScanlines()

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 65503558aaab8635899f62a280ead32d96ad5605..393fb43642439b84f4fe903ea691d55552a2a8b0 100644
--- a/src/codec/SkCodec_libgif.cpp
+++ b/src/codec/SkCodec_libgif.cpp
@@ -506,26 +506,38 @@ SkCodec::Result SkGifCodec::onStartScanlineDecode(const SkImageInfo& dstInfo,
     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++) {
@@ -539,6 +551,22 @@ int SkGifCodec::onGetScanlines(void* dst, int count, size_t 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;