SkPDFCanon

src/pdf/SkPDFCanon.h

+/*
+ * Copyright 2015 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+#ifndef SkPDFCanon_DEFINED
+#define SkPDFCanon_DEFINED
+
+#include "SkThread.h"
+#include "SkTDArray.h"
+
+class SkBaseMutex;
+class SkIRect;
+class SkMatrix;
+class SkPDFFont;
+class SkPDFGraphicState;
+class SkPDFShader;
+class SkPDFShaderState;
+class SkPaint;
+class SkShader;
+class SkTypeface;
+
+// This class's fields and methods will eventually become part of
+// SkPDFDocument/SkDocument_PDF.  For now, it exists as a singleton to
+// preflight that transition.  This replaces three global arrays in
+// SkPDFFont, SkPDFShader, and SkPDFGraphicsContext.
+//
+// Note that this class does not create, delete, reference or
+// dereference the SkPDFObject objects that it indexes.  It is up to
+// the caller to manage the lifetime of these objects.
+class SkPDFCanon {
+public:
+    static SkPDFCanon& GetCanon();
+
+    // This mutexes will be removed once this class is subsumed into
+    // SkPDFDocument.

[mtklein, 2015/01/20 21:59:51]

Let's be louder about how much of a minefield this is?

// IF YOU ARE LOOKING AT THIS API PLEASE DO NOT WRITE THE CHANGE 
// YOU ARE ABOUT TO WRITE WITHOUT TALKING TO HAL.

[hal.canary, 2015/01/21 17:07:50]

Done.

+    static SkBaseMutex& GetFontMutex();
+    static SkBaseMutex& GetShaderMutex();
+    static SkBaseMutex& GetPaintMutex();
+
+    SkPDFFont* findFont(uint32_t fontID,

[mtklein, 2015/01/20 21:59:51]

Add a brief comment for this?  The others look straightforward enough.

// Returns exact match if there is one.  If not, it returns NULL.
// If there is no exact match, but there is a related font, we still return
NULL, but also set *relatedFont.

[hal.canary, 2015/01/21 17:07:50]

Done.

+                        uint16_t glyphID,
+                        SkPDFFont** relatedFont) const;
+    void addFont(SkPDFFont* font, uint32_t fontID, uint16_t fGlyphID);
+    void removeFont(SkPDFFont*);
+
+    SkPDFShader* findShader(const SkPDFShaderState&) const;
+    void addShader(SkPDFShader*);
+    void removeShader(SkPDFShader*);
+
+    SkPDFGraphicState* findGraphicState(const SkPaint&) const;
+    void addGraphicState(SkPDFGraphicState*);
+    void removeGraphicState(SkPDFGraphicState*);
+
+private:
+    struct FontRec {
+        SkPDFFont* fFont;
+        uint32_t fFontID;
+        uint16_t fGlyphID;
+    };
+    SkTDArray<FontRec> fFontRecords;
+
+    SkTDArray<SkPDFShader*> fShaderRecords;
+
+    SkTDArray<SkPDFGraphicState*> fGraphicStateRecords;
+
+    SkBaseMutex* fFontMutex;

[djsollen, 2015/01/21 14:10:27]

why pointers to global variables? if the cannon is a singleton we should use the
standard SkMutex as a member variable on this object.

[mtklein, 2015/01/21 15:14:40]

I missed this the first time around.  I too don't get why we need the extra
indirection.

[hal.canary, 2015/01/21 17:07:50]

Acknowledged.

[hal.canary, 2015/01/21 17:07:50]

I forgot to delete those.

+    SkBaseMutex* fShaderMutex;
+    SkBaseMutex* fPaintMutex;
+
+    SkPDFCanon();
+    ~SkPDFCanon();
+    static SkPDFCanon* Create();

[djsollen, 2015/01/21 14:10:27]

Any reason to expose the Create/Destroy functions in the .h file? They can just
be declared as static in the cpp file.

[mtklein, 2015/01/21 15:14:40]

I believe this allows the constructor and destructor to be private, while
providing hooks for an SK_DECLARE_STATIC_LAZY_PTR.

[hal.canary, 2015/01/21 17:07:50]

Acknowledged.

[hal.canary, 2015/01/21 17:07:50]

I needed to because I made the constructor private.  But there's no good reason
for that, since in a later CL, SkPDFDocument will have a SkCanon as a member.

+    static void Destroy(SkPDFCanon*);
+};
+#endif  // SkPDFCanon_DEFINED