Step towards variable length effect keys.

include/gpu/GrBackendEffectFactory.h

Index: include/gpu/GrBackendEffectFactory.h
diff --git a/include/gpu/GrBackendEffectFactory.h b/include/gpu/GrBackendEffectFactory.h
index af8d5c76c577b9bfb3034c602ab82bf2e329e160..4933ff77ce0a676aab39eeea79b42d174e6766d9 100644
--- a/include/gpu/GrBackendEffectFactory.h
+++ b/include/gpu/GrBackendEffectFactory.h
@@ -12,6 +12,7 @@
 #include "SkTemplates.h"
 #include "SkThread.h"
 #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
@@ -27,23 +28,28 @@ class GrGLEffect;
 class GrGLCaps;
 class GrDrawEffect;
 

[robertphillips, 2014/07/10 12:15:50]

// The EffectKeyBuilder class helps assemble a key. It takes a pointer to the
desired destination but doesn't assume ownership of it ?

[bsalomon, 2014/07/10 17:24:34]

I didn't add the bit about ownership to the top doc since it seems more targeted
at effect writers. But I did add a comment on the member var about ownership.

+class EffectKeyBuilder {
+public:
+    EffectKeyBuilder(SkTArray<unsigned char, true>* data) : fData(data), fCount(0) {
+        SkASSERT(0 == fData->count() % sizeof(uint32_t));
+    }
+
+    void add32(uint32_t v) {
+        ++fCount;
+        fData->push_back_n(4, reinterpret_cast<uint8_t*>(&v));
+    }
+
+    size_t size() const { return sizeof(uint32_t) * fCount; }
+private:
+    SkTArray<uint8_t, true>* fData;

[robertphillips, 2014/07/10 12:15:50]

Couldn't we replace fCount with (fData->count()/4) ?

+    int fCount;

[bsalomon, 2014/07/10 17:24:34]

No, it is the count of uint32_ts added since this was created. fData's count
doesn't start at 0 when this object is created.

[robertphillips, 2014/07/10 18:13:17]

That probably needs a comment

+};
+
 class GrBackendEffectFactory : SkNoncopyable {
 public:
     typedef uint32_t EffectKey;
-    enum {
-        kNoEffectKey = 0,
-        kEffectKeyBits = 10,
-        /**
-         * The framework automatically includes coord transforms and texture accesses in their
-         * effect's EffectKey, so effects don't need to account for them in GenKey().
-         */
-        kTextureKeyBits = 4,
-        kTransformKeyBits = 6,
-        kAttribKeyBits = 6,
-        kClassIDBits = 6
-    };
 

[robertphillips, 2014/07/10 12:15:50]

// Return true ... ?

[bsalomon, 2014/07/10 17:24:34]

The subclass GrTBackendEffectFactory has a comment. Added some language about
failure.

-    virtual EffectKey glEffectKey(const GrDrawEffect&, const GrGLCaps&) const = 0;
+    virtual bool getGLEffectKey(const GrDrawEffect&, const GrGLCaps&, EffectKeyBuilder*) const = 0;
     virtual GrGLEffect* createGLInstance(const GrDrawEffect&) const = 0;
 
     bool operator ==(const GrBackendEffectFactory& b) const {
@@ -54,11 +60,7 @@ public:
     }
 
     virtual const char* name() const = 0;

[robertphillips, 2014/07/10 12:15:50]

Remove this '\' ?

[bsalomon, 2014/07/10 17:24:34]

Done.

-
-    static EffectKey GetTransformKey(EffectKey key) {
-        return key >> (kEffectKeyBits + kTextureKeyBits) & ((1U << kTransformKeyBits) - 1);
-    }
-
+\
 protected:
     enum {
         kIllegalEffectClassID = 0,
@@ -69,18 +71,15 @@ protected:
     }
     virtual ~GrBackendEffectFactory() {}
 
-    static EffectKey GenID() {
-        SkDEBUGCODE(static const int32_t kClassIDBits = 8 * sizeof(EffectKey) -
-                           kTextureKeyBits - kEffectKeyBits - kAttribKeyBits);
+    static int32_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;
-        SkASSERT(id < (1 << kClassIDBits));
-        return static_cast<EffectKey>(id);
+        return id;
     }
 
-    EffectKey fEffectClassID;
+    int32_t fEffectClassID;
 
 private:
     static int32_t fCurrEffectClassID;