Refactor SkPDFImage

src/pdf/SkPDFImage.h

Index: src/pdf/SkPDFImage.h
diff --git a/src/pdf/SkPDFImage.h b/src/pdf/SkPDFImage.h
index 31f894086ee3949e20e952d205ed818ac3b01f7d..ed637273b1f257e4def164c5a5546329a87a6e06 100644
--- a/src/pdf/SkPDFImage.h
+++ b/src/pdf/SkPDFImage.h
@@ -11,7 +11,7 @@
 #define SkPDFImage_DEFINED
 
 #include "SkPDFDevice.h"
-#include "SkPDFImageStream.h"
+#include "SkPDFStream.h"
 #include "SkPDFTypes.h"
 #include "SkRefCnt.h"
 
@@ -27,7 +27,7 @@ struct SkIRect;
 // We could play the same trick here as is done in SkPDFGraphicState, storing
 // a copy of the Bitmap object (not the pixels), the pixel generation number,
 // and settings used from the paint to canonicalize image objects.
-class SkPDFImage : public SkPDFImageStream {
+class SkPDFImage : public SkPDFStream {
 public:
     /** Create a new Image XObject to represent the passed bitmap.
      *  @param bitmap   The image to encode.
@@ -48,24 +48,68 @@ public:
      */
     SkPDFImage* addSMask(SkPDFImage* mask);
 
+    bool isEmpty() {
+        return fSrcRect.isEmpty();
+    }
+
     // The SkPDFObject interface.
     virtual void getResources(const SkTSet<SkPDFObject*>& knownResourceObjects,
                               SkTSet<SkPDFObject*>* newResourceObjects);
 
 private:
+    SkBitmap fBitmap;
+    SkIRect fSrcRect;
+    EncodeToDCTStream fEncoder;
+
     SkTDArray<SkPDFObject*> fResources;
 
     /** Create a PDF image XObject. Entries for the image properties are
      *  automatically added to the stream dictionary.
-     *  @param imageData  The final raw bits representing the image.
-     *  @param bitmap     The image parameters to use (Config, etc).
+     *  @param bitmap     The image, along with parameters (like config).
      *  @param srcRect    The clipping applied to bitmap before generating
      *                    imageData.
-     *  @param alpha      Is this the alpha channel of the bitmap.
-     *  @param paint      Used to calculate alpha, masks, etc.
+     *  @param encoder    A function used to encode the bitmap for compression.
+     *                    May be NULL.
+     */
+    SkPDFImage(const SkBitmap& bitmap, const SkIRect& srcRect,

[vandebo (ex-Chrome), 2013/08/21 22:37:17]

Maybe there should be one constructor that makes an "empty" image with various
InitFromFoo methods.

[ducky, 2013/08/22 03:48:06]

I kind of like having a self-contained constructor, instead of having two parts
(and having to remember to do both parts)...
I also think the isValid (isEmpty in this case) pattern for checking if
construction succeeded is used elsewhere in the PDF code as well.

+               EncodeToDCTStream encoder);
+
+    /** Create a PDF image XObject from a stream and bitmap. The stream is used
+     *  for the image data, while the bitmap contains contains parameters.

[vandebo (ex-Chrome), 2013/08/21 22:37:17]

contains contains

[ducky, 2013/08/22 03:48:06]

Done.

+     *  This is intended to create an alpha SMask, and the behavior when used
+     *  in other ways is undefined (as in, don't do it).
+     *  @param stream     Uncompressed stream containing image data.
+     *  @param bitmap     The image, along with parameters (like config).
+     *  @param srcRect    The clipping applied to bitmap before generating
+                          imageData.
+     */
+    SkPDFImage(SkStream* stream, const SkBitmap& bitmap,
+               const SkIRect& srcRect);
+
+    /** Copy constructor, used to generate substitutes.
+     *  @param image      The SkPDFImage to copy.
      */
-    SkPDFImage(SkStream* imageData, const SkBitmap& bitmap,
-               const SkIRect& srcRect, bool alpha, EncodeToDCTStream encoder);
+    SkPDFImage(SkPDFImage& pdfImage);
+
+
+    /** Initialize the image XObject parameters based on fBitmap and
+     *  fSrcRect, which MUST have been initialized before calling this.
+     *  Image data is not touched.
+     *  @param isAlpha    Whether or not this image is the alpha channel
+     *                    (SMask) for another image XObject.
+     */
+    void initImageParams(bool isAlpha);
+
+    /** Returns the fEncoder encoded version of the image data, or NULL
+     *  if not possible (or desired).
+     */
+    SkStream* getCompressedStream();

[vandebo (ex-Chrome), 2013/08/21 22:37:17]

nit: getDCTCompressedStream()

[ducky, 2013/08/22 03:48:06]

Removed, other style changes rendered this obsolete.

+
+    // Populate the stream dictionary.  This method returns false if
+    // fSubstitute should be used.
+    virtual bool populate(SkPDFCatalog* catalog);
+
+    typedef SkPDFStream INHERITED;
 };
 
 #endif