Add SkPDFResourceDict class, refactor existing code

src/pdf/SkPDFDevice.cpp

Index: src/pdf/SkPDFDevice.cpp
diff --git a/src/pdf/SkPDFDevice.cpp b/src/pdf/SkPDFDevice.cpp
index db0825af3a9f39d19dcc2a009690484cd32e8137..1592e40551fb7159220dfa533ae47e859cc92485 100644
--- a/src/pdf/SkPDFDevice.cpp
+++ b/src/pdf/SkPDFDevice.cpp
@@ -20,6 +20,7 @@
 #include "SkPDFFormXObject.h"
 #include "SkPDFGraphicState.h"
 #include "SkPDFImage.h"
+#include "SkPDFResourceDict.h"
 #include "SkPDFShader.h"
 #include "SkPDFStream.h"
 #include "SkPDFTypes.h"
@@ -435,10 +436,14 @@ void GraphicStackState::updateDrawingState(const GraphicStateEntry& state) {
     // PDF treats a shader as a color, so we only set one or the other.
     if (state.fShaderIndex >= 0) {
         if (state.fShaderIndex != currentEntry()->fShaderIndex) {
-            fContentStream->writeText("/Pattern CS /Pattern cs /P");
-            fContentStream->writeDecAsText(state.fShaderIndex);
-            fContentStream->writeText(" SCN /P");
-            fContentStream->writeDecAsText(state.fShaderIndex);
+            fContentStream->writeText("/Pattern CS /Pattern cs /");
+            fContentStream->writeText(SkPDFResourceDict::getResourceName(

[vandebo (ex-Chrome), 2013/07/12 21:58:58]

Since this is called twice with the same index, you probably want to store the
result and use it twice.

[ducky, 2013/07/12 23:49:32]

Done.

+                    SkPDFResourceDict::kPattern_ResourceType,
+                    state.fShaderIndex).c_str());
+            fContentStream->writeText(" SCN /");
+            fContentStream->writeText(SkPDFResourceDict::getResourceName(
+                    SkPDFResourceDict::kPattern_ResourceType,
+                    state.fShaderIndex).c_str());
             fContentStream->writeText(" scn\n");
             currentEntry()->fShaderIndex = state.fShaderIndex;
         }
@@ -1134,123 +1139,46 @@ void SkPDFDevice::setDrawingArea(DrawingArea drawingArea) {
     fDrawingArea = drawingArea;
 }
 
-SkPDFDict* SkPDFDevice::getResourceDict() {
+SkPDFResourceDict* SkPDFDevice::getResourceDict() {
     if (NULL == fResourceDict) {
-        fResourceDict = SkNEW(SkPDFDict);
+        fResourceDict = SkNEW(SkPDFResourceDict);
 
         if (fGraphicStateResources.count()) {
-            SkAutoTUnref<SkPDFDict> extGState(new SkPDFDict());
             for (int i = 0; i < fGraphicStateResources.count(); i++) {
-                SkString nameString("G");
-                nameString.appendS32(i);
-                extGState->insert(
-                        nameString.c_str(),
-                        new SkPDFObjRef(fGraphicStateResources[i]))->unref();
+                fResourceDict->insertResourceAsRef(
+                        SkPDFResourceDict::kExtGState_ResourceType,
+                        i, fGraphicStateResources[i]);
             }
-            fResourceDict->insert("ExtGState", extGState.get());
         }
 
         if (fXObjectResources.count()) {
-            SkAutoTUnref<SkPDFDict> xObjects(new SkPDFDict());
             for (int i = 0; i < fXObjectResources.count(); i++) {
-                SkString nameString("X");
-                nameString.appendS32(i);
-                xObjects->insert(
-                        nameString.c_str(),
-                        new SkPDFObjRef(fXObjectResources[i]))->unref();
+                fResourceDict->insertResourceAsRef(
+                        SkPDFResourceDict::kXObject_ResourceType,
+                        i, fXObjectResources[i]);
             }
-            fResourceDict->insert("XObject", xObjects.get());
         }
 
         if (fFontResources.count()) {
-            SkAutoTUnref<SkPDFDict> fonts(new SkPDFDict());
             for (int i = 0; i < fFontResources.count(); i++) {
-                SkString nameString("F");
-                nameString.appendS32(i);
-                fonts->insert(nameString.c_str(),
-                              new SkPDFObjRef(fFontResources[i]))->unref();
+                fResourceDict->insertResourceAsRef(
+                        SkPDFResourceDict::kFont_ResourceType,
+                        i, fFontResources[i]);
             }
-            fResourceDict->insert("Font", fonts.get());
         }
 
         if (fShaderResources.count()) {
             SkAutoTUnref<SkPDFDict> patterns(new SkPDFDict());
             for (int i = 0; i < fShaderResources.count(); i++) {
-                SkString nameString("P");
-                nameString.appendS32(i);
-                patterns->insert(nameString.c_str(),
-                                 new SkPDFObjRef(fShaderResources[i]))->unref();
+                fResourceDict->insertResourceAsRef(
+                        SkPDFResourceDict::kPattern_ResourceType,
+                        i, fShaderResources[i]);
             }
-            fResourceDict->insert("Pattern", patterns.get());
         }
-
-        // For compatibility, add all proc sets (only used for output to PS
-        // devices).
-        const char procs[][7] = {"PDF", "Text", "ImageB", "ImageC", "ImageI"};
-        SkAutoTUnref<SkPDFArray> procSets(new SkPDFArray());
-        procSets->reserve(SK_ARRAY_COUNT(procs));
-        for (size_t i = 0; i < SK_ARRAY_COUNT(procs); i++)
-            procSets->appendName(procs[i]);
-        fResourceDict->insert("ProcSet", procSets.get());
     }
     return fResourceDict;
 }
 
-void SkPDFDevice::getResources(const SkTSet<SkPDFObject*>& knownResourceObjects,
-                               SkTSet<SkPDFObject*>* newResourceObjects,
-                               bool recursive) const {
-    // 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++) {
-        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++) {
-        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++) {
-        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++) {
-        if (!knownResourceObjects.contains(fShaderResources[i]) &&
-                !newResourceObjects->contains(fShaderResources[i])) {
-            newResourceObjects->add(fShaderResources[i]);
-            fShaderResources[i]->ref();
-            if (recursive) {
-                fShaderResources[i]->getResources(knownResourceObjects,
-                                                  newResourceObjects);
-            }
-        }
-    }
-}
-
 const SkTDArray<SkPDFFont*>& SkPDFDevice::getFontResources() const {
     return fFontResources;
 }
@@ -1782,8 +1710,10 @@ void SkPDFDevice::updateFont(const SkPaint& paint, uint16_t glyphID,
             contentEntry->fState.fTextSize != paint.getTextSize() ||
             !contentEntry->fState.fFont->hasGlyph(glyphID)) {
         int fontIndex = getFontResourceIndex(typeface, glyphID);
-        contentEntry->fContent.writeText("/F");
-        contentEntry->fContent.writeDecAsText(fontIndex);
+        contentEntry->fContent.writeText("/");
+        contentEntry->fContent.writeText(SkPDFResourceDict::getResourceName(

[vandebo (ex-Chrome), 2013/07/12 21:58:58]

This statement looks kind of ugly.  Do you think it would look nicer if the
function appended directly to the stream?

[ducky, 2013/07/12 23:49:32]

I think it's going to look ugly either way, because you need the two
SkPDFResourceDict:: qualifiers and each takes up a huge chunk of space. I like
how right now it's at least contentEntry->fContent.writeText all the way down,
so it's consistent.
Adding a stream method also seems to be a bit specialized, even though it may
remove a few characters of code and would be heavily used.

It might make sense to allow a SkWStream to write a SkString directly (instead
of needing a c_str() in the caller), but it's probably only syntactic sugar. And
my last CL messing with core classes didn't go over so well either...

In this function on line 1718 (new), it also might make more sense to do
contentEntry->fContent.writeScalarAsText, rather than SkPDFScalar::Append. Do
you know if there is any difference between the two, and why this version was
preferred?

+                SkPDFResourceDict::kFont_ResourceType,
+                fontIndex).c_str());
         contentEntry->fContent.writeText(" ");
         SkPDFScalar::Append(paint.getTextSize(), &contentEntry->fContent);
         contentEntry->fContent.writeText(" Tf\n");