Revert of Makes GrGLProgramDesc's key store the lengths as well as offsets of the effect keys. (https://codereview.chromium.org/379113004/)

Reason for revert:
Most likely candidate for Valgrind failures:

[21:10:08.755668] ==3036== Use of uninitialised value of size 8
[21:10:08.755753] ==3036==    at 0x734AB2: GrGpuGL::ProgramCache::getProgram(GrGLProgramDesc const&, GrEffectStage const**, GrEffectStage const**) (GrGpuGL_program.cpp:107)
[21:10:08.755788] ==3036==    by 0x734ED2: GrGpuGL::flushGraphicsState(GrGpu::DrawType, GrDeviceCoordTexture const*) (GrGpuGL_program.cpp:253)
[21:10:08.755811] ==3036==    by 0x6E81C2: GrGpu::setupClipAndFlushState(GrGpu::DrawType, GrDeviceCoordTexture const*, GrDrawState::AutoRestoreEffects*, SkRect const*) (GrGpu.cpp:350)
[21:10:08.755837] ==3036==    by 0x6E9BE8: GrGpu::onDraw(GrDrawTarget::DrawInfo const&) (GrGpu.cpp:390)
[21:10:08.755858] ==3036==    by 0x6EEECE: GrInOrderDrawBuffer::flush() (GrDrawTarget.h:506)
[21:10:08.755879] ==3036==    by 0x6D0EB4: GrContext::flush(int) (GrContext.cpp:1327)
[21:10:08.755900] ==3036==    by 0x6D3F8F: GrContext::writeTexturePixels(GrTexture*, int, int, int, int, GrPixelConfig, void const*, unsigned long, unsigned int) (GrContext.cpp:1349)
[21:10:08.755922] ==3036==    by 0x6D39D7: GrContext::writeRenderTargetPixels(GrRenderTarget*, int, int, int, int, GrPixelConfig, void const*, unsigned long, unsigned int) (GrContext.cpp:1632)
[21:10:08.755949] ==3036==    by 0x6FFDF3: GrRenderTarget::writePixels(int, int, int, int, GrPixelConfig, void const*, unsigned long, unsigned int) (GrRenderTarget.cpp:45)
[21:10:08.755978] ==3036==    by 0x735563: SkGpuDevice::onWritePixels(SkImageInfo const&, void const*, unsigned long, int, int) (SkGpuDevice.cpp:280)
[21:10:08.756003] ==3036==    by 0x57A048: SkBaseDevice::writePixels(SkImageInfo const&, void const*, unsigned long, int, int) (SkDevice.cpp:106)
[21:10:08.756025] ==3036==    by 0x56D0AE: SkCanvas::writePixels(SkImageInfo const&, void const*, unsigned long, int, int) (SkCanvas.cpp:700)
[21:10:08.756050] ==3036==    by 0x56D156: SkCanvas::writePixels(SkBitmap const&, int, int) (SkCanvas.cpp:652)
[21:10:08.756077] ==3036==    by 0x5109B6: test_WritePixels(skiatest::Reporter*, GrContextFactory*) (WritePixelsTest.cpp:464)
[21:10:08.756099] ==3036==    by 0x51114C: skiatest::WritePixelsClass::onRun(skiatest::Reporter*) (WritePixelsTest.cpp:361)
[21:10:08.756122] ==3036==    by 0x406BE8: skiatest::Test::run() (Test.cpp:107)
[21:10:08.756145] ==3036==    by 0x4064C2: SkTestRunnable::run() (skia_test.cpp:109)
[21:10:08.756167] ==3036==    by 0x405D1A: tool_main(int, char**) (skia_test.cpp:221)
[21:10:08.756189] ==3036==    by 0x405F75: main (skia_test.cpp:239)
[21:10:08.756211] ==3036==  Uninitialised value was created by a stack allocation
[21:10:08.756233] ==3036==    at 0x734CC8: GrGpuGL::flushGraphicsState(GrGpu::DrawType, GrDeviceCoordTexture const*) (GrGpuGL_program.cpp:213)

Original issue's description:
> 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.
>
> Committed: https://skia.googlesource.com/skia/+/c0ea398aff8254e31152cbb94c9ab6150428e252

