Fill incomplete images in SkCodec parent class

src/codec/SkScaledCodec.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 "SkCodecPriv.h"
 #include "SkScaledCodec.h"
 #include "SkStream.h"
@@ skipping 160 matching lines @@
     }
     if (sampleYPtr) {
         *sampleYPtr = sampleY;
     }
 }
 
 // TODO: Implement subsetting in onGetPixels which works when and when not sampling 
 
 SkCodec::Result SkScaledCodec::onGetPixels(const SkImageInfo& requestedInfo, void* dst,
                                            size_t rowBytes, const Options& options,
-                                           SkPMColor ctable[], int* ctableCount) {
+                                           SkPMColor ctable[], int* ctableCount,
+                                           int* rowsDecoded) {
 
     if (options.fSubset) {
         // Subsets are not supported.
         return kUnimplemented;
-    } 
+    }
 
     // FIXME: If no scaling/subsets are requested, we can call fCodec->getPixels.
     Result result = fCodec->startScanlineDecode(requestedInfo, &options, ctable, ctableCount);
     if (kSuccess == result) {
         // native decode supported
         switch (fCodec->getScanlineOrder()) {
             case SkCodec::kTopDown_SkScanlineOrder:
             case SkCodec::kBottomUp_SkScanlineOrder:
             case SkCodec::kNone_SkScanlineOrder:
-                return fCodec->getScanlines(dst, requestedInfo.height(), rowBytes);
+                if (fCodec->getScanlines(dst, requestedInfo.height(), rowBytes) !=
+                        requestedInfo.height()) {
+                    // fCodec has already handled filling uninitialized memory.
+                    *rowsDecoded = requestedInfo.height();
+                    return kIncompleteInput;
+                }
+                return kSuccess;
             case SkCodec::kOutOfOrder_SkScanlineOrder: {
                 for (int y = 0; y < requestedInfo.height(); y++) {
                     int dstY = fCodec->nextScanline();
                     void* dstPtr = SkTAddOffset<void>(dst, rowBytes * dstY);
-                    result = fCodec->getScanlines(dstPtr, 1, rowBytes);
-                    // FIXME (msarett): Make the SkCodec base class take care of filling
-                    // uninitialized pixels so we can return immediately on kIncompleteInput.
-                    if (kSuccess != result && kIncompleteInput != result) {
-                        return result;
+                    if (1 != fCodec->getScanlines(dstPtr, 1, rowBytes)) {
+                        *rowsDecoded = y + 1;

[scroggo, 2015/10/01 14:48:32]

Why is this y+1 and not y? (Same for other instances below)

[msarett, 2015/10/01 18:14:14]

The failed call to getScanlines(1) will fill the scanline that it failed to get.
 Adding a comment.

+                        return kIncompleteInput;
                     }
                 }
-                return result;
+                return kSuccess;
             }
         }
     }
 
     if (kInvalidScale != result) {
         // no scaling requested
         return result;
     }
 
     // scaling requested
@@ skipping 15 matching lines @@
     // update codec with new info
     // FIXME: The previous call to start returned kInvalidScale. This call may
     // require a rewind. (skbug.com/4284)
     result = fCodec->startScanlineDecode(info, &options, ctable, ctableCount);
     if (kSuccess != result) {
         return result;
     }
 
     switch(fCodec->getScanlineOrder()) {
         case SkCodec::kTopDown_SkScanlineOrder: {
-            result = fCodec->skipScanlines(Y0);
-            if (kSuccess != result && kIncompleteInput != result) {
-                return result;
+            if (!fCodec->skipScanlines(Y0)) {
+                *rowsDecoded = 0;
+                return kIncompleteInput;
             }
             for (int y = 0; y < dstHeight; y++) {
-                result = fCodec->getScanlines(dst, 1, rowBytes);
-                if (kSuccess != result && kIncompleteInput != result) {
-                    return result;
+                if (1 != fCodec->getScanlines(dst, 1, rowBytes)) {
+                    *rowsDecoded = y + 1;
+                    return kIncompleteInput;
                 }
                 if (y < dstHeight - 1) {
-                    result = fCodec->skipScanlines(sampleY - 1);
-                    if (kSuccess != result && kIncompleteInput != result) {
-                        return result;
+                    if (!fCodec->skipScanlines(sampleY - 1)) {
+                        *rowsDecoded = y + 1;
+                        return kIncompleteInput;
                     }
                 }
                 dst = SkTAddOffset<void>(dst, rowBytes);
             }
-            return result;
+            return kSuccess;
         }
         case SkCodec::kBottomUp_SkScanlineOrder:
         case SkCodec::kOutOfOrder_SkScanlineOrder: {
-            for (int y = 0; y < srcHeight; y++) {
+            Result result = kSuccess;
+            int y;
+            for (y = 0; y < srcHeight; y++) {
                 int srcY = fCodec->nextScanline();
                 if (is_coord_necessary(srcY, sampleY, dstHeight)) {
                     void* dstPtr = SkTAddOffset<void>(dst, rowBytes * get_dst_coord(srcY, sampleY));
-                    result = fCodec->getScanlines(dstPtr, 1, rowBytes);
-                    if (kSuccess != result && kIncompleteInput != result) {
-                        return result;
+                    if (1 != fCodec->getScanlines(dstPtr, 1, rowBytes)) {
+                        result = kIncompleteInput;
+                        break;
                     }
                 } else {
-                    result = fCodec->skipScanlines(1);
-                    if (kSuccess != result && kIncompleteInput != result) {
-                        return result;
+                    if (!fCodec->skipScanlines(1)) {
+                        result = kIncompleteInput;
+                        break;
                     }
                 }
             }
+
+            // We handle filling uninitialized memory here instead of in the parent class.
+            // The parent class does not know that we are sampling.
+            if (kIncompleteInput == result) {
+                const SkImageInfo fillInfo = requestedInfo.makeWH(requestedInfo.width(), 1);
+                const uint32_t fillValue = this->getFillValue(fillInfo.colorType(),
+                        fillInfo.alphaType());
+                for (; y < srcHeight; y++) {
+                    int srcY = fCodec->nextScanline(y);
+                    if (is_coord_necessary(srcY, sampleY, dstHeight)) {
+                        void* dstRow = SkTAddOffset<void>(dst,
+                                rowBytes * get_dst_coord(srcY, sampleY));
+                        SkSwizzler::Fill(dstRow, fillInfo, rowBytes, fillValue,
+                                options.fZeroInitialized);
+                    }
+                }
+                *rowsDecoded = dstHeight;
+            }
             return result;
         }
         case SkCodec::kNone_SkScanlineOrder: {
             SkAutoMalloc storage(srcHeight * rowBytes);
             uint8_t* storagePtr = static_cast<uint8_t*>(storage.get());
-            result = fCodec->getScanlines(storagePtr, srcHeight, rowBytes);
-            if (kSuccess != result && kIncompleteInput != result) {
-                return result;
-            }
+            int scanlines = (int) fCodec->getScanlines(storagePtr, srcHeight, rowBytes);

[scroggo, 2015/10/01 14:48:32]

It's weird to me that getScanlines returns one type, which you often cast to an
int. Can you use the same type throughout? Is there a reason they are different?

[msarett, 2015/10/01 18:14:14]

Yeah I also thought this was strange.

I feel strongly that it should return an unsigned int.  But unfortunately all of
our other APIs that *should* be unsigned are signed (ex: width, height,
nextScanline etc.)

I think its probably to best to be consistent and use int.  Maybe this is
something that we can clean up in the future (or that we don't care about)?

[scroggo, 2015/10/01 20:48:57]

+1 to consistency. I'm not sure why width and height are stored as ints...

             storagePtr += Y0 * rowBytes;
-            for (int y = 0; y < dstHeight; y++) {
+            scanlines -= Y0;
+            int y = 0;
+            while(y < dstHeight && scanlines > 0) {
                 memcpy(dst, storagePtr, rowBytes);
                 storagePtr += sampleY * rowBytes;
                 dst = SkTAddOffset<void>(dst, rowBytes);
+                scanlines -= sampleY;
+                y++;
             }
-            return result;
+            if (y < dstHeight) {
+                // fCodec has already handled filling uninitialized memory.
+                *rowsDecoded = dstHeight;
+                return kIncompleteInput;
+            }
+            return kSuccess;
         }
         default:
             SkASSERT(false);
             return kUnimplemented;
     }
 }
+
+uint32_t SkScaledCodec::onGetFillValue(SkColorType colorType, SkAlphaType alphaType) const {
+    return fCodec->getFillValue(colorType, alphaType);
+}
+
+SkCodec::SkScanlineOrder SkScaledCodec::onGetScanlineOrder() const {
+    return fCodec->getScanlineOrder();
+}