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
diff --git a/src/pdf/SkPDFBitmap.cpp b/src/pdf/SkPDFBitmap.cpp
index f1a053f..ba26cd8 100644
--- a/src/pdf/SkPDFBitmap.cpp
+++ b/src/pdf/SkPDFBitmap.cpp
@@ -249,7 +249,7 @@
~PDFAlphaBitmap() {}
void emitObject(SkWStream*,
const SkPDFObjNumMap&,
- const SkPDFSubstituteMap&) override;
+ const SkPDFSubstituteMap&) const override;
private:
const SkBitmap fBitmap;
@@ -257,7 +257,7 @@
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 @@
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 @@
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 @@
: 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());
diff --git a/src/pdf/SkPDFFont.cpp b/src/pdf/SkPDFFont.cpp
index d8755ae..017b016 100644
--- a/src/pdf/SkPDFFont.cpp
+++ b/src/pdf/SkPDFFont.cpp
@@ -1011,7 +1011,7 @@
#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);
}
diff --git a/src/pdf/SkPDFFontImpl.h b/src/pdf/SkPDFFontImpl.h
index cc7a4a1..6147ad5 100644
--- a/src/pdf/SkPDFFontImpl.h
+++ b/src/pdf/SkPDFFontImpl.h
@@ -18,9 +18,9 @@
virtual bool multiByteGlyphs() const { return true; }
virtual SkPDFFont* getFontSubset(const SkPDFGlyphSet* usage);
#ifdef SK_DEBUG
- virtual void emitObject(SkWStream*,
- const SkPDFObjNumMap&,
- const SkPDFSubstituteMap&);
+ void emitObject(SkWStream*,
+ const SkPDFObjNumMap&,
+ const SkPDFSubstituteMap&) const override;
#endif
private:
diff --git a/src/pdf/SkPDFGraphicState.cpp b/src/pdf/SkPDFGraphicState.cpp
index 6cf6645..55f3b39 100644
--- a/src/pdf/SkPDFGraphicState.cpp
+++ b/src/pdf/SkPDFGraphicState.cpp
@@ -195,9 +195,10 @@
return SkRef(noSMaskGraphicState.get());
}
-void SkPDFGraphicState::emitObject(SkWStream* stream,
- const SkPDFObjNumMap& objNumMap,
- const SkPDFSubstituteMap& substitutes) {
+void SkPDFGraphicState::emitObject(
+ SkWStream* stream,
+ const SkPDFObjNumMap& objNumMap,
+ const SkPDFSubstituteMap& substitutes) const {
SkAutoTUnref<SkPDFDict> dict(SkNEW_ARGS(SkPDFDict, ("ExtGState")));
dict->insertName("Type", "ExtGState");
diff --git a/src/pdf/SkPDFGraphicState.h b/src/pdf/SkPDFGraphicState.h
index 1b11fe4..45a32d4 100644
--- a/src/pdf/SkPDFGraphicState.h
+++ b/src/pdf/SkPDFGraphicState.h
@@ -32,9 +32,9 @@
// Override emitObject so that we can populate the dictionary on
// demand.
- virtual void emitObject(SkWStream* stream,
- const SkPDFObjNumMap& objNumMap,
- const SkPDFSubstituteMap& substitutes);
+ void emitObject(SkWStream* stream,
+ const SkPDFObjNumMap& objNumMap,
+ 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
diff --git a/src/pdf/SkPDFStream.cpp b/src/pdf/SkPDFStream.cpp
index d21205c..69ce317 100644
--- a/src/pdf/SkPDFStream.cpp
+++ b/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) {
- if (fState == kUnused_State) {
- 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());
- }
+ const SkPDFSubstituteMap& substitutes) const {
+ SkASSERT(fCompressedData);
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());
- SkAssertResult(fDataStream->rewind());
+ stream->writeStream(dup.get(), dup->getLength());
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.
- // SkStreamRewindableFromSkStream will try stream->duplicate().
- fDataStream.reset(SkStreamRewindableFromSkStream(stream));
- SkASSERT(fDataStream.get());
-}
+ // Code assumes that the stream starts at the beginning.
-size_t SkPDFStream::dataSize() const {
- SkASSERT(fDataStream->hasLength());
- return fDataStream->getLength();
+ SkDynamicMemoryWStream compressedData;
+ SkDeflateWStream deflateWStream(&compressedData);
+ 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);
}
diff --git a/src/pdf/SkPDFStream.h b/src/pdf/SkPDFStream.h
index d29d9b1..c5961d4 100644
--- a/src/pdf/SkPDFStream.h
+++ b/src/pdf/SkPDFStream.h
@@ -27,50 +27,38 @@
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);
-
- size_t dataSize() const;
-
- void setState(State state) {
- fState = state;
+ void setData(SkData* data) {
+ SkMemoryStream memoryStream(data);
+ this->setData(&memoryStream);
}
private:
- // Indicates what form (or if) the stream has been requested.
- State fState;
-
- SkAutoTDelete<SkStreamRewindable> fDataStream;
+ SkAutoTDelete<SkStreamRewindable> fCompressedData;
typedef SkPDFDict INHERITED;
};
diff --git a/src/pdf/SkPDFTypes.cpp b/src/pdf/SkPDFTypes.cpp
index 619870a..2fe408b 100644
--- a/src/pdf/SkPDFTypes.cpp
+++ b/src/pdf/SkPDFTypes.cpp
@@ -260,7 +260,7 @@
#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 @@
void SkPDFArray::reserve(int length) { fValues.setReserve(length); }
void SkPDFArray::emitObject(SkWStream* stream,
- const SkPDFObjNumMap& objNumMap,
- const SkPDFSubstituteMap& substitutes) {
+ const SkPDFObjNumMap& objNumMap,
+ const SkPDFSubstituteMap& substitutes) const {
stream->writeText("[");
for (int i = 0; i < fValues.count(); i++) {
fValues[i].emitObject(stream, objNumMap, substitutes);
@@ -353,7 +353,7 @@
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);
diff --git a/src/pdf/SkPDFTypes.h b/src/pdf/SkPDFTypes.h
index 3df2e23..23a30b9 100644
--- a/src/pdf/SkPDFTypes.h
+++ b/src/pdf/SkPDFTypes.h
@@ -40,7 +40,7 @@
// 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 @@
// 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 @@
// 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;