Fill incomplete images in SkCodec parent class

Rather than implementing some sort of "fill" in every
SkCodec subclass for incomplete images, let's make the
parent class handle this situation.

This includes an API change to SkCodec.h

SkCodec::getScanlines() now returns the number of lines it
read successfully, rather than an SkCodec::Result enum.
getScanlines() most often fails on an incomplete input, in
which case it is useful to know how many lines were
successfully decoded - this provides more information than
kIncomplete vs kSuccess.  We do lose information when the
API is used improperly, as we are no longer able to return
kInvalidParameter or kScanlineNotStarted.

Known Issues:
Does not work for incomplete fFrameIsSubset gifs.
Does not work for incomplete icos.

BUG=skia:

Review URL: https://codereview.chromium.org/1332053002

2 files changed, 123 insertions, 94 deletions

diff --git a/tests/CodexTest.cpp b/tests/CodexTest.cpp
index 12b42e3ebb..91fb6897b5 100644
--- a/tests/CodexTest.cpp
+++ b/tests/CodexTest.cpp
@@ -8,6 +8,7 @@
 #include "Resources.h"
 #include "SkBitmap.h"
 #include "SkCodec.h"
+#include "SkData.h"
 #include "SkMD5.h"
 #include "SkRandom.h"
 #include "SkScaledCodec.h"
