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 2e2255633538b4b4c9317fa76459c48891f5c834..33a6caad7b80d60ee2d210bc93d65d11de3499fe 100644
--- a/dm/DMSrcSink.cpp
+++ b/dm/DMSrcSink.cpp
@@ -28,6 +28,8 @@
 #include "SkStream.h"
 #include "SkTLogic.h"
 #include "SkXMLWriter.h"
+#include "SkScaledCodec.h"
+#include "SkSwizzler.h"
 
 DEFINE_bool(multiPage, false, "For document-type backends, render the source"
             " into multiple pages");
@@ -340,36 +342,25 @@ Error CodecSrc::draw(SkCanvas* canvas) const {
                 return Error::Nonfatal("Could not start scanline decoder");
             }
 
-            SkCodec::Result result = SkCodec::kUnimplemented;
+            void* dst = bitmap.getAddr(0, 0);
+            size_t rowBytes = bitmap.rowBytes();
+            uint32_t height = decodeInfo.height();
             switch (codec->getScanlineOrder()) {
                 case SkCodec::kTopDown_SkScanlineOrder:
                 case SkCodec::kBottomUp_SkScanlineOrder:
                 case SkCodec::kNone_SkScanlineOrder:
-                    result = codec->getScanlines(bitmap.getAddr(0, 0),
-                            decodeInfo.height(), bitmap.rowBytes());
+                    codec->getScanlines(dst, height, rowBytes);

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

getScanlines() now fills when it cannot get all of the lines, so we do not need
to inspect the return value.

                     break;
                 case SkCodec::kOutOfOrder_SkScanlineOrder: {
                     for (int y = 0; y < decodeInfo.height(); y++) {
-                        int dstY = codec->nextScanline();
+                        int dstY = codec->nextScanline(y);
                         void* dstPtr = bitmap.getAddr(0, dstY);
-                        result = codec->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);
-                        }
+                        codec->getScanlines(dstPtr, 1, bitmap.rowBytes());

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

Don't we want to break out of the loop if 0 is returned?

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

We could do that.  But then we need to iterate over the remaining lines and fill
line by line.  This duplicates the fill code we already have in SkCodec.cpp. 
See Patch Set 4 for this implementation.

Alternatively, here we keep calling getScanlines, and every time it fails, it
will fill the line.  I know we've discussed that we don't really like this
approach.  I think I prefer it here because it simplifies our test code.  But I
don't feel strongly and would be happy to change.

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

Oh yeah, now I remember talking about this, but I don't remember if we came to a
decision.

I guess I sort of imagine an error to mean that calling getScanlines again does
not work, but you're right, this does simplify things here in this testing code.
Maybe add a comment explaining this?

[msarett, 2015/10/01 22:34:51]

Acknowledged.

                     }
                     break;
                 }
             }
 
-            switch (result) {
-                case SkCodec::kSuccess:
-                case SkCodec::kIncompleteInput:
-                    break;
-                default:
-                    return SkStringPrintf("%s failed with error message %d",
-                                          fPath.c_str(), (int) result);
-            }
             canvas->drawBitmap(bitmap, 0, 0);
             break;
         }
@@ -430,31 +421,18 @@ Error CodecSrc::draw(SkCanvas* canvas) const {
                         }
                     }
                     //skip to first line of subset
-                    const SkCodec::Result skipResult = codec->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);
+                    if (!codec->skipScanlines(y)) {
+                        return Error::Nonfatal("Image is incomplete, could not get scanlines");

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

Since we've returned an error, we will not be able to look at the output, right?
I think we still want to draw the output.

Same for the rest of our incomplete images.

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

Yeah the way the test is written now, we will only see the output if the image
is at least half complete.  We can do better than this.

                     }
+
                     //create and set size of subsetBm
                     SkBitmap subsetBm;
                     SkIRect bounds = SkIRect::MakeWH(subsetWidth, subsetHeight);
                     bounds.setXYWH(0, 0, currentSubsetWidth, currentSubsetHeight);
                     SkAssertResult(largestSubsetBm.extractSubset(&subsetBm, bounds));
                     SkAutoLockPixels autlockSubsetBm(subsetBm, true);
-                    const SkCodec::Result subsetResult =
-                            codec->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);
-                    }
+                    codec->getScanlines(buffer, currentSubsetHeight, rowBytes);

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

Note that you do something different if skipScanlines reveals the image to be
incomplete versus when getScanlines does. I think this is the correct behavior
(continue)

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

Acknowledged.

+
                     const size_t bpp = decodeInfo.bytesPerPixel();
                     /*
                      * we copy all the lines at once becuase when calling getScanlines for

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

This isn't due to your CL, but it seems like this comment no longer goes with
the code it applies to?

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

Yeah I'm a bit confused by this comment.  Removing it.

@@ -494,30 +472,21 @@ 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");
             }
+
             for (int i = 0; i < numStripes; i += 2) {
                 // Skip a stripe
                 const int linesToSkip = SkTMin(stripeHeight, height - i * stripeHeight);
-                SkCodec::Result result = codec->skipScanlines(linesToSkip);
-                switch (result) {
-                    case SkCodec::kSuccess:
-                    case SkCodec::kIncompleteInput:
-                        break;
-                    default:
-                        return SkStringPrintf("Cannot skip scanlines for %s.", fPath.c_str());
+                if (!codec->skipScanlines(linesToSkip)) {
+                    return Error::Nonfatal("Image is incomplete, could not get scanlines");

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

Not only was the image incomplete, but it was so incomplete that we did not
decode anything.

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

This could be improved as well.

                 }
 
                 // Read a stripe
                 const int startY = (i + 1) * stripeHeight;
                 const int linesToRead = SkTMin(stripeHeight, height - startY);
                 if (linesToRead > 0) {
-                    result = codec->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());
+                    if ((int) codec->getScanlines(bitmap.getAddr(0, startY), linesToRead,
+                            bitmap.rowBytes()) < linesToRead) {
+                        return Error::Nonfatal("Image is incomplete, could not get scanlines");
                     }
                 }
             }
@@ -532,26 +501,16 @@ Error CodecSrc::draw(SkCanvas* canvas) const {
                 // Read a stripe
                 const int startY = i * stripeHeight;
                 const int linesToRead = SkTMin(stripeHeight, height - startY);
-                SkCodec::Result result = codec->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());
+                if ((int) codec->getScanlines(bitmap.getAddr(0, startY), linesToRead,
+                        bitmap.rowBytes()) < 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 = codec->skipScanlines(linesToSkip);
-                    switch (result) {
-                        case SkCodec::kSuccess:
-                        case SkCodec::kIncompleteInput:
-                            break;
-                        default:
-                            return SkStringPrintf("Cannot skip scanlines for %s.", fPath.c_str());
+                    if (!codec->skipScanlines(linesToSkip)) {
+                        return Error::Nonfatal("Image is incomplete, could not get scanlines");
                     }
                 }
             }