SkPDF: add drop() virtual to release resources early.
Call drop() after calling emitObject() on top-level objects. In Debug
mode, assert that each object is emited exactly once by asserting that
emitObject is never called after drop(). Same for addResources().
To make sure that top level objects don't get deleted prematurely,
SkPDFObjNumMap takes a reference.
Motivation: save RAM. Allow even earlier serialization with later
changes.
Also: Switch some SkTDArrays to SkTArrays.
BUG=skia:5087
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1790023003
Review URL: https://codereview.chromium.org/1790023003
diff --git a/bench/PDFBench.cpp b/bench/PDFBench.cpp
index 920a02d..75b05de 100644
--- a/bench/PDFBench.cpp
+++ b/bench/PDFBench.cpp
@@ -30,7 +30,7 @@
SkPDFObjNumMap objNumMap;
objNumMap.addObjectRecursively(object, substitutes);
for (int i = 0; i < objNumMap.objects().count(); ++i) {
- SkPDFObject* object = objNumMap.objects()[i];
+ SkPDFObject* object = objNumMap.objects()[i].get();
wStream.writeDecAsText(i + 1);
wStream.writeText(" 0 obj\n");
object->emitObject(&wStream, objNumMap, substitutes);
diff --git a/src/pdf/SkPDFBitmap.cpp b/src/pdf/SkPDFBitmap.cpp
index fdc3303..5e617de 100644
--- a/src/pdf/SkPDFBitmap.cpp
+++ b/src/pdf/SkPDFBitmap.cpp
@@ -382,13 +382,14 @@
// This SkPDFObject only outputs the alpha layer of the given bitmap.
class PDFAlphaBitmap final : public SkPDFObject {
public:
- PDFAlphaBitmap(const SkImage* image) : fImage(SkRef(image)) {}
- ~PDFAlphaBitmap() {}
+ PDFAlphaBitmap(const SkImage* image) : fImage(SkRef(image)) { SkASSERT(image); }
void emitObject(SkWStream* stream,
const SkPDFObjNumMap& objNumMap,
const SkPDFSubstituteMap& subs) const override {
+ SkASSERT(fImage);
emit_image_xobject(stream, fImage.get(), true, nullptr, objNumMap, subs);
}
+ void drop() override { fImage = nullptr; }
private:
sk_sp<const SkImage> fImage;
@@ -404,22 +405,25 @@
void emitObject(SkWStream* stream,
const SkPDFObjNumMap& objNumMap,
const SkPDFSubstituteMap& subs) const override {
+ SkASSERT(fImage);
emit_image_xobject(stream, fImage.get(), false, fSMask, objNumMap, subs);
}
void addResources(SkPDFObjNumMap* catalog,
const SkPDFSubstituteMap& subs) const override {
+ SkASSERT(fImage);
if (fSMask.get()) {
SkPDFObject* obj = subs.getSubstitute(fSMask.get());
SkASSERT(obj);
catalog->addObjectRecursively(obj, subs);
}
}
+ void drop() override { fImage = nullptr; fSMask = nullptr; }
PDFDefaultBitmap(const SkImage* image, SkPDFObject* smask)
- : fImage(SkRef(image)), fSMask(smask) {}
+ : fImage(SkRef(image)), fSMask(smask) { SkASSERT(fImage); }
private:
sk_sp<const SkImage> fImage;
- const sk_sp<SkPDFObject> fSMask;
+ sk_sp<SkPDFObject> fSMask;
};
} // namespace
@@ -437,15 +441,17 @@
sk_sp<SkData> fData;
bool fIsYUV;
PDFJpegBitmap(SkISize size, SkData* data, bool isYUV)
- : fSize(size), fData(SkRef(data)), fIsYUV(isYUV) {}
+ : fSize(size), fData(SkRef(data)), fIsYUV(isYUV) { SkASSERT(data); }
void emitObject(SkWStream*,
const SkPDFObjNumMap&,
const SkPDFSubstituteMap&) const override;
+ void drop() override { fData = nullptr; }
};
void PDFJpegBitmap::emitObject(SkWStream* stream,
const SkPDFObjNumMap& objNumMap,
const SkPDFSubstituteMap& substituteMap) const {
+ SkASSERT(fData);
SkPDFDict pdfDict("XObject");
pdfDict.insertName("Subtype", "Image");
pdfDict.insertInt("Width", fSize.width());
diff --git a/src/pdf/SkPDFDocument.cpp b/src/pdf/SkPDFDocument.cpp
index 4895dc5..670908e 100644
--- a/src/pdf/SkPDFDocument.cpp
+++ b/src/pdf/SkPDFDocument.cpp
@@ -224,7 +224,7 @@
emit_pdf_header(stream);
SkTDArray<int32_t> offsets;
for (int i = 0; i < objNumMap.objects().count(); ++i) {
- SkPDFObject* object = objNumMap.objects()[i];
+ SkPDFObject* object = objNumMap.objects()[i].get();
size_t offset = stream->bytesWritten();
// This assert checks that size(pdf_header) > 0 and that
// the output stream correctly reports bytesWritten().
@@ -236,6 +236,7 @@
stream->writeText(" 0 obj\n"); // Generation number is always 0.
object->emitObject(stream, objNumMap, substitutes);
stream->writeText("\nendobj\n");
+ object->drop();
}
int32_t xRefFileOffset = SkToS32(stream->bytesWritten() - baseOffset);
@@ -256,7 +257,7 @@
// The page tree has both child and parent pointers, so it creates a
// reference cycle. We must clear that cycle to properly reclaim memory.
for (int i = 0; i < pageTree.count(); i++) {
- pageTree[i]->clear();
+ pageTree[i]->drop();
}
pageTree.safeUnrefAll();
pages.unrefAll();
diff --git a/src/pdf/SkPDFFont.cpp b/src/pdf/SkPDFFont.cpp
index ac98433..9e78461 100644
--- a/src/pdf/SkPDFFont.cpp
+++ b/src/pdf/SkPDFFont.cpp
@@ -1434,3 +1434,10 @@
}
return *canon->fCanEmbedTypeface.set(id, canEmbed);
}
+
+void SkPDFFont::drop() {
+ fTypeface = nullptr;
+ fFontInfo = nullptr;
+ fDescriptor = nullptr;
+ this->SkPDFDict::drop();
+}
diff --git a/src/pdf/SkPDFFont.h b/src/pdf/SkPDFFont.h
index 16c5a76..4a01c41 100644
--- a/src/pdf/SkPDFFont.h
+++ b/src/pdf/SkPDFFont.h
@@ -193,6 +193,8 @@
static bool Find(uint32_t fontID, uint16_t glyphID, int* index);
+ void drop() override;
+
private:
sk_sp<SkTypeface> fTypeface;
diff --git a/src/pdf/SkPDFStream.cpp b/src/pdf/SkPDFStream.cpp
index 8b8f323..193723d 100644
--- a/src/pdf/SkPDFStream.cpp
+++ b/src/pdf/SkPDFStream.cpp
@@ -15,6 +15,11 @@
SkPDFStream::~SkPDFStream() {}
+void SkPDFStream::drop() {
+ fCompressedData.reset(nullptr);
+ this->SkPDFDict::drop();
+}
+
void SkPDFStream::emitObject(SkWStream* stream,
const SkPDFObjNumMap& objNumMap,
const SkPDFSubstituteMap& substitutes) const {
diff --git a/src/pdf/SkPDFStream.h b/src/pdf/SkPDFStream.h
index c5961d4..611e51b 100644
--- a/src/pdf/SkPDFStream.h
+++ b/src/pdf/SkPDFStream.h
@@ -43,6 +43,7 @@
void emitObject(SkWStream* stream,
const SkPDFObjNumMap& objNumMap,
const SkPDFSubstituteMap& substitutes) const override;
+ void drop() override;
protected:
/* Create a PDF stream with no data. The setData method must be called to
diff --git a/src/pdf/SkPDFTypes.cpp b/src/pdf/SkPDFTypes.cpp
index e6fe744..b8727a9 100644
--- a/src/pdf/SkPDFTypes.cpp
+++ b/src/pdf/SkPDFTypes.cpp
@@ -263,21 +263,26 @@
////////////////////////////////////////////////////////////////////////////////
-SkPDFArray::SkPDFArray() {}
-SkPDFArray::~SkPDFArray() {
- for (SkPDFUnion& value : fValues) {
- value.~SkPDFUnion();
- }
+SkPDFArray::SkPDFArray() { SkDEBUGCODE(fDumped = false;) }
+
+SkPDFArray::~SkPDFArray() { this->drop(); }
+
+void SkPDFArray::drop() {
fValues.reset();
+ SkDEBUGCODE(fDumped = true;)
}
int SkPDFArray::size() const { return fValues.count(); }
-void SkPDFArray::reserve(int length) { fValues.setReserve(length); }
+void SkPDFArray::reserve(int length) {
+ // TODO(halcanary): implement SkTArray<T>::reserve() or change the
+ // contstructor of SkPDFArray to take reserve size.
+}
void SkPDFArray::emitObject(SkWStream* stream,
const SkPDFObjNumMap& objNumMap,
const SkPDFSubstituteMap& substitutes) const {
+ SkASSERT(!fDumped);
stream->writeText("[");
for (int i = 0; i < fValues.count(); i++) {
fValues[i].emitObject(stream, objNumMap, substitutes);
@@ -290,13 +295,14 @@
void SkPDFArray::addResources(SkPDFObjNumMap* catalog,
const SkPDFSubstituteMap& substitutes) const {
+ SkASSERT(!fDumped);
for (const SkPDFUnion& value : fValues) {
value.addResources(catalog, substitutes);
}
}
void SkPDFArray::append(SkPDFUnion&& value) {
- new (fValues.append()) SkPDFUnion(std::move(value));
+ fValues.emplace_back(std::move(value));
}
void SkPDFArray::appendInt(int32_t value) {
@@ -337,11 +343,19 @@
///////////////////////////////////////////////////////////////////////////////
-SkPDFDict::SkPDFDict() {}
+SkPDFDict::~SkPDFDict() { this->drop(); }
-SkPDFDict::~SkPDFDict() { this->clear(); }
+void SkPDFDict::drop() {
+ fRecords.reset();
+ SkDEBUGCODE(fDumped = true;)
+}
-SkPDFDict::SkPDFDict(const char type[]) { this->insertName("Type", type); }
+SkPDFDict::SkPDFDict(const char type[]) {
+ SkDEBUGCODE(fDumped = false;)
+ if (type) {
+ this->insertName("Type", type);
+ }
+}
void SkPDFDict::emitObject(SkWStream* stream,
const SkPDFObjNumMap& objNumMap,
@@ -354,6 +368,7 @@
void SkPDFDict::emitAll(SkWStream* stream,
const SkPDFObjNumMap& objNumMap,
const SkPDFSubstituteMap& substitutes) const {
+ SkASSERT(!fDumped);
for (int i = 0; i < fRecords.count(); i++) {
fRecords[i].fKey.emitObject(stream, objNumMap, substitutes);
stream->writeText(" ");
@@ -366,41 +381,48 @@
void SkPDFDict::addResources(SkPDFObjNumMap* catalog,
const SkPDFSubstituteMap& substitutes) const {
+ SkASSERT(!fDumped);
for (int i = 0; i < fRecords.count(); i++) {
fRecords[i].fKey.addResources(catalog, substitutes);
fRecords[i].fValue.addResources(catalog, substitutes);
}
}
-void SkPDFDict::set(SkPDFUnion&& name, SkPDFUnion&& value) {
- Record* rec = fRecords.append();
- SkASSERT(name.isName());
- new (&rec->fKey) SkPDFUnion(std::move(name));
- new (&rec->fValue) SkPDFUnion(std::move(value));
+SkPDFDict::Record::Record(SkPDFUnion&& k, SkPDFUnion&& v)
+ : fKey(std::move(k)), fValue(std::move(v)) {}
+
+SkPDFDict::Record::Record(SkPDFDict::Record&& o)
+ : fKey(std::move(o.fKey)), fValue(std::move(o.fValue)) {}
+
+SkPDFDict::Record& SkPDFDict::Record::operator=(SkPDFDict::Record&& o) {
+ fKey = std::move(o.fKey);
+ fValue = std::move(o.fValue);
+ return *this;
}
int SkPDFDict::size() const { return fRecords.count(); }
void SkPDFDict::insertObjRef(const char key[], sk_sp<SkPDFObject> objSp) {
- this->set(SkPDFUnion::Name(key), SkPDFUnion::ObjRef(std::move(objSp)));
+ fRecords.emplace_back(SkPDFUnion::Name(key), SkPDFUnion::ObjRef(std::move(objSp)));
}
+
void SkPDFDict::insertObjRef(const SkString& key, sk_sp<SkPDFObject> objSp) {
- this->set(SkPDFUnion::Name(key), SkPDFUnion::ObjRef(std::move(objSp)));
+ fRecords.emplace_back(SkPDFUnion::Name(key), SkPDFUnion::ObjRef(std::move(objSp)));
}
void SkPDFDict::insertObject(const char key[], sk_sp<SkPDFObject> objSp) {
- this->set(SkPDFUnion::Name(key), SkPDFUnion::Object(std::move(objSp)));
+ fRecords.emplace_back(SkPDFUnion::Name(key), SkPDFUnion::Object(std::move(objSp)));
}
void SkPDFDict::insertObject(const SkString& key, sk_sp<SkPDFObject> objSp) {
- this->set(SkPDFUnion::Name(key), SkPDFUnion::Object(std::move(objSp)));
+ fRecords.emplace_back(SkPDFUnion::Name(key), SkPDFUnion::Object(std::move(objSp)));
}
void SkPDFDict::insertBool(const char key[], bool value) {
- this->set(SkPDFUnion::Name(key), SkPDFUnion::Bool(value));
+ fRecords.emplace_back(SkPDFUnion::Name(key), SkPDFUnion::Bool(value));
}
void SkPDFDict::insertInt(const char key[], int32_t value) {
- this->set(SkPDFUnion::Name(key), SkPDFUnion::Int(value));
+ fRecords.emplace_back(SkPDFUnion::Name(key), SkPDFUnion::Int(value));
}
void SkPDFDict::insertInt(const char key[], size_t value) {
@@ -408,39 +430,46 @@
}
void SkPDFDict::insertScalar(const char key[], SkScalar value) {
- this->set(SkPDFUnion::Name(key), SkPDFUnion::Scalar(value));
+ fRecords.emplace_back(SkPDFUnion::Name(key), SkPDFUnion::Scalar(value));
}
void SkPDFDict::insertName(const char key[], const char name[]) {
- this->set(SkPDFUnion::Name(key), SkPDFUnion::Name(name));
+ fRecords.emplace_back(SkPDFUnion::Name(key), SkPDFUnion::Name(name));
}
void SkPDFDict::insertName(const char key[], const SkString& name) {
- this->set(SkPDFUnion::Name(key), SkPDFUnion::Name(name));
+ fRecords.emplace_back(SkPDFUnion::Name(key), SkPDFUnion::Name(name));
}
void SkPDFDict::insertString(const char key[], const char value[]) {
- this->set(SkPDFUnion::Name(key), SkPDFUnion::String(value));
+ fRecords.emplace_back(SkPDFUnion::Name(key), SkPDFUnion::String(value));
}
void SkPDFDict::insertString(const char key[], const SkString& value) {
- this->set(SkPDFUnion::Name(key), SkPDFUnion::String(value));
-}
-
-void SkPDFDict::clear() {
- for (Record& rec : fRecords) {
- rec.fKey.~SkPDFUnion();
- rec.fValue.~SkPDFUnion();
- }
- fRecords.reset();
+ fRecords.emplace_back(SkPDFUnion::Name(key), SkPDFUnion::String(value));
}
////////////////////////////////////////////////////////////////////////////////
+SkPDFSharedStream::SkPDFSharedStream(SkStreamAsset* data)
+ : fAsset(data), fDict(new SkPDFDict) {
+ SkDEBUGCODE(fDumped = false;)
+ SkASSERT(data);
+}
+
+SkPDFSharedStream::~SkPDFSharedStream() { this->drop(); }
+
+void SkPDFSharedStream::drop() {
+ fAsset.reset();
+ fDict.reset(nullptr);
+ SkDEBUGCODE(fDumped = true;)
+}
+
void SkPDFSharedStream::emitObject(
SkWStream* stream,
const SkPDFObjNumMap& objNumMap,
const SkPDFSubstituteMap& substitutes) const {
+ SkASSERT(!fDumped);
SkDynamicMemoryWStream buffer;
SkDeflateWStream deflateWStream(&buffer);
// Since emitObject is const, this function doesn't change the dictionary.
@@ -467,6 +496,7 @@
void SkPDFSharedStream::addResources(
SkPDFObjNumMap* catalog, const SkPDFSubstituteMap& substitutes) const {
+ SkASSERT(!fDumped);
fDict->addResources(catalog, substitutes);
}
@@ -496,7 +526,7 @@
return false;
}
fObjectNumbers.set(obj, fObjectNumbers.count() + 1);
- fObjects.push(obj);
+ fObjects.emplace_back(sk_ref_sp(obj));
return true;
}
diff --git a/src/pdf/SkPDFTypes.h b/src/pdf/SkPDFTypes.h
index f93b030..f7b7b64 100644
--- a/src/pdf/SkPDFTypes.h
+++ b/src/pdf/SkPDFTypes.h
@@ -51,6 +51,14 @@
virtual void addResources(SkPDFObjNumMap* catalog,
const SkPDFSubstituteMap& substitutes) const {}
+ /**
+ * Release all resources associated with this SkPDFObject. It is
+ * an error to call emitObject() or addResources() after calling
+ * drop().
+ */
+ virtual void drop() {}
+
+ virtual ~SkPDFObject() {}
private:
typedef SkRefCnt INHERITED;
};
@@ -178,8 +186,6 @@
*/
class SkPDFArray final : public SkPDFObject {
public:
- static const int kMaxLen = 8191;
-
/** Create a PDF array. Maximum length is 8191.
*/
SkPDFArray();
@@ -191,6 +197,7 @@
const SkPDFSubstituteMap& substitutes) const override;
void addResources(SkPDFObjNumMap*,
const SkPDFSubstituteMap&) const override;
+ void drop() override;
/** The size of the array.
*/
@@ -215,9 +222,9 @@
void appendObjRef(sk_sp<SkPDFObject>);
private:
- SkTDArray<SkPDFUnion> fValues;
+ SkTArray<SkPDFUnion> fValues;
void append(SkPDFUnion&& value);
- typedef SkPDFObject INHERITED;
+ SkDEBUGCODE(bool fDumped;)
};
/** \class SkPDFDict
@@ -226,14 +233,10 @@
*/
class SkPDFDict : public SkPDFObject {
public:
- /** Create a PDF dictionary. Maximum number of entries is 4095.
+ /** Create a PDF dictionary.
+ * @param type The value of the Type entry, nullptr for no type.
*/
- SkPDFDict();
-
- /** Create a PDF dictionary with a Type entry.
- * @param type The value of the Type entry.
- */
- explicit SkPDFDict(const char type[]);
+ explicit SkPDFDict(const char type[] = nullptr);
virtual ~SkPDFDict();
@@ -243,6 +246,7 @@
const SkPDFSubstituteMap& substitutes) const override;
void addResources(SkPDFObjNumMap*,
const SkPDFSubstituteMap&) const override;
+ void drop() override;
/** The size of the dictionary.
*/
@@ -270,10 +274,6 @@
void insertString(const char key[], const char value[]);
void insertString(const char key[], const SkString& value);
- /** Remove all entries from the dictionary.
- */
- void clear();
-
/** Emit the dictionary, without the "<<" and ">>".
*/
void emitAll(SkWStream* stream,
@@ -284,13 +284,14 @@
struct Record {
SkPDFUnion fKey;
SkPDFUnion fValue;
+ Record(SkPDFUnion&&, SkPDFUnion&&);
+ Record(Record&&);
+ Record& operator=(Record&&);
+ Record(const Record&) = delete;
+ Record& operator=(const Record&) = delete;
};
- SkTDArray<Record> fRecords;
- static const int kMaxLen = 4095;
-
- void set(SkPDFUnion&& name, SkPDFUnion&& value);
-
- typedef SkPDFObject INHERITED;
+ SkTArray<Record> fRecords;
+ SkDEBUGCODE(bool fDumped;)
};
/** \class SkPDFSharedStream
@@ -303,18 +304,20 @@
class SkPDFSharedStream final : public SkPDFObject {
public:
// Takes ownership of asset.
- SkPDFSharedStream(SkStreamAsset* data)
- : fAsset(data), fDict(new SkPDFDict) { SkASSERT(data); }
+ SkPDFSharedStream(SkStreamAsset* data);
+ ~SkPDFSharedStream();
SkPDFDict* dict() { return fDict.get(); }
void emitObject(SkWStream*,
const SkPDFObjNumMap&,
const SkPDFSubstituteMap&) const override;
void addResources(SkPDFObjNumMap*,
const SkPDFSubstituteMap&) const override;
+ void drop() override;
private:
SkAutoTDelete<SkStreamAsset> fAsset;
sk_sp<SkPDFDict> fDict;
+ SkDEBUGCODE(bool fDumped;)
typedef SkPDFObject INHERITED;
};
@@ -344,10 +347,10 @@
*/
int32_t getObjectNumber(SkPDFObject* obj) const;
- const SkTDArray<SkPDFObject*>& objects() const { return fObjects; }
+ const SkTArray<sk_sp<SkPDFObject>>& objects() const { return fObjects; }
private:
- SkTDArray<SkPDFObject*> fObjects;
+ SkTArray<sk_sp<SkPDFObject>> fObjects;
SkTHashMap<SkPDFObject*, int32_t> fObjectNumbers;
};