Add subsetting to SkScaledCodec

include/codec/SkCodec.h

Index: include/codec/SkCodec.h
diff --git a/include/codec/SkCodec.h b/include/codec/SkCodec.h
index 7ed93208e0ec7873799cb2008b702842bc2b4bbf..4fdc75cc2a92a03d69890f0fc707fae064fb980d 100644
--- a/include/codec/SkCodec.h
+++ b/include/codec/SkCodec.h
@@ -54,20 +54,7 @@ public:
      *  The returned value is the codec's suggestion for the closest valid
      *  scale that it can natively support
      */
-    SkISize getScaledDimensions(float desiredScale) const {
-        // 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);
-    }
+    SkISize getScaledDimensions(float desiredScale) const;
 
     /**
      *  Return (via desiredSubset) a subset which can decoded from this codec,
@@ -84,9 +71,7 @@ public:
      *  @return true if this codec supports decoding desiredSubset (as
      *      returned, potentially modified)
      */
-    bool getValidSubset(SkIRect* desiredSubset) const {
-        return this->onGetValidSubset(desiredSubset);
-    }
+    bool getValidSubset(SkIRect* desiredSubset) const;
 
     /**
      *  Format of the encoded data.
@@ -162,21 +147,79 @@ public:
         Options()
             : fZeroInitialized(kNo_ZeroInitialized)
             , fSubset(NULL)
+            , fScaledDimensions(SkISize::Make(0, 0))
+            , fScaledSubset(SkIRect::MakeEmpty())
         {}
 
         ZeroInitialized fZeroInitialized;
+
         /**
-         *  If not NULL, represents a subset of the original image to decode.
+         *  fSubset represents a subset of the original image to decode.
+         *  It must be within the bounds returned by getInfo().
+         *  If the EncodedFormat is kWEBP_SkEncodedFormat, the top and left
+         *  values must be even.
+         *  If fSubset is NULL, we are not performing a subset decode.
+         *
+         *  fScaledDimensions and fScaledSubset are used together to help
+         *  specify a scaled subset decode.  Both should be specified for a
+         *  scaled subset decode, and both should be zeroed otherwise.
          *
-         *  Must be within the bounds returned by getInfo().
+         *  If fScaledDimensions and fScaledSubset are zeroed, we are not
+         *  performing a scaled subset decode:
+         *      (1) If fSubset is non-NULL, we are performing an unscaled
+         *          subset decode, and the subset dimensions must match the
+         *          dstInfo dimensions.
+         *      (2) If fSubset is NULL and the dstInfo dimensions do not
+         *          match the original dimensions, we are performing a
+         *          scaled decode.
+         *      (3) If fSubset is NULL and the dstInfo dimensions match the
+         *          original dimensions, we are performing an unscaled, full
+         *          image decode.
          *
-         *  If the EncodedFormat is kWEBP_SkEncodedFormat (the only one which
-         *  currently supports subsets), the top and left values must be even.
+         *  If both are non-NULL we are performing a scaled subset decode:
+         *      fSubset must be non-NULL.
+         *      fScaledSubset must be within the bounds of fScaledDimensions.
+         *      The dimensions of fScaledSubset must match the dimensions
+         *      specified by dstInfo.
+         *
+         *  Usage:
+         *      Call getScaledSubsetDimensions() with the desired fSubset of
+         *      the original image and the desired scale.  The function will
+         *      populate the options object with valid values of
+         *      fScaledDimensions and fScaledSubset that can be decoded
+         *      efficiently.
          */
         SkIRect*        fSubset;
+        SkISize         fScaledDimensions;

[scroggo, 2015/10/02 18:27:03]

For fSubset, we decided to make it an unowned pointer, so that a client that
does not want to specify it need not pay the cost of initializing the SkIRect. I
think we should make the same decision for all three options.

[msarett, 2015/10/06 23:01:27]

I'll change to be consistent.

My concern is that a client will pass nullptr for these fields, since we are
responsible for initializing them.  We can't really deal with that because we
want the client to own the memory.

Not sure if it would seem to strange to a client to need to pass in valid
pointers to uninitialized objects.

[scroggo, 2015/10/07 17:49:39]

I don't think it's so bad. In this case it will just be an out variable from one
method (we do follow this pattern in other places, where we pass an
uninitialized variable into a method, which returns true if it initialized it),
and then an input variable to another (onGetPixels, I think?).

