SkScaledCodec class

include/codec/SkCodec.h

Index: include/codec/SkCodec.h
diff --git a/include/codec/SkCodec.h b/include/codec/SkCodec.h
index 1cdc88d4adcfc0dc4e028913348dd21c287e50e7..9d80592568664d795a9eb397d8bdcd1244778227 100644
--- a/include/codec/SkCodec.h
+++ b/include/codec/SkCodec.h
@@ -149,6 +149,7 @@ public:
         Options()
             : fZeroInitialized(kNo_ZeroInitialized)
             , fSubset(NULL)
+            , fSampled(false)
         {}
 
         ZeroInitialized fZeroInitialized;
@@ -161,6 +162,13 @@ public:
          *  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;
     };
 
     /**
@@ -244,6 +252,14 @@ public:
         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*);
 
@@ -289,6 +305,13 @@ protected:
 
     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,