Step towards variable length effect keys.

include/gpu/GrBackendEffectFactory.h

 /*
  * Copyright 2012 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #ifndef GrBackendEffectFactory_DEFINED
 #define GrBackendEffectFactory_DEFINED
 
 #include "GrTypes.h"
 #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
     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;
 

[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 {
         return fEffectClassID == b.fEffectClassID;
     }
     bool operator !=(const GrBackendEffectFactory& b) const {
         return !(*this == b);
     }
 
     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,
     };
 
     GrBackendEffectFactory() {
         fEffectClassID = kIllegalEffectClassID;
     }
     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;
 };
 
 #endif