Revert "Add a requiresVertexShader method to GrGLEffect"

This reverts commit 1a30a3af805b7ea688d4a0f0bfe373c204085a27. We're
going to take a different direction for vertexless shaders.

R=bsalomon@google.com

Author: cdalton@nvidia.com

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

git-svn-id: http://skia.googlecode.com/svn/trunk@11521 2bbb7eff-a529-9590-31e7-b0007b416f81

6 files changed, 64 insertions, 103 deletions

diff --git a/src/gpu/gl/GrGLEffect.h b/src/gpu/gl/GrGLEffect.h
index 2d7258e026..5df22811c4 100644
--- a/src/gpu/gl/GrGLEffect.h
+++ b/src/gpu/gl/GrGLEffect.h
@@ -51,13 +51,6 @@ public:
 
     virtual ~GrGLEffect();
 
-    /** Called when GrGLProgram is about to create its GrGLShaderBuilder. When possible, effects
-        should handle programs that don't have a vertex shader. But if an effect requires special
-        vertex processing that can't be accomplished with the fixed pipeline, it can override this
-        method and return true to guarantee the GrGLShaderBuilder in emitCode has a VertexBuilder.
-        */
-    virtual bool requiresVertexShader(const GrDrawEffect&) const { return false; }
-
     /** Called when the program stage should insert its code into the shaders. The code in each
         shader will be in its own block ({}) and so locally scoped names will not collide across
         stages.
diff --git a/src/gpu/gl/GrGLEffectMatrix.cpp b/src/gpu/gl/GrGLEffectMatrix.cpp
index 512bef96e7..880df12acf 100644
--- a/src/gpu/gl/GrGLEffectMatrix.cpp
+++ b/src/gpu/gl/GrGLEffectMatrix.cpp
@@ -48,7 +48,6 @@ GrSLType GrGLEffectMatrix::emitCode(GrGLShaderBuilder* builder,
                                     SkString* fsCoordName,
                                     SkString* vsCoordName,
                                     const char* suffix) {
-    // TODO: Handle vertexless shaders here before we start enabling them.
     GrGLShaderBuilder::VertexBuilder* vertexBuilder = builder->getVertexBuilder();
     SkASSERT(NULL != vertexBuilder);
 
diff --git a/src/gpu/gl/GrGLProgram.cpp b/src/gpu/gl/GrGLProgram.cpp
index ebc57827c5..ac9794d4d5 100644
--- a/src/gpu/gl/GrGLProgram.cpp
+++ b/src/gpu/gl/GrGLProgram.cpp
@@ -446,32 +446,10 @@ bool GrGLProgram::genProgram(const GrEffectStage* colorStages[],
     SkASSERT(0 == fProgramID);
 
     const GrGLProgramDesc::KeyHeader& header = fDesc.getHeader();
-    bool hasExplicitLocalCoords = -1 != header.fLocalCoordAttributeIndex;
 
-    // Get the coeffs for the Mode-based color filter, determine if color is needed.
-    SkXfermode::Coeff colorCoeff;
-    SkXfermode::Coeff filterColorCoeff;
-    SkAssertResult(
-        SkXfermode::ModeAsCoeff(static_cast<SkXfermode::Mode>(header.fColorFilterXfermode),
-                                &filterColorCoeff,
-                                &colorCoeff));
-    bool needColor, needFilterColor;
-    need_blend_inputs(filterColorCoeff, colorCoeff, &needFilterColor, &needColor);
+    bool needsVertexShader = true;
 
-    // Create the GL effects.
-    bool hasVertexShaderEffects = false;
-
-    SkTArray<GrDrawEffect> colorDrawEffects(needColor ? fDesc.numColorEffects() : 0);
-    if (needColor) {
-        this->buildGLEffects(&GrGLProgram::fColorEffects, colorStages, fDesc.numColorEffects(),
-                             hasExplicitLocalCoords, &colorDrawEffects, &hasVertexShaderEffects);
-    }
-
-    SkTArray<GrDrawEffect> coverageDrawEffects(fDesc.numCoverageEffects());
-    this->buildGLEffects(&GrGLProgram::fCoverageEffects, coverageStages, fDesc.numCoverageEffects(),
-                         hasExplicitLocalCoords, &coverageDrawEffects, &hasVertexShaderEffects);
-
-    GrGLShaderBuilder builder(fGpu->ctxInfo(), fUniformManager, fDesc, hasVertexShaderEffects);
+    GrGLShaderBuilder builder(fGpu->ctxInfo(), fUniformManager, fDesc, needsVertexShader);
 
     if (GrGLShaderBuilder::VertexBuilder* vertexBuilder = builder.getVertexBuilder()) {
         const char* viewMName;
@@ -508,6 +486,16 @@ bool GrGLProgram::genProgram(const GrEffectStage* colorStages[],
     SkString inColor;
     GrSLConstantVec knownColorValue = this->genInputColor(&builder, &inColor);
 
+    // Get the coeffs for the Mode-based color filter, determine if color is needed.
+    SkXfermode::Coeff colorCoeff;
+    SkXfermode::Coeff filterColorCoeff;
+    SkAssertResult(
+        SkXfermode::ModeAsCoeff(static_cast<SkXfermode::Mode>(header.fColorFilterXfermode),
+                                &filterColorCoeff,
+                                &colorCoeff));
+    bool needColor, needFilterColor;
+    need_blend_inputs(filterColorCoeff, colorCoeff, &needFilterColor, &needColor);
+
     // used in order for builder to return the per-stage uniform handles.
     typedef SkTArray<GrGLUniformManager::UniformHandle, true>* UniHandleArrayPtr;
     int maxColorOrCovEffectCnt = GrMax(fDesc.numColorEffects(), fDesc.numCoverageEffects());
@@ -516,17 +504,20 @@ bool GrGLProgram::genProgram(const GrEffectStage* colorStages[],
 
     if (needColor) {
         for (int e = 0; e < fDesc.numColorEffects(); ++e) {
-            glEffects[e] = fColorEffects[e].fGLEffect;
             effectUniformArrays[e] = &fColorEffects[e].fSamplerUnis;
         }
 
-        builder.emitEffects(glEffects.get(),
-                            colorDrawEffects.begin(),
+        builder.emitEffects(colorStages,
                             fDesc.effectKeys(),
                             fDesc.numColorEffects(),
                             &inColor,
                             &knownColorValue,
-                            effectUniformArrays.get());
+                            effectUniformArrays.get(),
+                            glEffects.get());
+
+        for (int e = 0; e < fDesc.numColorEffects(); ++e) {
+            fColorEffects[e].fGLEffect = glEffects[e];
+        }
     }
 
     // Insert the color filter. This will soon be replaced by a color effect.
@@ -557,17 +548,19 @@ bool GrGLProgram::genProgram(const GrEffectStage* colorStages[],
     GrSLConstantVec knownCoverageValue = this->genInputCoverage(&builder, &inCoverage);
 
     for (int e = 0; e < fDesc.numCoverageEffects(); ++e) {
-        glEffects[e] = fCoverageEffects[e].fGLEffect;
         effectUniformArrays[e] = &fCoverageEffects[e].fSamplerUnis;
     }
 
-    builder.emitEffects(glEffects.get(),
-                        coverageDrawEffects.begin(),
+    builder.emitEffects(coverageStages,
                         fDesc.getEffectKeys() + fDesc.numColorEffects(),
                         fDesc.numCoverageEffects(),
                         &inCoverage,
                         &knownCoverageValue,
-                        effectUniformArrays.get());
+                        effectUniformArrays.get(),
+                        glEffects.get());
+    for (int e = 0; e < fDesc.numCoverageEffects(); ++e) {
+        fCoverageEffects[e].fGLEffect = glEffects[e];
+    }
 
     // discard if coverage is zero
     if (header.fDiscardIfZeroCoverage && kOnes_GrSLConstantVec != knownCoverageValue) {
@@ -696,28 +689,6 @@ bool GrGLProgram::genProgram(const GrEffectStage* colorStages[],
     return true;
 }
 
-void GrGLProgram::buildGLEffects(SkTArray<EffectAndSamplers> GrGLProgram::* effectSet,
-                                 const GrEffectStage* stages[],
-                                 int count,
-                                 bool hasExplicitLocalCoords,
-                                 SkTArray<GrDrawEffect>* drawEffects,
-                                 bool* hasVertexShaderEffects) {
-    for (int e = 0; e < count; ++e) {
-        SkASSERT(NULL != stages[e] && NULL != stages[e]->getEffect());
-
-        const GrEffectStage& stage = *stages[e];
-        SkNEW_APPEND_TO_TARRAY(drawEffects, GrDrawEffect, (stage, hasExplicitLocalCoords));
-
-        const GrDrawEffect& drawEffect = (*drawEffects)[e];
-        GrGLEffect* effect = (this->*effectSet)[e].fGLEffect =
-            (*stage.getEffect())->getFactory().createGLInstance(drawEffect);
-
-        if (!*hasVertexShaderEffects && effect->requiresVertexShader(drawEffect)) {
-            *hasVertexShaderEffects = true;
-        }
-    }
-}
-
 bool GrGLProgram::bindOutputsAttribsAndLinkProgram(const GrGLShaderBuilder& builder,
                                                    bool bindColorOut,
                                                    bool bindDualSrcOut) {
diff --git a/src/gpu/gl/GrGLProgram.h b/src/gpu/gl/GrGLProgram.h
index b18b8ae31d..09f7668be2 100644
--- a/src/gpu/gl/GrGLProgram.h
+++ b/src/gpu/gl/GrGLProgram.h
@@ -178,14 +178,6 @@ private:
 
     void genGeometryShader(GrGLShaderBuilder::VertexBuilder* vertexBuilder) const;
 
-    // Creates a set of GrGLEffects and GrGLDrawEffects.
-    void buildGLEffects(SkTArray<EffectAndSamplers> GrGLProgram::* effectSet,
-                        const GrEffectStage* stages[],
-                        int count,
-                        bool hasExplicitLocalCoords,
-                        SkTArray<GrDrawEffect>* drawEffects,
-                        bool* hasVertexShaderEffects);
-
     // Creates a GL program ID, binds shader attributes to GL vertex attrs, and links the program
     bool bindOutputsAttribsAndLinkProgram(const GrGLShaderBuilder& builder,
                                           bool bindColorOut,
diff --git a/src/gpu/gl/GrGLShaderBuilder.cpp b/src/gpu/gl/GrGLShaderBuilder.cpp
index f73520c363..d11394a7b3 100644
--- a/src/gpu/gl/GrGLShaderBuilder.cpp
+++ b/src/gpu/gl/GrGLShaderBuilder.cpp
@@ -94,7 +94,7 @@ static const char kDstCopyColorName[] = "_dstColor";
 GrGLShaderBuilder::GrGLShaderBuilder(const GrGLContextInfo& ctxInfo,
                                      GrGLUniformManager& uniformManager,
                                      const GrGLProgramDesc& desc,
-                                     bool hasVertexShaderEffects)
+                                     bool needsVertexShader)
     : fUniforms(kVarsPerBlock)
     , fCtxInfo(ctxInfo)
     , fUniformManager(uniformManager)
@@ -106,8 +106,9 @@ GrGLShaderBuilder::GrGLShaderBuilder(const GrGLContextInfo& ctxInfo,
 
     const GrGLProgramDesc::KeyHeader& header = desc.getHeader();
 
-    // TODO: go vertexless when possible.
-    fVertexBuilder.reset(SkNEW_ARGS(VertexBuilder, (this, desc)));
+    if (needsVertexShader) {
+        fVertexBuilder.reset(SkNEW_ARGS(VertexBuilder, (this, desc)));
+    }
 
     // Emit code to read the dst copy textue if necessary.
     if (kNoDstRead_DstReadKey != header.fDstReadKey &&
@@ -216,7 +217,7 @@ void GrGLShaderBuilder::nameVariable(SkString* out, char prefix, const char* nam
 
 const char* GrGLShaderBuilder::dstColor() {
     if (fCodeStage.inStageCode()) {
-        const GrEffectRef& effect = *fCodeStage.effect();
+        const GrEffectRef& effect = *fCodeStage.effectStage()->getEffect();
         if (!effect->willReadDstColor()) {
             GrDebugCrash("GrGLEffect asked for dst color but its generating GrEffect "
                          "did not request access.");
@@ -362,7 +363,7 @@ GrGLUniformManager::UniformHandle GrGLShaderBuilder::addUniformArray(uint32_t vi
 
 const char* GrGLShaderBuilder::fragmentPosition() {
     if (fCodeStage.inStageCode()) {
-        const GrEffectRef& effect = *fCodeStage.effect();
+        const GrEffectRef& effect = *fCodeStage.effectStage()->getEffect();
         if (!effect->willReadFragmentPosition()) {
             GrDebugCrash("GrGLEffect asked for frag position but its generating GrEffect "
                          "did not request access.");
@@ -499,23 +500,24 @@ void GrGLShaderBuilder::finished(GrGLuint programID) {
 }
 
 void GrGLShaderBuilder::emitEffects(
-                        GrGLEffect* const glEffects[],
-                        const GrDrawEffect drawEffects[],
+                        const GrEffectStage* effectStages[],
                         const GrBackendEffectFactory::EffectKey effectKeys[],
                         int effectCnt,
                         SkString* fsInOutColor,
                         GrSLConstantVec* fsInOutColorKnownValue,
-                        SkTArray<GrGLUniformManager::UniformHandle, true>* effectSamplerHandles[]) {
+                        SkTArray<GrGLUniformManager::UniformHandle, true>* effectSamplerHandles[],
+                        GrGLEffect* glEffects[]) {
     bool effectEmitted = false;
 
     SkString inColor = *fsInOutColor;
     SkString outColor;
 
     for (int e = 0; e < effectCnt; ++e) {
-        const GrDrawEffect& drawEffect = drawEffects[e];
-        const GrEffectRef& effect = *drawEffect.effect();
+        SkASSERT(NULL != effectStages[e] && NULL != effectStages[e]->getEffect());
+        const GrEffectStage& stage = *effectStages[e];
+        const GrEffectRef& effect = *stage.getEffect();
 
-        CodeStage::AutoStageRestore csar(&fCodeStage, &effect);
+        CodeStage::AutoStageRestore csar(&fCodeStage, &stage);
 
         int numTextures = effect->numTextures();
         SkSTArray<8, GrGLShaderBuilder::TextureSampler> textureSamplers;
@@ -524,9 +526,11 @@ void GrGLShaderBuilder::emitEffects(
             textureSamplers[t].init(this, &effect->textureAccess(t), t);
             effectSamplerHandles[e]->push_back(textureSamplers[t].fSamplerUniform);
         }
+        GrDrawEffect drawEffect(stage, fVertexBuilder.get()
+                                       && fVertexBuilder->hasExplicitLocalCoords());
 
-        int numAttributes = drawEffect.getVertexAttribIndexCount();
-        const int* attributeIndices = drawEffect.getVertexAttribIndices();
+        int numAttributes = stage.getVertexAttribIndexCount();
+        const int* attributeIndices = stage.getVertexAttribIndices();
         SkSTArray<GrEffect::kMaxVertexAttribs, SkString> attributeNames;
         for (int a = 0; a < numAttributes; ++a) {
             // TODO: Make addAttribute mangle the name.
@@ -538,6 +542,8 @@ void GrGLShaderBuilder::emitEffects(
                                                attributeName);
         }
 
+        glEffects[e] = effect->getFactory().createGLInstance(drawEffect);
+
         if (kZeros_GrSLConstantVec == *fsInOutColorKnownValue) {
             // Effects have no way to communicate zeros, they treat an empty string as ones.
             this->nameVariable(&inColor, '\0', "input");
diff --git a/src/gpu/gl/GrGLShaderBuilder.h b/src/gpu/gl/GrGLShaderBuilder.h
index 5f2983d19d..3c3275ce36 100644
--- a/src/gpu/gl/GrGLShaderBuilder.h
+++ b/src/gpu/gl/GrGLShaderBuilder.h
@@ -108,7 +108,7 @@ public:
     GrGLShaderBuilder(const GrGLContextInfo&,
                       GrGLUniformManager&,
                       const GrGLProgramDesc&,
-                      bool hasVertexShaderEffects);
+                      bool needsVertexShader);
 
     /**
      * Use of these features may require a GLSL extension to be enabled. Shaders may not compile
@@ -257,13 +257,13 @@ public:
      * glEffects array is updated to contain the GrGLEffect generated for each entry in
      * effectStages.
      */
