SkScaledCodec class

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 131 matching lines @@
         kNo_ZeroInitialized,
     };
 
     /**
      *  Additional options to pass to getPixels.
      */
     struct Options {
         Options()
             : fZeroInitialized(kNo_ZeroInitialized)
             , fSubset(NULL)
+            , fSampled(false)
         {}
 
         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;
+        /**
+         * If true, the codec is being sampled

[scroggo, 2015/07/27 19:29:57]

This could be more concise, e.g.

Whether the codec is being sampled.

+         * If false, the codec is not being sampled
+         * The default value is false
+         * Used to determine if dstDimensions can be different than srcDimensions

[scroggo, 2015/07/27 19:29:57]

To be fair, dstDimensions can be different than srcDimensions in Jpeg or WebP,
without setting fSampled to true.

I think this is only used for sampling in the X direction? Maybe it should even
be an integer, representing the sample size in the x direction? I think that
would allow us to remove setSampleX, since it would always be set at the outset.

This might also let us check to make sure the dst width matches the desired
sampling.

[msarett, 2015/07/27 20:42:17]

+1 for some sort of int fSampleSize with 1 as the default.  As a warning, I ran
into bugs at one point because the default sampleSize is 0 when passed from
Android (and SkImageDecoder interprets this as the default, which is 1).

[scroggo, 2015/07/27 20:47:24]

We're in charge of the Android code, so we should make them use a sensible
default.

[emmaleer, 2015/07/28 14:19:16]

I've change bool fSampled to int fSampleX. setSampleX functions have been
removed. 

I've also added a check in onGetScanlineDecoder to make sure dst width matches
the desired sampling.
 

+         */
+        bool            fSampled;
     };
 
     /**
      *  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.
      *
@@ skipping 63 matching lines @@
      *  of the encoded data, but then never use any colors which have alpha
      *  less than 100%. This function can be called *after* decoding to
      *  determine if such an image truly had alpha. Calling it before decoding
      *  is undefined.
      *  FIXME: see skbug.com/3582.
      */
     bool reallyHasAlpha() const {
         return this->onReallyHasAlpha();
     }
 
+    /**
+     * returns true if the image is interlaced

[scroggo, 2015/07/27 19:29:57]

These two lines can be more concise, e.g.

  Returns whether the image is interlaced

I think it would also be good to explain why a client would want to call it.
e.g.

For an interlaced image, calling getScanlines once (regardless of the count
used) needs to read the entire image, therefore it is inefficient to call
getScanlines more than once. Instead, it should only ever be called with all the
rows needed.

I don't know if we want to be so explicit about the implementation, but I tend
to think we should say something about why someone might want to call it.

I also wonder if this makes more sense to be on the SkScanlineDecoder (this
question is moot if we merge it with SkCodec).

[emmaleer, 2015/07/28 14:19:16]

Yes, I think it's good to explain how the entire interlaced image must be read
regardless of the count.
I've added that comment in.
Also, I agree that it makes more sense to add this function to the
scanlineDecoder, since you only would need it if you are using a
scanlineDecoder.

+     * returns false if the image is not interlaced
+     */
+    bool isInterlaced() {
+        return this->onIsInterlaced();
+    }
+
 protected:
     SkCodec(const SkImageInfo&, SkStream*);
 
     virtual SkISize onGetScaledDimensions(float /* desiredScale */) const {
         // By default, scaling is not supported.
         return this->getInfo().dimensions();
     }
 
     virtual SkEncodedFormat onGetEncodedFormat() const = 0;
 
@@ skipping 25 matching lines @@
      */
     virtual SkScanlineDecoder* onGetScanlineDecoder(const SkImageInfo& dstInfo,
                                                     const Options& options,
                                                     SkPMColor ctable[],
                                                     int* ctableCount) {
         return NULL;
     }
 
     virtual bool onReallyHasAlpha() const { return false; }
 
+    /**
+     * returns true if the image is interlaced
+     * returns false if the image is not interlaced
+     * only certain file types can have interlaced images, those of which override this function
+     */
+    virtual bool onIsInterlaced() { return false; }
+
     enum RewindState {
         kRewound_RewindState,
         kNoRewindNecessary_RewindState,
         kCouldNotRewind_RewindState
     };
     /**
      *  If the stream was previously read, attempt to rewind.
      *  @returns:
      *      kRewound if the stream needed to be rewound, and the
      *               rewind succeeded.
@@ skipping 14 matching lines @@
     SkStream* stream() {
         return fStream.get();
     }
 
 private:
     const SkImageInfo       fInfo;
     SkAutoTDelete<SkStream> fStream;
     bool                    fNeedsRewind;
 };
 #endif // SkCodec_DEFINED