Change SkDecodingImageGenerator API

src/images/SkDecodingImageGenerator.h

Index: src/images/SkDecodingImageGenerator.h
diff --git a/src/images/SkDecodingImageGenerator.h b/src/images/SkDecodingImageGenerator.h
index dba234bcf19c400d5a005c5c5ac4efe057733267..654f532aa0e9db60c41fa9b54f81235837319778 100644
--- a/src/images/SkDecodingImageGenerator.h
+++ b/src/images/SkDecodingImageGenerator.h
@@ -8,11 +8,11 @@
 #ifndef SkDecodingImageGenerator_DEFINED
 #define SkDecodingImageGenerator_DEFINED
 
+#include "SkBitmap.h"
 #include "SkDiscardableMemory.h"
 #include "SkImageGenerator.h"
 #include "SkImageInfo.h"
 
-class SkBitmap;
 class SkStreamRewindable;
 
 /**
@@ -21,11 +21,14 @@ class SkStreamRewindable;
  */
 class SkDecodingImageGenerator : public SkImageGenerator {
 public:
+    struct Options;

[scroggo, 2013/12/12 22:33:46]

Perhaps a note here to look down below?

[hal.canary, 2013/12/13 15:48:48]

Done.  Moved Options to the top.

     /*
      *  The constructor will take a reference to the SkData.  The
      *  destructor will unref() it.
+     *
+     *  @param Options if NULL, use defaults;
      */
-    explicit SkDecodingImageGenerator(SkData* data);
+    SkDecodingImageGenerator(SkData* data, const Options* opts);

[scroggo, 2013/12/12 22:33:46]

I like your thinking here, to allow opts to be NULL, but I think Mike generally
frowns on pointer to const as a parameter. If you made it a const reference, the
only difference would be requiring the caller to create an object and then
calling the copy constructor, yes?

[hal.canary, 2013/12/13 15:48:48]

Done.

 
     /*
      *  The SkData version of this constructor is preferred.  If the
@@ -48,8 +51,11 @@ public:
      *                     (stream->duplicate()));
      *    ...
      *    SkDELETE(gen);
+     *
+     *  @param Options if NULL, use defaults;
      */
-    explicit SkDecodingImageGenerator(SkStreamRewindable* stream);
+    SkDecodingImageGenerator(SkStreamRewindable* stream,

[scroggo, 2013/12/12 22:33:46]

Do these get called directly? Or do callers call Install? Wondering if these
should be private.

[hal.canary, 2013/12/13 15:48:48]

I'm in the process of eliminating that use case.

+                             const Options* opts);
 
     virtual ~SkDecodingImageGenerator();
 
@@ -78,6 +84,7 @@ public:
      *  @return true iff successful.
      */
     static bool Install(SkData* data, SkBitmap* destination,
+                        const Options* opts,
                         SkDiscardableMemory::Factory* factory = NULL);
     /**
      *  Install the stream into the destination bitmap, using a new
@@ -108,13 +115,56 @@ public:
      *  @return true iff successful.
      */
     static bool Install(SkStreamRewindable* stream, SkBitmap* destination,
+                        const Options* opts,
                         SkDiscardableMemory::Factory* factory = NULL);
 
+    /**
+     *  These options will be passed on to the image decoder.  The
+     *  defaults are sensible.
+     *
+     *  @param fSampleSize If set to > 1, tells the decoder to return a
+     *         smaller than original bitmap, sampling 1 pixel for
+     *         every size pixels. e.g. if sample size is set to 3,
+     *         then the returned bitmap will be 1/3 as wide and high,
+     *         and will contain 1/9 as many pixels as the original.
+     *         Note: this is a hint, and the codec may choose to
+     *         ignore this, or only approximate the sample size.
+     *
+     *  @param fDitherImage Set to true if the the decoder should try
+     *         to dither the resulting image.  The default is true.
+     *
+     *  @param fPreferQualityOverSpeed Should the decoder should try to

[scroggo, 2013/12/12 22:33:46]

Whether*

[hal.canary, 2013/12/13 15:48:48]

removed this param, as per your suggestion.

+     *         decode the resulting image to a higher quality even at
+     *         the expense of the decoding speed.
+     *
+     *  @param fRequestedConfig If SkBitmap::kNo_Config, then use
+     *         whichever config the decoder wants.  Else try to use
+     *         this one. If this config won't work, getInfo() will

[scroggo, 2013/12/12 22:33:46]

This comment seems vague. getInfo() on which class?

[hal.canary, 2013/12/13 15:48:48]

Done.

+     *         return false.
+     */
+    struct Options {
+        int              fSampleSize;
+        bool             fDitherImage;
+        bool             fPreferQualityOverSpeed;
+        SkBitmap::Config fRequestedConfig;
+        Options()
+            : fSampleSize(1)
+            , fDitherImage(true)
+            , fPreferQualityOverSpeed(false)
+            , fRequestedConfig(SkBitmap::kNo_Config) { }
+        Options(int sampleSize, bool dither, bool quality,
+                SkBitmap::Config config)
+            : fSampleSize(sampleSize)
+            , fDitherImage(dither)
+            , fPreferQualityOverSpeed(quality)
+            , fRequestedConfig(config) { }
+    };
 private:
     SkData*             fData;
     SkStreamRewindable* fStream;
     SkImageInfo         fInfo;
     bool                fHasInfo;
     bool                fDoCopyTo;
+    Options             fOpts;

[scroggo, 2013/12/12 22:33:46]

Can be const?

[hal.canary, 2013/12/13 15:48:48]

Done.

 };
 #endif  // SkDecodingImageGenerator_DEFINED