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..12a49d59c47e6c6b89c248a7bd88341d35633da3 100644
--- a/src/images/SkDecodingImageGenerator.h
+++ b/src/images/SkDecodingImageGenerator.h
@@ -8,113 +8,134 @@
 #ifndef SkDecodingImageGenerator_DEFINED
 #define SkDecodingImageGenerator_DEFINED
 
-#include "SkDiscardableMemory.h"
+#include "SkBitmap.h"
 #include "SkImageGenerator.h"
-#include "SkImageInfo.h"
 
-class SkBitmap;
+class SkData;
 class SkStreamRewindable;
 
 /**
- * Calls into SkImageDecoder::DecodeMemoryToTarget to implement a
- * SkImageGenerator
+ *  An implementation of SkImageGenerator that calls into
+ *  SkImageDecoder.
  */
 class SkDecodingImageGenerator : public SkImageGenerator {
 public:
-    /*
-     *  The constructor will take a reference to the SkData.  The
-     *  destructor will unref() it.
-     */
-    explicit SkDecodingImageGenerator(SkData* data);
-
-    /*
-     *  The SkData version of this constructor is preferred.  If the
-     *  stream has an underlying SkData (such as a SkMemoryStream)
-     *  pass that in.
-     *
-     *  This object will unref the stream when done.  Since streams
-     *  have internal state (position), the caller should not pass a
-     *  shared stream in.  Pass either a new duplicated stream in or
-     *  transfer ownership of the stream.  In the latter case, be sure
-     *  that there are no other consumers of the stream who will
-     *  modify the stream's position.  This constructor asserts
-     *  stream->unique().
-     *
-     *  For example:
-     *    SkStreamRewindable* stream;
-     *    ...
-     *    SkImageGenerator* gen
-     *        = SkNEW_ARGS(SkDecodingImageGenerator,
-     *                     (stream->duplicate()));
-     *    ...
-     *    SkDELETE(gen);
-     */
-    explicit SkDecodingImageGenerator(SkStreamRewindable* stream);
-
     virtual ~SkDecodingImageGenerator();
-
     virtual SkData* refEncodedData() SK_OVERRIDE;
-
+    // This implementaion of getInfo() always returns true.
     virtual bool getInfo(SkImageInfo* info) SK_OVERRIDE;
-
     virtual bool getPixels(const SkImageInfo& info,
                            void* pixels,
                            size_t rowBytes) SK_OVERRIDE;
-
     /**
-     *  Install the SkData into the destination bitmap, using a new
-     *  SkDiscardablePixelRef and a new SkDecodingImageGenerator.
-     *
-     *  @param data Contains the encoded image data that will be used
-     *  by the SkDecodingImageGenerator.  Will be ref()ed.
+     *  These options will be passed on to the image decoder.  The
+     *  defaults are sensible.
      *
-     *  @param destination Upon success, this bitmap will be
-     *  configured and have a pixelref installed.
+     *  @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 factory If not NULL, this object will be used as a
-     *  source of discardable memory when decoding.  If NULL, then
-     *  SkDiscardableMemory::Create() will be called.
+     *  @param fDitherImage Set to true if the the decoder should try to
+     *         dither the resulting image when decoding to a smaller
+     *         color-space.  The default is true.
      *
-     *  @return true iff successful.
+     *  @param fRequestedColorType If not given, then use whichever

[scroggo, 2013/12/17 22:53:34]

What is this set to if not given?

[hal.canary, 2013/12/18 15:39:07]

The value of opts.fRequestedColorType is only referenced four times.  Each of
these is in a code path where opts.fRequestedColorType is true.  I hope valgrind
sees that and respects that.  I am running the valgrind bot to verify.

+     *         config the decoder wants.  Else try to use this color
+     *         type.  If the decoder won't support this color type,
+     *         SkDecodingImageGenerator::Create will return
+     *         NULL. kIndex_8_SkColorType is not supported.
      */
-    static bool Install(SkData* data, SkBitmap* destination,
-                        SkDiscardableMemory::Factory* factory = NULL);
+    struct Options {
+        Options()
+            : fSampleSize(1)
+            , fDitherImage(true)
+            , fUseRequestedColorType(false)
+            , fRequestedColorType() { }

[scroggo, 2013/12/17 22:53:34]

Will valgrind be okay with this? What does it get set to?

[hal.canary, 2013/12/18 15:39:07]

See above.

+        Options(int sampleSize, bool dither)
+            : fSampleSize(sampleSize)
+            , fDitherImage(dither)
+            , fUseRequestedColorType(false)
+            , fRequestedColorType() { }
+        Options(int sampleSize, bool dither, SkColorType colorType)

[scroggo, 2013/12/17 22:53:34]

Do we use all three constructors?

[hal.canary, 2013/12/18 15:39:07]

We use the default constructor in many of our tests.

Android will use the third constructor.

I include the second constructor for completeness (in case we want to set
sampleSize or dither without specifying a color type (these ought to be
orthogonal).  I exercise that in the unit tests only.

+            : fSampleSize(sampleSize)
+            , fDitherImage(dither)
+            , fUseRequestedColorType(true)
+            , fRequestedColorType(colorType) { }
+        const int         fSampleSize;
+        const bool        fDitherImage;
+        const bool        fUseRequestedColorType;
+        const SkColorType fRequestedColorType;
+    };
+
     /**
-     *  Install the stream into the destination bitmap, using a new
-     *  SkDiscardablePixelRef and a new SkDecodingImageGenerator.
+     *  These two functions return a SkImageGenerator that calls into
+     *  SkImageDecoder.  They return NULL on failure.
      *
-     *  The SkData version of this function is preferred.  If the
-     *  stream has an underlying SkData (such as a SkMemoryStream)
-     *  pass that in.
+     *  The SkData version of this function is preferred.  If the stream
+     *  has an underlying SkData (such as a SkMemoryStream) pass that in.
      *
-     *  @param stream The source of encoded data that will be passed
-     *  to the decoder.  The installed SkDecodingImageGenerator will
-     *  unref the stream when done.  If false is returned, this
-     *  function will perform the unref.  Since streams have internal
-     *  state (position), the caller should not pass a shared stream
-     *  in.  Pass either a new duplicated stream in or transfer
-     *  ownership of the stream.  In the latter case, be sure that
-     *  there are no other consumers of the stream who will modify the
-     *  stream's position.  This function will fail if
-     *  (!stream->unique()).
+     *  This object will unref the stream when done or on failure.  Since
+     *  streams have internal state (position), the caller should not pass
+     *  a shared stream in.  Pass either a new duplicated stream in or
+     *  transfer ownership of the stream.  This factory asserts
+     *  stream->unique().
      *
-     *  @param destination Upon success, this bitmap will be
-     *  configured and have a pixelref installed.
+     *  For example:
+     *    SkStreamRewindable* stream;
+     *    ...
+     *    SkImageGenerator* gen
+     *        = SkDecodingImageGenerator::Create(
+     *            stream->duplicate(), SkDecodingImageGenerator::Options());
+     *    ...
+     *    SkDELETE(gen);
      *
-     *  @param factory If not NULL, this object will be used as a
-     *  source of discardable memory when decoding.  If NULL, then
-     *  SkDiscardableMemory::Create() will be called.
+     *  @param Options (see above)
      *
-     *  @return true iff successful.
+     *  @return NULL on failure, a new SkImageGenerator on success.
      */
-    static bool Install(SkStreamRewindable* stream, SkBitmap* destination,
-                        SkDiscardableMemory::Factory* factory = NULL);
+    static SkImageGenerator* Create(SkStreamRewindable* stream,
+                                    const Options& opt);
+
+    /**
+     *  @param data Contains the encoded image data that will be used by
+     *         the SkDecodingImageGenerator.  Will be ref()ed by the
+     *         SkImageGenerator constructor and and unref()ed on deletion.
+     */
+    static SkImageGenerator* Create(SkData* data, const Options& opt);
 
 private:
-    SkData*             fData;
-    SkStreamRewindable* fStream;
-    SkImageInfo         fInfo;
-    bool                fHasInfo;
-    bool                fDoCopyTo;
+    SkData*                fData;
+    SkStreamRewindable*    fStream;
+    const SkImageInfo      fInfo;
+    const int              fSampleSize;
+    const bool             fDitherImage;
+    const SkBitmap::Config fRequestedConfig;
+    SkDecodingImageGenerator(SkData* data,
+                             SkStreamRewindable* stream,
+                             const SkImageInfo& info,
+                             int sampleSize,
+                             bool ditherImage,
+                             SkBitmap::Config requestedConfig);
+    static SkImageGenerator* Create(SkData*, SkStreamRewindable*,
+                                    const Options&);
+    typedef SkImageGenerator INHERITED;
 };
+
+//  // Example of most basic use case:
+//
+//  bool install_data(SkData* data, SkBitmap* dst) {
+//     return SkInstallDiscardablePixelRef(
+//         SkDecodingImageGenerator::Create(
+//             data, SkDecodingImageGenerator::Options()), dst, NULL);
+//  }
+//  bool install_stream(SkStreamRewindable* stream, SkBitmap* dst) {
+//     return SkInstallDiscardablePixelRef(
+//         SkDecodingImageGenerator::Create(
+//             stream, SkDecodingImageGenerator::Options()), dst, NULL);
+//  }
+
 #endif  // SkDecodingImageGenerator_DEFINED