@@ -75,14 +76,15 @@ SkIRect generate_random_subset(SkRandom* rand, int w, int h) {
 }
 
 static void test_codec(skiatest::Reporter* r, SkCodec* codec, SkBitmap& bm, const SkImageInfo& info,
-        const SkISize& size, bool supports565, SkMD5::Digest* digest,
-        const SkMD5::Digest* goodDigest) {
+        const SkISize& size, bool supports565, SkCodec::Result expectedResult,
+        SkMD5::Digest* digest, const SkMD5::Digest* goodDigest) {
+
     REPORTER_ASSERT(r, info.dimensions() == size);
     bm.allocPixels(info);
     SkAutoLockPixels autoLockPixels(bm);
 
     SkCodec::Result result = codec->getPixels(info, bm.getPixels(), bm.rowBytes());
-    REPORTER_ASSERT(r, result == SkCodec::kSuccess);
+    REPORTER_ASSERT(r, result == expectedResult);
 
     md5(bm, digest);
     if (goodDigest) {
@@ -92,16 +94,16 @@ static void test_codec(skiatest::Reporter* r, SkCodec* codec, SkBitmap& bm, cons
     {
         // Test decoding to 565
         SkImageInfo info565 = info.makeColorType(kRGB_565_SkColorType);
-        SkCodec::Result expected = (supports565 && info.alphaType() == kOpaque_SkAlphaType) ?
-                SkCodec::kSuccess : SkCodec::kInvalidConversion;
-        test_info(r, codec, info565, expected, nullptr);
+        SkCodec::Result expected565 = (supports565 && info.alphaType() == kOpaque_SkAlphaType) ?
+                expectedResult : SkCodec::kInvalidConversion;
+        test_info(r, codec, info565, expected565, nullptr);
     }
 
     // Verify that re-decoding gives the same result.  It is interesting to check this after
     // a decode to 565, since choosing to decode to 565 may result in some of the decode
     // options being modified.  These options should return to their defaults on another
     // decode to kN32, so the new digest should match the old digest.
-    test_info(r, codec, info, SkCodec::kSuccess, digest);
+    test_info(r, codec, info, expectedResult, digest);
 
     {
         // Check alpha type conversions
@@ -121,7 +123,7 @@ static void test_codec(skiatest::Reporter* r, SkCodec* codec, SkBitmap& bm, cons
                 otherAt = kPremul_SkAlphaType;
             }
             // The other non-opaque alpha type should always succeed, but not match.
-            test_info(r, codec, info.makeAlphaType(otherAt), SkCodec::kSuccess, nullptr);
+            test_info(r, codec, info.makeAlphaType(otherAt), expectedResult, nullptr);
         }
     }
 }
@@ -147,14 +149,24 @@ static void check(skiatest::Reporter* r,
                   SkISize size,
                   bool supportsScanlineDecoding,
                   bool supportsSubsetDecoding,
-                  bool supports565 = true) {
+                  bool supports565 = true,
+                  bool supportsIncomplete = true) {
 
     SkAutoTDelete<SkStream> stream(resource(path));
     if (!stream) {
         SkDebugf("Missing resource '%s'\n", path);
         return;
     }
-    SkAutoTDelete<SkCodec> codec(SkCodec::NewFromStream(stream.detach()));
+
+    SkAutoTDelete<SkCodec> codec(nullptr);
+    bool isIncomplete = supportsIncomplete;
+    if (isIncomplete) {
+        size_t size = stream->getLength();
+        SkAutoTUnref<SkData> data((SkData::NewFromStream(stream, 2 * size / 3)));
+        codec.reset(SkCodec::NewFromData(data));
+    } else {
+        codec.reset(SkCodec::NewFromStream(stream.detach()));
+    }
     if (!codec) {
         ERRORF(r, "Unable to decode '%s'", path);
         return;
@@ -164,14 +176,15 @@ static void check(skiatest::Reporter* r,
     SkMD5::Digest codecDigest;
     SkImageInfo info = codec->getInfo().makeColorType(kN32_SkColorType);
     SkBitmap bm;
-    test_codec(r, codec, bm, info, size, supports565, &codecDigest, nullptr);
+    SkCodec::Result expectedResult = isIncomplete ? SkCodec::kIncompleteInput : SkCodec::kSuccess;
+    test_codec(r, codec, bm, info, size, supports565, expectedResult, &codecDigest, nullptr);
 
     // Scanline decoding follows.
     // Need to call startScanlineDecode() first.
     REPORTER_ASSERT(r, codec->getScanlines(bm.getAddr(0, 0), 1, 0)
-            == SkCodec::kScanlineDecodingNotStarted);
+            == 0);
     REPORTER_ASSERT(r, codec->skipScanlines(1)
-            == SkCodec::kScanlineDecodingNotStarted);
+            == 0);
 
     const SkCodec::Result startResult = codec->startScanlineDecode(info);
     if (supportsScanlineDecoding) {
@@ -180,8 +193,10 @@ static void check(skiatest::Reporter* r,
         REPORTER_ASSERT(r, startResult == SkCodec::kSuccess);
 
         for (int y = 0; y < info.height(); y++) {
-            SkCodec::Result result = codec->getScanlines(bm.getAddr(0, y), 1, 0);
-            REPORTER_ASSERT(r, result == SkCodec::kSuccess);
+            const int lines = codec->getScanlines(bm.getAddr(0, y), 1, 0);
+            if (!isIncomplete) {
+                REPORTER_ASSERT(r, 1 == lines);
+            }
         }
         // verify that scanline decoding gives the same result.
         if (SkCodec::kTopDown_SkScanlineOrder == codec->getScanlineOrder()) {
@@ -190,19 +205,21 @@ static void check(skiatest::Reporter* r,
 
         // Cannot continue to decode scanlines beyond the end
         REPORTER_ASSERT(r, codec->getScanlines(bm.getAddr(0, 0), 1, 0)
-                == SkCodec::kInvalidParameters);
+                == 0);
 
         // Interrupting a scanline decode with a full decode starts from
         // scratch
         REPORTER_ASSERT(r, codec->startScanlineDecode(info) == SkCodec::kSuccess);
-        REPORTER_ASSERT(r, codec->getScanlines(bm.getAddr(0, 0), 1, 0)
-                == SkCodec::kSuccess);
+        const int lines = codec->getScanlines(bm.getAddr(0, 0), 1, 0);
+        if (!isIncomplete) {
+            REPORTER_ASSERT(r, lines == 1);
+        }
         REPORTER_ASSERT(r, codec->getPixels(bm.info(), bm.getPixels(), bm.rowBytes())
-                == SkCodec::kSuccess);
+                == expectedResult);
         REPORTER_ASSERT(r, codec->getScanlines(bm.getAddr(0, 0), 1, 0)
-                == SkCodec::kScanlineDecodingNotStarted);
+                == 0);
         REPORTER_ASSERT(r, codec->skipScanlines(1)
-                == SkCodec::kScanlineDecodingNotStarted);
+                == 0);
     } else {
         REPORTER_ASSERT(r, startResult == SkCodec::kUnimplemented);
     }
@@ -232,7 +249,7 @@ static void check(skiatest::Reporter* r,
                                                         &opts, nullptr, nullptr);
 
         if (supportsSubsetDecoding) {
-            REPORTER_ASSERT(r, result == SkCodec::kSuccess);
+            REPORTER_ASSERT(r, result == expectedResult);
             // Webp is the only codec that supports subsets, and it will have modified the subset
             // to have even left/top.
             REPORTER_ASSERT(r, SkIsAlign2(subset.fLeft) && SkIsAlign2(subset.fTop));
@@ -250,7 +267,15 @@ static void check(skiatest::Reporter* r,
             SkDebugf("Missing resource '%s'\n", path);
             return;
         }
-        SkAutoTDelete<SkCodec> codec(SkScaledCodec::NewFromStream(stream.detach()));
+
+        SkAutoTDelete<SkCodec> codec(nullptr);
+        if (isIncomplete) {
+            size_t size = stream->getLength();
+            SkAutoTUnref<SkData> data((SkData::NewFromStream(stream, 2 * size / 3)));
+            codec.reset(SkScaledCodec::NewFromData(data));
+        } else {
+            codec.reset(SkScaledCodec::NewFromStream(stream.detach()));
+        }
         if (!codec) {
             ERRORF(r, "Unable to decode '%s'", path);
             return;
@@ -258,7 +283,13 @@ static void check(skiatest::Reporter* r,
 
         SkBitmap bm;
         SkMD5::Digest scaledCodecDigest;
-        test_codec(r, codec, bm, info, size, supports565, &scaledCodecDigest, &codecDigest);
+        test_codec(r, codec, bm, info, size, supports565, expectedResult, &scaledCodecDigest,
+                &codecDigest);
+    }
+
+    // If we've just tested incomplete decodes, let's run the same test again on full decodes.
+    if (isIncomplete) {
+        check(r, path, size, supportsScanlineDecoding, supportsSubsetDecoding, supports565, false);
     }
 }
 
@@ -275,39 +306,45 @@ DEF_TEST(Codec, r) {
     check(r, "randPixels.bmp", SkISize::Make(8, 8), true, false);
 
     // ICO
+    // FIXME: We are not ready to test incomplete ICOs
     // These two tests examine interestingly different behavior:
     // Decodes an embedded BMP image
-    check(r, "color_wheel.ico", SkISize::Make(128, 128), false, false);
+    check(r, "color_wheel.ico", SkISize::Make(128, 128), false, false, true, false);
     // Decodes an embedded PNG image
-    check(r, "google_chrome.ico", SkISize::Make(256, 256), false, false);
+    check(r, "google_chrome.ico", SkISize::Make(256, 256), false, false, true, false);
 
     // GIF
-    check(r, "box.gif", SkISize::Make(200, 55), true, false);
-    check(r, "color_wheel.gif", SkISize::Make(128, 128), true, false);
-    check(r, "randPixels.gif", SkISize::Make(8, 8), true, false);
+    // FIXME: We are not ready to test incomplete GIFs
+    check(r, "box.gif", SkISize::Make(200, 55), true, false, true, false);
+    check(r, "color_wheel.gif", SkISize::Make(128, 128), true, false, true, false);
+    // randPixels.gif is too small to test incomplete
+    check(r, "randPixels.gif", SkISize::Make(8, 8), true, false, true, false);
 
     // JPG
     check(r, "CMYK.jpg", SkISize::Make(642, 516), true, false, false);
     check(r, "color_wheel.jpg", SkISize::Make(128, 128), true, false);
-    check(r, "grayscale.jpg", SkISize::Make(128, 128), true, false);
+    // grayscale.jpg is too small to test incomplete
+    check(r, "grayscale.jpg", SkISize::Make(128, 128), true, false, true, false);
     check(r, "mandrill_512_q075.jpg", SkISize::Make(512, 512), true, false);
-    check(r, "randPixels.jpg", SkISize::Make(8, 8), true, false);
+    // randPixels.jpg is too small to test incomplete
+    check(r, "randPixels.jpg", SkISize::Make(8, 8), true, false, true, false);
 
     // PNG
-    check(r, "arrow.png", SkISize::Make(187, 312), true, false);
-    check(r, "baby_tux.png", SkISize::Make(240, 246), true, false);
-    check(r, "color_wheel.png", SkISize::Make(128, 128), true, false);
-    check(r, "half-transparent-white-pixel.png", SkISize::Make(1, 1), true, false);
-    check(r, "mandrill_128.png", SkISize::Make(128, 128), true, false);
-    check(r, "mandrill_16.png", SkISize::Make(16, 16), true, false);
-    check(r, "mandrill_256.png", SkISize::Make(256, 256), true, false);
-    check(r, "mandrill_32.png", SkISize::Make(32, 32), true, false);
-    check(r, "mandrill_512.png", SkISize::Make(512, 512), true, false);
-    check(r, "mandrill_64.png", SkISize::Make(64, 64), true, false);
-    check(r, "plane.png", SkISize::Make(250, 126), true, false);
-    check(r, "plane_interlaced.png", SkISize::Make(250, 126), true, false);
-    check(r, "randPixels.png", SkISize::Make(8, 8), true, false);
-    check(r, "yellow_rose.png", SkISize::Make(400, 301), true, false);
+    check(r, "arrow.png", SkISize::Make(187, 312), true, false, true, false);
+    check(r, "baby_tux.png", SkISize::Make(240, 246), true, false, true, false);
+    check(r, "color_wheel.png", SkISize::Make(128, 128), true, false, true, false);
+    check(r, "half-transparent-white-pixel.png", SkISize::Make(1, 1), true, false, true, false);
+    check(r, "mandrill_128.png", SkISize::Make(128, 128), true, false, true, false);
+    check(r, "mandrill_16.png", SkISize::Make(16, 16), true, false, true, false);
+    check(r, "mandrill_256.png", SkISize::Make(256, 256), true, false, true, false);
+    check(r, "mandrill_32.png", SkISize::Make(32, 32), true, false, true, false);
+    check(r, "mandrill_512.png", SkISize::Make(512, 512), true, false, true, false);
+    check(r, "mandrill_64.png", SkISize::Make(64, 64), true, false, true, false);
+    check(r, "plane.png", SkISize::Make(250, 126), true, false, true, false);
+    // FIXME: We are not ready to test incomplete interlaced pngs
+    check(r, "plane_interlaced.png", SkISize::Make(250, 126), true, false, true, false);
+    check(r, "randPixels.png", SkISize::Make(8, 8), true, false, true, false);
+    check(r, "yellow_rose.png", SkISize::Make(400, 301), true, false, true, false);
 }
 
 // Test interlaced PNG in stripes, similar to DM's kStripe_Mode
@@ -361,12 +398,12 @@ DEF_TEST(Codec_stripes, r) {
     // Odd stripes
     for (int i = 1; i < numStripes; i += 2) {
         // Skip the even stripes
-        result = codec->skipScanlines(stripeHeight);
-        REPORTER_ASSERT(r, result == SkCodec::kSuccess);
+        bool skipResult = codec->skipScanlines(stripeHeight);
+        REPORTER_ASSERT(r, skipResult);
 
-        result = codec->getScanlines(bm.getAddr(0, i * stripeHeight), stripeHeight,
+        int linesDecoded = codec->getScanlines(bm.getAddr(0, i * stripeHeight), stripeHeight,
                                      bm.rowBytes());
-        REPORTER_ASSERT(r, result == SkCodec::kSuccess);
+        REPORTER_ASSERT(r, linesDecoded == stripeHeight);
     }
 
     // Even stripes
@@ -374,14 +411,14 @@ DEF_TEST(Codec_stripes, r) {
     REPORTER_ASSERT(r, result == SkCodec::kSuccess);
 
     for (int i = 0; i < numStripes; i += 2) {
-        result = codec->getScanlines(bm.getAddr(0, i * stripeHeight), stripeHeight,
+        int linesDecoded = codec->getScanlines(bm.getAddr(0, i * stripeHeight), stripeHeight,
                                      bm.rowBytes());
-        REPORTER_ASSERT(r, result == SkCodec::kSuccess);
+        REPORTER_ASSERT(r, linesDecoded == stripeHeight);
 
         // Skip the odd stripes
         if (i + 1 < numStripes) {
-            result = codec->skipScanlines(stripeHeight);
-            REPORTER_ASSERT(r, result == SkCodec::kSuccess);
+            bool skipResult = codec->skipScanlines(stripeHeight);
+            REPORTER_ASSERT(r, skipResult);
         }
     }
 
@@ -390,12 +427,12 @@ DEF_TEST(Codec_stripes, r) {
         result = codec->startScanlineDecode(info);
         REPORTER_ASSERT(r, result == SkCodec::kSuccess);
 
-        result = codec->skipScanlines(height - remainingLines);
-        REPORTER_ASSERT(r, result == SkCodec::kSuccess);
+        bool skipResult = codec->skipScanlines(height - remainingLines);
+        REPORTER_ASSERT(r, skipResult);
 
-        result = codec->getScanlines(bm.getAddr(0, height - remainingLines),
+        int linesDecoded = codec->getScanlines(bm.getAddr(0, height - remainingLines),
                                      remainingLines, bm.rowBytes());
-        REPORTER_ASSERT(r, result == SkCodec::kSuccess);
+        REPORTER_ASSERT(r, linesDecoded == remainingLines);
     }
 
     compare_to_good_digest(r, digest, bm);
diff --git a/tests/SwizzlerTest.cpp b/tests/SwizzlerTest.cpp
index 256a4b2bcd..95aaf38639 100644
--- a/tests/SwizzlerTest.cpp
+++ b/tests/SwizzlerTest.cpp
@@ -9,8 +9,10 @@
 #include "Test.h"
 
 // These are the values that we will look for to indicate that the fill was successful
-static const uint8_t kFillIndex = 0x1;
-static const uint32_t kFillColor = 0x22334455;
+static const uint8_t kFillIndex = 0x11;
+static const uint8_t kFillGray = 0x22;
+static const uint16_t kFill565 = 0x3344;
+static const uint32_t kFillColor = 0x55667788;
 
 static void check_fill(skiatest::Reporter* r,
                        const SkImageInfo& imageInfo,
@@ -18,8 +20,7 @@ static void check_fill(skiatest::Reporter* r,
                        uint32_t endRow,
                        size_t rowBytes,
                        uint32_t offset,
-                       uint32_t colorOrIndex,
-                       SkPMColor* colorTable) {
+                       uint32_t colorOrIndex) {
 
     // Calculate the total size of the image in bytes.  Use the smallest possible size.
     // The offset value tells us to adjust the pointer from the memory we allocate in order
@@ -34,16 +35,15 @@ static void check_fill(skiatest::Reporter* r,
     // Adjust the pointer in order to test on different memory alignments
     uint8_t* imageData = storage.get() + offset;
     uint8_t* imageStart = imageData + rowBytes * startRow;
-
-    // Fill image with the fill value starting at the indicated row
-    SkSwizzler::Fill(imageStart, imageInfo, rowBytes, endRow - startRow + 1, colorOrIndex,
-            colorTable, SkCodec::kNo_ZeroInitialized);
+    const SkImageInfo fillInfo = imageInfo.makeWH(imageInfo.width(), endRow - startRow + 1);
+    SkSampler::Fill(fillInfo, imageStart, rowBytes, colorOrIndex, SkCodec::kNo_ZeroInitialized);
 
     // Ensure that the pixels are filled properly
     // The bots should catch any memory corruption
     uint8_t* indexPtr = imageData + startRow * rowBytes;
     uint8_t* grayPtr = indexPtr;
     uint32_t* colorPtr = (uint32_t*) indexPtr;
+    uint16_t* color565Ptr = (uint16_t*) indexPtr;
     for (uint32_t y = startRow; y <= endRow; y++) {
         for (int32_t x = 0; x < imageInfo.width(); x++) {
             switch (imageInfo.colorType()) {
@@ -54,8 +54,10 @@ static void check_fill(skiatest::Reporter* r,
                     REPORTER_ASSERT(r, kFillColor == colorPtr[x]);
                     break;
                 case kGray_8_SkColorType:
-                    // We always fill kGray with black
-                    REPORTER_ASSERT(r, (uint8_t) kFillColor == grayPtr[x]);
+                    REPORTER_ASSERT(r, kFillGray == grayPtr[x]);
+                    break;
+                case kRGB_565_SkColorType:
+                    REPORTER_ASSERT(r, kFill565 == color565Ptr[x]);
                     break;
                 default:
                     REPORTER_ASSERT(r, false);
@@ -69,12 +71,6 @@ static void check_fill(skiatest::Reporter* r,
 
 // Test Fill() with different combinations of dimensions, alignment, and padding
 DEF_TEST(SwizzlerFill, r) {
-    // Set up a color table
-    SkPMColor colorTable[kFillIndex + 1];
-    colorTable[kFillIndex] = kFillColor;
-    // Apart from the fill index, we will leave the other colors in the color table uninitialized.
-    // If we incorrectly try to fill with this uninitialized memory, the bots will catch it.
-
     // Test on an invalid width and representative widths
     const uint32_t widths[] = { 0, 10, 50 };
 
@@ -83,48 +79,44 @@ DEF_TEST(SwizzlerFill, r) {
     const uint32_t heights[] = { 1, 5, 10 };
 
     // Test on interesting possibilities for row padding
-    const uint32_t paddings[] = { 0, 1, 2, 3, 4 };
+    const uint32_t paddings[] = { 0, 4 };
 
     // Iterate over test dimensions
     for (uint32_t width : widths) {
         for (uint32_t height : heights) {
 
             // Create image info objects
-            const SkImageInfo colorInfo = SkImageInfo::MakeN32(width, height,
-                kUnknown_SkAlphaType);
-            const SkImageInfo indexInfo = colorInfo.makeColorType(kIndex_8_SkColorType);
+            const SkImageInfo colorInfo = SkImageInfo::MakeN32(width, height, kUnknown_SkAlphaType);
             const SkImageInfo grayInfo = colorInfo.makeColorType(kGray_8_SkColorType);
+            const SkImageInfo indexInfo = colorInfo.makeColorType(kIndex_8_SkColorType);
+            const SkImageInfo color565Info = colorInfo.makeColorType(kRGB_565_SkColorType);
 
             for (uint32_t padding : paddings) {
 
                 // Calculate row bytes
-                size_t colorRowBytes = SkColorTypeBytesPerPixel(kN32_SkColorType) * width +
-                        padding;
-                size_t indexRowBytes = width + padding;
-                size_t grayRowBytes = indexRowBytes;
+                const size_t colorRowBytes = SkColorTypeBytesPerPixel(kN32_SkColorType) * width
+                        + padding;
+                const size_t indexRowBytes = width + padding;
+                const size_t grayRowBytes = indexRowBytes;
+                const size_t color565RowBytes =
+                        SkColorTypeBytesPerPixel(kRGB_565_SkColorType) * width + padding;
 
                 // If there is padding, we can invent an offset to change the memory alignment
-                for (uint32_t offset = 0; offset <= padding; offset++) {
+                for (uint32_t offset = 0; offset <= padding; offset += 4) {
 
                     // Test all possible start rows with all possible end rows
                     for (uint32_t startRow = 0; startRow < height; startRow++) {
                         for (uint32_t endRow = startRow; endRow < height; endRow++) {
 
-                            // Fill with an index that we use to look up a color
+                            // Test fill with each color type
                             check_fill(r, colorInfo, startRow, endRow, colorRowBytes, offset,
-                                    kFillIndex, colorTable);
-
-                            // Fill with a color
-                            check_fill(r, colorInfo, startRow, endRow, colorRowBytes, offset,
-                                    kFillColor, nullptr);
-
-                            // Fill with an index
+                                    kFillColor);
                             check_fill(r, indexInfo, startRow, endRow, indexRowBytes, offset,
-                                    kFillIndex, nullptr);
-
-                            // Fill a grayscale image
+                                    kFillIndex);
                             check_fill(r, grayInfo, startRow, endRow, grayRowBytes, offset,
-                                    kFillColor, nullptr);
+                                    kFillGray);
+                            check_fill(r, color565Info, startRow, endRow, color565RowBytes, offset,
+                                    kFill565);
                         }
                     }
                 }