Simplify reference management in SkPDF

src/pdf/SkPDFTypes.h

 
 /*
  * Copyright 2010 The Android Open Source Project
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 
 #ifndef SkPDFTypes_DEFINED
 #define SkPDFTypes_DEFINED
 
 #include "SkRefCnt.h"
 #include "SkScalar.h"
 #include "SkString.h"
 #include "SkTDArray.h"
 #include "SkTSet.h"
 #include "SkTypes.h"
 
 class SkPDFCatalog;
 class SkWStream;
 
+class SkPDFObject;
+typedef SkTSet<SkPDFObject*> SkPDFObjectSet;

[mtklein, 2015/01/26 23:38:59]

I generally prefer writing out the full names of these things instead of
typedefing, unless you're leaving an option to easily swap between two
implementations or something.

I'm also pretty skeptical of SkTSet in general, but I see that really isn't the
point here.   It's not clear that we need to maintain sorted and insertion
order.  Might be something to look to simplify later. 

[hal.canary, 2015/02/09 23:35:01]

I'm not sure that I need SkTSet.  I really need an abstract interface:

    {
      bool add(SkPDFObject*); // return false if already known
    }

for addResources(), qhich I will add to SkPDFCatalog in a later CL.

Then SkTSet<SkPDFObject*> (or something like it) can be an implementation
detail of SkPDFCatalog.

Or maybe I'll just implement Map<SkPDFbject*, ObjectIndex> in a
later CL.

+
 /** \class SkPDFObject
 
     A PDF Object is the base class for primitive elements in a PDF file.  A
     common subtype is used to ease the use of indirect object references,
     which are common in the PDF format.
 */
 class SkPDFObject : public SkRefCnt {
 public:
     SK_DECLARE_INST_COUNT(SkPDFObject)
 
     /** Subclasses must implement this method to print the object to the
      *  PDF file.
      *  @param catalog  The object catalog to use.
      *  @param stream   The writable output stream to send the output to.
      */
     virtual void emitObject(SkWStream* stream, SkPDFCatalog* catalog) = 0;
 
-    /** For non-primitive objects (i.e. objects defined outside this file),
-     *  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.
+    /**
+     *  An object's has a resource if it depends, via an indirect
+     *  reference on another object.  This method recursively adds
+     *  resouces for this object and for its resouces' resouces....

[mtklein, 2015/01/26 23:38:59]

This first sentence doesn't really make sense.  Is it

/** Adds all transitive dependencies of this object to resourceSet. */

?

It might make sense to mention why we pass catalog.  Can catalog be const*, or
const&?

[hal.canary, 2015/02/09 23:35:01]

Done.

      *
-     *  @param knownResourceObjects  The set of resources to be ignored.
-     *  @param newResourceObjects  The set to append dependant resources to.
+     *  Ther default implementation does nothing and is therefore
+     *  suitable for "leaf" objects.

[mtklein, 2015/01/26 23:38:59]

Instead of writing this note, you could just put the implementation here, {}.

[hal.canary, 2015/02/09 23:35:01]

You shouldn't put virtuals in a header file.

[mtklein, 2015/02/09 23:47:17]

Why not?

      */
-    virtual void getResources(const SkTSet<SkPDFObject*>& knownResourceObjects,
-                              SkTSet<SkPDFObject*>* newResourceObjects);
-
-    /** Static helper function to add a resource to a list.  The list takes
-     *  a reference.
-     * @param resource  The resource to add.
-     * @param list      The list to add the resource to.
-     */
-    static void AddResourceHelper(SkPDFObject* resource,
-                                  SkTDArray<SkPDFObject*>* list);
-
-    /** Static helper function to copy and reference the resources (and all
-     *   their subresources) into a new list.
-     * @param resources The resource list.
-     * @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);
+    virtual void addResources(SkPDFObjectSet* resourceSet,

[mtklein, 2015/01/26 23:38:59]

Can this method be const ?

+                              SkPDFCatalog* catalog);
 
 private:
     typedef SkRefCnt INHERITED;
 };
 
 /** \class SkPDFObjRef
 
     An indirect reference to a PDF object.
 */
 class SkPDFObjRef : public SkPDFObject {
 public:
     SK_DECLARE_INST_COUNT(SkPDFObjRef)
 
     /** Create a reference to an existing SkPDFObject.
      *  @param obj The object to reference.
      */
     explicit SkPDFObjRef(SkPDFObject* obj);
     virtual ~SkPDFObjRef();
 
     // The SkPDFObject interface.
     virtual void emitObject(SkWStream* stream, SkPDFCatalog* catalog) SK_OVERRIDE;
+    virtual void addResources(SkPDFObjectSet*, SkPDFCatalog*) SK_OVERRIDE;
 
 private:
     SkAutoTUnref<SkPDFObject> fObj;
 
     typedef SkPDFObject INHERITED;
 };
 
 /** \class SkPDFInt
 
     An integer object in a PDF.
@@ skipping 143 matching lines @@
 public:
     SK_DECLARE_INST_COUNT(SkPDFArray)
 
     /** Create a PDF array. Maximum length is 8191.
      */
     SkPDFArray();
     virtual ~SkPDFArray();
 
     // The SkPDFObject interface.
     virtual void emitObject(SkWStream* stream, SkPDFCatalog* catalog) SK_OVERRIDE;
+    virtual void addResources(SkPDFObjectSet*, SkPDFCatalog*) SK_OVERRIDE;
 
     /** The size of the array.
      */
     int size() { return fValue.count(); }
 
     /** Preallocate space for the given number of entries.
      *  @param length The number of array slots to preallocate.
      */
     void reserve(int length);
 
@@ skipping 51 matching lines @@
 
     /** Create a PDF dictionary with a Type entry.
      *  @param type   The value of the Type entry.
      */
     explicit SkPDFDict(const char type[]);
 
     virtual ~SkPDFDict();
 
     // The SkPDFObject interface.
     virtual void emitObject(SkWStream* stream, SkPDFCatalog* catalog) SK_OVERRIDE;
+    virtual void addResources(SkPDFObjectSet*, SkPDFCatalog*) SK_OVERRIDE;
 
     /** The size of the dictionary.
      */
     int size() const;
 
     /** Add the value to the dictionary with the given key.  Refs value.
      *  @param key   The key for this dictionary entry.
      *  @param value The value for this dictionary entry.
      *  @return The value argument is returned.
      */
@@ skipping 66 matching lines @@
 
     mutable SkMutex fMutex;  // protects modifications to fValue
     SkTDArray<struct Rec> fValue;
 
     SkPDFObject* append(SkPDFName* key, SkPDFObject* value);
 
     typedef SkPDFObject INHERITED;
 };
 
 #endif