Add the ability to decode a subset to SkCodec
authorscroggo <scroggo@chromium.org>
Wed, 22 Jul 2015 14:16:20 +0000 (07:16 -0700)
committerCommit bot <commit-bot@chromium.org>
Wed, 22 Jul 2015 14:16:20 +0000 (07:16 -0700)
This allows codecs that support subsets natively (i.e. WEBP) to do so.

Add a field on SkCodec::Options representing the subset.

Add a method on SkCodec to find a valid subset which approximately
matches a desired subset.

Implement subset decodes in SkWebpCodec.

Add a test in DM for decoding subsets.
Notice that we only start on even boundaries. This is due to the
way libwebp's API works. SkWEBPImageDecoder does not take this into
account, which results in visual artifacts.

FIXME: Subsets with scaling are not pixel identical, but close. (This
may be fine, though - they are not perceptually different. We'll just
need to mark another set of images in gold as valid, once
https://skbug.com/4038 is fixed, so we can tests scaled webp without
generating new images on each run.)

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

13 files changed:
dm/DM.cpp
dm/DMSrcSink.cpp
dm/DMSrcSink.h
include/codec/SkCodec.h
src/codec/SkCodec_libbmp.cpp
src/codec/SkCodec_libgif.cpp
src/codec/SkCodec_libico.cpp
src/codec/SkCodec_libpng.cpp
src/codec/SkCodec_wbmp.cpp
src/codec/SkJpegCodec.cpp
src/codec/SkWebpCodec.cpp
src/codec/SkWebpCodec.h
tests/CodexTest.cpp

index dd072dc..d472be6 100644 (file)
--- a/dm/DM.cpp
+++ b/dm/DM.cpp
@@ -257,6 +257,11 @@ static void push_codec_srcs(Path path) {
                 CodecSrc::kGetFromCanvas_DstColorType, scale));
         push_src("image", "stripe", new CodecSrc(path, CodecSrc::kStripe_Mode,
                 CodecSrc::kGetFromCanvas_DstColorType, scale));
+        // Note: The only codec which supports subsets natively is SkWebpCodec, which will never
+        // report kIndex_8 or kGray_8, so there is no need to test kSubset_mode with those color
+        // types specifically requested.
+        push_src("image", "codec_subset", new CodecSrc(path, CodecSrc::kSubset_Mode,
+                CodecSrc::kGetFromCanvas_DstColorType, scale));
     }
 }
 
index 38597b6..fd13313 100644 (file)
@@ -355,6 +355,83 @@ Error CodecSrc::draw(SkCanvas* canvas) const {
             canvas->drawBitmap(bitmap, 0, 0);
             break;
         }
