Add new style key information to several core effects

Covers some common geometry processors, texture effect, etc.

This also rearranges how fp keys are arranged in the overall key. We no
longer include the key size as part of the key - this made no sense.
Instead, we explicitly include the number of children. We also put all
data for one fp before any children, so the tree can be reconstructed
more-or-less top-down.

Finally, added an "addBool" helper that reads nicer than addBits(1)
everywhere.

Bug: skia:11372
Change-Id: I4e35257fb5923d88fe6d7522109a0b3f4c4017d4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/379059
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>

src/gpu/GrPrimitiveProcessor.cpp

@@ -14,15 +14,15 @@
  * the transform code is applied.
  */
 enum SampleFlag {
-    kExplicitlySampled_Flag      = 0b00001,  // GrFP::isSampledWithExplicitCoords()
+    kExplicitlySampled_Flag      = 0b0001,  // GrFP::isSampledWithExplicitCoords()
 
-    kNone_SampleMatrix_Flag      = 0b00100, // GrFP::sampleUsage()::hasMatrix() == false
+    kNone_SampleMatrix_Flag      = 0b0010, // GrFP::sampleUsage()::hasMatrix() == false
-    kUniform_SampleMatrix_Flag   = 0b01000, // GrFP::sampleUsage()::hasUniformMatrix()
+    kUniform_SampleMatrix_Flag   = 0b0100, // GrFP::sampleUsage()::hasUniformMatrix()
-    kVariable_SampleMatrix_Flag  = 0b01100, // GrFP::sampleUsage()::hasVariableMatrix()
+    kVariable_SampleMatrix_Flag  = 0b0110, // GrFP::sampleUsage()::hasVariableMatrix()
 
     // Currently, sample(matrix) only specializes on no-perspective or general.
     // FIXME add new flags as more matrix types are supported.
-    kPersp_Matrix_Flag           = 0b10000, // GrFP::sampleUsage()::fHasPerspective
+    kPersp_Matrix_Flag           = 0b1000, // GrFP::sampleUsage()::fHasPerspective
 };
 
 GrPrimitiveProcessor::GrPrimitiveProcessor(ClassID classID) : GrProcessor(classID) {}

src/gpu/GrPrimitiveProcessor.h

@@ -183,6 +183,8 @@ public:
      */
     static uint32_t ComputeCoordTransformsKey(const GrFragmentProcessor& fp);
 
+    static constexpr int kCoordTransformKeyBits = 4;
+
     /**
      * Sets a unique key on the GrProcessorKeyBuilder that is directly associated with this geometry
      * processor's GL backend implementation.
@@ -200,8 +202,9 @@ public:
 
         auto add_attributes = [=](const Attribute* attrs, int attrCount) {
             for (int i = 0; i < attrCount; ++i) {
-                b->add32(attrs[i].isInitialized() ? (attrs[i].cpuType() << 16) | attrs[i].gpuType()
+                const Attribute& attr = attrs[i];
-                                                  : ~0);
+                b->addBits(8, attr.isInitialized() ? attr.cpuType() : 0xff, "attrType");
+                b->addBits(8, attr.isInitialized() ? attr.gpuType() : 0xff, "attrGpuType");
             }
         };
         add_attributes(fVertexAttributes.fAttributes, fVertexAttributes.fRawCount);

src/gpu/GrProcessor.h

@@ -44,6 +44,10 @@ public:
         fKey->addBytes(numBytes, data, label);
     }
 
+    void addBool(bool b, const char* label) {
+        this->addBits(1, b, label);
+    }
+
     void add32(uint32_t v, const char* label = "unknown") {
         this->addBits(32, v, label);
     }

src/gpu/GrProgramDesc.cpp

@@ -80,38 +80,13 @@ static bool processor_meta_data_fits(uint32_t classID, size_t keySize) {
 }
 
 /**
- * A function which emits a meta key into the key builder.  This is required because shader code may
+ * Functions which emit a meta key into the key builder. These are required because shader code may
- * be dependent on properties of the effect that the effect itself doesn't use
+ * be dependent on properties of the effect that the effect itself doesn't use in its key (e.g. the
- * in its key (e.g. the pixel format of textures used). So we create a meta-key for
+ * pixel format of textures used). So we create a meta-key for every effect.
- * every effect using this function. It is also responsible for inserting the effect's class ID
+ * It is also responsible for inserting the effect's class ID which must be different for every
- * which must be different for every GrProcessor subclass. It can fail if an effect uses too many
+ * GrProcessor subclass. It can fail if an effect uses too many transforms, etc, for the space
- * transforms, etc, for the space allotted in the meta-key.  NOTE, both FPs and GPs share this
+ * allotted in the meta-key.
- * function because it is hairy, though FPs do not have attribs, and GPs do not have transforms
  */
