Update effect key for descriptor to not record offsets.

src/gpu/gl/GrGLProgramDesc.cpp

 /*
  * Copyright 2013 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 #include "GrGLProgramDesc.h"
 
 #include "GrGLProcessor.h"
 #include "GrBackendProcessorFactory.h"
@@ skipping 104 matching lines @@
  * in its key (e.g. the pixel format of textures used). So we create 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 GrProcessor subclass. It can fail if an effect uses too many
  * textures, transforms, etc, for the space allotted in the meta-key.  NOTE, both FPs and GPs share
  * this function because it is hairy, though FPs do not have attribs, and GPs do not have transforms
  */
 static bool get_meta_key(const GrProcessor& proc,
                          const GrGLCaps& caps,
                          uint32_t transformKey,
                          uint32_t attribKey,
-                         GrProcessorKeyBuilder* b,
-                         uint16_t* processorKeySize) {
+                         GrProcessorKeyBuilder* b) {
     const GrBackendProcessorFactory& factory = proc.getFactory();
     factory.getGLProcessorKey(proc, caps, b);
-    size_t size = b->size();
-    if (size > SK_MaxU16) {
-        *processorKeySize = 0; // suppresses a warning.
-        return false;
-    }
-    *processorKeySize = SkToU16(size);
+
+    size_t processorKeySize = b->size();
     uint32_t textureKey = gen_texture_key(proc, caps);
     uint32_t classID = proc.getFactory().classID();
 
     // 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 | classID) & kMetaKeyInvalidMask) {
+    if ((textureKey | transformKey | classID | processorKeySize) & kMetaKeyInvalidMask) {
         return false;
     }
 
     uint32_t* key = b->add32n(2);
     key[0] = (textureKey << 16 | transformKey);
-    key[1] = (classID << 16);
+    key[1] = (classID << 16 | processorKeySize);

[bsalomon, 2014/11/26 15:25:06]

fail if key size and classID overlap?

[egdaniel, 2014/11/26 15:47:50]

In practice the keys would have never overlapped, but there was a bug if the
size was sooo large that some bits > 32 were 1, bits 17-32 were 0's, and the
lower bits were whatever, it wouldn't have been caught by the previous check
above

     return true;
 }
 
