Fill incomplete images in SkCodec parent class

dm/DMSrcSink.cpp

Index: dm/DMSrcSink.cpp
diff --git a/dm/DMSrcSink.cpp b/dm/DMSrcSink.cpp
index 981e47d6c0bf84690c25c8a49392fa2e3abadc18..5229abdf5e909913aafdc7a273951cf9ac65a83a 100644
--- a/dm/DMSrcSink.cpp
+++ b/dm/DMSrcSink.cpp
@@ -352,35 +352,33 @@ Error CodecSrc::draw(SkCanvas* canvas) const {
                 return Error::Nonfatal("Could not start scanline decoder");
             }
 
-            SkCodec::Result result = SkCodec::kUnimplemented;
+            uint32_t lines = 0;

[scroggo, 2015/09/22 18:02:48]

Maybe this should be linesDecoded?

[msarett, 2015/09/23 13:22:39]

Done.

             switch (scanlineDecoder->getScanlineOrder()) {
                 case SkScanlineDecoder::kTopDown_SkScanlineOrder:
                 case SkScanlineDecoder::kBottomUp_SkScanlineOrder:
                 case SkScanlineDecoder::kNone_SkScanlineOrder:
-                    result = scanlineDecoder->getScanlines(bitmap.getAddr(0, 0),
+                    lines = scanlineDecoder->getScanlines(bitmap.getAddr(0, 0),
                             decodeInfo.height(), bitmap.rowBytes());
                     break;
                 case SkScanlineDecoder::kOutOfOrder_SkScanlineOrder: {
                     for (int y = 0; y < decodeInfo.height(); y++) {
                         int dstY = scanlineDecoder->getY();
                         void* dstPtr = bitmap.getAddr(0, dstY);
-                        result = scanlineDecoder->getScanlines(dstPtr, 1, bitmap.rowBytes());
-                        if (SkCodec::kSuccess != result && SkCodec::kIncompleteInput != result) {
-                            return SkStringPrintf("%s failed with error message %d",
-                                                  fPath.c_str(), (int) result);
+                        uint32_t line = scanlineDecoder->getScanlines(dstPtr, 1, bitmap.rowBytes());
+                        if (1 != line) {
+                            break;
                         }
+                        lines += line;
                     }
                     break;
                 }
             }
-
-            switch (result) {
-                case SkCodec::kSuccess:
-                case SkCodec::kIncompleteInput:
-                    break;
-                default:
-                    return SkStringPrintf("%s failed with error message %d",
-                                          fPath.c_str(), (int) result);
+            if ((int) lines < decodeInfo.height()) {
+                // We have not implemented a function in SkScanlineDecoder to fill remaining
+                // scanlines on an incomplete decode.  As long as we are the only client that
+                // uses SkScanlineDecoder, I think this is ok.  If this changes, maybe we want
+                // to consider providing an API for this?
+                return Error::Nonfatal("Image is incomplete, could not get scanlines");
             }
             canvas->drawBitmap(bitmap, 0, 0);
             break;
@@ -443,14 +441,9 @@ Error CodecSrc::draw(SkCanvas* canvas) const {
                         }
                     }
                     //skip to first line of subset
-                    const SkCodec::Result skipResult = decoder->skipScanlines(y);
-                    switch (skipResult) {
-                        case SkCodec::kSuccess:
-                        case SkCodec::kIncompleteInput:
-                            break;
-                        default:
-                            return SkStringPrintf("%s failed after attempting to skip %d scanlines"
-                                    "with error message %d", fPath.c_str(), y, (int) skipResult);
+                    int lines = decoder->skipScanlines(y);
+                    if (lines < y) {
+                        return Error::Nonfatal("Image is incomplete, could not get scanlines");
                     }
                     //create and set size of subsetBm
                     SkBitmap subsetBm;
@@ -458,15 +451,9 @@ Error CodecSrc::draw(SkCanvas* canvas) const {
                     bounds.setXYWH(0, 0, currentSubsetWidth, currentSubsetHeight);
                     SkAssertResult(largestSubsetBm.extractSubset(&subsetBm, bounds));
                     SkAutoLockPixels autlockSubsetBm(subsetBm, true);
-                    const SkCodec::Result subsetResult =
-                            decoder->getScanlines(buffer, currentSubsetHeight, rowBytes);
-                    switch (subsetResult) {
-                        case SkCodec::kSuccess:
-                        case SkCodec::kIncompleteInput:
-                            break;
-                        default:
-                            return SkStringPrintf("%s failed with error message %d",
-                                    fPath.c_str(), (int) subsetResult);
+                    lines = decoder->getScanlines(buffer, currentSubsetHeight, rowBytes);
+                    if (lines < currentSubsetHeight) {
+                        return Error::Nonfatal("Image is incomplete, could not get scanlines");

[scroggo, 2015/09/22 18:02:48]

This is going to change the behavior, right? Previously we did the same thing on
kSuccess and kIncompleteInput, but now we won't ever draw it and be able to look
at it in gold.

(Maybe this is okay, and correct, since we won't have filled in the rest of the
image, so we'll get uninitialized memory?)

[msarett, 2015/09/23 13:22:39]

Yes I should have been more explicit about behavior changes.

Before this CL, getScanlines() would fill the buffer on kIncompleteInput.  Now
getScanlines() just returns the number of lines successfully decoded and leaves
the remaining lines uninitialized.

Not sure if this is "correct", but this is what I intended.  Clients of
SkScanlineDecoder (SkScaledCodec is currently the only one) can and should fill
the uninitialized lines.  But a side effect is that we can't test the
SkScanlineDecoder on incomplete images from DM, which I think is ok.

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

Sure we can. We just need to make DM fill the buffer, now that it knows how many
scanlines were decoded.

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

kScanlineSubset_Mode now fills on incomplete images, instead of failing.  For
consistency, kScanline_Mode has also been modified to fill incomplete images.

                     }
                     const size_t bpp = decodeInfo.bytesPerPixel();
                     /*
@@ -508,30 +495,23 @@ Error CodecSrc::draw(SkCanvas* canvas) const {
                 // to run this test for image types that do not have this scanline ordering.
                 return Error::Nonfatal("Could not start top-down scanline decoder");
             }
+            int lines;

[scroggo, 2015/09/22 18:02:48]

I would lean towards removing this variable and instead making each of the below
lines look like:

if (decoder->skip/getScanlines() < linesToRead) {
  return Error::Nonfatal
}

The reason is because its value is never reused - you assign to it each time you
read from it.

[msarett, 2015/09/23 13:22:39]

Done.

             for (int i = 0; i < numStripes; i += 2) {
                 // Skip a stripe
                 const int linesToSkip = SkTMin(stripeHeight, height - i * stripeHeight);
-                SkCodec::Result result = decoder->skipScanlines(linesToSkip);
-                switch (result) {
-                    case SkCodec::kSuccess:
-                    case SkCodec::kIncompleteInput:
-                        break;
-                    default:
-                        return SkStringPrintf("Cannot skip scanlines for %s.", fPath.c_str());
+                lines = decoder->skipScanlines(linesToSkip);
+                if (lines < linesToSkip) {
+                    return Error::Nonfatal("Image is incomplete, could not get scanlines");
                 }
 
                 // Read a stripe
                 const int startY = (i + 1) * stripeHeight;
                 const int linesToRead = SkTMin(stripeHeight, height - startY);
                 if (linesToRead > 0) {
-                    result = decoder->getScanlines(bitmap.getAddr(0, startY),
-                            linesToRead, bitmap.rowBytes());
-                    switch (result) {
-                        case SkCodec::kSuccess:
-                        case SkCodec::kIncompleteInput:
-                            break;
-                        default:
-                            return SkStringPrintf("Cannot get scanlines for %s.", fPath.c_str());
+                    lines = decoder->getScanlines(bitmap.getAddr(0, startY), linesToRead,
+                            bitmap.rowBytes());
+                    if (lines < linesToRead) {
+                        return Error::Nonfatal("Image is incomplete, could not get scanlines");
                     }
                 }
             }
@@ -546,26 +526,18 @@ Error CodecSrc::draw(SkCanvas* canvas) const {
                 // Read a stripe
                 const int startY = i * stripeHeight;
                 const int linesToRead = SkTMin(stripeHeight, height - startY);
-                SkCodec::Result result = decoder->getScanlines(bitmap.getAddr(0, startY),
-                        linesToRead, bitmap.rowBytes());
-                switch (result) {
-                    case SkCodec::kSuccess:
-                    case SkCodec::kIncompleteInput:
-                        break;
-                    default:
-                        return SkStringPrintf("Cannot get scanlines for %s.", fPath.c_str());
+                lines = decoder->getScanlines(bitmap.getAddr(0, startY), linesToRead,
+                        bitmap.rowBytes());
+                if (lines < linesToRead) {
+                    return Error::Nonfatal("Image is incomplete, could not get scanlines");
                 }
 
                 // Skip a stripe
                 const int linesToSkip = SkTMin(stripeHeight, height - (i + 1) * stripeHeight);
                 if (linesToSkip > 0) {
-                    result = decoder->skipScanlines(linesToSkip);
-                    switch (result) {
-                        case SkCodec::kSuccess:
-                        case SkCodec::kIncompleteInput:
-                            break;
-                        default:
-                            return SkStringPrintf("Cannot skip scanlines for %s.", fPath.c_str());
+                    lines = decoder->skipScanlines(linesToSkip);
+                    if (lines < linesToSkip) {
+                        return Error::Nonfatal("Image is incomplete, could not get scanlines");
                     }
                 }
             }