Remove numPointers from SkShaderSnippet.

This undoes http://review.skia.org/550018, and also eliminates the
pointer-related additions to Stack in http://review.skia.org/550179.

When this was originally added, the plan was that pointers would not be
represented in the data payload at all, so we needed a mechanism to
track whether pointers were needed for each snippet type. This is now
handled by the existence of the kPointerIndex data payload field--
you'll assert on field-type mismatch/missing field if you skip the
pointer.

Change-Id: Ie868526d8d9da47819ea08e16aab1a5566f9cff8
Bug: skia:13428
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/550700
Commit-Queue: Robert Phillips <robertphillips@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>

src/core/SkPaintParamsKey.cpp

@@ -79,8 +79,6 @@ void SkPaintParamsKeyBuilder::beginBlock(int codeSnippetID) {
     fStack.back().fCurDataPayloadEntry = 0;
     fStack.back().fNumExpectedChildren = snippet->fNumChildren;
     fStack.back().fNumActualChildren = 0;
-    fStack.back().fNumExpectedPointers = snippet->fNumPointers;
-    fStack.back().fNumActualPointers = 0;
 #endif
 }
 
@@ -100,7 +98,6 @@ void SkPaintParamsKeyBuilder::endBlock() {
     SkASSERT(fStack.back().fCurDataPayloadEntry ==
              SkTo<int>(fStack.back().fDataPayloadExpectations.size()));
     SkASSERT(fStack.back().fNumActualChildren == fStack.back().fNumExpectedChildren);
-    SkASSERT(fStack.back().fNumActualPointers == fStack.back().fNumExpectedPointers);
     SkASSERT(!this->isLocked());
 
     int headerOffset = fStack.back().fHeaderOffset;
@@ -188,12 +185,6 @@ void SkPaintParamsKeyBuilder::addPointer(const void* ptr) {
         return;
     }
 
-#ifdef SK_DEBUG
-    StackFrame& frame = fStack.back();
-    SkASSERT(frame.fNumActualPointers < frame.fNumExpectedPointers);
-    frame.fNumActualPointers++;
-#endif
-
     SkDEBUGCODE(this->checkExpectations(SkPaintParamsKey::DataPayloadType::kPointerIndex, 1);)
     SkASSERT(!this->isLocked());
     SkASSERT(fPointerData.size() <= 0xFF);

src/core/SkPaintParamsKey.h

@@ -264,8 +264,6 @@ private:
         int fCurDataPayloadEntry = 0;
         int fNumExpectedChildren = 0;
         int fNumActualChildren = 0;
-        int fNumExpectedPointers = 0;
-        int fNumActualPointers = 0;
 #endif
     };
 

src/core/SkShaderCodeDictionary.cpp

@@ -643,13 +643,11 @@ bool SkShaderCodeDictionary::isValidID(int snippetID) const {
 }
 
 static constexpr int kNoChildren = 0;
