Makes GrGLProgramDesc's key store the lengths as well as offsets of the effect keys.

include/gpu/GrBackendEffectFactory.h

Index: include/gpu/GrBackendEffectFactory.h
diff --git a/include/gpu/GrBackendEffectFactory.h b/include/gpu/GrBackendEffectFactory.h
index 32f14f27460955a0f99939faaaef27a4d87ec9b3..791feab8e746cd82c51be91cfe281bdb92ce8ade 100644
--- a/include/gpu/GrBackendEffectFactory.h
+++ b/include/gpu/GrBackendEffectFactory.h
@@ -14,22 +14,12 @@
 #include "SkTypes.h"
 #include "SkTArray.h"
 
-/** Given a GrEffect of a particular type, creates the corresponding graphics-backend-specific
-    effect object. Also tracks equivalence of shaders generated via a key. Each factory instance
-    is assigned a generation ID at construction. The ID of the return of GrEffect::getFactory()
-    is used as a type identifier. Thus a GrEffect subclass must return a singleton from
-    getFactory(). GrEffect subclasses should use the derived class GrTBackendEffectFactory that is
-    templated on the GrEffect subclass as their factory object. It requires that the GrEffect
-    subclass has a nested class (or typedef) GLEffect which is its GL implementation and a subclass
-    of GrGLEffect.
- */
-
 class GrGLEffect;
 class GrGLCaps;
 class GrDrawEffect;
 
 /**
- * Used by effects to build their keys. It incorpates each per-effect key into a larger shader key.
+ * Used by effects to build their keys. It incorporates each per-effect key into a larger shader key.
  */
 class GrEffectKeyBuilder {
 public:
@@ -42,6 +32,14 @@ public:
         fData->push_back_n(4, reinterpret_cast<uint8_t*>(&v));
     }
 
+    /** Inserts count uint32_ts into the key. The returned pointer is only valid until the next
+        add*() call. */
+    uint32_t* SK_WARN_UNUSED_RESULT add32n(int count) {
+        SkASSERT(count > 0);
+        fCount += count;
+        return reinterpret_cast<uint32_t*>(fData->push_back_n(4 * count));
+    }
+
     size_t size() const { return sizeof(uint32_t) * fCount; }
 
 private:
@@ -49,43 +47,73 @@ private:
     int fCount;                     // number of uint32_ts added to fData by the effect.
 };
 
+/**
+ * Given a GrEffect of a particular type, creates the corresponding graphics-backend-specific
+ * effect object. Also tracks equivalence of shaders generated via a key. Each factory instance
+ * is assigned an ID at construction. The ID of the return of GrEffect::getFactory() is used as a
+ * type identifier. Thus, a GrEffect subclass must always return the same object from getFactory()
+ * and that factory object must be unique to the GrEffect subclass (and unique from any further
+ * derived subclasses).
+ *

[robertphillips, 2014/07/11 17:24:58]

This sentence seems a bit garbled

[bsalomon, 2014/07/11 17:52:56]

Done.

+ * It is recommended that GrEffect subclasses the derived class GrTBackendEffectFactory by
+ * writing their getFactory() method as:
+ *
+ * const GrBackendEffectFactory& MyEffect::getFactory() const {
+ *     return GrTBackendEffectFactory<MyEffect>::getInstance();
+ * }
+ *
+ * Using GrTBackendEffectFactory places a few constraints on the effect. See that class's comments.
+ */
 class GrBackendEffectFactory : SkNoncopyable {
 public:
     typedef uint32_t EffectKey;
 
-    virtual bool getGLEffectKey(const GrDrawEffect&, const GrGLCaps&, GrEffectKeyBuilder*) const = 0;
+    /** 
+     * Generates an effect's key. The key is based on the aspects of the GrEffect object's
+     * configuration that affect GLSL code generation. Two GrEffect instances that would cause
+     * this->createGLInstance()->emitCode() to produce different code must produce different keys.
+     */
+    virtual void getGLEffectKey(const GrDrawEffect&, const GrGLCaps&, GrEffectKeyBuilder*) const = 0;
+
+    /**
+     * Creates a GrGLEffect instance that is used both to generate code for the GrEffect in a GLSL
+     * program and to manage updating uniforms for the program when it is used.
+     */
     virtual GrGLEffect* createGLInstance(const GrDrawEffect&) const = 0;
 
-    bool operator ==(const GrBackendEffectFactory& b) const {
-        return fEffectClassID == b.fEffectClassID;
-    }
-    bool operator !=(const GrBackendEffectFactory& b) const {
-        return !(*this == b);
-    }
-
+    /**
+     * Produces a human-reable name for the effect.
+     */
     virtual const char* name() const = 0;
 
+    /**
+     * A unique value for every instance of this factory. It is automatically incorporated into the
+     * effect's key. This allows keys generated by getGLEffectKey() to only be unique within a
+     * GrEffect subclass and not necessarily across subclasses.
+     */
+    uint32_t effectClassID() const { return fEffectClassID; }
+
 protected:
+    GrBackendEffectFactory() : fEffectClassID(GenID()) {}
+    virtual ~GrBackendEffectFactory() {}
+
+private:
     enum {
         kIllegalEffectClassID = 0,
     };
 
-    GrBackendEffectFactory() {
-        fEffectClassID = kIllegalEffectClassID;
-    }
-    virtual ~GrBackendEffectFactory() {}
-
-    static int32_t GenID() {
+    static uint32_t GenID() {
         // fCurrEffectClassID has been initialized to kIllegalEffectClassID. The
         // atomic inc returns the old value not the incremented value. So we add
         // 1 to the returned value.
-        int32_t id = sk_atomic_inc(&fCurrEffectClassID) + 1;
+        uint32_t id = static_cast<uint32_t>(sk_atomic_inc(&fCurrEffectClassID)) + 1;

[robertphillips, 2014/07/11 17:24:58]

Can this even happen ?

[bsalomon, 2014/07/11 17:52:56]

After 4 billion calls, yeah :)

+        if (!id) {

[robertphillips, 2014/07/11 17:24:58]

This error message seems misleading

[bsalomon, 2014/07/11 17:52:56]

Done.

+            SkFAIL("This should only be called once per GrEffect subclass.");
+        }
         return id;
     }
 

[robertphillips, 2014/07/11 17:24:58]

const?

[bsalomon, 2014/07/11 17:52:56]

Done.

-    int32_t fEffectClassID;
-
-private:
+    uint32_t fEffectClassID;
     static int32_t fCurrEffectClassID;
 };