Fill incomplete images in SkCodec parent class

include/codec/SkScanlineDecoder.h

Index: include/codec/SkScanlineDecoder.h
diff --git a/include/codec/SkScanlineDecoder.h b/include/codec/SkScanlineDecoder.h
index 61184d644a15d9043e2fef92f3aa1e54a01e21e8..8c90b98ca040061f42d0afcc69d93c274c73c39c 100644
--- a/include/codec/SkScanlineDecoder.h
+++ b/include/codec/SkScanlineDecoder.h
@@ -100,16 +100,17 @@ public:
      *  @param countLines Number of lines to write.
      *  @param rowBytes Number of bytes per row. Must be large enough to hold
      *      a scanline based on the SkImageInfo used to create this object.
+     *  @return the number of lines successfully decoded
      */
-    SkCodec::Result getScanlines(void* dst, int countLines, size_t rowBytes) {
+    uint32_t getScanlines(void* dst, int countLines, size_t rowBytes) {
         SkASSERT(!fDstInfo.isEmpty());
         if ((rowBytes < fDstInfo.minRowBytes() && countLines > 1 ) || countLines <= 0
                 || fCurrScanline + countLines > fDstInfo.height()) {
             return SkCodec::kInvalidParameters;
         }
-        const SkCodec::Result result = this->onGetScanlines(dst, countLines, rowBytes);
-        fCurrScanline += countLines;
-        return result;
+        uint32_t decodedLines = this->onGetScanlines(dst, countLines, rowBytes);
+        fCurrScanline += decodedLines;
+        return decodedLines;
     }
 
     /**
@@ -121,18 +122,20 @@ public:
      *  NOTE: If skipped lines are the only lines with alpha, this default
      *  will make reallyHasAlpha return true, when it could have returned
      *  false.
+     *
+     *  @return true if the scanlines were successfully skipped, false otherwise

[scroggo, 2015/09/25 15:55:05]

I think we should further specify what this might mean. It seems like it means
that the data is incomplete, so it was not able to skip that many lines. Another
possibility appears to be bad parameters or we're going beyond the end of the
image. (On that note, going beyond the end of the image is a user error, but it
doesn't mean the image is incomplete. Not sure what to do there. Too bad for
dumb client?)

[msarett, 2015/10/01 12:44:52]

I will add to the comment.

      */
-    SkCodec::Result skipScanlines(int countLines) {
+    bool skipScanlines(int countLines) {
         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
             // can catch their bug.
             return SkCodec::kInvalidParameters;

[scroggo, 2015/09/25 15:55:05]

This return a bool. Not sure what though.

[msarett, 2015/10/01 12:44:52]

Making it return false.

         }
-        const SkCodec::Result result = this->onSkipScanlines(countLines);
-        fCurrScanline += countLines;
-        return result;
+        uint32_t skippedLines = this->onSkipScanlines(countLines);

[scroggo, 2015/09/25 15:55:05]

As suggested in the first patch set, this should be something like:

if (!this->onSkipScanlines(countLines)) {
  // In my merge CL (crrev.com/1365313002, which I'm almost ready for you
  // to review), I use -1 as a value to mean that start must be called. I think
  // this would be a case where we might want to reset it to -1.
  return false;
}

fCurrScanline += skippedLines;
return true;

[msarett, 2015/10/01 12:44:52]

Done.

+        fCurrScanline += skippedLines;
+        return skippedLines == (uint32_t) countLines;
     }
 
     /**
@@ -220,19 +223,31 @@ public:
     SkScanlineOrder getScanlineOrder() const { return this->onGetScanlineOrder(); }
 
     /**
-     *  Returns the y-coordinate of the next row to be returned by the scanline
-     *  decoder.  This will be overridden in the case of
-     *  kOutOfOrder_SkScanlineOrder and should be unnecessary in the case of
-     *  kNone_SkScanlineOrder.
+     *  Returns the y-coordinate of the next dst row to be returned by the scanline
+     *  decoder.
      */
     int getY() const {
-        SkASSERT(kNone_SkScanlineOrder != this->getScanlineOrder());
-        return this->onGetY();
+        return this->getY(fCurrScanline);
+    }
+
+    /**
+     *  Returns the y-coordinate of the dst row that corresponds to the
+     *  src y-coordinate.
+     */
+    int getY(int srcY) const;
+
+    uint32_t getFillValue(const SkImageInfo& dstInfo) const {
+        return fCodec->getFillValue(dstInfo);
+    }
+
+    void* getFillDst(void* dstStart, size_t rowBytes, uint32_t decodedScanlines) const {
+        return fCodec->getFillDst(dstStart, rowBytes, decodedScanlines);
     }
 
 protected:
-    SkScanlineDecoder(const SkImageInfo& srcInfo)
-        : fSrcInfo(srcInfo)
+    SkScanlineDecoder(SkCodec* codec, const SkImageInfo& srcInfo)
+        : fCodec(codec)
+        , fSrcInfo(srcInfo)
         , fDstInfo()
         , fOptions()
         , fCurrScanline(0) {}
@@ -251,27 +266,23 @@ protected:
      */
     virtual SkScanlineOrder onGetScanlineOrder() const { return kTopDown_SkScanlineOrder; }
 
-    /**
-     *  Most images will be kTopDown and will not need to override this function.
-     */
-    virtual int onGetY() const { return fCurrScanline; }
-
     const SkImageInfo& dstInfo() const { return fDstInfo; }
 
     const SkCodec::Options& options() const { return fOptions; }
 
 private:
-    const SkImageInfo   fSrcInfo;
-    SkImageInfo         fDstInfo;
-    SkCodec::Options    fOptions;
-    int                 fCurrScanline;
+    SkAutoTDelete<SkCodec> fCodec;
+    const SkImageInfo      fSrcInfo;
+    SkImageInfo            fDstInfo;
+    SkCodec::Options       fOptions;
+    int                    fCurrScanline;
 
     virtual SkCodec::Result onStart(const SkImageInfo& dstInfo,
                                     const SkCodec::Options& options,
                                     SkPMColor ctable[], int* ctableCount) = 0;
 
     // Naive default version just calls onGetScanlines on temp memory.
-    virtual SkCodec::Result onSkipScanlines(int countLines) {
+    virtual uint32_t onSkipScanlines(int countLines) {
         SkAutoMalloc storage(fDstInfo.minRowBytes());
         // Note that we pass 0 to rowBytes so we continue to use the same memory.
         // Also note that while getScanlines checks that rowBytes is big enough,
@@ -281,8 +292,7 @@ private:
         return this->onGetScanlines(storage.get(), countLines, 0);
     }
 
-    virtual SkCodec::Result onGetScanlines(void* dst, int countLines,
-                                                    size_t rowBytes) = 0;
+    virtual uint32_t onGetScanlines(void* dst, int countLines, size_t rowBytes) = 0;
 
 };
 #endif // SkScanlineDecoder_DEFINED