-static constexpr int kNoPointers = 0;
 
 // TODO: this version needs to be removed
 int SkShaderCodeDictionary::addUserDefinedSnippet(
         const char* name,
-        SkSpan<const DataPayloadField> dataPayloadExpectations,
-        int numPointers) {
+        SkSpan<const DataPayloadField> dataPayloadExpectations) {
 
     std::unique_ptr<SkShaderSnippet> entry(new SkShaderSnippet("UserDefined",
                                                                {}, // no uniforms
@@ -658,7 +656,6 @@ int SkShaderCodeDictionary::addUserDefinedSnippet(
                                                                name,
                                                                GenerateDefaultGlueCode,
                                                                kNoChildren,
-                                                               numPointers,
                                                                dataPayloadExpectations));
 
     // TODO: the memory for user-defined entries could go in the dictionary's arena but that
@@ -686,11 +683,8 @@ SkBlenderID SkShaderCodeDictionary::addUserDefinedBlender(sk_sp<SkRuntimeEffect>
                                                                "foo",
                                                                GenerateDefaultGlueCode,
                                                                kNoChildren,
-                                                               kNoPointers,
                                                                /*dataPayloadExpectations=*/{}));
 
-    // TODO(skia:13428): set `numPointers` to 1 here once actual pointer support is ready.
-
     // TODO: the memory for user-defined entries could go in the dictionary's arena but that
     // would have to be a thread safe allocation since the arena also stores entries for
     // 'fHash' and 'fEntryVector'
@@ -711,7 +705,6 @@ SkShaderCodeDictionary::SkShaderCodeDictionary() {
             kErrorName,
             GenerateDefaultGlueCode,
             kNoChildren,
-            kNoPointers,
             { }
     };
     fBuiltInCodeSnippets[(int) SkBuiltInCodeSnippetID::kSolidColorShader] = {
@@ -722,7 +715,6 @@ SkShaderCodeDictionary::SkShaderCodeDictionary() {
             kSolidShaderName,
             GenerateDefaultGlueCode,
             kNoChildren,
-            kNoPointers,
             { }
     };
     fBuiltInCodeSnippets[(int) SkBuiltInCodeSnippetID::kLinearGradientShader4] = {
@@ -733,7 +725,6 @@ SkShaderCodeDictionary::SkShaderCodeDictionary() {
             kLinearGradient4Name,
             GenerateDefaultGlueCode,
             kNoChildren,
-            kNoPointers,
             { }
     };
     fBuiltInCodeSnippets[(int) SkBuiltInCodeSnippetID::kLinearGradientShader8] = {
@@ -744,7 +735,6 @@ SkShaderCodeDictionary::SkShaderCodeDictionary() {
             kLinearGradient8Name,
             GenerateDefaultGlueCode,
             kNoChildren,
-            kNoPointers,
             { }
     };
     fBuiltInCodeSnippets[(int) SkBuiltInCodeSnippetID::kRadialGradientShader4] = {
@@ -755,7 +745,6 @@ SkShaderCodeDictionary::SkShaderCodeDictionary() {
             kRadialGradient4Name,
             GenerateDefaultGlueCode,
             kNoChildren,
-            kNoPointers,
             { }
     };
     fBuiltInCodeSnippets[(int) SkBuiltInCodeSnippetID::kRadialGradientShader8] = {
@@ -766,7 +755,6 @@ SkShaderCodeDictionary::SkShaderCodeDictionary() {
             kRadialGradient8Name,
             GenerateDefaultGlueCode,
             kNoChildren,
-            kNoPointers,
             { }
     };
     fBuiltInCodeSnippets[(int) SkBuiltInCodeSnippetID::kSweepGradientShader4] = {
@@ -777,7 +765,6 @@ SkShaderCodeDictionary::SkShaderCodeDictionary() {
             kSweepGradient4Name,
             GenerateDefaultGlueCode,
             kNoChildren,
-            kNoPointers,
             { }
     };
     fBuiltInCodeSnippets[(int) SkBuiltInCodeSnippetID::kSweepGradientShader8] = {
@@ -788,7 +775,6 @@ SkShaderCodeDictionary::SkShaderCodeDictionary() {
             kSweepGradient8Name,
             GenerateDefaultGlueCode,
             kNoChildren,
-            kNoPointers,
             { }
     };
     fBuiltInCodeSnippets[(int) SkBuiltInCodeSnippetID::kConicalGradientShader4] = {
@@ -799,7 +785,6 @@ SkShaderCodeDictionary::SkShaderCodeDictionary() {
             kConicalGradient4Name,
             GenerateDefaultGlueCode,
             kNoChildren,
-            kNoPointers,
             { }
     };
     fBuiltInCodeSnippets[(int) SkBuiltInCodeSnippetID::kConicalGradientShader8] = {
@@ -810,7 +795,6 @@ SkShaderCodeDictionary::SkShaderCodeDictionary() {
             kConicalGradient8Name,
             GenerateDefaultGlueCode,
             kNoChildren,
-            kNoPointers,
             { }
     };
     fBuiltInCodeSnippets[(int) SkBuiltInCodeSnippetID::kLocalMatrixShader] = {
@@ -821,7 +805,6 @@ SkShaderCodeDictionary::SkShaderCodeDictionary() {
             kLocalMatrixShaderName,
             GenerateDefaultGlueCode,
             kNumLocalMatrixShaderChildren,
-            kNoPointers,
             { }
     };
     fBuiltInCodeSnippets[(int) SkBuiltInCodeSnippetID::kImageShader] = {
@@ -832,7 +815,6 @@ SkShaderCodeDictionary::SkShaderCodeDictionary() {
             kImageShaderName,
             GenerateImageShaderGlueCode,
             kNoChildren,
-            kNoPointers,
             { }
     };
     fBuiltInCodeSnippets[(int) SkBuiltInCodeSnippetID::kBlendShader] = {
@@ -843,7 +825,6 @@ SkShaderCodeDictionary::SkShaderCodeDictionary() {
             kBlendShaderName,
             GenerateDefaultGlueCode,
             kNumBlendShaderChildren,
-            kNoPointers,
             { }
     };
     fBuiltInCodeSnippets[(int) SkBuiltInCodeSnippetID::kRuntimeShader] = {
@@ -854,7 +835,6 @@ SkShaderCodeDictionary::SkShaderCodeDictionary() {
             kRuntimeShaderName,
             GenerateDefaultGlueCode,
             kNoChildren,
-            /*numPointers=*/1,
             SkSpan(kRuntimeShaderDataPayload)
     };
     fBuiltInCodeSnippets[(int) SkBuiltInCodeSnippetID::kFixedFunctionBlender] = {
@@ -865,7 +845,6 @@ SkShaderCodeDictionary::SkShaderCodeDictionary() {
             "FF-blending",  // fixed function blending doesn't use static SkSL
             GenerateFixedFunctionBlenderGlueCode,
             kNoChildren,
-            kNoPointers,
             { }
     };
     fBuiltInCodeSnippets[(int) SkBuiltInCodeSnippetID::kShaderBasedBlender] = {
@@ -876,7 +855,6 @@ SkShaderCodeDictionary::SkShaderCodeDictionary() {
             kBlendHelperName,
             GenerateShaderBasedBlenderGlueCode,
             kNoChildren,
-            kNoPointers,
             { }
     };
 }

src/core/SkShaderCodeDictionary.h

@@ -61,7 +61,6 @@ struct SkShaderSnippet {
                     const char* functionName,
                     GenerateGlueCodeForEntry glueCodeGenerator,
                     int numChildren,
-                    int numPointers,
                     SkSpan<const SkPaintParamsKey::DataPayloadField> dataPayloadExpectations)
             : fName(name)
             , fUniforms(uniforms)
@@ -70,7 +69,6 @@ struct SkShaderSnippet {
             , fStaticFunctionName(functionName)
             , fGlueCodeGenerator(glueCodeGenerator)
             , fNumChildren(numChildren)
-            , fNumPointers(numPointers)
             , fDataPayloadExpectations(dataPayloadExpectations) {}
 
     std::string getMangledUniformName(int uniformIndex, int mangleId) const;
@@ -86,7 +84,6 @@ struct SkShaderSnippet {
     const char* fStaticFunctionName = nullptr;
     GenerateGlueCodeForEntry fGlueCodeGenerator = nullptr;
     int fNumChildren = 0;
-    int fNumPointers = 0;
     SkSpan<const SkPaintParamsKey::DataPayloadField> fDataPayloadExpectations;
 };
 
@@ -202,8 +199,7 @@ public:
     // It returns the code snippet ID to use to identify the supplied user-defined code
     // TODO: add hooks for user to actually provide code.
     int addUserDefinedSnippet(const char* name,
-                              SkSpan<const SkPaintParamsKey::DataPayloadField> expectations,
-                              int numPointers = 0);
+                              SkSpan<const SkPaintParamsKey::DataPayloadField> expectations);
 
     SkBlenderID addUserDefinedBlender(sk_sp<SkRuntimeEffect>);
 

tests/graphite/KeyTest.cpp

@@ -158,7 +158,7 @@ DEF_GRAPHITE_TEST_FOR_CONTEXTS(KeyEqualityDoesNotCheckPointers, reporter, contex
             {"ptrIndex", SkPaintParamsKey::DataPayloadType::kPointerIndex, 1},
     };
 
-    int userSnippetID = dict->addUserDefinedSnippet("key", SkSpan(kDataFields), /*numPointers=*/1);
+    int userSnippetID = dict->addUserDefinedSnippet("key", SkSpan(kDataFields));
 
     static constexpr uint8_t kData[kBlockDataSize] = {1, 2, 3, 4};
     int arbitraryData1 = 1;