Change SkDecodingImageGenerator API

src/images/SkDecodingImageGenerator.h

 /*
  * Copyright 2013 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #ifndef SkDecodingImageGenerator_DEFINED
 #define SkDecodingImageGenerator_DEFINED
 
+#include "SkBitmap.h"
 #include "SkDiscardableMemory.h"
 #include "SkImageGenerator.h"
 #include "SkImageInfo.h"
 
-class SkBitmap;
 class SkStreamRewindable;
 
 /**
  * Calls into SkImageDecoder::DecodeMemoryToTarget to implement a
  * SkImageGenerator
  */
 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
      *  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);
+     *
+     *  @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();
 
     virtual SkData* refEncodedData() SK_OVERRIDE;
 
     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.
      *
      *  @param destination Upon success, this bitmap will be
      *  configured and have a pixelref installed.
      *
      *  @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.
      *
      *  @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
      *  SkDiscardablePixelRef and a new SkDecodingImageGenerator.
      *
      *  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
@@ skipping 10 matching lines @@
      *  @param destination Upon success, this bitmap will be
      *  configured and have a pixelref installed.
      *
      *  @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.
      *
      *  @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