+        case kSubset_Mode: {
+            // Arbitrarily choose a divisor.
+            int divisor = 2;
+            // Total width/height of the image.
+            const int W = codec->getInfo().width();
+            const int H = codec->getInfo().height();
+            if (divisor > W || divisor > H) {
+                return Error::Nonfatal(SkStringPrintf("Cannot codec subset: divisor %d is too big "
+                                                      "for %s with dimensions (%d x %d)", divisor,
+                                                      fPath.c_str(), W, H));
+            }
+            // subset dimensions
+            // SkWebpCodec, the only one that supports subsets, requires even top/left boundaries.
+            const int w = SkAlign2(W / divisor);
+            const int h = SkAlign2(H / divisor);
+            SkIRect subset;
+            SkCodec::Options opts;
+            opts.fSubset = &subset;
+            SkBitmap subsetBm;
+            // We will reuse pixel memory from bitmap.
+            void* pixels = bitmap.getPixels();
+            // Keep track of left and top (for drawing subsetBm into canvas). We could use
+            // fScale * x and fScale * y, but we want integers such that the next subset will start
+            // where the last one ended. So we'll add decodeInfo.width() and height().
+            int left = 0;
+            for (int x = 0; x < W; x += w) {
+                int top = 0;
+                for (int y = 0; y < H; y+= h) {
+                    // Do not make the subset go off the edge of the image.
+                    const int preScaleW = SkTMin(w, W - x);
+                    const int preScaleH = SkTMin(h, H - y);
+                    subset.setXYWH(x, y, preScaleW, preScaleH);
+                    // And scale
+                    // FIXME: Should we have a version of getScaledDimensions that takes a subset
+                    // into account?
+                    decodeInfo = decodeInfo.makeWH(SkScalarRoundToInt(preScaleW * fScale),
+                                                   SkScalarRoundToInt(preScaleH * fScale));
+                    size_t rowBytes = decodeInfo.minRowBytes();
+                    if (!subsetBm.installPixels(decodeInfo, pixels, rowBytes, colorTable.get(),
+                                                NULL, NULL)) {
+                        return SkStringPrintf("could not install pixels for %s.", fPath.c_str());
+                    }
+                    const SkCodec::Result result = codec->getPixels(decodeInfo, pixels, rowBytes,
+                            &opts, colorPtr, colorCountPtr);
+                    switch (result) {
+                        case SkCodec::kSuccess:
+                        case SkCodec::kIncompleteInput:
+                            break;
+                        case SkCodec::kInvalidConversion:
+                            if (0 == (x|y)) {
+                                // First subset is okay to return unimplemented.
+                                return Error::Nonfatal("Incompatible colortype conversion");
+                            }
+                            // If the first subset succeeded, a later one should not fail.
+                            // fall through to failure
+                        case SkCodec::kUnimplemented:
+                            if (0 == (x|y)) {
+                                // First subset is okay to return unimplemented.
+                                return Error::Nonfatal("subset codec not supported");
+                            }
+                            // If the first subset succeeded, why would a later one fail?
+                            // fall through to failure
+                        default:
+                            return SkStringPrintf("subset codec failed to decode (%d, %d, %d, %d) "
+                                                  "from %s with dimensions (%d x %d)\t error %d",
+                                                  x, y, decodeInfo.width(), decodeInfo.height(),
+                                                  fPath.c_str(), W, H, result);
+                    }
+                    canvas->drawBitmap(subsetBm, SkIntToScalar(left), SkIntToScalar(top));
+                    // translate by the scaled height.
+                    top += decodeInfo.height();
+                }
+                // translate by the scaled width.
+                left += decodeInfo.width();
+            }
+            return "";
+        }
     }
     return "";
 }
index b7c28ed..b80c7c9 100644 (file)
@@ -99,6 +99,7 @@ public:
         kScanline_Mode,
         kScanline_Subset_Mode,
         kStripe_Mode, // Tests the skipping of scanlines
+        kSubset_Mode, // For codecs that support subsets directly.
     };
     enum DstColorType {
         kGetFromCanvas_DstColorType,
index cc635e0..1cdc88d 100644 (file)
@@ -58,6 +58,25 @@ public:
     }
 
     /**
+     *  Return (via desiredSubset) a subset which can decoded from this codec,
+     *  or false if this codec cannot decode subsets or anything similar to
+     *  desiredSubset.
+     *
+     *  @param desiredSubset In/out parameter. As input, a desired subset of
+     *      the original bounds (as specified by getInfo). If true is returned,
+     *      desiredSubset may have been modified to a subset which is
+     *      supported. Although a particular change may have been made to
+     *      desiredSubset to create something supported, it is possible other
+     *      changes could result in a valid subset.
+     *      If false is returned, desiredSubset's value is undefined.
+     *  @return true if this codec supports decoding desiredSubset (as
+     *      returned, potentially modified)
+     */
+    bool getValidSubset(SkIRect* desiredSubset) const {
+        return this->onGetValidSubset(desiredSubset);
+    }
+
+    /**
      *  Format of the encoded data.
      */
     SkEncodedFormat getEncodedFormat() const { return this->onGetEncodedFormat(); }
