Revert of GrFP can express distance vector field req., program builder declares variable for it (patchset #20 id:370001 of https://codereview.chromium.org/2114993002/ )

Reason for revert:
UBSAN says we're reading a bad bool here:
bool usesDistanceVectorField() const { return fUsesDistanceVectorField; }

../../../include/gpu/GrPaint.h:83:51: runtime error: load of value 239, which is not a valid value for type 'bool'
SUMMARY: AddressSanitizer: undefined-behavior ../../../include/gpu/GrPaint.h:83:51 in

Seems likely also the root of Valgrind failure:
https://luci-milo.appspot.com/swarming/task/30522e4f2241cb10

Original issue's description:
> GrFP can express distance vector field req.,  program builder declares variable for it
>
> This update allows fragment processors to require a field of vectors to the nearest edge. This requirement propagates:
>
> - from child FPs to their parent
> - from parent FPs to the GrPaint
> - from GrPaint through the PipelineBuilder into GrPipeline
> - acessed from GrPipeline by GrGLSLProgramBuilder
>
> GrGLSL generates a variable for the distance vector and passes it down to the GeometryProcessor->emitCode() method.
>
> This CL's base is the CL for adding the BevelNormalSource API: https://codereview.chromium.org/2080993002
>
> BUG=skia:
> GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2114993002
>
> Committed: https://skia.googlesource.com/skia/+/4ef6dfa7089c092c67b0d5ec34e89c1e319af196

TBR=egdaniel@google.com,robertphillips@google.com,bsalomon@google.com,dvonbeck@google.com
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=skia:

Review-Url: https://codereview.chromium.org/2201613002

17 files changed, 21 insertions, 136 deletions

diff --git a/src/core/SkNormalBevelSource.cpp b/src/core/SkNormalBevelSource.cpp
index a63e434c3f..feccb0cce8 100644
--- a/src/core/SkNormalBevelSource.cpp
+++ b/src/core/SkNormalBevelSource.cpp
@@ -8,7 +8,6 @@
 #include "SkNormalBevelSource.h"
 
 #include "SkNormalSource.h"
-#include "SkNormalSourcePriv.h"
 #include "SkPoint3.h"
 #include "SkReadBuffer.h"
 #include "SkWriteBuffer.h"
@@ -26,19 +25,17 @@ public:
         , fWidth(width)
         , fHeight(height) {
         this->initClassID<NormalBevelFP>();
-
-        fUsesDistanceVectorField = true;
     }
 
-    class GLSLNormalBevelFP : public GLSLNormalFP {
+    class GLSLNormalBevelFP : public GrGLSLFragmentProcessor {
     public:
         GLSLNormalBevelFP() {
             fPrevWidth = SkFloatToScalar(0.0f);
             fPrevHeight = SkFloatToScalar(0.0f);
         }
 
-        void onEmitCode(EmitArgs& args) override {
-            GrGLSLFPFragmentBuilder* fragBuilder = args.fFragBuilder;
+        void emitCode(EmitArgs& args) override {
+            GrGLSLFragmentBuilder* fragBuilder = args.fFragBuilder;
             GrGLSLUniformHandler* uniformHandler = args.fUniformHandler;
 
             const char* widthUniName = nullptr;
@@ -49,7 +46,7 @@ public:
             fHeightUni = uniformHandler->addUniform(kFragment_GrShaderFlag, kFloat_GrSLType,
                     kDefault_GrSLPrecision, "Height", &heightUniName);
 
-            fragBuilder->codeAppendf("%s = vec4(0.0, 0.0, 1.0, 0.0);", args.fOutputColor);
+            fragBuilder->codeAppendf("%s = vec4(0, 0, 1, 0);", args.fOutputColor);
         }
 
         static void GenKey(const GrProcessor& proc, const GrGLSLCaps&,
@@ -59,8 +56,7 @@ public:
         }
 
     protected:
-        void setNormalData(const GrGLSLProgramDataManager& pdman,
-                           const GrProcessor& proc) override {
+        void onSetData(const GrGLSLProgramDataManager& pdman, const GrProcessor& proc) override {
             const NormalBevelFP& normalBevelFP = proc.cast<NormalBevelFP>();
 
             if (fPrevWidth  != normalBevelFP.fWidth) {
diff --git a/src/core/SkNormalFlatSource.cpp b/src/core/SkNormalFlatSource.cpp
index bdd65b03e9..fcb1a4f3ad 100644
--- a/src/core/SkNormalFlatSource.cpp
+++ b/src/core/SkNormalFlatSource.cpp
@@ -8,7 +8,6 @@
 #include "SkNormalFlatSource.h"
 
 #include "SkNormalSource.h"
-#include "SkNormalSourcePriv.h"
 #include "SkPoint3.h"
 #include "SkReadBuffer.h"
 #include "SkWriteBuffer.h"
@@ -24,12 +23,12 @@ public:
         this->initClassID<NormalFlatFP>();
     }
 
-    class GLSLNormalFlatFP : public GLSLNormalFP {
+    class GLSLNormalFlatFP : public GrGLSLFragmentProcessor {
     public:
         GLSLNormalFlatFP() {}
 
-        void onEmitCode(EmitArgs& args) override {
-            GrGLSLFPFragmentBuilder* fragBuilder = args.fFragBuilder;
+        void emitCode(EmitArgs& args) override {
+            GrGLSLFragmentBuilder* fragBuilder = args.fFragBuilder;
 
             fragBuilder->codeAppendf("%s = vec4(0, 0, 1, 0);", args.fOutputColor);
         }
@@ -40,8 +39,7 @@ public:
         }
 
     protected:
-        void setNormalData(const GrGLSLProgramDataManager& pdman,
-                           const GrProcessor& proc) override {}
+        void onSetData(const GrGLSLProgramDataManager& pdman, const GrProcessor& proc) override {}
     };
 
     void onGetGLSLProcessorKey(const GrGLSLCaps& caps, GrProcessorKeyBuilder* b) const override {
diff --git a/src/core/SkNormalMapSource.cpp b/src/core/SkNormalMapSource.cpp
index c2cda099ab..2ecf1d310d 100644
--- a/src/core/SkNormalMapSource.cpp
+++ b/src/core/SkNormalMapSource.cpp
@@ -10,7 +10,6 @@
 #include "SkLightingShader.h"
 #include "SkMatrix.h"
 #include "SkNormalSource.h"
-#include "SkNormalSourcePriv.h"
 #include "SkPM4f.h"
 #include "SkReadBuffer.h"
 #include "SkWriteBuffer.h"
@@ -32,13 +31,13 @@ public:
         this->initClassID<NormalMapFP>();
     }
 
-    class GLSLNormalMapFP : public GLSLNormalFP {
+    class GLSLNormalMapFP : public GrGLSLFragmentProcessor {
     public:
         GLSLNormalMapFP()
             : fColumnMajorInvCTM22{0.0f} {}
 
-        void onEmitCode(EmitArgs& args) override {
-            GrGLSLFPFragmentBuilder* fragBuilder = args.fFragBuilder;
+        void emitCode(EmitArgs& args) override {
+            GrGLSLFragmentBuilder* fragBuilder = args.fFragBuilder;
             GrGLSLUniformHandler* uniformHandler = args.fUniformHandler;
 
             // add uniform
@@ -79,8 +78,7 @@ public:
         }
 
     protected:
-        void setNormalData(const GrGLSLProgramDataManager& pdman,
-                           const GrProcessor& proc) override {
+        void onSetData(const GrGLSLProgramDataManager& pdman, const GrProcessor& proc) override {
             const NormalMapFP& normalMapFP = proc.cast<NormalMapFP>();
 
             const SkMatrix& invCTM = normalMapFP.invCTM();
diff --git a/src/core/SkNormalSource.h b/src/core/SkNormalSource.h
index 4b09d0b411..8cbb3e4fae 100644
--- a/src/core/SkNormalSource.h
+++ b/src/core/SkNormalSource.h
@@ -111,7 +111,7 @@ public:
     /** Returns a normal source that generates a bevel for the given shape. UNIMPLEMENTED: Will
         return straight-up normals only.
 
-        @param  type   the type of bevel to add.
+        @param  type   the type of bevel to add
         @param  width  the width of the bevel, in source space. Must be positive.
         @param  height the height of the plateau, in source space. Can be positive, negative,
                        or zero. A negative height means the simulated bevels slope downwards.
diff --git a/src/core/SkNormalSourcePriv.h b/src/core/SkNormalSourcePriv.h
deleted file mode 100644
index ce8baf61e4..0000000000
--- a/src/core/SkNormalSourcePriv.h
+++ /dev/null
@@ -1,57 +0,0 @@
-/*
- * Copyright 2016 Google Inc.
- *
- * Use of this source code is governed by a BSD-style license that can be
- * found in the LICENSE file.
- */
-
-#ifndef SkNormalSourcePriv_DEFINED
-#define SkNormalSourcePriv_DEFINED
-
-#if SK_SUPPORT_GPU
-#include "glsl/GrGLSLFragmentProcessor.h"
-#include "glsl/GrGLSLFragmentShaderBuilder.h"
-
-/* GLSLFragmentProcessors for NormalSourceImpls must sub-class this class and override onEmitCode,
- * and setNormalData calls, as well as all other calls FPs normally override, except for the 2
- * defined in this superclass.
- * This class exists to intercept emitCode calls and emit <0, 0, 1> if the FP requires a distance
- * vector but the GP doesn't provide it. onSetData calls need to be intercepted too because
- * uniform handlers will be invalid in subclasses where onEmitCode isn't called.
- * We don't need to adjust the key here since the use of a given GP (through its class ID already in
- * the key), will determine what code gets emitted here.
- */
-class GLSLNormalFP : public GrGLSLFragmentProcessor {
-public:
-    GLSLNormalFP()
-        : fDidIntercept(false) {}
-
-    void emitCode(EmitArgs& args) final override {
-        if (args.fFp.usesDistanceVectorField() && !args.fGpImplementsDistanceVector) {
-            GrGLSLFPFragmentBuilder* fragBuilder = args.fFragBuilder;
-            fragBuilder->codeAppendf("// GLSLNormalFP intercepted emitCode call, GP does not "
-                                             "implement required distance vector feature\n");
-            fragBuilder->codeAppendf("%s = vec4(0, 0, 1, 0);", args.fOutputColor);
-
-            fDidIntercept = true;
-        } else {
-            this->onEmitCode(args);
-        }
-    }
-
-    void onSetData(const GrGLSLProgramDataManager& pdman, const GrProcessor& proc) final override {
-        if (!fDidIntercept) {
-            this->setNormalData(pdman, proc);
-        }
-    }
-
-protected:
-    virtual void onEmitCode(EmitArgs& args) = 0;
-    virtual void setNormalData(const GrGLSLProgramDataManager& pdman, const GrProcessor& proc) = 0;
-
-private:
-    bool fDidIntercept;
-};
-#endif
-
-#endif
diff --git a/src/gpu/GrFragmentProcessor.cpp b/src/gpu/GrFragmentProcessor.cpp
index a8a4898e78..fdba610356 100644
--- a/src/gpu/GrFragmentProcessor.cpp
+++ b/src/gpu/GrFragmentProcessor.cpp
@@ -106,9 +106,6 @@ int GrFragmentProcessor::registerChildProcessor(sk_sp<GrFragmentProcessor> child
     if (child->usesLocalCoords()) {
         fUsesLocalCoords = true;
     }
-    if (child->usesDistanceVectorField()) {
-        fUsesDistanceVectorField = true;
-    }
 
     int index = fChildProcessors.count();
     fChildProcessors.push_back(child.release());
diff --git a/src/gpu/GrPipeline.cpp b/src/gpu/GrPipeline.cpp
index c60a94be7e..7767bf220d 100644
--- a/src/gpu/GrPipeline.cpp
+++ b/src/gpu/GrPipeline.cpp
@@ -48,9 +48,6 @@ GrPipeline* GrPipeline::CreateAt(void* memory, const CreateArgs& args,
     if (builder.getAllowSRGBInputs()) {
         pipeline->fFlags |= kAllowSRGBInputs_Flag;
     }
-    if (builder.getUsesDistanceVectorField()) {
-        pipeline->fFlags |= kUsesDistanceVectorField_Flag;
-    }
     if (args.fHasStencilClip) {
         pipeline->fFlags |= kHasStencilClip_Flag;
     }
diff --git a/src/gpu/GrPipeline.h b/src/gpu/GrPipeline.h
index 1457dbc3f5..5ec572d0ae 100644
--- a/src/gpu/GrPipeline.h
+++ b/src/gpu/GrPipeline.h
@@ -160,9 +160,6 @@ public:
     bool getAllowSRGBInputs() const {
         return SkToBool(fFlags & kAllowSRGBInputs_Flag);
     }
-    bool usesDistanceVectorField() const {
-        return SkToBool(fFlags & kUsesDistanceVectorField_Flag);
-    }
     bool hasStencilClip() const {
         return SkToBool(fFlags & kHasStencilClip_Flag);
     }
@@ -209,8 +206,7 @@ private:
         kSnapVertices_Flag                  = 0x2,
         kDisableOutputConversionToSRGB_Flag = 0x4,
         kAllowSRGBInputs_Flag               = 0x8,
-        kUsesDistanceVectorField_Flag       = 0x10,
-        kHasStencilClip_Flag                = 0x20,
+        kHasStencilClip_Flag                = 0x10
     };
 
     typedef GrPendingIOResource<GrRenderTarget, kWrite_GrIOType> RenderTarget;
diff --git a/src/gpu/GrPipelineBuilder.cpp b/src/gpu/GrPipelineBuilder.cpp
index 864d6f1bc6..60cbc62cf2 100644
--- a/src/gpu/GrPipelineBuilder.cpp
+++ b/src/gpu/GrPipelineBuilder.cpp
@@ -41,8 +41,6 @@ GrPipelineBuilder::GrPipelineBuilder(const GrPaint& paint, bool useHWAA)
                    paint.getDisableOutputConversionToSRGB());
     this->setState(GrPipelineBuilder::kAllowSRGBInputs_Flag,
                    paint.getAllowSRGBInputs());
-    this->setState(GrPipelineBuilder::kUsesDistanceVectorField_Flag,
-                   paint.usesDistanceVectorField());
 }
 
 //////////////////////////////////////////////////////////////////////////////s
diff --git a/src/gpu/GrPipelineBuilder.h b/src/gpu/GrPipelineBuilder.h
index 0c33eb344f..dca7f36034 100644
--- a/src/gpu/GrPipelineBuilder.h
+++ b/src/gpu/GrPipelineBuilder.h
@@ -221,13 +221,7 @@ public:
          */
         kAllowSRGBInputs_Flag = 0x08,
 
-        /**
-         * Signals that one or more FPs need access to the distance vector field to the nearest
-         * edge
-         */
-        kUsesDistanceVectorField_Flag = 0x10,
-
-        kLast_Flag = kUsesDistanceVectorField_Flag,
+        kLast_Flag = kAllowSRGBInputs_Flag,
     };
 
     bool isHWAntialias() const { return SkToBool(fFlags & kHWAntialias_Flag); }
@@ -237,8 +231,6 @@ public:
         return SkToBool(fFlags & kDisableOutputConversionToSRGB_Flag); }
     bool getAllowSRGBInputs() const {
         return SkToBool(fFlags & kAllowSRGBInputs_Flag); }
-    bool getUsesDistanceVectorField() const {
-        return SkToBool(fFlags & kUsesDistanceVectorField_Flag); }
 
     /**
      * Enable render state settings.
diff --git a/src/gpu/GrPrimitiveProcessor.h b/src/gpu/GrPrimitiveProcessor.h
index d27a6f31c7..0d5280fbd3 100644
--- a/src/gpu/GrPrimitiveProcessor.h
+++ b/src/gpu/GrPrimitiveProcessor.h
@@ -221,10 +221,6 @@ public:
         return 0.0;
     }
 
-    /* Sub-class should override and return true if this primitive processor implements the distance
-     * vector field, a field of vectors to the nearest point in the edge of the shape.  */
-    virtual bool implementsDistanceVector() const { return false; }
-
 protected:
     GrPrimitiveProcessor() : fVertexStride(0) {}
 
diff --git a/src/gpu/glsl/GrGLSLFragmentProcessor.cpp b/src/gpu/glsl/GrGLSLFragmentProcessor.cpp
index 46945bdc6b..9533d5a8cb 100644
--- a/src/gpu/glsl/GrGLSLFragmentProcessor.cpp
+++ b/src/gpu/glsl/GrGLSLFragmentProcessor.cpp
@@ -107,8 +107,7 @@ void GrGLSLFragmentProcessor::internalEmitChild(int childIndex, const char* inpu
                        inputColor,
                        childCoords,
                        childTexSamplers,
-                       childBufferSamplers,
-                       args.fGpImplementsDistanceVector);
+                       childBufferSamplers);
     this->childProcessor(childIndex)->emitCode(childArgs);
     fragBuilder->codeAppend("}\n");
 
diff --git a/src/gpu/glsl/GrGLSLFragmentProcessor.h b/src/gpu/glsl/GrGLSLFragmentProcessor.h
index c6eb016a32..43614f2279 100644
--- a/src/gpu/glsl/GrGLSLFragmentProcessor.h
+++ b/src/gpu/glsl/GrGLSLFragmentProcessor.h
@@ -61,8 +61,7 @@ public:
                  const char* inputColor,
                  const GrGLSLTransformedCoordsArray& coords,
                  const SamplerHandle* texSamplers,
-                 const SamplerHandle* bufferSamplers,
-                 bool gpImplementsDistanceVector)
+                 const SamplerHandle* bufferSamplers)
             : fFragBuilder(fragBuilder)
             , fUniformHandler(uniformHandler)
             , fGLSLCaps(caps)
@@ -71,8 +70,7 @@ public:
             , fInputColor(inputColor)
             , fCoords(coords)
             , fTexSamplers(texSamplers)
-            , fBufferSamplers(bufferSamplers)
-            , fGpImplementsDistanceVector(gpImplementsDistanceVector){}
+            , fBufferSamplers(bufferSamplers) {}
         GrGLSLFPFragmentBuilder* fFragBuilder;
         GrGLSLUniformHandler* fUniformHandler;
         const GrGLSLCaps* fGLSLCaps;
@@ -82,7 +80,6 @@ public:
         const GrGLSLTransformedCoordsArray& fCoords;
         const SamplerHandle* fTexSamplers;
         const SamplerHandle* fBufferSamplers;
-        bool fGpImplementsDistanceVector;
     };
 
     virtual void emitCode(EmitArgs&) = 0;
diff --git a/src/gpu/glsl/GrGLSLFragmentShaderBuilder.cpp b/src/gpu/glsl/GrGLSLFragmentShaderBuilder.cpp
index e6ecf8c171..5d1ba511b7 100644
--- a/src/gpu/glsl/GrGLSLFragmentShaderBuilder.cpp
+++ b/src/gpu/glsl/GrGLSLFragmentShaderBuilder.cpp
@@ -189,10 +189,6 @@ const char* GrGLSLFragmentShaderBuilder::fragmentPosition() {
     }
 }
 
-const char* GrGLSLFragmentShaderBuilder::distanceVectorName() const {
-    return "fsDistanceVector";
-}
-
 void GrGLSLFragmentShaderBuilder::appendOffsetToSample(const char* sampleIdx, Coordinates coords) {
     SkASSERT(fProgramBuilder->header().fSamplePatternKey);
     SkDEBUGCODE(fUsedProcessorFeatures |= GrProcessor::kSampleLocations_RequiredFeature);
diff --git a/src/gpu/glsl/GrGLSLFragmentShaderBuilder.h b/src/gpu/glsl/GrGLSLFragmentShaderBuilder.h
index 6845376261..614b04f54b 100644
--- a/src/gpu/glsl/GrGLSLFragmentShaderBuilder.h
+++ b/src/gpu/glsl/GrGLSLFragmentShaderBuilder.h
@@ -96,10 +96,6 @@ public:
      */
     virtual void maskSampleCoverage(const char* mask, bool invert = false) = 0;
 
-    /** Returns a variable name that represents a vector to the nearest edge of the shape, in source
-        space coordinates. */
-    virtual const char* distanceVectorName() const = 0;
-
     /**
      * Fragment procs with child procs should call these functions before/after calling emitCode
      * on a child proc.
@@ -170,7 +166,6 @@ public:
     virtual SkString ensureFSCoords2D(const GrGLSLTransformedCoordsArray& coords,
                                       int index) override;
     const char* fragmentPosition() override;
-    const char* distanceVectorName() const override;
 
     // GrGLSLFPFragmentBuilder interface.
     void appendOffsetToSample(const char* sampleIdx, Coordinates) override;
@@ -240,7 +235,6 @@ private:
     bool       fHasSecondaryOutput;
     uint8_t    fUsedSampleOffsetArrays;
     bool       fHasInitializedSampleMask;
-    SkString   fDistanceVectorOutput;
 
 #ifdef SK_DEBUG
     // some state to verify shaders and effects are consistent, this is reset between effects by
diff --git a/src/gpu/glsl/GrGLSLPrimitiveProcessor.h b/src/gpu/glsl/GrGLSLPrimitiveProcessor.h
index a94099656b..d726027686 100644
--- a/src/gpu/glsl/GrGLSLPrimitiveProcessor.h
+++ b/src/gpu/glsl/GrGLSLPrimitiveProcessor.h
@@ -42,7 +42,6 @@ public:
                  const GrPrimitiveProcessor& gp,
                  const char* outputColor,
                  const char* outputCoverage,
-                 const char* distanceVectorName,
                  const SamplerHandle* texSamplers,
                  const SamplerHandle* bufferSamplers,
                  const TransformsIn& transformsIn,
@@ -55,7 +54,6 @@ public:
             , fGP(gp)
             , fOutputColor(outputColor)
             , fOutputCoverage(outputCoverage)
-            , fDistanceVectorName(distanceVectorName)
             , fTexSamplers(texSamplers)
             , fBufferSamplers(bufferSamplers)
             , fTransformsIn(transformsIn)
@@ -68,7 +66,6 @@ public:
         const GrPrimitiveProcessor& fGP;
         const char* fOutputColor;
         const char* fOutputCoverage;
-        const char* fDistanceVectorName;
         const SamplerHandle* fTexSamplers;
         const SamplerHandle* fBufferSamplers;
         const TransformsIn& fTransformsIn;
@@ -81,6 +78,7 @@ public:
      */
     virtual void emitCode(EmitArgs&) = 0;
 
+
     /** A GrGLSLPrimitiveProcessor instance can be reused with any GrGLSLPrimitiveProcessor that
         produces the same stage key; this function reads data from a GrGLSLPrimitiveProcessor and
         uploads any uniform variables required  by the shaders created in emitCode(). The
diff --git a/src/gpu/glsl/GrGLSLProgramBuilder.cpp b/src/gpu/glsl/GrGLSLProgramBuilder.cpp
index 1f28f19bb9..64bc0640b9 100644
--- a/src/gpu/glsl/GrGLSLProgramBuilder.cpp
+++ b/src/gpu/glsl/GrGLSLProgramBuilder.cpp
@@ -87,13 +87,6 @@ void GrGLSLProgramBuilder::emitAndInstallPrimProc(const GrPrimitiveProcessor& pr
     this->nameExpression(outputColor, "outputColor");
     this->nameExpression(outputCoverage, "outputCoverage");
 
-    const char* distanceVectorName = nullptr;
-    if (this->fPipeline.usesDistanceVectorField() && proc.implementsDistanceVector()) {
-        distanceVectorName = fFS.distanceVectorName();
-        fFS.codeAppend( "// Un-normalized vector to the closed geometric edge (in source space)\n");
-        fFS.codeAppendf("vec2 %s;", distanceVectorName);
-    }
-
     // Enclose custom code in a block to avoid namespace conflicts
     SkString openBrace;
     openBrace.printf("{ // Stage %d, %s\n", fStageIndex, proc.name());
@@ -115,7 +108,6 @@ void GrGLSLProgramBuilder::emitAndInstallPrimProc(const GrPrimitiveProcessor& pr
                                            proc,
                                            outputColor->c_str(),
                                            outputCoverage->c_str(),
-                                           distanceVectorName,
                                            texSamplers.begin(),
                                            bufferSamplers.begin(),
                                            fCoordTransforms,
@@ -169,9 +161,7 @@ void GrGLSLProgramBuilder::emitAndInstallFragProc(const GrFragmentProcessor& fp,
                                            input.isOnes() ? nullptr : input.c_str(),
                                            fOutCoords[index],
                                            texSamplers.begin(),
-                                           bufferSamplers.begin(),
-                                           this->primitiveProcessor().implementsDistanceVector());
-
+                                           bufferSamplers.begin());
     fragProc->emitCode(args);
 
     // We have to check that effects and the code they emit are consistent, ie if an effect