Add scaled subset API to SkCodec

include/codec/SkCodec.h

 /*
  * Copyright 2015 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #ifndef SkCodec_DEFINED
 #define SkCodec_DEFINED
 
@@ skipping 53 matching lines @@
 
         // 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);
     }
 
     /**
-     *  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(); }
 
     /**
      *  Used to describe the result of a call to getPixels().
      *
      *  Result is the union of possible results from subclasses.
      */
     enum Result {
@@ skipping 52 matching lines @@
          */
         kNo_ZeroInitialized,
     };
 
     /**
      *  Additional options to pass to getPixels.
      */
     struct Options {
         Options()
             : fZeroInitialized(kNo_ZeroInitialized)
-            , fSubset(NULL)
+            , fSubset(nullptr)
+            , fScaledDimensions(nullptr)
+            , fScaledSubset(nullptr)
         {}
 
         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.
+         *  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 non-NULL for a
+         *  scaled subset decode, and both should be NULL otherwise.
+         *
+         *  If fScaledDimensions and fScaledSubset are NULL, 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

[scroggo, 2015/10/12 20:47:06]

Maybe it is worth noting that (2) and (3) are the same as if there was no
Options object?

+         *          original dimensions, we are performing an unscaled, full
+         *          image decode.
+         *
+         *  If both are non-NULL we are performing a scaled subset decode:

[scroggo, 2015/10/12 20:47:06]

Which "both"? fScaledDimensions and fScaledSubset?

+         *      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.
          *
          *  In getPixels, we will attempt to decode the exact rectangular
          *  subset specified by fSubset.
          *
          *  In a scanline decode, it does not make sense to specify a subset
          *  top or subset height, since the client already controls which rows
          *  to get and which rows to skip.  During scanline decodes, we will
          *  require that the subset top be zero and the subset height be equal
          *  to the full height.  We will, however, use the values of
          *  subset left and subset width to decode partial scanlines on calls
          *  to getScanlines().
          */
         SkIRect*        fSubset;
+        SkISize*        fScaledDimensions;
+        SkIRect*        fScaledSubset;
     };
 
     /**
+     *  @param desiredScale The requested scale factor.
+     *  @param options      In/out parameter.
+     *                      options->fSubset (in/out) specifies the

[scroggo, 2015/10/12 20:47:06]

If we wanted to, we could make options an optional parameter (haha) and combine
this with getScaledDimensions. I'm not sure whether that's better. (My suspicion
is that since most codecs do not support subsets at all, it makes sense to keep
them separate, so asking for a scale does not worry about subsetting, but I do
think it's at least worth considering.)

Sort of on the other extreme, it seems like SkCodec needs to do a lot of work to
handle subsets, which are only supported by two subclasses. I wonder if we
should refactor things so that SkCodec doesn't need to know anything about
subsets. The sticky point here is webp, which supports subsets natively instead
of supporting scanline decoding. I think we have the following options:
- This approach, where SkCodec has to deal with subsets
- Sort of a hybrid approach, where SkCodec knows minimally about subsets (in
order to support webp). All of this new functionality goes directly on
SkScaledCodec (which I think will need to pass through to SkWebpCodec). Maybe
SkScaledCodec's API looks very different, as it will be focused on handling
subsets.
- All subset handling is done inside SkScaledCodec. Webp will potentially be
very slow, since we do not have a scanline decoder. (We could write a dumb one
though, which decodes 16 scanlines (the blocks that webp are stored in) and
stores them, and then returns one line at a time).
- Maybe SkScaledCodec has its own interface, for which webp has its own
implementation.

There are probably more options I haven't thought of, and it will probably make
sense to discuss in person.

+     *                      requested subset in terms of the original image
+     *                      dimensions.  This may be modified to a subset
+     *                      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.

[scroggo, 2015/10/12 20:47:06]

"in terms of fScaledDimensions" is not intuitive to me.

Do you mean it specifies fSubset after scaling the entire image to
fScaledDimensions? I think something like this may be more clear.

+     *
+     *  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

[scroggo, 2015/10/12 20:47:06]

You've removed getValidSubset, so I assume the intent is to use this function
with desiredScale == 1.0f, but the comments do not describe that use case.

It seems like there should be an option to pass 1.0f and just a subset, and the
caller need never worry about fScaledDimensions or fScaledSubset, since they are
doing neither.

On that note, you do not mention who owns fScaledDimensions or fScaledSubset.
I'm assuming that the client does the following:

Options options;
options.fSubset = &some_rect;
options.fScaledDimensions = &uninitialized_dim;
options.fScaledSubset = &uninitialized_rect;

if (codec->getScaledSubsetDimensions(scale, options)) {
  ...
}

So they handle the pointers.

+     *               successfully set to values that the codec supports.
+     *          false otherwise.
+     */
+    bool getScaledSubsetDimensions(float desiredScale, const Options& options) const;