R=robertphillips@google.com, jvanverth@google.com, bsalomon@google.com
TBR=bsalomon@google.com, jvanverth@google.com, robertphillips@google.com
NOTREECHECKS=true
NOTRY=true

Author: mtklein@google.com

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

4 files changed, 42 insertions, 113 deletions

diff --git a/src/gpu/gl/GrGLProgramDesc.cpp b/src/gpu/gl/GrGLProgramDesc.cpp
index 1807b84888..2c260cda52 100644
--- a/src/gpu/gl/GrGLProgramDesc.cpp
+++ b/src/gpu/gl/GrGLProgramDesc.cpp
@@ -14,14 +14,13 @@
 
 #include "SkChecksum.h"
 
-bool GrGLProgramDesc::GetEffectKeyAndUpdateStats(const GrEffectStage& stage,
-                                                 const GrGLCaps& caps,
-                                                 bool useExplicitLocalCoords,
-                                                 GrEffectKeyBuilder* b,
-                                                 uint16_t* effectKeySize,
-                                                 bool* setTrueIfReadsDst,
-                                                 bool* setTrueIfReadsPos,
-                                                 bool* setTrueIfHasVertexCode) {
+static inline bool get_key_and_update_stats(const GrEffectStage& stage,
+                                            const GrGLCaps& caps,
+                                            bool useExplicitLocalCoords,
+                                            GrEffectKeyBuilder* b, 
+                                            bool* setTrueIfReadsDst,
+                                            bool* setTrueIfReadsPos,
+                                            bool* setTrueIfHasVertexCode) {
     const GrBackendEffectFactory& factory = stage.getEffect()->getFactory();
     GrDrawEffect drawEffect(stage, useExplicitLocalCoords);
     if (stage.getEffect()->willReadDstColor()) {
@@ -33,17 +32,7 @@ bool GrGLProgramDesc::GetEffectKeyAndUpdateStats(const GrEffectStage& stage,
     if (stage.getEffect()->hasVertexCode()) {
         *setTrueIfHasVertexCode = true;
     }
-    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;
+    return factory.getGLEffectKey(drawEffect, caps, b);
 }
 
 bool GrGLProgramDesc::Build(const GrDrawState& drawState,
@@ -116,54 +105,43 @@ bool GrGLProgramDesc::Build(const GrDrawState& drawState,
     if (!skipCoverage) {
         numStages += drawState.numCoverageStages() - firstEffectiveCoverageStage;
     }
-    GR_STATIC_ASSERT(0 == kEffectKeyOffsetsAndLengthOffset % sizeof(uint32_t));
+    GR_STATIC_ASSERT(0 == kEffectKeyLengthsOffset % 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(kEffectKeyOffsetsAndLengthOffset + 2 * sizeof(uint32_t) * numStages);
+    desc->fKey.push_back_n(kEffectKeyLengthsOffset + sizeof(uint32_t) * numStages);
 
-    int offsetAndSizeIndex = 0;
+    size_t offset = desc->fKey.count();
+    int offsetIndex = 0;
 
     bool effectKeySuccess = true;
     if (!skipColor) {
         for (int s = firstEffectiveColorStage; s < drawState.numColorStages(); ++s) {
-            uint16_t* offsetAndSize =
-                reinterpret_cast<uint16_t*>(desc->fKey.begin() + kEffectKeyOffsetsAndLengthOffset +
-                                            offsetAndSizeIndex * 2 * sizeof(uint16_t));
+            uint32_t* offsetLocation = reinterpret_cast<uint32_t*>(desc->fKey.begin() +
+                                                                   kEffectKeyLengthsOffset +
+                                                                   offsetIndex * sizeof(uint32_t));
+            *offsetLocation = offset;
+            ++offsetIndex;
 
             GrEffectKeyBuilder b(&desc->fKey);
-            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;
+            effectKeySuccess |= get_key_and_update_stats(drawState.getColorStage(s), gpu->glCaps(),
+                                                         requiresLocalCoordAttrib, &b, &readsDst,
+                                                         &readFragPosition, &hasVertexCode);
+            offset += b.size();
         }
     }
     if (!skipCoverage) {
         for (int s = firstEffectiveCoverageStage; s < drawState.numCoverageStages(); ++s) {
-            uint16_t* offsetAndSize =
-                reinterpret_cast<uint16_t*>(desc->fKey.begin() + kEffectKeyOffsetsAndLengthOffset +
-                                            offsetAndSizeIndex * 2 * sizeof(uint16_t));
-
+            uint32_t* offsetLocation = reinterpret_cast<uint32_t*>(desc->fKey.begin() +
+                                                                   kEffectKeyLengthsOffset +
+                                                                   offsetIndex * sizeof(uint32_t));
+            *offsetLocation = offset;
+            ++offsetIndex;
             GrEffectKeyBuilder b(&desc->fKey);
-            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;
+            effectKeySuccess |= get_key_and_update_stats(drawState.getCoverageStage(s),
+                                                         gpu->glCaps(), requiresLocalCoordAttrib,
+                                                         &b, &readsDst, &readFragPosition,
+                                                         &hasVertexCode);
+            offset += b.size();
         }
     }
     if (!effectKeySuccess) {
diff --git a/src/gpu/gl/GrGLProgramDesc.h b/src/gpu/gl/GrGLProgramDesc.h
index c8aae19503..d7652f473c 100644
--- a/src/gpu/gl/GrGLProgramDesc.h
+++ b/src/gpu/gl/GrGLProgramDesc.h
@@ -172,20 +172,14 @@ private:
     // 1. uint32_t for total key length.
     // 2. uint32_t for a checksum.
     // 3. Header struct defined above.
-    // 4. An array of offsets to effect keys and their sizes (see 5). uint16_t for each
-    //    offset and size.
+    // 4. uint32_t offsets to beginning of every effects' key (see 5).
     // 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)),
-        // Part 4.
-        // This is the offset in the overall key to the array of per-effect offset,length pairs.
-        kEffectKeyOffsetsAndLengthOffset = kHeaderOffset + kHeaderSize,
+        kEffectKeyLengthsOffset = kHeaderOffset + kHeaderSize,
     };
 
     template<typename T, size_t OFFSET> T* atOffset() {
@@ -200,16 +194,6 @@ 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>(); }
@@ -228,11 +212,9 @@ private:
         }
 
         EffectKey get(int index) const {
-            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)];
+            const uint32_t* offsets = reinterpret_cast<const uint32_t*>(fDesc->fKey.begin() +
+                                                                        kEffectKeyLengthsOffset);
+            uint32_t offset = offsets[fBaseIndex + index];
             return *reinterpret_cast<const EffectKey*>(fDesc->fKey.begin() + offset);
         }
     private:
@@ -243,7 +225,7 @@ private:
     enum {
         kMaxPreallocEffects = 8,
         kIntsPerEffect      = 4,    // This is an overestimate of the average effect key size.
-        kPreAllocSize = kEffectKeyOffsetsAndLengthOffset +
+        kPreAllocSize = kEffectKeyLengthsOffset +
                         kMaxPreallocEffects * sizeof(uint32_t) * kIntsPerEffect,
     };
 
diff --git a/src/gpu/gl/GrGLProgramEffects.cpp b/src/gpu/gl/GrGLProgramEffects.cpp
index 9936aa54ad..65d14fde11 100644
--- a/src/gpu/gl/GrGLProgramEffects.cpp
+++ b/src/gpu/gl/GrGLProgramEffects.cpp
@@ -114,27 +114,6 @@ 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 c4d884392a..5a2fefd7d4 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,14 +31,11 @@ public:
     typedef GrGLUniformManager::UniformHandle UniformHandle;
 
     /**
-     * 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.
+     * These methods generate different portions of an effect's final key.
      */
-    static bool GenEffectMetaKey(const GrDrawEffect&, const GrGLCaps&, GrEffectKeyBuilder*);
+    static EffectKey GenAttribKey(const GrDrawEffect&);
+    static EffectKey GenTransformKey(const GrDrawEffect&);
+    static EffectKey GenTextureKey(const GrDrawEffect&, const GrGLCaps&);
 
     virtual ~GrGLProgramEffects();
 
@@ -101,13 +98,6 @@ 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) {