Fill incomplete images in SkCodec parent class

src/codec/SkCodec.cpp

Index: src/codec/SkCodec.cpp
diff --git a/src/codec/SkCodec.cpp b/src/codec/SkCodec.cpp
index 1a901a97ac4314b210dd6deeeebf14d4957760e9..bafca852554db21a1e15fdda6b5b3c2a0dc2de45 100644
--- a/src/codec/SkCodec.cpp
+++ b/src/codec/SkCodec.cpp
@@ -167,11 +167,26 @@ SkCodec::Result SkCodec::getPixels(const SkImageInfo& info, void* pixels, size_t
         return kInvalidScale;
     }
 
-    const Result result = this->onGetPixels(info, pixels, rowBytes, *options, ctable, ctableCount);
+    // On an incomplete decode, the subclass will specify the number of scanlines that it decoded
+    // successfully.
+    int rowsDecoded = 0;
+    const Result result = this->onGetPixels(info, pixels, rowBytes, *options, ctable, ctableCount,
+            &rowsDecoded);
 
     if ((kIncompleteInput == result || kSuccess == result) && ctableCount) {
         SkASSERT(*ctableCount >= 0 && *ctableCount <= 256);
     }
+
+    // A return value of kIncompleteInput indicates a truncated image stream.
+    // In this case, we will fill any uninitialized memory with a default value.
+    // Some subclasses will take care of filling any uninitialized memory on
+    // their own.  They indicate that all of the memory has been filled by
+    // setting rowsDecoded equal to the height.
+    if (kIncompleteInput == result && rowsDecoded != info.height()) {
+        this->fillIncompleteImage(pixels, info, rowBytes, options->fZeroInitialized, info.height(),
+                rowsDecoded);
+    }
+
     return result;
 }
 
@@ -233,36 +248,101 @@ SkCodec::Result SkCodec::startScanlineDecode(const SkImageInfo& dstInfo) {
     return this->startScanlineDecode(dstInfo, nullptr, nullptr, nullptr);
 }
 
-SkCodec::Result SkCodec::getScanlines(void* dst, int countLines, size_t rowBytes) {
+int SkCodec::getScanlines(void* dst, int countLines, size_t rowBytes) {
     if (fCurrScanline < 0) {
-        return kScanlineDecodingNotStarted;
+        return 0;
     }
 
     SkASSERT(!fDstInfo.isEmpty());
-
     if (countLines <= 0 || fCurrScanline + countLines > fDstInfo.height()) {
-        return kInvalidParameters;
+        return 0;
     }
 
-    const Result result = this->onGetScanlines(dst, countLines, rowBytes);
+    const uint32_t linesDecoded = this->onGetScanlines(dst, countLines, rowBytes);

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

This should be an int. (There may be a few other places where you're still
mixing types.) I think one of our bots will catch you on this, since you're
comparing unsigned to signed down below.

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

Yeah it wouldn't surprise me if I haven't caught all of these...  Hopefully the
bots will help.

+    if (linesDecoded < countLines) {
+        this->fillIncompleteImage(dst, this->dstInfo(), rowBytes, this->options().fZeroInitialized,
+                countLines, linesDecoded);
+    }
     fCurrScanline += countLines;
-    return result;
+    return linesDecoded;
 }
 
-SkCodec::Result SkCodec::skipScanlines(int countLines) {
+bool SkCodec::skipScanlines(int countLines) {
     if (fCurrScanline < 0) {
-        return kScanlineDecodingNotStarted;
+        return false;
     }
 
     SkASSERT(!fDstInfo.isEmpty());
-    if (fCurrScanline + countLines > fDstInfo.height()) {
+    if (countLines < 0 || fCurrScanline + countLines > fDstInfo.height()) {
         // Arguably, we could just skip the scanlines which are remaining,
-        // and return kSuccess. We choose to return invalid so the client
+        // and return true. We choose to return false so the client
         // can catch their bug.
-        return SkCodec::kInvalidParameters;
+        return false;
     }
 
-    const Result result = this->onSkipScanlines(countLines);
+    bool result = this->onSkipScanlines(countLines);
     fCurrScanline += countLines;
     return result;
 }
+
+int SkCodec::outputScanline(int inputScanline) const {
+    SkASSERT(0 <= inputScanline && inputScanline < this->getInfo().height());
+    return this->onOutputScanline(inputScanline);
+}
+
+int SkCodec::onOutputScanline(int inputScanline) const {
+    switch (this->getScanlineOrder()) {
+        case kTopDown_SkScanlineOrder:
+        case kNone_SkScanlineOrder:
+            return inputScanline;
+        case kBottomUp_SkScanlineOrder:
+            return this->getInfo().height() - inputScanline - 1;
+        case kOutOfOrder_SkScanlineOrder:
+            // This case indicates an interlaced gif and is implemented by SkGifCodec.
+            SkASSERT(false);
+            return 0;
+    }
+}
+
+static void fill_proc(SkSampler* sampler, void* dst, const SkImageInfo& info, size_t rowBytes,

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

This order should probably match getPixels (and fillIncompleteImage, once you
update it) - info, dst, rowBytes. (I do not feel strongly about the other
parameters' ordering).

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

Done.

+        uint32_t colorOrIndex, SkCodec::ZeroInitialized zeroInit) {
+    if (sampler) {
+        sampler->fill(dst, info, rowBytes, colorOrIndex, zeroInit);
+    } else {
+        SkSampler::Fill(dst, info, rowBytes, colorOrIndex, zeroInit);
+    }
+}
+
+void SkCodec::fillIncompleteImage(void* dst, const SkImageInfo& info, size_t rowBytes,
+        ZeroInitialized zeroInit, int linesRequested, int linesDecoded) {
+
+    void* fillDst;
+    const uint32_t fillValue = this->getFillValue(info.colorType(), info.alphaType());
+    const int linesRemaining = linesRequested - linesDecoded;
+    SkSampler* sampler = this->getSampler(false);
+
+    switch (this->getScanlineOrder()) {
+        case kTopDown_SkScanlineOrder:
+        case kNone_SkScanlineOrder: {
+            const SkImageInfo fillInfo = info.makeWH(info.width(), linesRemaining);
+            fillDst = SkTAddOffset<void>(dst, linesDecoded * rowBytes);
+            fill_proc(sampler, fillDst, fillInfo, rowBytes, fillValue, zeroInit);
+            break;
+        }
+        case kBottomUp_SkScanlineOrder: {
+            fillDst = dst;
+            const SkImageInfo fillInfo = info.makeWH(info.width(), linesRemaining);
+            fill_proc(sampler, fillDst, fillInfo, rowBytes, fillValue, zeroInit);
+            break;
+        }
+        case kOutOfOrder_SkScanlineOrder: {
+            SkASSERT(1 == linesRequested || this->getInfo().height() == linesRequested);
+            const SkImageInfo fillInfo = info.makeWH(info.width(), 1);
+            for (int srcY = linesDecoded; srcY < linesRequested; srcY++) {
+                fillDst = SkTAddOffset<void>(dst, this->outputScanline(srcY) * rowBytes);
+                fill_proc(sampler, fillDst, fillInfo, rowBytes, fillValue, zeroInit);
+            }
+            break;
+        }
+    }
+}