Add subsetting to SkScanlineDecoder

src/codec/SkCodec.cpp

Index: src/codec/SkCodec.cpp
diff --git a/src/codec/SkCodec.cpp b/src/codec/SkCodec.cpp
index 3ad58f3077633dd94f87becf1ce0a63984383a01..9e1d9f463d95f0d11e52770800035f550a0560a1 100644
--- a/src/codec/SkCodec.cpp
+++ b/src/codec/SkCodec.cpp
@@ -111,6 +111,34 @@ bool SkCodec::rewindIfNeeded() {
     return this->onRewind();
 }
 
+SkISize SkCodec::getScaledDimensions(float desiredScale) const {

[scroggo, 2015/10/08 20:16:07]

This appears to be moved here unchanged. What was the motivation for it? It
seems like an independent CL (unless something else depends on it?).

[msarett, 2015/10/09 20:03:44]

There's no good reason, except that it helped me to see everything in one place.

All it does is add clutter to this CL, moving it back.

+    // Negative and zero scales are errors.
+    SkASSERT(desiredScale > 0.0f);
+    if (desiredScale <= 0.0f) {
+        return SkISize::Make(0, 0);
+    }
+
+    // Upscaling is not supported. Return the original size if the client
+    // requests an upscale.
+    if (desiredScale >= 1.0f) {
+        return this->getInfo().dimensions();
+    }
+    return this->onGetScaledDimensions(desiredScale);
+}
+
+bool SkCodec::getValidSubset(SkIRect* desiredSubset) const {
+    if (!desiredSubset) {
+        return false;
+    }
+
+    SkIRect dimensions  = SkIRect::MakeSize(this->getInfo().dimensions());

[scroggo, 2015/10/08 20:16:07]

This check seems unnecessary for subclasses which do not support subsets at all.

[msarett, 2015/10/09 20:03:44]

Agreed.  I think my motivation was to refactor shared code, but now I'm noticing
that it's not even shared in this CL.

+    if (!dimensions.contains(*desiredSubset)) {
+        return false;
+    }
+
+    return this->onGetValidSubset(desiredSubset);
+}
+
 SkCodec::Result SkCodec::getPixels(const SkImageInfo& info, void* pixels, size_t rowBytes,
                                    const Options* options, SkPMColor ctable[], int* ctableCount) {
     if (kUnknown_SkColorType == info.colorType()) {
@@ -198,6 +226,7 @@ SkCodec::Result SkCodec::startScanlineDecode(const SkImageInfo& dstInfo,
         const SkCodec::Options* options, SkPMColor ctable[], int* ctableCount) {
     // Reset fCurrScanline in case of failure.
     fCurrScanline = -1;
+
     // Ensure that valid color ptrs are passed in for kIndex8 color type
     if (kIndex_8_SkColorType == dstInfo.colorType()) {
         if (nullptr == ctable || nullptr == ctableCount) {
@@ -220,10 +249,14 @@ SkCodec::Result SkCodec::startScanlineDecode(const SkImageInfo& dstInfo,
     if (nullptr == options) {
         options = &optsStorage;
     } else if (options->fSubset) {
-        SkIRect subset(*options->fSubset);
-        if (!this->onGetValidSubset(&subset) || subset != *options->fSubset) {
-            // FIXME: How to differentiate between not supporting subset at all
-            // and not supporting this particular subset?
+        SkIRect size = SkIRect::MakeSize(dstInfo.dimensions());
+        if (!size.contains(*options->fSubset)) {
+            return kInvalidInput;
+        }
+
+        // We only support subsetting in the x-dimension for scanline decoder.

[scroggo, 2015/10/08 20:16:07]

We haven't really talked about how scanline decoding should handle fSubset. It
seems it will be different between getPixels and startScanlineDecode. We should
document that in the header.

[msarett, 2015/10/09 20:03:44]

Will add some documentation.

This check is kind of a stop-gap.  It will most likely be refactored again in
the next CL.

+        // Subsetting in the y-dimension can be accomplished using skipScanlines().
+        if (options->fSubset->top() != 0 || options->fSubset->height() != dstInfo.height()) {
             return kUnimplemented;

[scroggo, 2015/10/08 20:16:07]

Maybe this should be kInvalidInput?

[msarett, 2015/10/09 20:03:44]

Done.

         }
     }
@@ -233,14 +266,14 @@ SkCodec::Result SkCodec::startScanlineDecode(const SkImageInfo& dstInfo,
         return kInvalidScale;
     }
 
+    fDstInfo = dstInfo;
+    fOptions = *options;
     const Result result = this->onStartScanlineDecode(dstInfo, *options, ctable, ctableCount);
     if (result != SkCodec::kSuccess) {
         return result;
     }
 
     fCurrScanline = 0;
-    fDstInfo = dstInfo;
-    fOptions = *options;
     return kSuccess;
 }