Simplify reference management in SkPDF
Prior to this change, SkPDFObject subclasses were required
to track their resources separately from the document
structure. (An object has a resource if it depends, via an
indirect reference, on another object). This led to a lot
of extra code to duplicate effort. I replace the
getResources() function with the much simpler addResources()
function. I only define a non-trivial addResources() method
on arrays, dictionaries, and indirect object references.
All other specialized classes simply rely on their parent
class's implementation.
SkPDFObject::addResources() works by recursively walking the
directed graph of object (direct and indirect) references
and adding resources to a set. It doesn't matter that there
are closed loops in the graph, since we check the set before
walking down a branch.
- Add SkPDFObject::addResources() virtual function, with
four implementations
- Remove SkPDFObject::getResources() virtual function and
all implementations.
- Remove SkPDFObject::GetResourcesHelper()
- Remove SkPDFObject::AddResourceHelper()
- In SkPDFCatalog::findObjectIndex(), add an object to the
catalog if it doesn't exist yet.
- SkPDFCatalog::setSubstitute() no longer sets up resources
- SkPDFDocument.cpp no longer needs the Streamer object
- SkPDFDocument.cpp calls fDocCatalog->addResources to build
the resource list.
- SkPDFFont::addResource() removed
- All SkPDF-::fResource sets removed (they are redundant).
- removed SkPDFImage::addSMask() function
- SkPDFResourceDict::getReferencedResources() removed.
Motivation: this removes quite a bit of code and makes the
objects slightly slimmer in memory. Most importantly, this
will lead the way towards removing SkPDFObject's inheritance
from SkRefCnt, which will greatly simplify everything.
Testing: I usually test changes to the PDF backend by
comparing checksums of PDF files rendered from GMs and SKPs
before and after the change. This change both re-orders and
re-numbers the indirect PDF objects. I used the qpdf
program to normalize the PDFs and then compared the
normalized outputs from before and after the change; they
matched.
Review URL: https://codereview.chromium.org/870333002
diff --git a/src/pdf/SkPDFTypes.cpp b/src/pdf/SkPDFTypes.cpp
index b0763a3..7902e59 100644
--- a/src/pdf/SkPDFTypes.cpp
+++ b/src/pdf/SkPDFTypes.cpp
@@ -17,36 +17,6 @@
#define SNPRINTF snprintf
#endif
-///////////////////////////////////////////////////////////////////////////////
-
-void SkPDFObject::getResources(const SkTSet<SkPDFObject*>& knownResourceObjects,
- SkTSet<SkPDFObject*>* newResourceObjects) {}
-
-void SkPDFObject::AddResourceHelper(SkPDFObject* resource,
- SkTDArray<SkPDFObject*>* list) {
- list->push(resource);
- resource->ref();
-}
-
-void SkPDFObject::GetResourcesHelper(
- const SkTDArray<SkPDFObject*>* resources,
- const SkTSet<SkPDFObject*>& knownResourceObjects,
- SkTSet<SkPDFObject*>* newResourceObjects) {
- if (resources->count()) {
- newResourceObjects->setReserve(
- newResourceObjects->count() + resources->count());
- for (int i = 0; i < resources->count(); i++) {
- if (!knownResourceObjects.contains((*resources)[i]) &&
- !newResourceObjects->contains((*resources)[i])) {
- newResourceObjects->add((*resources)[i]);
- (*resources)[i]->ref();
- (*resources)[i]->getResources(knownResourceObjects,
- newResourceObjects);
- }
- }
- }
-}
-
////////////////////////////////////////////////////////////////////////////////
SkPDFObjRef::SkPDFObjRef(SkPDFObject* obj) : fObj(obj) {
@@ -56,8 +26,17 @@
SkPDFObjRef::~SkPDFObjRef() {}
void SkPDFObjRef::emitObject(SkWStream* stream, SkPDFCatalog* catalog) {
- catalog->emitObjectNumber(stream, fObj.get());
- stream->writeText(" R");
+ stream->writeDecAsText(catalog->getObjectNumber(fObj.get()));
+ stream->writeText(" 0 R"); // Generation number is always 0.
+}
+
+void SkPDFObjRef::addResources(SkTSet<SkPDFObject*>* resourceSet,
+ SkPDFCatalog* catalog) const {
+ SkPDFObject* obj = catalog->getSubstituteObject(fObj);
+ SkASSERT(obj);
+ if (resourceSet->add(obj)) {
+ obj->addResources(resourceSet, catalog);
+ }
}
////////////////////////////////////////////////////////////////////////////////
@@ -277,6 +256,14 @@
stream->writeText("]");
}
+void SkPDFArray::addResources(SkTSet<SkPDFObject*>* resourceSet,
+ SkPDFCatalog* catalog) const {
+ for (int i = 0; i < fValue.count(); i++) {
+ catalog->getSubstituteObject(fValue[i])
+ ->addResources(resourceSet, catalog);
+ }
+}
+
void SkPDFArray::reserve(int length) {
SkASSERT(length <= kMaxLen);
fValue.setReserve(length);
@@ -347,6 +334,17 @@
stream->writeText(">>");
}
+void SkPDFDict::addResources(SkTSet<SkPDFObject*>* resourceSet,
+ SkPDFCatalog* catalog) const {
+ for (int i = 0; i < fValue.count(); i++) {
+ SkASSERT(fValue[i].key);
+ SkASSERT(fValue[i].value);
+ fValue[i].key->addResources(resourceSet, catalog);
+ catalog->getSubstituteObject(fValue[i].value)
+ ->addResources(resourceSet, catalog);
+ }
+}
+
SkPDFObject* SkPDFDict::append(SkPDFName* key, SkPDFObject* value) {
SkASSERT(key);
SkASSERT(value);