SkPDF: Simplify PDFStream / emitObject() const

Compress SkPDFStream's data on setData(), not emitObject(); no longer stateful.

SkPDFObject::emitObject is now const.  This makes it easier to reason about state.

Minimal performance gains.

Review URL: https://codereview.chromium.org/1304493002

src/pdf/SkPDFBitmap.cpp

@@ -249,7 +249,7 @@ public:
     ~PDFAlphaBitmap() {}
     void emitObject(SkWStream*,
                     const SkPDFObjNumMap&,
-                    const SkPDFSubstituteMap&) override;
+                    const SkPDFSubstituteMap&) const override;
 
 private:
     const SkBitmap fBitmap;
@@ -257,7 +257,7 @@ private:
 
 void PDFAlphaBitmap::emitObject(SkWStream* stream,
                                 const SkPDFObjNumMap& objNumMap,
-                                const SkPDFSubstituteMap& substitutes) {
+                                const SkPDFSubstituteMap& substitutes) const {
     SkAutoLockPixels autoLockPixels(fBitmap);
     SkASSERT(fBitmap.colorType() != kIndex_8_SkColorType ||
              fBitmap.getColorTable());
@@ -293,7 +293,7 @@ public:
     const SkAutoTUnref<SkPDFObject> fSMask;
     void emitObject(SkWStream*,
                     const SkPDFObjNumMap&,
-                    const SkPDFSubstituteMap&) override;
+                    const SkPDFSubstituteMap&) const override;
     void addResources(SkPDFObjNumMap*,
                       const SkPDFSubstituteMap&) const override;
     PDFDefaultBitmap(const SkBitmap& bm, SkPDFObject* smask)
