resubmit https://code.google.com/p/skia/source/detail?r=7883 (in the meantime we

added capability to collect minidump and callstack if buildbot fails with heap
coruption in windows, and a NPE bug was fixed in SkPDFDocument, when document was destroyed without ever beeing used and a field was NULL + a few minor conflicts have been resolved)

git-svn-id: http://skia.googlecode.com/svn/trunk@8487 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 90379f9..f8261b5 100644 (file)
--- a/include/pdf/SkPDFDevice.h
+++ b/include/pdf/SkPDFDevice.h
@@ -30,6 +30,7 @@ class SkPDFGraphicState;
 class SkPDFObject;
 class SkPDFShader;
 class SkPDFStream;
+template <typename T> class SK_API SkTSet;
 
 // Private classes.
 struct ContentEntry;
@@ -132,12 +133,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 cb786c7..8f4ee48 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
 
@@ -79,7 +80,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 49c0404..8690b3e 100644 (file)
--- a/src/pdf/SkPDFCatalog.cpp
+++ b/src/pdf/SkPDFCatalog.cpp
@@ -171,12 +171,16 @@ 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 number of duplicates.
+    // If there are duplicates, there is a bug and we mess ref counting.
+    SkDEBUGCODE(int duplicates =) targetSet->mergeInto(newResourceObjects);
+    SkASSERT(duplicates == 0);
 }
 
 SkPDFObject* SkPDFCatalog::getSubstituteObject(SkPDFObject* object) {
@@ -190,22 +194,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 6865b0f..6298f7b 100644 (file)
--- a/src/pdf/SkPDFDevice.cpp
+++ b/src/pdf/SkPDFDevice.cpp
@@ -29,6 +29,7 @@
 #include "SkTextFormatParams.h"
 #include "SkTemplates.h"
 #include "SkTypefacePriv.h"
+#include "SkTSet.h"
 
 // Utility functions
 
@@ -1191,39 +1192,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 7ee2778..062263d 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,20 @@ SkPDFDocument::~SkPDFDocument() {
         fPageTree[i]->clear();
     }
     fPageTree.safeUnrefAll();
-    fPageResources.safeUnrefAll();
+
+    if (fFirstPageResources) {
+        fFirstPageResources->safeUnrefAll();
+    }
+    if (fOtherPageResources) {
+        fOtherPageResources->safeUnrefAll();
+    }
+
     fSubstitutes.safeUnrefAll();
 
     fDocCatalog->unref();
     SkSafeUnref(fTrailerDict);
+    SkDELETE(fFirstPageResources);
+    SkDELETE(fOtherPageResources);
 }
 
 bool SkPDFDocument::emitPDF(SkWStream* stream) {
@@ -90,6 +92,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;
@@ -109,19 +114,41 @@ bool SkPDFDocument::emitPDF(SkWStream* stream) {
 
         SkPDFDict* dests = SkNEW(SkPDFDict);  // fPageResources owns reference
         fCatalog->addObject(dests, true /* onFirstPage */);
-        fPageResources.push(dests);
+        fFirstPageResources->add(dests);
 
         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.
+        SkDEBUGCODE(int duplicates =) knownResources.mergeInto(*fFirstPageResources);
+        SkASSERT(duplicates == 0);
+
         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());
-            fPages[i]->appendDestinations(dests);
-            if (i == 0) {
+            if (i == 1) {
                 firstPage = false;
-                fSecondPageFirstResourceIndex = fPageResources.count();
+                SkDEBUGCODE(duplicates =) knownResources.mergeInto(*fOtherPageResources);
+            }
+            SkTSet<SkPDFObject*> newResources;
+            fPages[i]->finalizePage(
+                fCatalog.get(), firstPage, knownResources, &newResources);
+            addResourcesToCatalog(firstPage, &newResources, fCatalog.get());
+            if (firstPage) {
+                SkDEBUGCODE(duplicates =) fFirstPageResources->mergeInto(newResources);
+            } else {
+                SkDEBUGCODE(duplicates =) fOtherPageResources->mergeInto(newResources);
             }
+            SkASSERT(duplicates == 0);
+
+            SkDEBUGCODE(duplicates =) knownResources.mergeInto(newResources);
+            SkASSERT(duplicates == 0);
+
+            fPages[i]->appendDestinations(dests);
         }
 
         fDocCatalog->insert("Dests", SkNEW_ARGS(SkPDFObjRef, (dests)))->unref();
@@ -135,8 +162,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.
@@ -154,11 +181,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,
@@ -170,8 +195,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
@@ -188,10 +213,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 34f2fee..883682f 100644 (file)
--- a/src/pdf/SkPDFFont.cpp
+++ b/src/pdf/SkPDFFont.cpp
@@ -722,8 +722,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 cda2b50..f5d358f 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 c11a9a6..884e6db 100644 (file)
--- a/src/pdf/SkPDFFormXObject.cpp
+++ b/src/pdf/SkPDFFormXObject.cpp
@@ -20,12 +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.
+    SkTSet<SkPDFObject*> emptySet;
+    device->getResources(emptySet, &fResources, false);
 
     SkAutoTUnref<SkStream> content(device->content());
     setData(content.get());
@@ -60,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 6ea535f..9cf2145 100644 (file)
--- a/src/pdf/SkPDFGraphicState.cpp
+++ b/src/pdf/SkPDFGraphicState.cpp
@@ -64,8 +64,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 3533a2c..f7889f1 100644 (file)
--- a/src/pdf/SkPDFImage.cpp
+++ b/src/pdf/SkPDFImage.cpp
@@ -284,8 +284,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 a4203f6..d41466f 100644 (file)
--- a/src/pdf/SkPDFImage.h
+++ b/src/pdf/SkPDFImage.h
@@ -47,7 +47,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 4cdc000..d940f41 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 285a2f5..2ce773c 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);
 
     /** Add destinations for this page to the supplied dictionary.
      *  @param dict       Dictionary to add destinations to.

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 d479a22..ed02d2b 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 98223ae..5ed6386 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 7f6bc34..3b54d17 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();
         }
     }