[reed1, 2015/10/12 20:21:00]

wow, pretty complicated rules for calling this. Are all combos required for
android?
Can we have 2-3 separate calls (w/ diff names / parameters) to distinguish the
different variations?
e.g.
    getScaledDimensions(float, SkIRect* inOut);
    getSubsetDimensions(float, SkIRect* inOut);
   ...

[scroggo, 2015/10/12 20:47:07]

The options object does not appear to be const.

+
+    /**
      *  Decode into the given pixels, a block of memory of size at
      *  least (info.fHeight - 1) * rowBytes + (info.fWidth *
      *  bytesPerPixel)
      *
      *  Repeated calls to this function should give the same results,
      *  allowing the PixelRef to be immutable.
      *
      *  @param info A description of the format (config, size)
      *         expected by the caller.  This can simply be identical
      *         to the info returned by getInfo().
@@ skipping 200 matching lines @@
     int outputScanline(int inputScanline) const;
 
 protected:
     SkCodec(const SkImageInfo&, SkStream*);
 
     virtual SkISize onGetScaledDimensions(float /* desiredScale */) const {
         // By default, scaling is not supported.
         return this->getInfo().dimensions();
     }
 
-    // FIXME: What to do about subsets??
+    virtual bool onGetScaledSubsetDimensions(float desiredScale, const Options& options) const {
+        // By default, scaled subsetting is not supported.
+        return false;
+    }
+
     /**
      *  Subclasses should override if they support dimensions other than the
      *  srcInfo's.
      */
     virtual bool onDimensionsSupported(const SkISize&) {
         return false;
     }
 
+    /**
+     *  Subclasses should override depending on how they support subsets.
+     *
+     *  The default implementation only supports subsetting in the
+     *  x-dimension during a scanline decode.
+     */
+    virtual bool onSubsetSupported(const SkIRect&, bool isScanlineDecode);
+
+    /**
+     *  Subclasses should override depenging on how they support scaled subsetting.
+     *
+     *  If the scale is supported, the default implementation will support subsetting
+     *  in the x-dimension during a scanline decode.
+     */
+    virtual bool onScaledSubsetSupported(const Options&, bool isScanlineDecode);
+
     virtual SkEncodedFormat onGetEncodedFormat() const = 0;
 
     /**
      * @param rowsDecoded When the encoded image stream is incomplete, this function
      *                    will return kIncompleteInput and rowsDecoded will be set to
      *                    the number of scanlines that were successfully decoded.
      *                    This will allow getPixels() to fill the uninitialized memory.
      */
     virtual Result onGetPixels(const SkImageInfo& info,
                                void* pixels, size_t rowBytes, const Options&,
@@ skipping 100 matching lines @@
      *  The codec may choose to cache the information about scale and subset.
      *  Either way, the same information will be passed to onGetPixels/onStart
      *  on success.
      *
      *  This must return true for a size returned from getScaledDimensions.
      */
     bool dimensionsSupported(const SkISize& dim) {
         return dim == fSrcInfo.dimensions() || this->onDimensionsSupported(dim);
     }
 
+    /**
+     *  Return whether decoding this subset is supported.
+     */
+    bool subsetSupported(const SkIRect& subset, bool isScanlineDecode);
+
+    /**
+     *  Return whether a decode with the specified options is supported
+     */
+    bool scaledSubsetSupported(const SkISize& dim, const Options& options, bool isScanlineDecode);
+
     // Methods for scanline decoding.
     virtual SkCodec::Result onStartScanlineDecode(const SkImageInfo& dstInfo,
             const SkCodec::Options& options, SkPMColor ctable[], int* ctableCount) {
         return kUnimplemented;
     }
 
     // Naive default version just calls onGetScanlines on temp memory.
     virtual bool onSkipScanlines(int countLines) {
         // FIXME (msarett): Make this a pure virtual and always override this.
         SkAutoMalloc storage(fDstInfo.minRowBytes());
@@ skipping 31 matching lines @@
      *  May create a sampler, if one is not currently being used. Otherwise, does
      *  not affect ownership.
      *
      *  Only valid during scanline decoding.
      */
     virtual SkSampler* getSampler(bool createIfNecessary) { return nullptr; }
 
     friend class SkScaledCodec;
 };
 #endif // SkCodec_DEFINED