@@ -345,7 +345,7 @@ static SkPDFArray* make_indexed_color_space(const SkColorTable* table) {
 
 void PDFDefaultBitmap::emitObject(SkWStream* stream,
                                   const SkPDFObjNumMap& objNumMap,
-                                  const SkPDFSubstituteMap& substitutes) {
+                                  const SkPDFSubstituteMap& substitutes) const {
     SkAutoLockPixels autoLockPixels(fBitmap);
     SkASSERT(fBitmap.colorType() != kIndex_8_SkColorType ||
              fBitmap.getColorTable());
@@ -407,12 +407,12 @@ public:
         : SkPDFBitmap(bm), fData(SkRef(data)), fIsYUV(isYUV) {}
     void emitObject(SkWStream*,
                     const SkPDFObjNumMap&,
-                    const SkPDFSubstituteMap&) override;
+                    const SkPDFSubstituteMap&) const override;
 };
 
 void PDFJpegBitmap::emitObject(SkWStream* stream,
                                const SkPDFObjNumMap& objNumMap,
-                               const SkPDFSubstituteMap& substituteMap) {
+                               const SkPDFSubstituteMap& substituteMap) const {
     SkPDFDict pdfDict("XObject");
     pdfDict.insertName("Subtype", "Image");
     pdfDict.insertInt("Width", fBitmap.width());

src/pdf/SkPDFFont.cpp

@@ -1011,7 +1011,7 @@ SkPDFFont* SkPDFType0Font::getFontSubset(const SkPDFGlyphSet* subset) {
 #ifdef SK_DEBUG
 void SkPDFType0Font::emitObject(SkWStream* stream,
                                 const SkPDFObjNumMap& objNumMap,
-                                const SkPDFSubstituteMap& substitutes) {
+                                const SkPDFSubstituteMap& substitutes) const {
     SkASSERT(fPopulated);
     return INHERITED::emitObject(stream, objNumMap, substitutes);
 }

src/pdf/SkPDFFontImpl.h

@@ -18,9 +18,9 @@ public:
     virtual bool multiByteGlyphs() const { return true; }
     virtual SkPDFFont* getFontSubset(const SkPDFGlyphSet* usage);
 #ifdef SK_DEBUG
-    virtual void emitObject(SkWStream*,
+    void emitObject(SkWStream*,
-                            const SkPDFObjNumMap&,
+                    const SkPDFObjNumMap&,
-                            const SkPDFSubstituteMap&);
+                    const SkPDFSubstituteMap&) const override;
 #endif
 
 private:

src/pdf/SkPDFGraphicState.cpp

@@ -195,9 +195,10 @@ SkPDFDict* SkPDFGraphicState::GetNoSMaskGraphicState() {
     return SkRef(noSMaskGraphicState.get());
 }
 
-void SkPDFGraphicState::emitObject(SkWStream* stream,
+void SkPDFGraphicState::emitObject(
-                                   const SkPDFObjNumMap& objNumMap,
+        SkWStream* stream,
-                                   const SkPDFSubstituteMap& substitutes) {
+        const SkPDFObjNumMap& objNumMap,
+        const SkPDFSubstituteMap& substitutes) const {
     SkAutoTUnref<SkPDFDict> dict(SkNEW_ARGS(SkPDFDict, ("ExtGState")));
     dict->insertName("Type", "ExtGState");
 

src/pdf/SkPDFGraphicState.h

@@ -32,9 +32,9 @@ public:
 
     // Override emitObject so that we can populate the dictionary on
     // demand.
-    virtual void emitObject(SkWStream* stream,
+    void emitObject(SkWStream* stream,
-                            const SkPDFObjNumMap& objNumMap,
+                    const SkPDFObjNumMap& objNumMap,
-                            const SkPDFSubstituteMap& substitutes);
+                    const SkPDFSubstituteMap& substitutes) const override;
 
     /** Get the graphic state for the passed SkPaint. The reference count of
      *  the object is incremented and it is the caller's responsibility to

src/pdf/SkPDFStream.cpp

@@ -13,58 +13,43 @@
 #include "SkStream.h"
 #include "SkStreamPriv.h"
 
-SkPDFStream::SkPDFStream(SkStream* stream) : fState(kUnused_State) {
-    this->setData(stream);
-}
-
-SkPDFStream::SkPDFStream(SkData* data) : fState(kUnused_State) {
-    this->setData(data);
-}
-
 SkPDFStream::~SkPDFStream() {}
 
 void SkPDFStream::emitObject(SkWStream* stream,
                              const SkPDFObjNumMap& objNumMap,
-                             const SkPDFSubstituteMap& substitutes) {
+                             const SkPDFSubstituteMap& substitutes) const {
-    if (fState == kUnused_State) {
+    SkASSERT(fCompressedData);
-        fState = kNoCompression_State;
-        SkDynamicMemoryWStream compressedData;
-        SkDeflateWStream deflateWStream(&compressedData);
-        SkAssertResult(SkStreamCopy(&deflateWStream, fDataStream.get()));
-        deflateWStream.finalize();
-        SkAssertResult(fDataStream->rewind());
-        if (compressedData.getOffset() < this->dataSize()) {
-            SkAutoTDelete<SkStream> compressed(
-                    compressedData.detachAsStream());
-            this->setData(compressed.get());
-            this->insertName("Filter", "FlateDecode");
-        }
-        fState = kCompressed_State;
-        this->insertInt("Length", this->dataSize());
-    }
     this->INHERITED::emitObject(stream, objNumMap, substitutes);
+    // Note: emitObject isn't marked const, but could be in the future
+    SkAutoTDelete<SkStreamRewindable> dup(fCompressedData->duplicate());
+    SkASSERT(dup);
+    SkASSERT(dup->hasLength());
     stream->writeText(" stream\n");
-    stream->writeStream(fDataStream.get(), fDataStream->getLength());
+    stream->writeStream(dup.get(), dup->getLength());
-    SkAssertResult(fDataStream->rewind());
     stream->writeText("\nendstream");
 }
 
-SkPDFStream::SkPDFStream() : fState(kUnused_State) {}
-
-void SkPDFStream::setData(SkData* data) {
-    // FIXME: Don't swap if the data is the same.
-    fDataStream.reset(SkNEW_ARGS(SkMemoryStream, (data)));
-}
-
 void SkPDFStream::setData(SkStream* stream) {
+    SkASSERT(!fCompressedData);  // Only call this function once.
     SkASSERT(stream);
-    // Code assumes that the stream starts at the beginning and is rewindable.
+    // Code assumes that the stream starts at the beginning.
-    // SkStreamRewindableFromSkStream will try stream->duplicate().
-    fDataStream.reset(SkStreamRewindableFromSkStream(stream));
-    SkASSERT(fDataStream.get());
-}
 
-size_t SkPDFStream::dataSize() const {
+    SkDynamicMemoryWStream compressedData;
-    SkASSERT(fDataStream->hasLength());
+    SkDeflateWStream deflateWStream(&compressedData);
-    return fDataStream->getLength();
+    SkStreamCopy(&deflateWStream, stream);
+    deflateWStream.finalize();
+    size_t length = compressedData.bytesWritten();
+
+    if (stream->hasLength()) {
+        SkAutoTDelete<SkStreamRewindable> dup(stream->duplicate());
+        if (dup && dup->hasLength() &&
+            dup->getLength() <= length + strlen("/Filter_/FlateDecode_")) {
+            this->insertInt("Length", dup->getLength());
+            fCompressedData.reset(dup.detach());
+            return;
+        }
+    }
+    fCompressedData.reset(compressedData.detachAsStream());
+    this->insertName("Filter", "FlateDecode");
+    this->insertInt("Length", length);
 }

src/pdf/SkPDFStream.h

@@ -27,50 +27,38 @@ class SkPDFStream : public SkPDFDict {
 public:
     /** Create a PDF stream. A Length entry is automatically added to the
      *  stream dictionary.
-     *  @param data   The data part of the stream.  Will be ref()ed.
+     *  @param data   The data part of the stream.  Will not take ownership.
      */
-    explicit SkPDFStream(SkData* data);
+    explicit SkPDFStream(SkData* data) { this->setData(data); }
 
     /** Create a PDF stream. A Length entry is automatically added to the
      *  stream dictionary.
-     *  @param stream The data part of the stream.  Will be duplicate()d.
+     *  @param stream The data part of the stream.  Will not take ownership.
      */
-    explicit SkPDFStream(SkStream* stream);
+    explicit SkPDFStream(SkStream* stream) { this->setData(stream); }
 
     virtual ~SkPDFStream();
 
     // The SkPDFObject interface.
     void emitObject(SkWStream* stream,
                     const SkPDFObjNumMap& objNumMap,
-                    const SkPDFSubstituteMap& substitutes) override;
+                    const SkPDFSubstituteMap& substitutes) const override;
 
 protected:
-    enum State {
-        kUnused_State,         //!< The stream hasn't been requested yet.
-        kNoCompression_State,  //!< The stream's been requested in an
-                               //   uncompressed form.
-        kCompressed_State,     //!< The stream's already been compressed.
-    };
-
     /* Create a PDF stream with no data.  The setData method must be called to
      * set the data.
      */
-    SkPDFStream();
+    SkPDFStream() {}
 
-    void setData(SkData* data);
+    /** Only call this function once. */
     void setData(SkStream* stream);
-
+    void setData(SkData* data) {
-    size_t dataSize() const;
+        SkMemoryStream memoryStream(data);
-
+        this->setData(&memoryStream);
-    void setState(State state) {
-        fState = state;
     }
 
 private:
-    // Indicates what form (or if) the stream has been requested.
+    SkAutoTDelete<SkStreamRewindable> fCompressedData;
-    State fState;
-
-    SkAutoTDelete<SkStreamRewindable> fDataStream;
 
     typedef SkPDFDict INHERITED;
 };

src/pdf/SkPDFTypes.cpp

@@ -260,7 +260,7 @@ SkPDFUnion SkPDFUnion::Object(SkPDFObject* ptr) {
 #if 0  // Enable if needed.
 void SkPDFAtom::emitObject(SkWStream* stream,
                            const SkPDFObjNumMap& objNumMap,
-                           const SkPDFSubstituteMap& substitutes) {
+                           const SkPDFSubstituteMap& substitutes) const {
     fValue.emitObject(stream, objNumMap, substitutes);
 }
 void SkPDFAtom::addResources(SkPDFObjNumMap* map,
@@ -284,8 +284,8 @@ int SkPDFArray::size() const { return fValues.count(); }
 void SkPDFArray::reserve(int length) { fValues.setReserve(length); }
 
 void SkPDFArray::emitObject(SkWStream* stream,
-                             const SkPDFObjNumMap& objNumMap,
+                            const SkPDFObjNumMap& objNumMap,
-                             const SkPDFSubstituteMap& substitutes) {
+                            const SkPDFSubstituteMap& substitutes) const {
     stream->writeText("[");
     for (int i = 0; i < fValues.count(); i++) {
         fValues[i].emitObject(stream, objNumMap, substitutes);
@@ -353,7 +353,7 @@ SkPDFDict::SkPDFDict(const char type[]) { this->insertName("Type", type); }
 
 void SkPDFDict::emitObject(SkWStream* stream,
                            const SkPDFObjNumMap& objNumMap,
-                           const SkPDFSubstituteMap& substitutes) {
+                           const SkPDFSubstituteMap& substitutes) const {
     stream->writeText("<<");
     for (int i = 0; i < fRecords.count(); i++) {
         fRecords[i].fKey.emitObject(stream, objNumMap, substitutes);

src/pdf/SkPDFTypes.h

@@ -40,7 +40,7 @@ public:
     // TODO(halcanary): make this method const
     virtual void emitObject(SkWStream* stream,
                             const SkPDFObjNumMap& objNumMap,
-                            const SkPDFSubstituteMap& substitutes) = 0;
+                            const SkPDFSubstituteMap& substitutes) const = 0;
 
     /**
      *  Adds all transitive dependencies of this object to the
@@ -200,7 +200,7 @@ public:
     // The SkPDFObject interface.
     void emitObject(SkWStream* stream,
                     const SkPDFObjNumMap& objNumMap,
-                    const SkPDFSubstituteMap& substitutes) override;
+                    const SkPDFSubstituteMap& substitutes) const override;
     void addResources(SkPDFObjNumMap*,
                       const SkPDFSubstituteMap&) const override;
 
@@ -255,7 +255,7 @@ public:
     // The SkPDFObject interface.
     void emitObject(SkWStream* stream,
                     const SkPDFObjNumMap& objNumMap,
-                    const SkPDFSubstituteMap& substitutes) override;
+                    const SkPDFSubstituteMap& substitutes) const override;
     void addResources(SkPDFObjNumMap*,
                       const SkPDFSubstituteMap&) const override;