-struct GeometryProcessorKeyBuilder {

[bsalomon, 2014/11/26 15:25:06]

good riddance.

-    typedef GrGeometryProcessor StagedProcessor;
-    static bool GetProcessorKey(const GrGeometryProcessor& gp,
-                                const GrGLCaps& caps,
-                                bool,
-                                GrProcessorKeyBuilder* b,
-                                uint16_t* keySize) {
-        /* 0 because no transforms on a GP */
-        return get_meta_key(gp, caps, 0, gen_attrib_key(gp), b, keySize);
-    }
-};
-
-struct FragmentProcessorKeyBuilder {
-    typedef GrPendingFragmentStage StagedProcessor;
-    static bool GetProcessorKey(const GrPendingFragmentStage& fps,
-                                const GrGLCaps& caps,
-                                bool useLocalCoords,
-                                GrProcessorKeyBuilder* b,
-                                uint16_t* keySize) {
-        /* 0 because no attribs on a fP */
-        return get_meta_key(*fps.getProcessor(), caps, gen_transform_key(fps, useLocalCoords), 0,
-                            b, keySize);
-    }
-};
-
-
-template <class ProcessorKeyBuilder>
-bool GrGLProgramDescBuilder::BuildStagedProcessorKey(
-        const typename ProcessorKeyBuilder::StagedProcessor& stage,
-        const GrGLCaps& caps,
-        bool requiresLocalCoordAttrib,
-        GrProgramDesc* desc,
-        int* offsetAndSizeIndex) {
-    GrProcessorKeyBuilder b(&desc->fKey);
-    uint16_t processorKeySize;
-    uint32_t processorOffset = desc->fKey.count();
-    if (processorOffset > SK_MaxU16 ||
-            !ProcessorKeyBuilder::GetProcessorKey(stage, caps, requiresLocalCoordAttrib, &b,
-                                                  &processorKeySize)){
-        desc->fKey.reset();
-        return false;
-    }
-
-    uint16_t* offsetAndSize =
-            reinterpret_cast<uint16_t*>(desc->fKey.begin() + kProcessorKeyOffsetsAndLengthOffset +
-                                        *offsetAndSizeIndex * 2 * sizeof(uint16_t));
-    offsetAndSize[0] = SkToU16(processorOffset);
-    offsetAndSize[1] = processorKeySize;
-    ++(*offsetAndSizeIndex);
-    return true;
-}
-
 bool GrGLProgramDescBuilder::Build(const GrOptDrawState& optState,
                                    const GrProgramDesc::DescInfo& descInfo,
                                    GrGpu::DrawType drawType,
                                    GrGpuGL* gpu,
                                    GrProgramDesc* desc) {
     bool inputColorIsUsed = descInfo.fInputColorIsUsed;
     bool inputCoverageIsUsed = descInfo.fInputCoverageIsUsed;
 
     // The descriptor is used as a cache key. Thus when a field of the
     // descriptor will not affect program generation (because of the attribute
     // bindings in use or other descriptor field settings) it should be set
     // to a canonical value to avoid duplicate programs with different keys.
 
     bool requiresLocalCoordAttrib = descInfo.fRequiresLocalCoordAttrib;
 
-    int numStages = optState.numTotalStages();
-
-    GR_STATIC_ASSERT(0 == kProcessorKeyOffsetsAndLengthOffset % sizeof(uint32_t));
-    // Make room for everything up to and including the array of offsets to effect keys.
+    GR_STATIC_ASSERT(0 == kProcessorKeysOffset % sizeof(uint32_t));
+    // Make room for everything up to the effect keys.
     desc->fKey.reset();
-    desc->fKey.push_back_n(kProcessorKeyOffsetsAndLengthOffset + 2 * sizeof(uint16_t) * numStages);
-
-    int offsetAndSizeIndex = 0;
+    desc->fKey.push_back_n(kProcessorKeysOffset);
 
     // We can only have one effect which touches the vertex shader
     if (optState.hasGeometryProcessor()) {
-        if (!BuildStagedProcessorKey<GeometryProcessorKeyBuilder>(*optState.getGeometryProcessor(),
-                                                                  gpu->glCaps(),
-                                                                  false,
-                                                                  desc,
-                                                                  &offsetAndSizeIndex)) {
+        const GrGeometryProcessor& gp = *optState.getGeometryProcessor();
+        GrProcessorKeyBuilder b(&desc->fKey);
+        if (!get_meta_key(gp, gpu->glCaps(), 0, gen_attrib_key(gp), &b)) {
+            desc->fKey.reset();
             return false;
         }
     }
 
     for (int s = 0; s < optState.numFragmentStages(); ++s) {
-        if (!BuildStagedProcessorKey<FragmentProcessorKeyBuilder>(optState.getFragmentStage(s),
-                                                                  gpu->glCaps(),
-                                                                  requiresLocalCoordAttrib,
-                                                                  desc,
-                                                                  &offsetAndSizeIndex)) {
+        const GrPendingFragmentStage& fps = optState.getFragmentStage(s);
+        GrProcessorKeyBuilder b(&desc->fKey);
+        if (!get_meta_key(*fps.getProcessor(), gpu->glCaps(),
+                         gen_transform_key(fps, requiresLocalCoordAttrib), 0, &b)) {
+            desc->fKey.reset();
             return false;
         }
     }
 
     // --------DO NOT MOVE HEADER ABOVE THIS LINE--------------------------------------------------
     // Because header is a pointer into the dynamic array, we can't push any new data into the key
     // below here.
     GLKeyHeader* header = desc->atOffset<GLKeyHeader, kHeaderOffset>();
 
     // make sure any padding in the header is zeroed.
@@ skipping 86 matching lines @@
     }
 
     header->fPrimaryOutputType = descInfo.fPrimaryOutputType;
     header->fSecondaryOutputType = descInfo.fSecondaryOutputType;
 
     header->fColorEffectCnt = optState.numColorStages();
     header->fCoverageEffectCnt = optState.numCoverageStages();
     desc->finalize();
     return true;
 }