+        SkIRect         fScaledSubset;
     };
 
     /**
+     *  @param desiredScale The requested scale factor.
+     *  @param options      In/out parameter.
+     *                      options->fSubset (in/out) specifies the
+     *                      requested subset in terms of the original image
+     *                      dimensions.  This may be modified to a subset

[scroggo, 2015/10/02 18:27:03]

So if I understand correctly, this is analogous to getValidSubset? Why not
combine the two? If someone wants a non-scaled subset, they can always use a
scale of 1.

[msarett, 2015/10/06 23:01:27]

I'm in favor of combining.  I had considered that previously but wasn't sure.

+     *                      that can be decoded more efficiently.
+     *                      options->fScaledDimensions is an output and
+     *                      specifies the best scaled (full image) output
+     *                      dimensions that can be achieved for the
+     *                      desiredScale.
+     *                      options->fScaledSubset is an output and specifies
+     *                      the fSubset in terms of fScaledDimensions.
+     *
+     *  The codec may not be able to scale and subset efficiently to the exact
+     *  scale and subset requested, so fScaledDimensions and fScaledSubset may
+     *  be approximate.  These output values are the codec's suggestion for
+     *  the closest valid scaled subset that it can support.
+     *
+     *  @return true if fScaledDimensions and fScaledSubset have been
+     *               successfully set to values that the codec supports.
+     *          false otherwise.
+     */
+    bool getScaledSubsetDimensions(float desiredScale, Options* options) const;
+
+    /**
      *  Decode into the given pixels, a block of memory of size at
      *  least (info.fHeight - 1) * rowBytes + (info.fWidth *
      *  bytesPerPixel)
@@ -255,14 +298,25 @@ public:
      *      dstInfo.colorType() is kIndex8, this should be non-NULL.  It will
      *      be modified to the true size of the color table (<= 256) after
      *      decoding the palette.
+     *  @param subsetLeft The left offset at which to begin decoding each
+     *      scanline.  This enables partial scanline decodes.
+     *  @param subsetWidth The number of pixels to decode in each scanline row.

[scroggo, 2015/10/02 18:27:03]

My first thought was that this could be represented by the dstInfo, but that
represents a scale, so I suppose we do need a separate value for this.

Should these be on the Options object? We already have a long list of parameters
on startScanlineDecode. I suppose moving to the Options object means having a
long list of items there, but it has two advantages:

- The options have names
- It separates the optional from the required

On that note, I think we should consider moving the ctable stuff to the Options
object.

[msarett, 2015/10/06 23:01:27]

I'm in favor of putting all of this on the Options object.

      *  @return Enum representing success or reason for failure.
      */
     Result startScanlineDecode(const SkImageInfo& dstInfo, const SkCodec::Options* options,
-                 SkPMColor ctable[], int* ctableCount);
+                 SkPMColor ctable[], int* ctableCount, int subsetLeft, int subsetWidth);
+
+    /**
+     *  Simplified version of startScanlineDecode() that does not enable partial
+     *  scanline decoding.
+     */
+    Result startScanlineDecode(const SkImageInfo& dstInfo, const SkCodec::Options* options,
+            SkPMColor ctable[], int* ctableCount);
 
     /**
      *  Simplified version of startScanlineDecode() that asserts that info is NOT
-     *  kIndex8_SkColorType and uses the default Options.
+     *  kIndex8_SkColorType, uses the default Options, and does not enable
+     *  partial scanline decoding.
      */
     Result startScanlineDecode(const SkImageInfo& dstInfo);
 
@@ -397,6 +451,16 @@ protected:
         return this->getInfo().dimensions();
     }
 
+    virtual bool onGetValidSubset(SkIRect* /* desiredSubset */) const {
+        // By default, subsets are not supported.
+        return false;
+    }
+
+    virtual bool onGetScaledSubsetDimensions(float desiredScale, Options* options) const {
+        // By default, scaled subsetting is not supported.
+        return false;
+    }
+
     virtual SkEncodedFormat onGetEncodedFormat() const = 0;
 
     /**
@@ -410,11 +474,6 @@ protected:
                                SkPMColor ctable[], int* ctableCount,
                                int* rowsDecoded) = 0;
 
-    virtual bool onGetValidSubset(SkIRect* /* desiredSubset */) const {
-        // By default, subsets are not supported.
-        return false;
-    }
-
     virtual bool onReallyHasAlpha() const { return false; }
 
     /**
@@ -493,6 +552,8 @@ protected:
 
     const SkCodec::Options& options() const { return fOptions; }
 
+    int subsetWidth() const { return fSubsetWidth; }
+
 private:
     const SkImageInfo       fSrcInfo;
     SkAutoTDelete<SkStream> fStream;
@@ -501,10 +562,12 @@ private:
     SkImageInfo             fDstInfo;
     SkCodec::Options        fOptions;
     int                     fCurrScanline;
+    int                     fSubsetWidth;
 
     // Methods for scanline decoding.
     virtual SkCodec::Result onStartScanlineDecode(const SkImageInfo& dstInfo,
-            const SkCodec::Options& options, SkPMColor ctable[], int* ctableCount) {
+            const SkCodec::Options& options, SkPMColor ctable[], int* ctableCount, int subsetLeft,
+            int subsetWidth) {
         return kUnimplemented;
     }