SKPDF: refactor pdfcatalog and pdfdocument

src/pdf/SkPDFCatalog.h

Index: src/pdf/SkPDFCatalog.h
diff --git a/src/pdf/SkPDFCatalog.h b/src/pdf/SkPDFCatalog.h
index 1bdac936d47c57cdf1112bfbcf22d447ba8c198c..d7f182a63c3ca25288e333c7de1dc5ec7047d7d0 100644
--- a/src/pdf/SkPDFCatalog.h
+++ b/src/pdf/SkPDFCatalog.h
@@ -10,10 +10,10 @@
 #ifndef SkPDFCatalog_DEFINED
 #define SkPDFCatalog_DEFINED
 
-#include <sys/types.h>
-
+#include "SkChecksum.h"

[mtklein, 2015/03/24 16:03:06]

probably unneeded?

[hal.canary, 2015/03/24 20:08:05]

moved to SkTHash where it belongs.

 #include "SkPDFTypes.h"
 #include "SkTDArray.h"
+#include "SkTHash.h"
 
 /** \class SkPDFCatalog
 
@@ -22,87 +22,50 @@
 */
 class SkPDFCatalog {
 public:
-    /** Create a PDF catalog.
-     */
-    SkPDFCatalog();
+    SkPDFCatalog() {}
     ~SkPDFCatalog();
 
-    /** Add the passed object to the catalog.  Refs obj.
-     *  @param obj         The object to add.
-     *  @param onFirstPage Is the object on the first page.
-     *  @return The obj argument is returned.
-     */
-    SkPDFObject* addObject(SkPDFObject* obj, bool onFirstPage);
-
-    /** Inform the catalog of the object's position in the final stream.
-     *  The object should already have been added to the catalog.
+    /** Add the passed object to the catalog.
      *  @param obj         The object to add.
-     *  @param offset      The byte offset in the output stream of this object.
+     *  @return True iff the object was not already added to the catalog.
      */
-    void setFileOffset(SkPDFObject* obj, off_t offset);
+    bool addObject(SkPDFObject* obj) {
+        if (fObjectNumbers.find(obj)) {

[mtklein, 2015/03/24 16:03:06]

These functions working with hashes are all short, but I'm not sure they're good
candidates for inlining.  find() and set() are rather non-trivial methods, and
as template methods, they'll inline through.  We probably want to keep those out
of line.

[hal.canary, 2015/03/24 20:08:05]

Done.

+            return false;
+        }
+        fObjectNumbers.set(obj, fObjectNumbers.count() + 1);
+        return true;
+    }
 
     /** Get the object number for the passed object.
      *  @param obj         The object of interest.
      */
-    int32_t getObjectNumber(SkPDFObject* obj);
-
-    /** Output the cross reference table for objects in the catalog.
-     *  Returns the total number of objects.
-     *  @param stream      The writable output stream to send the output to.
-     *  @param firstPage   If true, include first page objects only, otherwise
-     *                     include all objects not on the first page.
-     */
-    int32_t emitXrefTable(SkWStream* stream, bool firstPage);
+    int32_t getObjectNumber(SkPDFObject* obj) {

[mtklein, 2015/03/24 16:03:06]

Think it's worth merging addObject and getObjectNumber into 

int32_t findOrAdd(SkPDFObject* o) {
  if (int32_t* n = fObjectNumbers.find(o)) {
    return *n;
  }
  int32_t n = fObjectNumbers.count()+1;
  fObjectNumbers.set(o, n);
  return n;
}

?

[hal.canary, 2015/03/24 20:08:05]

No.  I will need to know if the object has been added to the catalog for the
next CL (which does away with SkTSet).

We need to get the object number at a different point.

+        int32_t* objectNumberFound = fObjectNumbers.find(obj);
+        SkASSERT(objectNumberFound);
+        return *objectNumberFound;
+    }
 
     /** Set substitute object for the passed object.
+        Refs substitute.
      */
-    void setSubstitute(SkPDFObject* original, SkPDFObject* substitute);
+    void setSubstitute(SkPDFObject* original, SkPDFObject* substitute) {
+        SkASSERT(original != substitute);
+        SkASSERT(!fSubstituteMap.find(original));
+        fSubstituteMap.set(original, SkRef(substitute));
+    }
 
     /** Find and return any substitute object set for the passed object. If
      *  there is none, return the passed object.
      */
-    SkPDFObject* getSubstituteObject(SkPDFObject* object);
+    SkPDFObject* getSubstituteObject(SkPDFObject* object) {
+        SkPDFObject** found = fSubstituteMap.find(object);
+        return found ? *found : object;
+    }
 
 private:
-    struct Rec {
-        Rec(SkPDFObject* object, bool onFirstPage)
-            : fObject(object),
-              fFileOffset(0),
-              fObjNumAssigned(false),
-              fOnFirstPage(onFirstPage) {
-        }
-        SkPDFObject* fObject;
-        off_t fFileOffset;
-        bool fObjNumAssigned;
-        bool fOnFirstPage;
-    };
-
-    struct SubstituteMapping {
-        SubstituteMapping(SkPDFObject* original, SkPDFObject* substitute)
-            : fOriginal(original), fSubstitute(substitute) {
-        }
-        SkPDFObject* fOriginal;
-        SkPDFObject* fSubstitute;
-    };
-
-    // TODO(vandebo): Make this a hash if it's a performance problem.
-    SkTDArray<Rec> fCatalog;
-
-    // TODO(arthurhsu): Make this a hash if it's a performance problem.
-    SkTDArray<SubstituteMapping> fSubstituteMap;
-    SkTSet<SkPDFObject*> fSubstituteResourcesFirstPage;
-    SkTSet<SkPDFObject*> fSubstituteResourcesRemaining;
-
-    // Number of objects on the first page.
-    uint32_t fFirstPageCount;
-    // Next object number to assign (on page > 1).
-    uint32_t fNextObjNum;
-    // Next object number to assign on the first page.
-    uint32_t fNextFirstPageObjNum;
-
-    int findObjectIndex(SkPDFObject* obj);
-
-    int assignObjNum(SkPDFObject* obj);
+    SkTHashMap<SkPDFObject*, int32_t> fObjectNumbers;
+    SkTHashMap<SkPDFObject*, SkPDFObject*> fSubstituteMap;
 };
 
 #endif