-    void emitEffects(GrGLEffect* const glEffects[],
-                     const GrDrawEffect drawEffects[],
+    void emitEffects(const GrEffectStage* effectStages[],
                      const GrBackendEffectFactory::EffectKey effectKeys[],
                      int effectCnt,
                      SkString*  inOutFSColor,
                      GrSLConstantVec* fsInOutColorKnownValue,
-                     SkTArray<GrGLUniformManager::UniformHandle, true>* effectSamplerHandles[]);
+                     SkTArray<GrGLUniformManager::UniformHandle, true>* effectSamplerHandles[],
+                     GrGLEffect* glEffects[]);
 
     GrGLUniformManager::UniformHandle getRTHeightUniform() const { return fRTHeightUniform; }
     GrGLUniformManager::UniformHandle getDstCopyTopLeftUniform() const {
@@ -389,16 +389,16 @@ private:
 private:
     class CodeStage : public SkNoncopyable {
     public:
-        CodeStage() : fNextIndex(0), fCurrentIndex(-1), fEffect(NULL) {}
+        CodeStage() : fNextIndex(0), fCurrentIndex(-1), fEffectStage(NULL) {}
 
         bool inStageCode() const {
             this->validate();
-            return NULL != fEffect;
+            return NULL != fEffectStage;
         }
 
-        const GrEffectRef* effect() const {
+        const GrEffectStage* effectStage() const {
             this->validate();
-            return fEffect;
+            return fEffectStage;
         }
 
         int stageIndex() const {
@@ -408,34 +408,34 @@ private:
 
         class AutoStageRestore : public SkNoncopyable {
         public:
-            AutoStageRestore(CodeStage* codeStage, const GrEffectRef* effect) {
+            AutoStageRestore(CodeStage* codeStage, const GrEffectStage* newStage) {
                 SkASSERT(NULL != codeStage);
                 fSavedIndex = codeStage->fCurrentIndex;
-                fSavedEffect = codeStage->fEffect;
+                fSavedEffectStage = codeStage->fEffectStage;
 
-                if (NULL == effect) {
+                if (NULL == newStage) {
                     codeStage->fCurrentIndex = -1;
                 } else {
                     codeStage->fCurrentIndex = codeStage->fNextIndex++;
                 }
-                codeStage->fEffect = effect;
+                codeStage->fEffectStage = newStage;
 
                 fCodeStage = codeStage;
             }
             ~AutoStageRestore() {
                 fCodeStage->fCurrentIndex = fSavedIndex;
-                fCodeStage->fEffect = fSavedEffect;
+                fCodeStage->fEffectStage = fSavedEffectStage;
             }
         private:
-            CodeStage*            fCodeStage;
-            int                   fSavedIndex;
-            const GrEffectRef*    fSavedEffect;
+            CodeStage*              fCodeStage;
+            int                     fSavedIndex;
+            const GrEffectStage*    fSavedEffectStage;
         };
     private:
-        void validate() const { SkASSERT((NULL == fEffect) == (-1 == fCurrentIndex)); }
-        int                   fNextIndex;
-        int                   fCurrentIndex;
-        const GrEffectRef*    fEffect;
+        void validate() const { SkASSERT((NULL == fEffectStage) == (-1 == fCurrentIndex)); }
+        int                     fNextIndex;
+        int                     fCurrentIndex;
+        const GrEffectStage*    fEffectStage;
     } fCodeStage;
 
     /**