@@ -128,9 +147,20 @@ public:
      */
     struct Options {
         Options()
-            : fZeroInitialized(kNo_ZeroInitialized) {}
+            : fZeroInitialized(kNo_ZeroInitialized)
+            , fSubset(NULL)
+        {}
 
         ZeroInitialized fZeroInitialized;
+        /**
+         *  If not NULL, represents a subset of the original image to decode.
+         *
+         *  Must be within the bounds returned by getInfo().
+         *
+         *  If the EncodedFormat is kWEBP_SkEncodedFormat (the only one which
+         *  currently supports subsets), the top and left values must be even.
+         */
+        SkIRect*        fSubset;
     };
 
     /**
@@ -228,6 +258,11 @@ protected:
                                void* pixels, size_t rowBytes, const Options&,
                                SkPMColor ctable[], int* ctableCount) = 0;
 
+    virtual bool onGetValidSubset(SkIRect* /* desiredSubset */) const {
+        // By default, subsets are not supported.
+        return false;
+    }
+
     /**
      *  Override if your codec supports scanline decoding.
      *
index 3ac4b0b..bd5d2ca 100644 (file)
@@ -585,6 +585,10 @@ SkCodec::Result SkBmpCodec::onGetPixels(const SkImageInfo& dstInfo,
             return kCouldNotRewind;
         }
     }
+    if (opts.fSubset) {
+        // Subsets are not supported.
+        return kUnimplemented;
+    }
     if (dstInfo.dimensions() != this->getInfo().dimensions()) {
         SkCodecPrintf("Error: scaling not supported.\n");
         return kInvalidScale;
index fb578f2..9b15151 100644 (file)
@@ -257,6 +257,10 @@ SkCodec::Result SkGifCodec::onGetPixels(const SkImageInfo& dstInfo,
     }
 
     // Check for valid input parameters
+    if (opts.fSubset) {
+        // Subsets are not supported.
+        return kUnimplemented;
+    }
     if (dstInfo.dimensions() != this->getInfo().dimensions()) {
         return gif_error("Scaling not supported.\n", kInvalidScale);
     }
index 97404af..7df4879 100644 (file)
@@ -229,6 +229,10 @@ SkCodec::Result SkIcoCodec::onGetPixels(const SkImageInfo& dstInfo,
                                         void* dst, size_t dstRowBytes,
                                         const Options& opts, SkPMColor* ct,
                                         int* ptr) {
+    if (opts.fSubset) {
+        // Subsets are not supported.
+        return kUnimplemented;
+    }
     // We return invalid scale if there is no candidate image with matching
     // dimensions.
     Result result = kInvalidScale;
index 7f9aeaa..553233d 100644 (file)
@@ -517,6 +517,10 @@ SkCodec::Result SkPngCodec::onGetPixels(const SkImageInfo& requestedInfo, void*
     if (!conversion_possible(requestedInfo, this->getInfo())) {
         return kInvalidConversion;
     }
+    if (options.fSubset) {
+        // Subsets are not supported.
+        return kUnimplemented;
+    }
     if (requestedInfo.dimensions() != this->getInfo().dimensions()) {
         return kInvalidScale;
     }
index 9709a68..35ac808 100644 (file)
@@ -103,7 +103,7 @@ SkEncodedFormat SkWbmpCodec::onGetEncodedFormat() const {
 SkCodec::Result SkWbmpCodec::onGetPixels(const SkImageInfo& info,
                                          void* pixels,
                                          size_t rowBytes,
-                                         const Options&,
+                                         const Options& options,
                                          SkPMColor ctable[],
                                          int* ctableCount) {
     SkCodec::RewindState rewindState = this->rewindIfNeeded();
@@ -112,6 +112,10 @@ SkCodec::Result SkWbmpCodec::onGetPixels(const SkImageInfo& info,
     } else if (rewindState == kRewound_RewindState) {
         (void)read_header(this->stream(), NULL);
     }
+    if (options.fSubset) {
+        // Subsets are not supported.
+        return kUnimplemented;
+    }
     if (info.dimensions() != this->getInfo().dimensions()) {
         return kInvalidScale;
     }
index 28e1e12..5acc0b3 100644 (file)
@@ -308,6 +308,11 @@ SkCodec::Result SkJpegCodec::onGetPixels(const SkImageInfo& dstInfo,
         return fDecoderMgr->returnFailure("could not rewind stream", kCouldNotRewind);
     }
 
+    if (options.fSubset) {
+        // Subsets are not supported.
+        return kUnimplemented;
+    }
+
     // Get a pointer to the decompress info since we will use it quite frequently
     jpeg_decompress_struct* dinfo = fDecoderMgr->dinfo();
 
index 32a8b78..fea557d 100644 (file)
@@ -125,8 +125,26 @@ static WEBP_CSP_MODE webp_decode_mode(SkColorType ct, bool premultiply) {
 // is arbitrary.
 static const size_t BUFFER_SIZE = 4096;
 
+bool SkWebpCodec::onGetValidSubset(SkIRect* desiredSubset) const {
+    if (!desiredSubset) {
+        return false;
+    }
+
+    SkIRect bounds = SkIRect::MakeSize(this->getInfo().dimensions());
+    if (!desiredSubset->intersect(bounds)) {
+        return false;
+    }
+
+    // As stated below, libwebp snaps to even left and top. Make sure top and left are even, so we
+    // decode this exact subset.
+    // Leave right and bottom unmodified, so we suggest a slightly larger subset than requested.
+    desiredSubset->fLeft = (desiredSubset->fLeft >> 1) << 1;
+    desiredSubset->fTop = (desiredSubset->fTop >> 1) << 1;
+    return true;
+}
+
 SkCodec::Result SkWebpCodec::onGetPixels(const SkImageInfo& dstInfo, void* dst, size_t rowBytes,
-                                         const Options&, SkPMColor*, int*) {
+                                         const Options& options, SkPMColor*, int*) {
     switch (this->rewindIfNeeded()) {
         case kCouldNotRewind_RewindState:
             return kCouldNotRewind;
@@ -153,12 +171,48 @@ SkCodec::Result SkWebpCodec::onGetPixels(const SkImageInfo& dstInfo, void* dst,
     // Free any memory associated with the buffer. Must be called last, so we declare it first.
     SkAutoTCallVProc<WebPDecBuffer, WebPFreeDecBuffer> autoFree(&(config.output));
 
-    SkISize dimensions = dstInfo.dimensions();
-    if (this->getInfo().dimensions() != dimensions) {
+    SkIRect bounds = SkIRect::MakeSize(this->getInfo().dimensions());
+    if (options.fSubset) {
+        // Caller is requesting a subset.
+        if (!bounds.contains(*options.fSubset)) {
+            // The subset is out of bounds.
+            return kInvalidParameters;
+        }
+
+        bounds = *options.fSubset;
+
+        // This is tricky. libwebp snaps the top and left to even values. We could let libwebp
+        // do the snap, and return a subset which is a different one than requested. The problem
+        // with that approach is that the caller may try to stitch subsets together, and if we
+        // returned different subsets than requested, there would be artifacts at the boundaries.
+        // Instead, we report that we cannot support odd values for top and left..
+        if (!SkIsAlign2(bounds.fLeft) || !SkIsAlign2(bounds.fTop)) {
+            return kInvalidParameters;
+        }
+
+#ifdef SK_DEBUG
+        {
+            // Make a copy, since getValidSubset can change its input.
+            SkIRect subset(bounds);
+            // That said, getValidSubset should *not* change its input, in this case; otherwise
+            // getValidSubset does not match the actual subsets we can do.
+            SkASSERT(this->getValidSubset(&subset) && subset == bounds);
+        }
+#endif
+
+        config.options.use_cropping = 1;
+        config.options.crop_left = bounds.fLeft;
+        config.options.crop_top = bounds.fTop;
+        config.options.crop_width = bounds.width();
+        config.options.crop_height = bounds.height();
+    }
+
+    SkISize dstDimensions = dstInfo.dimensions();
+    if (bounds.size() != dstDimensions) {
         // Caller is requesting scaling.
         config.options.use_scaling = 1;
-        config.options.scaled_width = dimensions.width();
-        config.options.scaled_height = dimensions.height();
+        config.options.scaled_width = dstDimensions.width();
+        config.options.scaled_height = dstDimensions.height();
     }
 
     config.output.colorspace = webp_decode_mode(dstInfo.colorType(),
index 9ea6a94..1fd3acb 100644 (file)
@@ -30,6 +30,8 @@ protected:
     }
 
     SkISize onGetScaledDimensions(float desiredScale) const override;
+
+    bool onGetValidSubset(SkIRect* /* desiredSubset */) const override;
 private:
     SkWebpCodec(const SkImageInfo&, SkStream*);
 
