Use SkSet to fix issue when pdf generates an exp number of resources.

The problem fixed - http://code.google.com/p/skia/issues/detail?id=940 - is that getResources will recursively obtain all child resource recursively without checking for duplicates.

If we have lots of duplicates, then we try to build a very large vector (exponential with the number of nodes usually) and sooner or later we end up using too much memory and crash.

A possible solution could have been to make sure resources do not have duplicates, but that requirement is impractical, and it this leaves the solution fragile, if there is any issue in the tree,  we crash.

When we emit the pdf, the large number of duplicates is not an issue, because SkPDFCatalog::addObject will deal with duplicates.

I have run the gm with --config pdf, and the images are 100% same bits, while the pdfs have the same size but some very small changes, the order of some objects.
Review URL: https://codereview.appspot.com/6744050

git-svn-id: http://skia.googlecode.com/svn/trunk@7883 2bbb7eff-a529-9590-31e7-b0007b416f81

diff --git a/gyp/pdf.gyp b/gyp/pdf.gyp
index 7b3b9ea..081df01 100644 (file)
--- a/gyp/pdf.gyp
+++ b/gyp/pdf.gyp
@@ -12,6 +12,7 @@
       'include_dirs': [
         '../include/config',
         '../include/core',
+        '../include/images',
         '../include/pdf',
         '../src/core', # needed to get SkGlyphCache.h and SkTextFormatParams.h
         '../src/utils', # needed to get SkBitSet.h
@@ -43,6 +44,7 @@
         '../src/pdf/SkPDFTypes.h',
         '../src/pdf/SkPDFUtils.cpp',
         '../src/pdf/SkPDFUtils.h',
+        '../src/pdf/SkTSet.h',
       ],
       # This section makes all targets that depend on this target
       # #define SK_SUPPORT_PDF and have access to the pdf header files.

diff --git a/include/pdf/SkPDFDevice.h b/include/pdf/SkPDFDevice.h
index a13d617..f42ecb8 100644 (file)
--- a/include/pdf/SkPDFDevice.h
+++ b/include/pdf/SkPDFDevice.h
@@ -29,6 +29,7 @@ class SkPDFGraphicState;
 class SkPDFObject;
 class SkPDFShader;
 class SkPDFStream;
+template <typename T> class SK_API SkTSet;
 
 // Private classes.
 struct ContentEntry;
@@ -130,12 +131,19 @@ public:
     SK_API SkPDFDict* getResourceDict();
 
     /** Get the list of resources (PDF objects) used on this page.
-     *  @param resourceList A list to append the resources to.
+     *  This method will add to newResourceObjects any objects that this method
+     *  depends on, but not already in knownResourceObjects. This might operate
+     *  recursively so if this object depends on another object and that object
+     *  depends on two more, all three objects will be added.
+     *
+     *  @param knownResourceObjects  The set of resources to be ignored.
+     *  @param newResourceObjects  The set to append dependant resources to.
      *  @param recursive    If recursive is true, get the resources of the
      *                      device's resources recursively. (Useful for adding
      *                      objects to the catalog.)
      */