-static bool gen_fp_meta_key(const GrFragmentProcessor& fp,
-                            const GrCaps& caps,
-                            uint32_t transformKey,
-                            GrProcessorKeyBuilder* b) {
-    size_t processorKeySize = b->sizeInBits();
-    uint32_t classID = fp.classID();
-
-    if (!processor_meta_data_fits(classID, processorKeySize)) {
-        return false;
-    }
-
-    fp.visitTextureEffects([&](const GrTextureEffect& te) {
-        const GrBackendFormat& backendFormat = te.view().proxy()->backendFormat();
-        uint32_t samplerKey = sampler_key(backendFormat.textureType(), te.view().swizzle(), caps);
-        b->add32(samplerKey);
-        caps.addExtraSamplerKey(b, te.samplerState(), backendFormat);
-    });
-
-    b->addBits(kClassIDBits, classID,          "fpClassID");
-    b->addBits(kKeySizeBits, processorKeySize, "fpKeySize");
-    b->add32(transformKey,                     "fpTransforms");
-    return true;
-}
-
 static bool gen_pp_meta_key(const GrPrimitiveProcessor& pp,
                             const GrCaps& caps,
                             GrProcessorKeyBuilder* b) {
@@ -145,6 +120,21 @@ static bool gen_xp_meta_key(const GrXferProcessor& xp, GrProcessorKeyBuilder* b)
 static bool gen_frag_proc_and_meta_keys(const GrFragmentProcessor& fp,
                                         const GrCaps& caps,
                                         GrProcessorKeyBuilder* b) {
+    b->addString([&](){ return fp.name(); });
+    b->addBits(kClassIDBits, fp.classID(), "fpClassID");
+    b->addBits(GrPrimitiveProcessor::kCoordTransformKeyBits,
+               GrPrimitiveProcessor::ComputeCoordTransformsKey(fp), "fpTransforms");
+
+    fp.visitTextureEffects([&](const GrTextureEffect& te) {
+        const GrBackendFormat& backendFormat = te.view().proxy()->backendFormat();
+        uint32_t samplerKey = sampler_key(backendFormat.textureType(), te.view().swizzle(), caps);
+        b->add32(samplerKey, "fpSamplerKey");
+        caps.addExtraSamplerKey(b, te.samplerState(), backendFormat);
+    });
+
+    fp.getGLSLProcessorKey(*caps.shaderCaps(), b);
+    b->add32(fp.numChildProcessors(), "fpNumChildren");
+
     for (int i = 0; i < fp.numChildProcessors(); ++i) {
         if (auto child = fp.childProcessor(i)) {
             if (!gen_frag_proc_and_meta_keys(*child, caps, b)) {
@@ -152,14 +142,12 @@ static bool gen_frag_proc_and_meta_keys(const GrFragmentProcessor& fp,
             }
         } else {
             // Fold in a sentinel value as the "class ID" for any null children
-            b->add32(GrProcessor::ClassID::kNull_ClassID);
+            b->addString([&](){ return "Null"; });
+            b->addBits(kClassIDBits, GrProcessor::ClassID::kNull_ClassID, "fpClassID");
         }
     }
 
-    b->addString([&](){ return fp.name(); });
+    return true;
-    fp.getGLSLProcessorKey(*caps.shaderCaps(), b);
-
-    return gen_fp_meta_key(fp, caps, GrPrimitiveProcessor::ComputeCoordTransformsKey(fp), b);
 }
 
 bool GrProgramDesc::Build(GrProgramDesc* desc,

src/gpu/effects/GrBitmapTextGeoProc.cpp

@@ -94,12 +94,11 @@ public:
                               const GrShaderCaps&,
                               GrProcessorKeyBuilder* b) {
         const GrBitmapTextGeoProc& btgp = proc.cast<GrBitmapTextGeoProc>();
-        uint32_t key = 0;
+        b->addBool(btgp.usesW(), "usesW");
-        key |= btgp.usesW() ? 0x1 : 0x0;
+        static_assert(kLast_GrMaskFormat < (1u << 2));
-        key |= btgp.maskFormat() << 1;
+        b->addBits(2, btgp.maskFormat(), "maskFormat");
-        key |= ComputeMatrixKey(btgp.localMatrix()) << 2;
+        b->addBits(kMatrixKeyBits, ComputeMatrixKey(btgp.localMatrix()), "localMatrixType");
-        b->add32(key);
+        b->add32(btgp.numTextureSamplers(),"numTextures");
-        b->add32(btgp.numTextureSamplers());
     }
 
 private:

src/gpu/effects/GrBitmapTextGeoProc.h

@@ -43,7 +43,7 @@ public:
 
     ~GrBitmapTextGeoProc() override {}
 
-    const char* name() const override { return "Texture"; }
+    const char* name() const override { return "BitmapText"; }
 
     const Attribute& inPosition() const { return fInPosition; }
     const Attribute& inColor() const { return fInColor; }

src/gpu/effects/GrTextureEffect.cpp

@@ -765,17 +765,17 @@ std::unique_ptr<GrGLSLFragmentProcessor> GrTextureEffect::onMakeProgramImpl() co
 
 void GrTextureEffect::onGetGLSLProcessorKey(const GrShaderCaps&, GrProcessorKeyBuilder* b) const {
     auto m0 = static_cast<uint32_t>(fShaderModes[0]);
-    SkASSERT(SkTFitsIn<uint8_t>(m0));
+    b->addBits(8, m0, "shaderMode0");
 
     auto m1 = static_cast<uint32_t>(fShaderModes[1]);
-    SkASSERT(SkTFitsIn<uint8_t>(m1));
+    b->addBits(8, m1, "shaderMode1");
 
     // The origin only affects the shader code when we're doing last minute normalization
     // for lazy proxies.
-    uint32_t normInfo = fLazyProxyNormalization ? (this->view().origin() << 1 | 0x1) : 0x0;
+    b->addBool(fLazyProxyNormalization, "normalization");
-    SkASSERT(SkTFitsIn<uint8_t>(normInfo));
+    if (fLazyProxyNormalization) {
-
+        b->addBits(1, this->view().origin(), "origin");
-    b->add32((m0 << 16) | m1 << 8 | normInfo);
+    }
 }
 
 bool GrTextureEffect::onIsEqual(const GrFragmentProcessor& other) const {

src/gpu/effects/generated/GrArithmeticProcessor.cpp

@@ -63,7 +63,7 @@ std::unique_ptr<GrGLSLFragmentProcessor> GrArithmeticProcessor::onMakeProgramImp
 }
 void GrArithmeticProcessor::onGetGLSLProcessorKey(const GrShaderCaps& caps,
                                                   GrProcessorKeyBuilder* b) const {
-    b->addBits(1, (uint32_t)enforcePMColor, "enforcePMColor");
+    b->addBool(enforcePMColor, "enforcePMColor");
 }
 bool GrArithmeticProcessor::onIsEqual(const GrFragmentProcessor& other) const {
     const GrArithmeticProcessor& that = other.cast<GrArithmeticProcessor>();

src/gpu/effects/generated/GrClampFragmentProcessor.cpp

@@ -50,7 +50,7 @@ std::unique_ptr<GrGLSLFragmentProcessor> GrClampFragmentProcessor::onMakeProgram
 }
 void GrClampFragmentProcessor::onGetGLSLProcessorKey(const GrShaderCaps& caps,
                                                      GrProcessorKeyBuilder* b) const {
-    b->addBits(1, (uint32_t)clampToPremul, "clampToPremul");
+    b->addBool(clampToPremul, "clampToPremul");
 }
 bool GrClampFragmentProcessor::onIsEqual(const GrFragmentProcessor& other) const {
     const GrClampFragmentProcessor& that = other.cast<GrClampFragmentProcessor>();

src/gpu/effects/generated/GrColorMatrixFragmentProcessor.cpp

@@ -92,9 +92,9 @@ std::unique_ptr<GrGLSLFragmentProcessor> GrColorMatrixFragmentProcessor::onMakeP
 }
 void GrColorMatrixFragmentProcessor::onGetGLSLProcessorKey(const GrShaderCaps& caps,
                                                            GrProcessorKeyBuilder* b) const {
-    b->addBits(1, (uint32_t)unpremulInput, "unpremulInput");
+    b->addBool(unpremulInput, "unpremulInput");
-    b->addBits(1, (uint32_t)clampRGBOutput, "clampRGBOutput");
+    b->addBool(clampRGBOutput, "clampRGBOutput");
-    b->addBits(1, (uint32_t)premulOutput, "premulOutput");
+    b->addBool(premulOutput, "premulOutput");
 }
 bool GrColorMatrixFragmentProcessor::onIsEqual(const GrFragmentProcessor& other) const {
     const GrColorMatrixFragmentProcessor& that = other.cast<GrColorMatrixFragmentProcessor>();

src/gpu/effects/generated/GrOverrideInputFragmentProcessor.cpp

@@ -67,7 +67,7 @@ std::unique_ptr<GrGLSLFragmentProcessor> GrOverrideInputFragmentProcessor::onMak
 }
 void GrOverrideInputFragmentProcessor::onGetGLSLProcessorKey(const GrShaderCaps& caps,
                                                              GrProcessorKeyBuilder* b) const {
-    b->addBits(1, (uint32_t)useUniform, "useUniform");
+    b->addBool(useUniform, "useUniform");
     if (!useUniform) {
         uint16_t red = SkFloatToHalf(literalColor.fR);
         uint16_t green = SkFloatToHalf(literalColor.fG);

src/gpu/effects/generated/GrRectBlurEffect.cpp

@@ -159,9 +159,9 @@ void GrRectBlurEffect::onGetGLSLProcessorKey(const GrShaderCaps& caps,
     bool highPrecision = ((abs(rect.left()) > 16000.0 || abs(rect.top()) > 16000.0) ||
                           abs(rect.right()) > 16000.0) ||
                          abs(rect.bottom()) > 16000.0;
-    b->addBits(1, (uint32_t)highPrecision, "highPrecision");
+    b->addBool(highPrecision, "highPrecision");
-    b->addBits(1, (uint32_t)applyInvVM, "applyInvVM");
+    b->addBool(applyInvVM, "applyInvVM");
-    b->addBits(1, (uint32_t)isFast, "isFast");
+    b->addBool(isFast, "isFast");
 }
 bool GrRectBlurEffect::onIsEqual(const GrFragmentProcessor& other) const {
     const GrRectBlurEffect& that = other.cast<GrRectBlurEffect>();

src/gpu/gradients/generated/GrClampedGradientEffect.cpp

@@ -93,8 +93,8 @@ std::unique_ptr<GrGLSLFragmentProcessor> GrClampedGradientEffect::onMakeProgramI
 }
 void GrClampedGradientEffect::onGetGLSLProcessorKey(const GrShaderCaps& caps,
                                                     GrProcessorKeyBuilder* b) const {
-    b->addBits(1, (uint32_t)makePremul, "makePremul");
+    b->addBool(makePremul, "makePremul");
-    b->addBits(1, (uint32_t)layoutPreservesOpacity, "layoutPreservesOpacity");
+    b->addBool(layoutPreservesOpacity, "layoutPreservesOpacity");
 }
 bool GrClampedGradientEffect::onIsEqual(const GrFragmentProcessor& other) const {
     const GrClampedGradientEffect& that = other.cast<GrClampedGradientEffect>();

src/gpu/gradients/generated/GrTiledGradientEffect.cpp

@@ -79,9 +79,9 @@ std::unique_ptr<GrGLSLFragmentProcessor> GrTiledGradientEffect::onMakeProgramImp
 }
 void GrTiledGradientEffect::onGetGLSLProcessorKey(const GrShaderCaps& caps,
                                                   GrProcessorKeyBuilder* b) const {
-    b->addBits(1, (uint32_t)mirror, "mirror");
+    b->addBool(mirror, "mirror");
-    b->addBits(1, (uint32_t)makePremul, "makePremul");
+    b->addBool(makePremul, "makePremul");
-    b->addBits(1, (uint32_t)layoutPreservesOpacity, "layoutPreservesOpacity");
+    b->addBool(layoutPreservesOpacity, "layoutPreservesOpacity");
 }
 bool GrTiledGradientEffect::onIsEqual(const GrFragmentProcessor& other) const {
     const GrTiledGradientEffect& that = other.cast<GrTiledGradientEffect>();

src/gpu/gradients/generated/GrTwoPointConicalGradientLayout.cpp

@@ -151,11 +151,11 @@ std::unique_ptr<GrGLSLFragmentProcessor> GrTwoPointConicalGradientLayout::onMake
 void GrTwoPointConicalGradientLayout::onGetGLSLProcessorKey(const GrShaderCaps& caps,
                                                             GrProcessorKeyBuilder* b) const {
     b->addBits(2, (uint32_t)type, "type");
-    b->addBits(1, (uint32_t)isRadiusIncreasing, "isRadiusIncreasing");
+    b->addBool(isRadiusIncreasing, "isRadiusIncreasing");
-    b->addBits(1, (uint32_t)isFocalOnCircle, "isFocalOnCircle");
+    b->addBool(isFocalOnCircle, "isFocalOnCircle");
-    b->addBits(1, (uint32_t)isWellBehaved, "isWellBehaved");
+    b->addBool(isWellBehaved, "isWellBehaved");
-    b->addBits(1, (uint32_t)isSwapped, "isSwapped");
+    b->addBool(isSwapped, "isSwapped");
-    b->addBits(1, (uint32_t)isNativelyFocal, "isNativelyFocal");
+    b->addBool(isNativelyFocal, "isNativelyFocal");
 }
 bool GrTwoPointConicalGradientLayout::onIsEqual(const GrFragmentProcessor& other) const {
     const GrTwoPointConicalGradientLayout& that = other.cast<GrTwoPointConicalGradientLayout>();

src/gpu/ops/GrAAConvexPathRenderer.cpp

@@ -618,9 +618,8 @@ public:
                                   const GrShaderCaps&,
                                   GrProcessorKeyBuilder* b) {
             const QuadEdgeEffect& qee = gp.cast<QuadEdgeEffect>();
-            uint32_t key = (uint32_t) qee.fUsesLocalCoords;
+            b->addBool(qee.fUsesLocalCoords, "usesLocalCoords");
-            key |= ComputeMatrixKey(qee.fLocalMatrix) << 1;
+            b->addBits(kMatrixKeyBits, ComputeMatrixKey(qee.fLocalMatrix), "localMatrixType");
-            b->add32(key);
         }
 
         void setData(const GrGLSLProgramDataManager& pdman,

src/gpu/ops/GrFillRRectOp.cpp

@@ -307,7 +307,8 @@ public:
     const char* name() const final { return "GrFillRRectOp::Processor"; }
 
     void getGLSLProcessorKey(const GrShaderCaps& caps, GrProcessorKeyBuilder* b) const final {
-        b->add32(((uint32_t)fFlags << 16) | (uint32_t)fAAType);
+        b->addBits(4, (uint32_t)fFlags,  "flags");
+        b->addBits(2, (uint32_t)fAAType, "aaType");
     }
 
     GrGLSLPrimitiveProcessor* createGLSLInstance(const GrShaderCaps&) const final;

src/gpu/ops/GrOvalOpFactory.cpp

@@ -206,14 +206,12 @@ private:
                            const GrShaderCaps&,
                            GrProcessorKeyBuilder* b) {
             const CircleGeometryProcessor& cgp = gp.cast<CircleGeometryProcessor>();
-            uint32_t key;
+            b->addBool(cgp.fStroke,                                     "stroked");
-            key = cgp.fStroke ? 0x01 : 0x0;
+            b->addBool(cgp.fInClipPlane.isInitialized(),                "clipPlane");
-            key |= cgp.fInClipPlane.isInitialized() ? 0x02 : 0x0;
+            b->addBool(cgp.fInIsectPlane.isInitialized(),               "isectPlane");
-            key |= cgp.fInIsectPlane.isInitialized() ? 0x04 : 0x0;
+            b->addBool(cgp.fInUnionPlane.isInitialized(),               "unionPlane");
-            key |= cgp.fInUnionPlane.isInitialized() ? 0x08 : 0x0;
+            b->addBool(cgp.fInRoundCapCenters.isInitialized(),          "roundCapCenters");
-            key |= cgp.fInRoundCapCenters.isInitialized() ? 0x10 : 0x0;
+            b->addBits(kMatrixKeyBits, ComputeMatrixKey(cgp.fLocalMatrix), "localMatrixType");
-            key |= (ComputeMatrixKey(cgp.fLocalMatrix) << 16);
-            b->add32(key);
         }
 
         void setData(const GrGLSLProgramDataManager& pdman,
@@ -472,7 +470,7 @@ private:
                            GrProcessorKeyBuilder* b) {
             const ButtCapDashedCircleGeometryProcessor& bcscgp =
                     gp.cast<ButtCapDashedCircleGeometryProcessor>();
-            b->add32(ComputeMatrixKey(bcscgp.fLocalMatrix));
+            b->addBits(kMatrixKeyBits, ComputeMatrixKey(bcscgp.fLocalMatrix), "localMatrixType");
         }
 
         void setData(const GrGLSLProgramDataManager& pdman,
@@ -660,9 +658,8 @@ private:
                            const GrShaderCaps&,
                            GrProcessorKeyBuilder* b) {
             const EllipseGeometryProcessor& egp = gp.cast<EllipseGeometryProcessor>();
-            uint32_t key = egp.fStroke ? 0x1 : 0x0;
+            b->addBool(egp.fStroke, "stroked");
-            key |= ComputeMatrixKey(egp.fLocalMatrix) << 1;
+            b->addBits(kMatrixKeyBits, ComputeMatrixKey(egp.fLocalMatrix), "localMatrixType");
-            b->add32(key);
         }
 
         void setData(const GrGLSLProgramDataManager& pdman,
@@ -853,9 +850,8 @@ private:
                            const GrShaderCaps&,
                            GrProcessorKeyBuilder* b) {
             const DIEllipseGeometryProcessor& diegp = gp.cast<DIEllipseGeometryProcessor>();
-            uint32_t key = static_cast<uint32_t>(diegp.fStyle);
+            b->addBits(2, static_cast<uint32_t>(diegp.fStyle), "style");
-            key |= ComputeMatrixKey(diegp.fViewMatrix) << 10;
+            b->addBits(kMatrixKeyBits, ComputeMatrixKey(diegp.fViewMatrix), "viewMatrixType");
-            b->add32(key);
         }
 
         void setData(const GrGLSLProgramDataManager& pdman,

src/gpu/ops/GrQuadPerEdgeAA.cpp

@@ -567,29 +567,33 @@ public:
 
     void getGLSLProcessorKey(const GrShaderCaps&, GrProcessorKeyBuilder* b) const override {
         // texturing, device-dimensions are single bit flags
-        uint32_t x = (fTexSubset.isInitialized() ? 0 : 0x1)
+        b->addBool(fTexSubset.isInitialized(),    "subset");
-                   | (fSampler.isInitialized()   ? 0 : 0x2)
+        b->addBool(fSampler.isInitialized(),      "textured");
-                   | (fNeedsPerspective          ? 0 : 0x4)
+        b->addBool(fNeedsPerspective,             "perspective");
-                   | (fSaturate == Saturate::kNo ? 0 : 0x8);
+        b->addBool((fSaturate == Saturate::kYes), "saturate");
-        // local coords require 2 bits (3 choices), 00 for none, 01 for 2d, 10 for 3d
+
+        b->addBool(fLocalCoord.isInitialized(),   "hasLocalCoords");
         if (fLocalCoord.isInitialized()) {
-            x |= kFloat3_GrVertexAttribType == fLocalCoord.cpuType() ? 0x10 : 0x20;
+            // 2D (0) or 3D (1)
+            b->addBits(1, (kFloat3_GrVertexAttribType == fLocalCoord.cpuType()), "localCoordsType");
         }
-        // similar for colors, 00 for none, 01 for bytes, 10 for half-floats
+        b->addBool(fColor.isInitialized(),        "hasColor");
         if (fColor.isInitialized()) {
-            x |= kUByte4_norm_GrVertexAttribType == fColor.cpuType() ? 0x40 : 0x80;
+            // bytes (0) or floats (1)
+            b->addBits(1, (kFloat4_GrVertexAttribType == fColor.cpuType()), "colorType");
         }
         // and coverage mode, 00 for none, 01 for withposition, 10 for withcolor, 11 for
         // position+geomsubset
+        uint32_t coverageKey = 0;
         SkASSERT(!fGeomSubset.isInitialized() || fCoverageMode == CoverageMode::kWithPosition);
         if (fCoverageMode != CoverageMode::kNone) {
-            x |= fGeomSubset.isInitialized()
+            coverageKey = fGeomSubset.isInitialized()
-                         ? 0x300
+                                  ? 0x3
-                         : (CoverageMode::kWithPosition == fCoverageMode ? 0x100 : 0x200);
+                                  : (CoverageMode::kWithPosition == fCoverageMode ? 0x1 : 0x2);
         }
+        b->addBits(2, coverageKey, "coverageMode");
 
-        b->add32(GrColorSpaceXform::XformKey(fTextureColorSpaceXform.get()));
+        b->add32(GrColorSpaceXform::XformKey(fTextureColorSpaceXform.get()), "colorSpaceXform");
-        b->add32(x);
     }
 
     GrGLSLPrimitiveProcessor* createGLSLInstance(const GrShaderCaps& caps) const override {

src/sksl/SkSLCPPCodeGenerator.cpp

@@ -1320,7 +1320,7 @@ void CPPCodeGenerator::writeGetKey() {
                     this->writef("    b->add32(sk_bit_cast<uint32_t>(%s), \"%s\");\n",
                                  HCodeGenerator::FieldName(name).c_str(), name);
                 } else if (varType.isBoolean()) {
-                    this->writef("    b->addBits(1, (uint32_t) %s, \"%s\");\n",
+                    this->writef("    b->addBool(%s, \"%s\");\n",
                                  HCodeGenerator::FieldName(name).c_str(), name);
                 } else if (varType.isEnum()) {
                     this->writef("    b->addBits(%d, (uint32_t) %s, \"%s\");\n",

tests/sksl/fp/GrConditionalInUniform.cpp

@@ -55,7 +55,7 @@ std::unique_ptr<GrGLSLFragmentProcessor> GrConditionalInUniform::onMakeProgramIm
     return std::make_unique<GrGLSLConditionalInUniform>();
 }
 void GrConditionalInUniform::onGetGLSLProcessorKey(const GrShaderCaps& caps, GrProcessorKeyBuilder* b) const {
-    b->addBits(1, (uint32_t) test, "test");
+    b->addBool(test, "test");
 }
 bool GrConditionalInUniform::onIsEqual(const GrFragmentProcessor& other) const {
     const GrConditionalInUniform& that = other.cast<GrConditionalInUniform>();