index f468205..82e490a 100644 (file)
@@ -9,6 +9,7 @@
 #include "SkBitmap.h"
 #include "SkCodec.h"
 #include "SkMD5.h"
+#include "SkRandom.h"
 #include "SkScanlineDecoder.h"
 #include "Test.h"
 
@@ -41,10 +42,23 @@ static void compare_to_good_digest(skiatest::Reporter* r, const SkMD5::Digest& g
     REPORTER_ASSERT(r, digest == goodDigest);
 }
 
+SkIRect generate_random_subset(SkRandom* rand, int w, int h) {
+    SkIRect rect;
+    do {
+        rect.fLeft = rand->nextRangeU(0, w);
+        rect.fTop = rand->nextRangeU(0, h);
+        rect.fRight = rand->nextRangeU(0, w);
+        rect.fBottom = rand->nextRangeU(0, h);
+        rect.sort();
+    } while (rect.isEmpty());
+    return rect;
+}
+
 static void check(skiatest::Reporter* r,
                   const char path[],
                   SkISize size,
-                  bool supportsScanlineDecoding) {
+                  bool supportsScanlineDecoding,
+                  bool supportsSubsetDecoding) {
     SkAutoTDelete<SkStream> stream(resource(path));
     if (!stream) {
         SkDebugf("Missing resource '%s'\n", path);
@@ -102,53 +116,88 @@ static void check(skiatest::Reporter* r,
     } else {
         REPORTER_ASSERT(r, !scanlineDecoder);
     }
+
+    // The rest of this function tests decoding subsets, and will decode an arbitrary number of
+    // random subsets.
+    // Do not attempt to decode subsets of an image of only once pixel, since there is no
+    // meaningful subset.
+    if (size.width() * size.height() == 1) {
+        return;
+    }
+
+    SkRandom rand;
+    SkIRect subset;
+    SkCodec::Options opts;
+    opts.fSubset = &subset;
+    for (int i = 0; i < 5; i++) {
+        subset = generate_random_subset(&rand, size.width(), size.height());
+        SkASSERT(!subset.isEmpty());
+        const bool supported = codec->getValidSubset(&subset);
+        REPORTER_ASSERT(r, supported == supportsSubsetDecoding);
+
+        SkImageInfo subsetInfo = info.makeWH(subset.width(), subset.height());
+        SkBitmap bm;
+        bm.allocPixels(subsetInfo);
+        const SkCodec::Result result = codec->getPixels(bm.info(), bm.getPixels(), bm.rowBytes(),
+                                                        &opts, NULL, NULL);
+
+        if (supportsSubsetDecoding) {
+            REPORTER_ASSERT(r, result == SkCodec::kSuccess);
+            // 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));
+        } else {
+            // No subsets will work.
+            REPORTER_ASSERT(r, result == SkCodec::kUnimplemented);
+        }
+    }
 }
 
 DEF_TEST(Codec, r) {
     // WBMP
-    check(r, "mandrill.wbmp", SkISize::Make(512, 512), false);
+    check(r, "mandrill.wbmp", SkISize::Make(512, 512), false, false);
 
     // WEBP
-    check(r, "baby_tux.webp", SkISize::Make(386, 395), false);
-    check(r, "color_wheel.webp", SkISize::Make(128, 128), false);
-    check(r, "yellow_rose.webp", SkISize::Make(400, 301), false);
+    check(r, "baby_tux.webp", SkISize::Make(386, 395), false, true);
+    check(r, "color_wheel.webp", SkISize::Make(128, 128), false, true);
+    check(r, "yellow_rose.webp", SkISize::Make(400, 301), false, true);
 
     // BMP
-    check(r, "randPixels.bmp", SkISize::Make(8, 8), false);
+    check(r, "randPixels.bmp", SkISize::Make(8, 8), false, false);
 
     // ICO
     // These two tests examine interestingly different behavior:
     // Decodes an embedded BMP image
-    check(r, "color_wheel.ico", SkISize::Make(128, 128), false);
+    check(r, "color_wheel.ico", SkISize::Make(128, 128), false, false);
     // Decodes an embedded PNG image
-    check(r, "google_chrome.ico", SkISize::Make(256, 256), false);
+    check(r, "google_chrome.ico", SkISize::Make(256, 256), false, false);
 
     // GIF
-    check(r, "box.gif", SkISize::Make(200, 55), false);
-    check(r, "color_wheel.gif", SkISize::Make(128, 128), false);
-    check(r, "randPixels.gif", SkISize::Make(8, 8), false);
+    check(r, "box.gif", SkISize::Make(200, 55), false, false);
+    check(r, "color_wheel.gif", SkISize::Make(128, 128), false, false);
+    check(r, "randPixels.gif", SkISize::Make(8, 8), false, false);
 
     // JPG
-    check(r, "CMYK.jpg", SkISize::Make(642, 516), true);
-    check(r, "color_wheel.jpg", SkISize::Make(128, 128), true);
-    check(r, "grayscale.jpg", SkISize::Make(128, 128), true);
-    check(r, "mandrill_512_q075.jpg", SkISize::Make(512, 512), true);
-    check(r, "randPixels.jpg", SkISize::Make(8, 8), true);
+    check(r, "CMYK.jpg", SkISize::Make(642, 516), true, false);
+    check(r, "color_wheel.jpg", SkISize::Make(128, 128), true, false);
+    check(r, "grayscale.jpg", SkISize::Make(128, 128), true, false);
+    check(r, "mandrill_512_q075.jpg", SkISize::Make(512, 512), true, false);
+    check(r, "randPixels.jpg", SkISize::Make(8, 8), true, false);
 
     // PNG
-    check(r, "arrow.png", SkISize::Make(187, 312), true);
-    check(r, "baby_tux.png", SkISize::Make(240, 246), true);
-    check(r, "color_wheel.png", SkISize::Make(128, 128), true);
-    check(r, "half-transparent-white-pixel.png", SkISize::Make(1, 1), true);
-    check(r, "mandrill_128.png", SkISize::Make(128, 128), true);
-    check(r, "mandrill_16.png", SkISize::Make(16, 16), true);
-    check(r, "mandrill_256.png", SkISize::Make(256, 256), true);
-    check(r, "mandrill_32.png", SkISize::Make(32, 32), true);
-    check(r, "mandrill_512.png", SkISize::Make(512, 512), true);
-    check(r, "mandrill_64.png", SkISize::Make(64, 64), true);
-    check(r, "plane.png", SkISize::Make(250, 126), true);
-    check(r, "randPixels.png", SkISize::Make(8, 8), true);
-    check(r, "yellow_rose.png", SkISize::Make(400, 301), true);
+    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, "randPixels.png", SkISize::Make(8, 8), true, false);
+    check(r, "yellow_rose.png", SkISize::Make(400, 301), true, false);
 }
 
 static void test_invalid_stream(skiatest::Reporter* r, const void* stream, size_t len) {