-    SK_API void getResources(SkTDArray<SkPDFObject*>* resourceList,
+    SK_API void getResources(const SkTSet<SkPDFObject*>& knownResourceObjects,
+                             SkTSet<SkPDFObject*>* newResourceObjects,
                              bool recursive) const;
 
     /** Get the fonts used on this device.

diff --git a/include/pdf/SkPDFDocument.h b/include/pdf/SkPDFDocument.h
index 167634e..443e8c2 100644 (file)
--- a/include/pdf/SkPDFDocument.h
+++ b/include/pdf/SkPDFDocument.h
@@ -21,6 +21,7 @@ class SkPDFDict;
 class SkPDFPage;
 class SkPDFObject;
 class SkWStream;
+template <typename T> class SK_API SkTSet;
 
 /** \class SkPDFDocument
 
@@ -76,7 +77,8 @@ private:
     SkTDArray<SkPDFPage*> fPages;
     SkTDArray<SkPDFDict*> fPageTree;
     SkPDFDict* fDocCatalog;
-    SkTDArray<SkPDFObject*> fPageResources;
+    SkTSet<SkPDFObject*>* fFirstPageResources;
+    SkTSet<SkPDFObject*>* fOtherPageResources;
     SkTDArray<SkPDFObject*> fSubstitutes;
     int fSecondPageFirstResourceIndex;
 

diff --git a/src/pdf/SkPDFCatalog.cpp b/src/pdf/SkPDFCatalog.cpp
index c0f6fb0..a5a5a0e 100644 (file)
--- a/src/pdf/SkPDFCatalog.cpp
+++ b/src/pdf/SkPDFCatalog.cpp
@@ -168,12 +168,15 @@ void SkPDFCatalog::setSubstitute(SkPDFObject* original,
     fSubstituteMap.append(1, &newMapping);
 
     // Add resource objects of substitute object to catalog.
-    SkTDArray<SkPDFObject*>* targetList = getSubstituteList(onFirstPage);
-    int existingSize = targetList->count();
-    newMapping.fSubstitute->getResources(targetList);
-    for (int i = existingSize; i < targetList->count(); ++i) {
-        addObject((*targetList)[i], onFirstPage);
-    }
+    SkTSet<SkPDFObject*>* targetSet = getSubstituteList(onFirstPage);
+    SkTSet<SkPDFObject*> newResourceObjects;
+    newMapping.fSubstitute->getResources(*targetSet, &newResourceObjects);
+    for (int i = 0; i < newResourceObjects.count(); ++i) {
+        addObject(newResourceObjects[i], onFirstPage);
+    }
+    // mergeInto returns the numbr of duplicates.
+    // If there are duplicates, there is a bug and we mess ref counting.
+    SkASSERT(targetSet->mergeInto(newResourceObjects) == 0);
 }
 
 SkPDFObject* SkPDFCatalog::getSubstituteObject(SkPDFObject* object) {
@@ -187,22 +190,22 @@ SkPDFObject* SkPDFCatalog::getSubstituteObject(SkPDFObject* object) {
 
 off_t SkPDFCatalog::setSubstituteResourcesOffsets(off_t fileOffset,
                                                   bool firstPage) {
-    SkTDArray<SkPDFObject*>* targetList = getSubstituteList(firstPage);
+    SkTSet<SkPDFObject*>* targetSet = getSubstituteList(firstPage);
     off_t offsetSum = fileOffset;
-    for (int i = 0; i < targetList->count(); ++i) {
-        offsetSum += setFileOffset((*targetList)[i], offsetSum);
+    for (int i = 0; i < targetSet->count(); ++i) {
+        offsetSum += setFileOffset((*targetSet)[i], offsetSum);
     }
     return offsetSum - fileOffset;
 }
 
 void SkPDFCatalog::emitSubstituteResources(SkWStream *stream, bool firstPage) {
-    SkTDArray<SkPDFObject*>* targetList = getSubstituteList(firstPage);
-    for (int i = 0; i < targetList->count(); ++i) {
-        (*targetList)[i]->emit(stream, this, true);
+    SkTSet<SkPDFObject*>* targetSet = getSubstituteList(firstPage);
+    for (int i = 0; i < targetSet->count(); ++i) {
+        (*targetSet)[i]->emit(stream, this, true);
     }
 }
 
-SkTDArray<SkPDFObject*>* SkPDFCatalog::getSubstituteList(bool firstPage) {
+SkTSet<SkPDFObject*>* SkPDFCatalog::getSubstituteList(bool firstPage) {
     return firstPage ? &fSubstituteResourcesFirstPage :
                        &fSubstituteResourcesRemaining;
 }

diff --git a/src/pdf/SkPDFCatalog.h b/src/pdf/SkPDFCatalog.h
index d5825ac..c7c6d6e 100644 (file)
--- a/src/pdf/SkPDFCatalog.h
+++ b/src/pdf/SkPDFCatalog.h
@@ -115,8 +115,8 @@ private:
 
     // TODO(arthurhsu): Make this a hash if it's a performance problem.
     SkTDArray<SubstituteMapping> fSubstituteMap;
-    SkTDArray<SkPDFObject*> fSubstituteResourcesFirstPage;
-    SkTDArray<SkPDFObject*> fSubstituteResourcesRemaining;
+    SkTSet<SkPDFObject*> fSubstituteResourcesFirstPage;
+    SkTSet<SkPDFObject*> fSubstituteResourcesRemaining;
 
     // Number of objects on the first page.
     uint32_t fFirstPageCount;
@@ -131,7 +131,7 @@ private:
 
     int assignObjNum(SkPDFObject* obj);
 
-    SkTDArray<SkPDFObject*>* getSubstituteList(bool firstPage);
+    SkTSet<SkPDFObject*>* getSubstituteList(bool firstPage);
 };
 
 #endif

diff --git a/src/pdf/SkPDFDevice.cpp b/src/pdf/SkPDFDevice.cpp
index aabc6b3..7fb6db0 100644 (file)
--- a/src/pdf/SkPDFDevice.cpp
+++ b/src/pdf/SkPDFDevice.cpp
@@ -32,6 +32,7 @@
 #include "SkTemplates.h"
 #include "SkTypeface.h"
 #include "SkTypes.h"
+#include "SkTSet.h"
 
 // Utility functions
 
@@ -1175,39 +1176,57 @@ SkPDFDict* SkPDFDevice::getResourceDict() {
     return fResourceDict;
 }
 
-void SkPDFDevice::getResources(SkTDArray<SkPDFObject*>* resourceList,
+void SkPDFDevice::getResources(const SkTSet<SkPDFObject*>& knownResourceObjects,
+                               SkTSet<SkPDFObject*>* newResourceObjects,
                                bool recursive) const {
-    resourceList->setReserve(resourceList->count() +
-                             fGraphicStateResources.count() +
-                             fXObjectResources.count() +
-                             fFontResources.count() +
-                             fShaderResources.count());
+    // TODO: reserve not correct if we need to recursively explore.
+    newResourceObjects->setReserve(newResourceObjects->count() +
+                                   fGraphicStateResources.count() +
+                                   fXObjectResources.count() +
+                                   fFontResources.count() +
+                                   fShaderResources.count());
     for (int i = 0; i < fGraphicStateResources.count(); i++) {
-        resourceList->push(fGraphicStateResources[i]);
-        fGraphicStateResources[i]->ref();
-        if (recursive) {
-            fGraphicStateResources[i]->getResources(resourceList);
+        if (!knownResourceObjects.contains(fGraphicStateResources[i]) &&
+                !newResourceObjects->contains(fGraphicStateResources[i])) {
+            newResourceObjects->add(fGraphicStateResources[i]);
+            fGraphicStateResources[i]->ref();
+            if (recursive) {
+                fGraphicStateResources[i]->getResources(knownResourceObjects, 
+                                                        newResourceObjects);
+            }
         }
     }
     for (int i = 0; i < fXObjectResources.count(); i++) {
-        resourceList->push(fXObjectResources[i]);
-        fXObjectResources[i]->ref();
-        if (recursive) {
-            fXObjectResources[i]->getResources(resourceList);
+        if (!knownResourceObjects.contains(fXObjectResources[i]) &&
+                !newResourceObjects->contains(fXObjectResources[i])) {
+            newResourceObjects->add(fXObjectResources[i]);
+            fXObjectResources[i]->ref();
+            if (recursive) {
+                fXObjectResources[i]->getResources(knownResourceObjects, 
+                                                   newResourceObjects);
+            }
         }
     }
     for (int i = 0; i < fFontResources.count(); i++) {
-        resourceList->push(fFontResources[i]);
-        fFontResources[i]->ref();
-        if (recursive) {
-            fFontResources[i]->getResources(resourceList);
+        if (!knownResourceObjects.contains(fFontResources[i]) &&
+                !newResourceObjects->contains(fFontResources[i])) {
+            newResourceObjects->add(fFontResources[i]);
+            fFontResources[i]->ref();
+            if (recursive) {
+                fFontResources[i]->getResources(knownResourceObjects, 
+                                                newResourceObjects);
+            }
         }
     }
     for (int i = 0; i < fShaderResources.count(); i++) {
-        resourceList->push(fShaderResources[i]);
-        fShaderResources[i]->ref();
-        if (recursive) {
-            fShaderResources[i]->getResources(resourceList);
+        if (!knownResourceObjects.contains(fShaderResources[i]) &&
+                !newResourceObjects->contains(fShaderResources[i])) {
+            newResourceObjects->add(fShaderResources[i]);
+            fShaderResources[i]->ref();
+            if (recursive) {
+                fShaderResources[i]->getResources(knownResourceObjects, 
+                                                  newResourceObjects);
+            }
         }
     }
 }

diff --git a/src/pdf/SkPDFDocument.cpp b/src/pdf/SkPDFDocument.cpp
index c7266d8..fc09286 100644 (file)
--- a/src/pdf/SkPDFDocument.cpp
+++ b/src/pdf/SkPDFDocument.cpp
@@ -14,21 +14,13 @@
 #include "SkPDFPage.h"
 #include "SkPDFTypes.h"
 #include "SkStream.h"
+#include "SkTSet.h"
 
-// Add the resources, starting at firstIndex to the catalog, removing any dupes.
-// A hash table would be really nice here.
-static void addResourcesToCatalog(int firstIndex, bool firstPage,
-                          SkTDArray<SkPDFObject*>* resourceList,
-                          SkPDFCatalog* catalog) {
-    for (int i = firstIndex; i < resourceList->count(); i++) {
-        int index = resourceList->find((*resourceList)[i]);
-        if (index != i) {
-            (*resourceList)[i]->unref();
-            resourceList->removeShuffle(i);
-            i--;
-        } else {
-            catalog->addObject((*resourceList)[i], firstPage);
-        }
+static void addResourcesToCatalog(bool firstPage,
+                                  SkTSet<SkPDFObject*>* resourceSet,
+                                  SkPDFCatalog* catalog) {
+    for (int i = 0; i < resourceSet->count(); i++) {
+        catalog->addObject((*resourceSet)[i], firstPage);
     }
 }
 
@@ -57,11 +49,12 @@ static void perform_font_subsetting(SkPDFCatalog* catalog,
 
 SkPDFDocument::SkPDFDocument(Flags flags)
         : fXRefFileOffset(0),
-          fSecondPageFirstResourceIndex(0),
           fTrailerDict(NULL) {
     fCatalog.reset(new SkPDFCatalog(flags));
     fDocCatalog = SkNEW_ARGS(SkPDFDict, ("Catalog"));
     fCatalog->addObject(fDocCatalog, true);
+    fFirstPageResources = NULL;
+    fOtherPageResources = NULL;
 }
 
 SkPDFDocument::~SkPDFDocument() {
@@ -73,11 +66,14 @@ SkPDFDocument::~SkPDFDocument() {
         fPageTree[i]->clear();
     }
     fPageTree.safeUnrefAll();
-    fPageResources.safeUnrefAll();
+    fFirstPageResources->safeUnrefAll();
+    fOtherPageResources->safeUnrefAll();
     fSubstitutes.safeUnrefAll();
 
     fDocCatalog->unref();
     SkSafeUnref(fTrailerDict);
+    SkDELETE(fFirstPageResources);
+    SkDELETE(fOtherPageResources);
 }
 
 bool SkPDFDocument::emitPDF(SkWStream* stream) {
@@ -90,6 +86,9 @@ bool SkPDFDocument::emitPDF(SkWStream* stream) {
         }
     }
 
+    fFirstPageResources = SkNEW(SkTSet<SkPDFObject*>);
+    fOtherPageResources = SkNEW(SkTSet<SkPDFObject*>);
+
     // We haven't emitted the document before if fPageTree is empty.
     if (fPageTree.isEmpty()) {
         SkPDFDict* pageTreeRoot;
@@ -108,15 +107,35 @@ bool SkPDFDocument::emitPDF(SkWStream* stream) {
         */
 
         bool firstPage = true;
+        /* The references returned in newResources are transfered to
+         * fFirstPageResources or fOtherPageResources depending on firstPage and
+         * knownResources doesn't have a reference but just relies on the other
+         * two sets to maintain a reference.
+         */
+        SkTSet<SkPDFObject*> knownResources;
+
+        // mergeInto returns the number of duplicates.
+        // If there are duplicates, there is a bug and we mess ref counting.
+        int duplicates = 0;
+        knownResources.mergeInto(*fFirstPageResources);
         for (int i = 0; i < fPages.count(); i++) {
-            int resourceCount = fPageResources.count();
-            fPages[i]->finalizePage(fCatalog.get(), firstPage, &fPageResources);
-            addResourcesToCatalog(resourceCount, firstPage, &fPageResources,
-                                  fCatalog.get());
-            if (i == 0) {
+            if (i == 1) {
                 firstPage = false;
-                fSecondPageFirstResourceIndex = fPageResources.count();
+                duplicates = knownResources.mergeInto(*fOtherPageResources);
+            }
+            SkTSet<SkPDFObject*> newResources;
+            fPages[i]->finalizePage(
+                fCatalog.get(), firstPage, knownResources, &newResources);
+            addResourcesToCatalog(firstPage, &newResources, fCatalog.get());
+            if (firstPage) {
+                duplicates = fFirstPageResources->mergeInto(newResources);
+            } else {
+                duplicates = fOtherPageResources->mergeInto(newResources);
             }
+            SkASSERT(duplicates == 0);             
+
+            duplicates = knownResources.mergeInto(newResources);
+            SkASSERT(duplicates == 0); 
         }
 
         // Build font subsetting info before proceeding.
@@ -128,8 +147,8 @@ bool SkPDFDocument::emitPDF(SkWStream* stream) {
         fileOffset += fCatalog->setFileOffset(fPages[0], fileOffset);
         fileOffset += fPages[0]->getPageSize(fCatalog.get(),
                 (size_t) fileOffset);
-        for (int i = 0; i < fSecondPageFirstResourceIndex; i++) {
-            fileOffset += fCatalog->setFileOffset(fPageResources[i],
+        for (int i = 0; i < fFirstPageResources->count(); i++) {
+            fileOffset += fCatalog->setFileOffset((*fFirstPageResources)[i],
                                                   fileOffset);
         }
         // Add the size of resources of substitute objects used on page 1.
@@ -147,11 +166,9 @@ bool SkPDFDocument::emitPDF(SkWStream* stream) {
             fileOffset += fPages[i]->getPageSize(fCatalog.get(), fileOffset);
         }
 
-        for (int i = fSecondPageFirstResourceIndex;
-                 i < fPageResources.count();
-                 i++) {
-            fileOffset += fCatalog->setFileOffset(fPageResources[i],
-                                                  fileOffset);
+        for (int i = 0; i < fOtherPageResources->count(); i++) {
+            fileOffset += fCatalog->setFileOffset(
+                (*fOtherPageResources)[i], fileOffset);
         }
 
         fileOffset += fCatalog->setSubstituteResourcesOffsets(fileOffset,
@@ -163,8 +180,8 @@ bool SkPDFDocument::emitPDF(SkWStream* stream) {
     fDocCatalog->emitObject(stream, fCatalog.get(), true);
     fPages[0]->emitObject(stream, fCatalog.get(), true);
     fPages[0]->emitPage(stream, fCatalog.get());
-    for (int i = 0; i < fSecondPageFirstResourceIndex; i++) {
-        fPageResources[i]->emit(stream, fCatalog.get(), true);
+    for (int i = 0; i < fFirstPageResources->count(); i++) {
+        (*fFirstPageResources)[i]->emit(stream, fCatalog.get(), true);
     }
     fCatalog->emitSubstituteResources(stream, true);
     // TODO(vandebo): Support linearized format
@@ -181,10 +198,8 @@ bool SkPDFDocument::emitPDF(SkWStream* stream) {
         fPages[i]->emitPage(stream, fCatalog.get());
     }
 
-    for (int i = fSecondPageFirstResourceIndex;
-            i < fPageResources.count();
-            i++) {
-        fPageResources[i]->emit(stream, fCatalog.get(), true);
+    for (int i = 0; i < fOtherPageResources->count(); i++) {
+        (*fOtherPageResources)[i]->emit(stream, fCatalog.get(), true);
     }
 
     fCatalog->emitSubstituteResources(stream, false);

diff --git a/src/pdf/SkPDFFont.cpp b/src/pdf/SkPDFFont.cpp
index 89b4fc9..af58ac7 100644 (file)
--- a/src/pdf/SkPDFFont.cpp
+++ b/src/pdf/SkPDFFont.cpp
@@ -716,8 +716,9 @@ SkPDFFont::~SkPDFFont() {
     fResources.unrefAll();
 }
 
-void SkPDFFont::getResources(SkTDArray<SkPDFObject*>* resourceList) {
-    GetResourcesHelper(&fResources, resourceList);
+void SkPDFFont::getResources(const SkTSet<SkPDFObject*>& knownResourceObjects,
+                             SkTSet<SkPDFObject*>* newResourceObjects) {
+    GetResourcesHelper(&fResources, knownResourceObjects, newResourceObjects);
 }
 
 SkTypeface* SkPDFFont::typeface() {

diff --git a/src/pdf/SkPDFFont.h b/src/pdf/SkPDFFont.h
index 693b911..d3aae89 100644 (file)
--- a/src/pdf/SkPDFFont.h
+++ b/src/pdf/SkPDFFont.h
@@ -81,7 +81,8 @@ class SkPDFFont : public SkPDFDict {
 public:
     virtual ~SkPDFFont();
 
-    virtual void getResources(SkTDArray<SkPDFObject*>* resourceList);
+    virtual void getResources(const SkTSet<SkPDFObject*>& knownResourceObjects,
+                              SkTSet<SkPDFObject*>* newResourceObjects);
 
     /** Returns the typeface represented by this class. Returns NULL for the
      *  default typeface.

diff --git a/src/pdf/SkPDFFormXObject.cpp b/src/pdf/SkPDFFormXObject.cpp
index 5e33995..884e6db 100644 (file)
--- a/src/pdf/SkPDFFormXObject.cpp
+++ b/src/pdf/SkPDFFormXObject.cpp
@@ -20,16 +20,8 @@ SkPDFFormXObject::SkPDFFormXObject(SkPDFDevice* device) {
     // We don't want to keep around device because we'd have two copies
     // of content, so reference or copy everything we need (content and
     // resources).
-    device->getResources(&fResources, false);
-
-    // Fail fast if in the tree of resources a child references a parent.
-    // If there is an issue, getResources will end up consuming all memory.
-    // TODO: A better approach might be for all SkPDFObject to keep track
-    // of possible cycles.
-#ifdef SK_DEBUG
-    SkTDArray<SkPDFObject*> dummy_resourceList;
-    getResources(&dummy_resourceList);
-#endif
+    SkTSet<SkPDFObject*> emptySet;
+    device->getResources(emptySet, &fResources, false);
 
     SkAutoTUnref<SkStream> content(device->content());
     setData(content.get());
@@ -64,6 +56,10 @@ SkPDFFormXObject::~SkPDFFormXObject() {
     fResources.unrefAll();
 }
 
-void SkPDFFormXObject::getResources(SkTDArray<SkPDFObject*>* resourceList) {
-    GetResourcesHelper(&fResources, resourceList);
+void SkPDFFormXObject::getResources(
+        const SkTSet<SkPDFObject*>& knownResourceObjects,
+        SkTSet<SkPDFObject*>* newResourceObjects) {
+    GetResourcesHelper(&fResources.toArray(),
+                       knownResourceObjects,
+                       newResourceObjects);
 }

diff --git a/src/pdf/SkPDFFormXObject.h b/src/pdf/SkPDFFormXObject.h
index 0c49152..b1a6f74 100644 (file)
--- a/src/pdf/SkPDFFormXObject.h
+++ b/src/pdf/SkPDFFormXObject.h
@@ -39,10 +39,11 @@ public:
     virtual ~SkPDFFormXObject();
 
     // The SkPDFObject interface.
-    virtual void getResources(SkTDArray<SkPDFObject*>* resourceList);
+    virtual void getResources(const SkTSet<SkPDFObject*>& knownResourceObjects,
+                              SkTSet<SkPDFObject*>* newResourceObjects);
 
 private:
-    SkTDArray<SkPDFObject*> fResources;
+    SkTSet<SkPDFObject*> fResources;
 };
 
 #endif

diff --git a/src/pdf/SkPDFGraphicState.cpp b/src/pdf/SkPDFGraphicState.cpp
index 558deb7..61929b3 100644 (file)
--- a/src/pdf/SkPDFGraphicState.cpp
+++ b/src/pdf/SkPDFGraphicState.cpp
@@ -59,8 +59,10 @@ SkPDFGraphicState::~SkPDFGraphicState() {
     fResources.unrefAll();
 }
 
-void SkPDFGraphicState::getResources(SkTDArray<SkPDFObject*>* resourceList) {
-    GetResourcesHelper(&fResources, resourceList);
+void SkPDFGraphicState::getResources(
+        const SkTSet<SkPDFObject*>& knownResourceObjects,
+        SkTSet<SkPDFObject*>* newResourceObjects) {
+    GetResourcesHelper(&fResources, knownResourceObjects, newResourceObjects);
 }
 
 void SkPDFGraphicState::emitObject(SkWStream* stream, SkPDFCatalog* catalog,

diff --git a/src/pdf/SkPDFGraphicState.h b/src/pdf/SkPDFGraphicState.h
index af01737..64f34f8 100644 (file)
--- a/src/pdf/SkPDFGraphicState.h
+++ b/src/pdf/SkPDFGraphicState.h
@@ -30,7 +30,8 @@ class SkPDFGraphicState : public SkPDFDict {
 public:
     virtual ~SkPDFGraphicState();
 
-    virtual void getResources(SkTDArray<SkPDFObject*>* resourceList);
+    virtual void getResources(const SkTSet<SkPDFObject*>& knownResourceObjects,
+                              SkTSet<SkPDFObject*>* newResourceObjects);
 
     // Override emitObject and getOutputSize so that we can populate
     // the dictionary on demand.

diff --git a/src/pdf/SkPDFImage.cpp b/src/pdf/SkPDFImage.cpp
index 1b93f6e..9c57fba 100644 (file)
--- a/src/pdf/SkPDFImage.cpp
+++ b/src/pdf/SkPDFImage.cpp
@@ -287,8 +287,9 @@ SkPDFImage* SkPDFImage::addSMask(SkPDFImage* mask) {
     return mask;
 }
 
-void SkPDFImage::getResources(SkTDArray<SkPDFObject*>* resourceList) {
-    GetResourcesHelper(&fResources, resourceList);
+void SkPDFImage::getResources(const SkTSet<SkPDFObject*>& knownResourceObjects,
+                              SkTSet<SkPDFObject*>* newResourceObjects) {
+    GetResourcesHelper(&fResources, knownResourceObjects, newResourceObjects);
 }
 
 SkPDFImage::SkPDFImage(SkStream* imageData, const SkBitmap& bitmap,

diff --git a/src/pdf/SkPDFImage.h b/src/pdf/SkPDFImage.h
index 48b0157..69aa7da 100644 (file)
--- a/src/pdf/SkPDFImage.h
+++ b/src/pdf/SkPDFImage.h
@@ -49,7 +49,8 @@ public:
     SkPDFImage* addSMask(SkPDFImage* mask);
 
     // The SkPDFObject interface.
-    virtual void getResources(SkTDArray<SkPDFObject*>* resourceList);
+    virtual void getResources(const SkTSet<SkPDFObject*>& knownResourceObjects,
+                              SkTSet<SkPDFObject*>* newResourceObjects);
 
 private:
     SkTDArray<SkPDFObject*> fResources;

diff --git a/src/pdf/SkPDFPage.cpp b/src/pdf/SkPDFPage.cpp
index f47f8ff..cad1099 100644 (file)
--- a/src/pdf/SkPDFPage.cpp
+++ b/src/pdf/SkPDFPage.cpp
@@ -21,7 +21,8 @@ SkPDFPage::SkPDFPage(SkPDFDevice* content)
 SkPDFPage::~SkPDFPage() {}
 
 void SkPDFPage::finalizePage(SkPDFCatalog* catalog, bool firstPage,
-                             SkTDArray<SkPDFObject*>* resourceObjects) {
+                             const SkTSet<SkPDFObject*>& knownResourceObjects,
+                             SkTSet<SkPDFObject*>* newResourceObjects) {
     if (fContentStream.get() == NULL) {
         insert("Resources", fDevice->getResourceDict());
         SkSafeUnref(this->insert("MediaBox", fDevice->copyMediaBox()));
@@ -38,7 +39,7 @@ void SkPDFPage::finalizePage(SkPDFCatalog* catalog, bool firstPage,
         insert("Contents", new SkPDFObjRef(fContentStream.get()))->unref();
     }
     catalog->addObject(fContentStream.get(), firstPage);
-    fDevice->getResources(resourceObjects, true);
+    fDevice->getResources(knownResourceObjects, newResourceObjects, true);
 }
 
 off_t SkPDFPage::getPageSize(SkPDFCatalog* catalog, off_t fileOffset) {

diff --git a/src/pdf/SkPDFPage.h b/src/pdf/SkPDFPage.h
index 72ba335..17fd437 100644 (file)
--- a/src/pdf/SkPDFPage.h
+++ b/src/pdf/SkPDFPage.h
@@ -40,13 +40,15 @@ public:
      *  that the page is part of.
      *  @param catalog         The catalog to add page content objects to.
      *  @param firstPage       Indicate if this is the first page of a document.
-     *  @param resourceObjects All the resource objects (recursively) used on
+     *  @param newResourceObjects All the resource objects (recursively) used on
      *                         the page are added to this array.  This gives
      *                         the caller a chance to deduplicate resources
      *                         across pages.
+     *  @param knownResourceObjects  The set of resources to be ignored.
      */
     void finalizePage(SkPDFCatalog* catalog, bool firstPage,
-                      SkTDArray<SkPDFObject*>* resourceObjects);
+                      const SkTSet<SkPDFObject*>& knownResourceObjects,
+                      SkTSet<SkPDFObject*>* newResourceObjects);
 
     /** Determine the size of the page content and store to the catalog
      *  the offsets of all nonresource-indirect objects that make up the page

diff --git a/src/pdf/SkPDFShader.cpp b/src/pdf/SkPDFShader.cpp
index 7958de3..b3e57cb 100644 (file)
--- a/src/pdf/SkPDFShader.cpp
+++ b/src/pdf/SkPDFShader.cpp
@@ -425,8 +425,11 @@ public:
 
     virtual bool isValid() { return fResources.count() > 0; }
 
-    void getResources(SkTDArray<SkPDFObject*>* resourceList) {
-        GetResourcesHelper(&fResources, resourceList);
+    void getResources(const SkTSet<SkPDFObject*>& knownResourceObjects,
+                      SkTSet<SkPDFObject*>* newResourceObjects) {
+        GetResourcesHelper(&fResources,
+                           knownResourceObjects,
+                           newResourceObjects);
     }
 
 private:
@@ -448,12 +451,15 @@ public:
 
     virtual bool isValid() { return size() > 0; }
 
-    void getResources(SkTDArray<SkPDFObject*>* resourceList) {
-        GetResourcesHelper(&fResources, resourceList);
+    void getResources(const SkTSet<SkPDFObject*>& knownResourceObjects,
+                      SkTSet<SkPDFObject*>* newResourceObjects) {
+        GetResourcesHelper(&fResources.toArray(),
+                           knownResourceObjects,
+                           newResourceObjects);
     }
 
 private:
-    SkTDArray<SkPDFObject*> fResources;
+    SkTSet<SkPDFObject*> fResources;
     SkAutoTDelete<const SkPDFShader::State> fState;
 };
 
@@ -832,7 +838,7 @@ SkPDFImageShader::SkPDFImageShader(SkPDFShader::State* state) : fState(state) {
     // Put the canvas into the pattern stream (fContent).
     SkAutoTUnref<SkStream> content(pattern.content());
     setData(content.get());
-    pattern.getResources(&fResources, false);
+    pattern.getResources(fResources, &fResources, false);
 
     insertName("Type", "Pattern");
     insertInt("PatternType", 1);

diff --git a/src/pdf/SkPDFTypes.cpp b/src/pdf/SkPDFTypes.cpp
index 59250c8..6c7120b 100644 (file)
--- a/src/pdf/SkPDFTypes.cpp
+++ b/src/pdf/SkPDFTypes.cpp
@@ -41,7 +41,8 @@ size_t SkPDFObject::getOutputSize(SkPDFCatalog* catalog, bool indirect) {
     return buffer.getOffset();
 }
 
-void SkPDFObject::getResources(SkTDArray<SkPDFObject*>* resourceList) {}
+void SkPDFObject::getResources(const SkTSet<SkPDFObject*>& knownResourceObjects,
+                               SkTSet<SkPDFObject*>* newResourceObjects) {}
 
 void SkPDFObject::emitIndirectObject(SkWStream* stream, SkPDFCatalog* catalog) {
     catalog->emitObjectNumber(stream, this);
@@ -61,14 +62,21 @@ void SkPDFObject::AddResourceHelper(SkPDFObject* resource,
     resource->ref();
 }
 
-void SkPDFObject::GetResourcesHelper(SkTDArray<SkPDFObject*>* resources,
-                                     SkTDArray<SkPDFObject*>* result) {
+void SkPDFObject::GetResourcesHelper(
+        const SkTDArray<SkPDFObject*>* resources,
+        const SkTSet<SkPDFObject*>& knownResourceObjects,
+        SkTSet<SkPDFObject*>* newResourceObjects) {
     if (resources->count()) {
-        result->setReserve(result->count() + resources->count());
+        newResourceObjects->setReserve(
+            newResourceObjects->count() + resources->count());
         for (int i = 0; i < resources->count(); i++) {
-            result->push((*resources)[i]);
-            (*resources)[i]->ref();
-            (*resources)[i]->getResources(result);
+            if (!knownResourceObjects.contains((*resources)[i]) &&
+                    !newResourceObjects->contains((*resources)[i])) {
+                newResourceObjects->add((*resources)[i]);
+                (*resources)[i]->ref();
+                (*resources)[i]->getResources(knownResourceObjects,
+                                              newResourceObjects);
+            }
         }
     }
 }

diff --git a/src/pdf/SkPDFTypes.h b/src/pdf/SkPDFTypes.h
index 03799d0..49fbbde 100644 (file)
--- a/src/pdf/SkPDFTypes.h
+++ b/src/pdf/SkPDFTypes.h
@@ -14,6 +14,7 @@
 #include "SkScalar.h"
 #include "SkString.h"
 #include "SkTDArray.h"
+#include "SkTSet.h"
 #include "SkTypes.h"
 
 class SkPDFCatalog;
@@ -38,13 +39,16 @@ public:
     virtual size_t getOutputSize(SkPDFCatalog* catalog, bool indirect);
 
     /** For non-primitive objects (i.e. objects defined outside this file),
-     *  this method will add to resourceList any objects that this method
-     *  depends on.  This operates recursively so if this object depends on
-     *  another object and that object depends on two more, all three objects
-     *  will be added.
-     *  @param resourceList  The list to append dependant resources to.
+     *  this method will add to newResourceObjects any objects that this method
+     *  depends on, but not already in knownResourceObjects. This operates
+     *  recursively so if this object depends on another object and that object
+     *  depends on two more, all three objects will be added.
+     *
+     *  @param knownResourceObjects  The set of resources to be ignored.
+     *  @param newResourceObjects  The set to append dependant resources to.
      */
-    virtual void getResources(SkTDArray<SkPDFObject*>* resourceList);
+    virtual void getResources(const SkTSet<SkPDFObject*>& knownResourceObjects,
+                              SkTSet<SkPDFObject*>* newResourceObjects);
 
     /** Emit this object unless the catalog has a substitute object, in which
      *  case emit that.
@@ -74,10 +78,16 @@ public:
     /** Static helper function to copy and reference the resources (and all
      *   their subresources) into a new list.
      * @param resources The resource list.
-     * @param result    The list to add to.
-     */
-    static void GetResourcesHelper(SkTDArray<SkPDFObject*>* resources,
-                                   SkTDArray<SkPDFObject*>* result);
+     * @param newResourceObjects All the resource objects (recursively) used on
+     *                         the page are added to this array.  This gives
+     *                         the caller a chance to deduplicate resources
+     *                         across pages.
+     * @param knownResourceObjects  The set of resources to be ignored.
+     */
+    static void GetResourcesHelper(
+            const SkTDArray<SkPDFObject*>* resources,
+            const SkTSet<SkPDFObject*>& knownResourceObjects,
+            SkTSet<SkPDFObject*>* newResourceObjects);
 
 protected:
     /** Subclasses must implement this method to print the object to the

diff --git a/tests/PDFPrimitivesTest.cpp b/tests/PDFPrimitivesTest.cpp
index 6e7d616..8041a8f 100644 (file)
--- a/tests/PDFPrimitivesTest.cpp
+++ b/tests/PDFPrimitivesTest.cpp
@@ -21,10 +21,10 @@
 
 class SkPDFTestDict : public SkPDFDict {
 public:
-    void getResources(SkTDArray<SkPDFObject*>* resourceList) {
-        resourceList->setReserve(resourceList->count() + fResources.count());
+  virtual void getResources(const SkTSet<SkPDFObject*>& knownResourceObjects,
+                            SkTSet<SkPDFObject*>* newResourceObjects) {
         for (int i = 0; i < fResources.count(); i++) {
-            resourceList->push(fResources[i]);
+            newResourceObjects->add(fResources[i]);
             fResources[i]->ref();
         }
     }