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

Makes it possible to use GrBackendEffectFactories other than GrTBEF by moving meta-key generation out of GrTBEF.

Cleans up docs around GrBackendEffectFactory.

R=robertphillips@google.com, jvanverth@google.com

Author: bsalomon@google.com

Review URL: https://codereview.chromium.org/379113004

7 files changed, 213 insertions, 124 deletions

diff --git a/include/gpu/GrBackendEffectFactory.h b/include/gpu/GrBackendEffectFactory.h
index 32f14f2746..ef9e436401 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,74 @@ 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. It also tracks equivalence of shaders generated via a key. The factory for an
+ * effect is accessed via GrEffect::getFactory(). Each factory instance is assigned an ID at
+ * construction. The ID 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).
+ *
+ * Rather than subclassing this class themselves, it is recommended that GrEffect authors use 
+ * the templated subclass 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;
+        if (!id) {
+            SkFAIL("This should never wrap as it should only be called once for each GrEffect "
+                   "subclass.");
+        }
         return id;
     }
 
-    int32_t fEffectClassID;
-
-private:
+    const uint32_t fEffectClassID;
     static int32_t fCurrEffectClassID;
 };
 
diff --git a/include/gpu/GrTBackendEffectFactory.h b/include/gpu/GrTBackendEffectFactory.h
index cd4c0a49e9..251e8c7768 100644
--- a/include/gpu/GrTBackendEffectFactory.h
+++ b/include/gpu/GrTBackendEffectFactory.h
@@ -13,7 +13,26 @@
 #include "gl/GrGLProgramEffects.h"
 
 /**
- * Implements GrBackendEffectFactory for a GrEffect subclass as a singleton.
+ * Implements GrBackendEffectFactory for a GrEffect subclass as a singleton. This can be used by
+ * most GrEffect subclasses to implement the GrEffect::getFactory() method:
+ *
+ * const GrBackendEffectFactory& MyEffect::getFactory() const {
+ *     return GrTBackendEffectFactory<MyEffect>::getInstance();
+ * }
+ *
+ * Using this class requires that the GrEffect subclass always produces the same GrGLEffect
+ * subclass. Additionally, It adds the following requirements to the GrEffect and GrGLEffect
+ * subclasses:
+ *
+ * 1. The GrGLEffect used by GrEffect subclass MyEffect must be named or typedef'ed to
+ *    MyEffect::GLEffect.
+ * 2. MyEffect::GLEffect must have a static function:
+ *      EffectKey GenKey(const GrDrawEffect, const GrGLCaps&)
+ *    which generates a key that maps 1 to 1 with code variations emitted by
+ *    MyEffect::GLEffect::emitCode().
+ * 3. MyEffect must have a static function:
+ *      const char* Name()
+ *    which returns a human-readable name for the effect.
  */
 template <typename EffectClass>
 class GrTBackendEffectFactory : public GrBackendEffectFactory {
@@ -21,35 +40,17 @@ class GrTBackendEffectFactory : public GrBackendEffectFactory {
 public:
     typedef typename EffectClass::GLEffect GLEffect;
 
-    /** Returns a human-readable name that is accessible via GrEffect or
-        GrGLEffect and is consistent between the two of them.
-     */
+    /** Returns a human-readable name for the effect. Implemented using GLEffect::Name as described
+     *  in this class's comment. */
     virtual const char* name() const SK_OVERRIDE { return EffectClass::Name(); }
 
-    /** Generates an effect's key. This enables caching of generated shaders. Part of the
-        id identifies the GrEffect subclass. The remainder is based on the aspects of the
-        GrEffect object's configuration that affect GLSL code generation. If this fails
-        then program generation should be aborted. Failure occurs if the effect uses more
-        transforms, attributes, or textures than the key has space for. */
-    virtual bool getGLEffectKey(const GrDrawEffect& drawEffect,
+
+    /** Implemented using GLEffect::GenKey as described in this class's comment. */
+    virtual void getGLEffectKey(const GrDrawEffect& drawEffect,
                                 const GrGLCaps& caps,
                                 GrEffectKeyBuilder* b) const SK_OVERRIDE {
-        SkASSERT(kIllegalEffectClassID != fEffectClassID);
         EffectKey effectKey = GLEffect::GenKey(drawEffect, caps);
-        EffectKey textureKey = GrGLProgramEffects::GenTextureKey(drawEffect, caps);
-        EffectKey transformKey = GrGLProgramEffects::GenTransformKey(drawEffect);
-        EffectKey attribKey = GrGLProgramEffects::GenAttribKey(drawEffect);
-        static const uint32_t kMetaKeyInvalidMask = ~((uint32_t) SK_MaxU16);
-        if ((textureKey | transformKey | attribKey | fEffectClassID) & kMetaKeyInvalidMask) {
-            return false;
-        }
-
-        // effectKey must be first because it is what will be returned by
-        // GrGLProgramDesc::EffectKeyProvider and passed to the GrGLEffect as its key. 
         b->add32(effectKey);
-        b->add32(textureKey << 16 | transformKey);
-        b->add32(fEffectClassID << 16 | attribKey);
-        return true;
     }
 
     /** Returns a new instance of the appropriate *GL* implementation class
@@ -59,8 +60,7 @@ public:
         return SkNEW_ARGS(GLEffect, (*this, drawEffect));
     }
 
-    /** This class is a singleton. This function returns the single instance.
-     */
+    /** This class is a singleton. This function returns the single instance. */
     static const GrBackendEffectFactory& getInstance() {
         static SkAlignedSTStorage<1, GrTBackendEffectFactory> gInstanceMem;
         static const GrTBackendEffectFactory* gInstance;
@@ -72,9 +72,7 @@ public:
     }
 
 protected:
-    GrTBackendEffectFactory() {
-        fEffectClassID = GenID();
-    }
+    GrTBackendEffectFactory() {}
 };
 
 #endif
diff --git a/src/gpu/gl/GrGLProgramDesc.cpp b/src/gpu/gl/GrGLProgramDesc.cpp
index 2c260cda52..1807b84888 100644
--- a/src/gpu/gl/GrGLProgramDesc.cpp
+++ b/src/gpu/gl/GrGLProgramDesc.cpp
@@ -14,13 +14,14 @@
 
 #include "SkChecksum.h"
 
-static inline bool get_key_and_update_stats(const GrEffectStage& stage,
-                                            const GrGLCaps& caps,
-                                            bool useExplicitLocalCoords,
-                                            GrEffectKeyBuilder* b, 
-                                            bool* setTrueIfReadsDst,
-                                            bool* setTrueIfReadsPos,
-                                            bool* setTrueIfHasVertexCode) {
+bool GrGLProgramDesc::GetEffectKeyAndUpdateStats(const GrEffectStage& stage,
+                                                 const GrGLCaps& caps,
+                                                 bool useExplicitLocalCoords,
+                                                 GrEffectKeyBuilder* b,
+                                                 uint16_t* effectKeySize,
+                                                 bool* setTrueIfReadsDst,
+                                                 bool* setTrueIfReadsPos,
+                                                 bool* setTrueIfHasVertexCode) {
     const GrBackendEffectFactory& factory = stage.getEffect()->getFactory();
     GrDrawEffect drawEffect(stage, useExplicitLocalCoords);
     if (stage.getEffect()->willReadDstColor()) {
@@ -32,7 +33,17 @@ static inline bool get_key_and_update_stats(const GrEffectStage& stage,
     if (stage.getEffect()->hasVertexCode()) {
         *setTrueIfHasVertexCode = true;
     }
-    return factory.getGLEffectKey(drawEffect, caps, b);
+    factory.getGLEffectKey(drawEffect, caps, b);
+    size_t size = b->size();
+    if (size > SK_MaxU16) {
+        *effectKeySize = 0; // suppresses a warning.
+        return false;
+    }
+    *effectKeySize = SkToU16(size);
+    if (!GrGLProgramEffects::GenEffectMetaKey(drawEffect, caps, b)) {
+        return false;
+    }
+    return true;
 }
 
 bool GrGLProgramDesc::Build(const GrDrawState& drawState,
@@ -105,43 +116,54 @@ bool GrGLProgramDesc::Build(const GrDrawState& drawState,
     if (!skipCoverage) {
         numStages += drawState.numCoverageStages() - firstEffectiveCoverageStage;
     }
-    GR_STATIC_ASSERT(0 == kEffectKeyLengthsOffset % sizeof(uint32_t));
+    GR_STATIC_ASSERT(0 == kEffectKeyOffsetsAndLengthOffset % sizeof(uint32_t));
     // Make room for everything up to and including the array of offsets to effect keys.
     desc->fKey.reset();
-    desc->fKey.push_back_n(kEffectKeyLengthsOffset + sizeof(uint32_t) * numStages);
+    desc->fKey.push_back_n(kEffectKeyOffsetsAndLengthOffset + 2 * sizeof(uint32_t) * numStages);
 
-    size_t offset = desc->fKey.count();
-    int offsetIndex = 0;
+    int offsetAndSizeIndex = 0;
 
     bool effectKeySuccess = true;
     if (!skipColor) {
         for (int s = firstEffectiveColorStage; s < drawState.numColorStages(); ++s) {
-            uint32_t* offsetLocation = reinterpret_cast<uint32_t*>(desc->fKey.begin() +
-                                                                   kEffectKeyLengthsOffset +
-                                                                   offsetIndex * sizeof(uint32_t));
-            *offsetLocation = offset;
-            ++offsetIndex;
+            uint16_t* offsetAndSize =
+                reinterpret_cast<uint16_t*>(desc->fKey.begin() + kEffectKeyOffsetsAndLengthOffset +
+                                            offsetAndSizeIndex * 2 * sizeof(uint16_t));
 
             GrEffectKeyBuilder b(&desc->fKey);
-            effectKeySuccess |= get_key_and_update_stats(drawState.getColorStage(s), gpu->glCaps(),
-                                                         requiresLocalCoordAttrib, &b, &readsDst,
-                                                         &readFragPosition, &hasVertexCode);
-            offset += b.size();
+            uint16_t effectKeySize;
+            uint32_t effectOffset = desc->fKey.count();
+            effectKeySuccess |= GetEffectKeyAndUpdateStats(
+                                    drawState.getColorStage(s), gpu->glCaps(),
+                                    requiresLocalCoordAttrib, &b,
+                                    &effectKeySize, &readsDst,
+                                    &readFragPosition, &hasVertexCode);
+            effectKeySuccess |= (effectOffset <= SK_MaxU16);
+
+            offsetAndSize[0] = SkToU16(effectOffset);
+            offsetAndSize[1] = effectKeySize;
+            ++offsetAndSizeIndex;
         }
     }
     if (!skipCoverage) {
         for (int s = firstEffectiveCoverageStage; s < drawState.numCoverageStages(); ++s) {
-            uint32_t* offsetLocation = reinterpret_cast<uint32_t*>(desc->fKey.begin() +
-                                                                   kEffectKeyLengthsOffset +
-                                                                   offsetIndex * sizeof(uint32_t));
-            *offsetLocation = offset;
-            ++offsetIndex;
+            uint16_t* offsetAndSize =
+                reinterpret_cast<uint16_t*>(desc->fKey.begin() + kEffectKeyOffsetsAndLengthOffset +
+                                            offsetAndSizeIndex * 2 * sizeof(uint16_t));
+
             GrEffectKeyBuilder b(&desc->fKey);
-            effectKeySuccess |= get_key_and_update_stats(drawState.getCoverageStage(s),
-                                                         gpu->glCaps(), requiresLocalCoordAttrib,
-                                                         &b, &readsDst, &readFragPosition,
-                                                         &hasVertexCode);
-            offset += b.size();
+            uint16_t effectKeySize;
+            uint32_t effectOffset = desc->fKey.count();
+            effectKeySuccess |= GetEffectKeyAndUpdateStats(
+                                    drawState.getCoverageStage(s), gpu->glCaps(),
+                                    requiresLocalCoordAttrib, &b,
+                                    &effectKeySize, &readsDst,
+                                    &readFragPosition, &hasVertexCode);
+            effectKeySuccess |= (effectOffset <= SK_MaxU16);
+
+            offsetAndSize[0] = SkToU16(effectOffset);
+            offsetAndSize[1] = effectKeySize;
+            ++offsetAndSizeIndex;
         }
     }
     if (!effectKeySuccess) {
diff --git a/src/gpu/gl/GrGLProgramDesc.h b/src/gpu/gl/GrGLProgramDesc.h
index d7652f473c..c8aae19503 100644
--- a/src/gpu/gl/GrGLProgramDesc.h
+++ b/src/gpu/gl/GrGLProgramDesc.h
@@ -172,14 +172,20 @@ private:
     // 1. uint32_t for total key length.
     // 2. uint32_t for a checksum.
     // 3. Header struct defined above.
-    // 4. uint32_t offsets to beginning of every effects' key (see 5).
+    // 4. An array of offsets to effect keys and their sizes (see 5). uint16_t for each
+    //    offset and size.
     // 5. per-effect keys. Each effect's key is a variable length array of uint32_t.
     enum {
+        // Part 1.
         kLengthOffset = 0,
+        // Part 2.
         kChecksumOffset = kLengthOffset + sizeof(uint32_t),
+        // Part 3.
         kHeaderOffset = kChecksumOffset + sizeof(uint32_t),
         kHeaderSize = SkAlign4(sizeof(KeyHeader)),
-        kEffectKeyLengthsOffset = kHeaderOffset + kHeaderSize,
+        // Part 4.
+        // This is the offset in the overall key to the array of per-effect offset,length pairs.
+        kEffectKeyOffsetsAndLengthOffset = kHeaderOffset + kHeaderSize,
     };
 
     template<typename T, size_t OFFSET> T* atOffset() {
@@ -194,6 +200,16 @@ private:
 
     KeyHeader* header() { return this->atOffset<KeyHeader, kHeaderOffset>(); }
 
+    // Shared code between setRandom() and Build().
+    static bool GetEffectKeyAndUpdateStats(const GrEffectStage& stage,
+                                           const GrGLCaps& caps,
+                                           bool useExplicitLocalCoords,
+                                           GrEffectKeyBuilder* b,
+                                           uint16_t* effectKeySize,
+                                           bool* setTrueIfReadsDst,
+                                           bool* setTrueIfReadsPos,
+                                           bool* setTrueIfHasVertexCode);
+
     void finalize();
 
     const KeyHeader& getHeader() const { return *this->atOffset<KeyHeader, kHeaderOffset>(); }
@@ -212,9 +228,11 @@ private:
         }
 
         EffectKey get(int index) const {
-            const uint32_t* offsets = reinterpret_cast<const uint32_t*>(fDesc->fKey.begin() +
-                                                                        kEffectKeyLengthsOffset);
-            uint32_t offset = offsets[fBaseIndex + index];
+            const uint16_t* offsets = reinterpret_cast<const uint16_t*>(
+                fDesc->fKey.begin() + kEffectKeyOffsetsAndLengthOffset);
+            // We store two uint16_ts per effect, one for the offset to the effect's key and one for
+            // its length. Here we just need the offset.
+            uint16_t offset = offsets[2 * (fBaseIndex + index)];
             return *reinterpret_cast<const EffectKey*>(fDesc->fKey.begin() + offset);
         }
     private:
@@ -225,7 +243,7 @@ private:
     enum {
         kMaxPreallocEffects = 8,
         kIntsPerEffect      = 4,    // This is an overestimate of the average effect key size.
-        kPreAllocSize = kEffectKeyLengthsOffset +
+        kPreAllocSize = kEffectKeyOffsetsAndLengthOffset +
                         kMaxPreallocEffects * sizeof(uint32_t) * kIntsPerEffect,
     };
 
diff --git a/src/gpu/gl/GrGLProgramEffects.cpp b/src/gpu/gl/GrGLProgramEffects.cpp
index 65d14fde11..9936aa54ad 100644
--- a/src/gpu/gl/GrGLProgramEffects.cpp
+++ b/src/gpu/gl/GrGLProgramEffects.cpp
@@ -114,6 +114,27 @@ SkMatrix get_transform_matrix(const GrDrawEffect& drawEffect, int transformIdx)
 
 ////////////////////////////////////////////////////////////////////////////////
 
+bool GrGLProgramEffects::GenEffectMetaKey(const GrDrawEffect& drawEffect, const GrGLCaps& caps,
+                                          GrEffectKeyBuilder* b) {
+
+    EffectKey textureKey = GrGLProgramEffects::GenTextureKey(drawEffect, caps);
+    EffectKey transformKey = GrGLProgramEffects::GenTransformKey(drawEffect);
+    EffectKey attribKey = GrGLProgramEffects::GenAttribKey(drawEffect);
+    uint32_t classID = drawEffect.effect()->getFactory().effectClassID();
+
+    // Currently we allow 16 bits for each of the above portions of the meta-key. Fail if they
+    // don't fit.
+    static const uint32_t kMetaKeyInvalidMask = ~((uint32_t) SK_MaxU16);
+    if ((textureKey | transformKey | attribKey | classID) & kMetaKeyInvalidMask) {
+        return false;
+    }
+
+    uint32_t* key = b->add32n(2);
+    key[0] = (textureKey << 16 | transformKey);
+    key[1] = (classID << 16 | attribKey);
+    return true;
+}
+
 EffectKey GrGLProgramEffects::GenAttribKey(const GrDrawEffect& drawEffect) {
     EffectKey key = 0;
     int numAttributes = drawEffect.getVertexAttribIndexCount();
diff --git a/src/gpu/gl/GrGLProgramEffects.h b/src/gpu/gl/GrGLProgramEffects.h
index 5a2fefd7d4..c4d884392a 100644
--- a/src/gpu/gl/GrGLProgramEffects.h
+++ b/src/gpu/gl/GrGLProgramEffects.h
@@ -9,9 +9,9 @@
 #define GrGLProgramEffects_DEFINED
 
 #include "GrBackendEffectFactory.h"
+#include "GrGLUniformManager.h"
 #include "GrTexture.h"
 #include "GrTextureAccess.h"
-#include "GrGLUniformManager.h"
 
 class GrEffect;
 class GrEffectStage;
@@ -31,11 +31,14 @@ public:
     typedef GrGLUniformManager::UniformHandle UniformHandle;
 
     /**
-     * These methods generate different portions of an effect's final key.
+     * This class emits some of the code inserted into the shaders for an effect. The code it
+     * creates may be dependent on properties of the effect that the effect itself doesn't use
+     * in its key (e.g. the pixel format of textures used). So this class inserts a meta-key for
+     * every effect using this function. It is also responsible for inserting the effect's class ID
+     * which must be different for every GrEffect subclass. It can fail if an effect uses too many
+     * textures, attributes, etc for the space allotted in the meta-key.
      */
-    static EffectKey GenAttribKey(const GrDrawEffect&);
-    static EffectKey GenTransformKey(const GrDrawEffect&);
-    static EffectKey GenTextureKey(const GrDrawEffect&, const GrGLCaps&);
+    static bool GenEffectMetaKey(const GrDrawEffect&, const GrGLCaps&, GrEffectKeyBuilder*);
 
     virtual ~GrGLProgramEffects();
 
@@ -98,6 +101,13 @@ public:
     typedef SkTArray<TextureSampler> TextureSamplerArray;
 
 protected:
+    /**
+     * Helpers for GenEffectMetaKey.
+     */
+    static EffectKey GenAttribKey(const GrDrawEffect&);
+    static EffectKey GenTransformKey(const GrDrawEffect&);
+    static EffectKey GenTextureKey(const GrDrawEffect&, const GrGLCaps&);
+
     GrGLProgramEffects(int reserveCount)
         : fGLEffects(reserveCount)
         , fSamplers(reserveCount) {
diff --git a/tests/GLProgramsTest.cpp b/tests/GLProgramsTest.cpp
index dd0f80f42f..2264385902 100644
--- a/tests/GLProgramsTest.cpp
+++ b/tests/GLProgramsTest.cpp
@@ -35,42 +35,33 @@ bool GrGLProgramDesc::setRandom(SkRandom* random,
     int numStages = numColorStages + numCoverageStages;
     fKey.reset();
 
-    GR_STATIC_ASSERT(0 == kEffectKeyLengthsOffset % sizeof(uint32_t));
+    GR_STATIC_ASSERT(0 == kEffectKeyOffsetsAndLengthOffset % sizeof(uint32_t));
 
     // Make room for everything up to and including the array of offsets to effect keys.
-    fKey.push_back_n(kEffectKeyLengthsOffset + sizeof(uint32_t) * numStages);
-
-    size_t offset = fKey.count();
-    int offsetIndex = 0;
+    fKey.push_back_n(kEffectKeyOffsetsAndLengthOffset + 2 * sizeof(uint16_t) * numStages);
 
     bool dstRead = false;
     bool fragPos = false;
     bool vertexCode = false;
     for (int s = 0; s < numStages; ++s) {
-        uint32_t* offsetLocation = reinterpret_cast<uint32_t*>(fKey.begin() +
-                                                               kEffectKeyLengthsOffset +
-                                                               offsetIndex * sizeof(uint32_t));
-        *offsetLocation = offset;
-        ++offsetIndex;
-
-        const GrBackendEffectFactory& factory = stages[s]->getEffect()->getFactory();
+        uint16_t* offsetAndSize = reinterpret_cast<uint16_t*>(fKey.begin() +
+                                                              kEffectKeyOffsetsAndLengthOffset +
+                                                              s * 2 * sizeof(uint16_t));
+        uint32_t effectKeyOffset = fKey.count();
+        if (effectKeyOffset > SK_MaxU16) {
+            fKey.reset();
+            return false;
+        }
         GrDrawEffect drawEffect(*stages[s], useLocalCoords);
         GrEffectKeyBuilder b(&fKey);
-        if (!factory.getGLEffectKey(drawEffect, gpu->glCaps(), &b)) {
+        uint16_t effectKeySize;
+        if (!GetEffectKeyAndUpdateStats(*stages[s], gpu->glCaps(), useLocalCoords, &b,
+                                        &effectKeySize, &dstRead, &fragPos, &vertexCode)) {
             fKey.reset();
             return false;
         }
-        if (stages[s]->getEffect()->willReadDstColor()) {
-            dstRead = true;
-        }
-        if (stages[s]->getEffect()->willReadFragmentPosition()) {
-            fragPos = true;
-        }
-        if (stages[s]->getEffect()->hasVertexCode()) {
-            vertexCode = true;
-        }
-
-        offset += b.size();
+        offsetAndSize[0] = effectKeyOffset;
+        offsetAndSize[1] = effectKeySize;
     }
 
     KeyHeader* header = this->header();