Move loops that chain together effects into GrGLShaderBuilder from GrGLProgram.

src/gpu/gl/GrGLShaderBuilder.cpp

Index: src/gpu/gl/GrGLShaderBuilder.cpp
diff --git a/src/gpu/gl/GrGLShaderBuilder.cpp b/src/gpu/gl/GrGLShaderBuilder.cpp
index b379c784324176d7caea456b34d7efb133274942..c5840c241c6759d8098eea105b38cf8b9ff05156 100644
--- a/src/gpu/gl/GrGLShaderBuilder.cpp
+++ b/src/gpu/gl/GrGLShaderBuilder.cpp
@@ -103,7 +103,6 @@ GrGLShaderBuilder::GrGLShaderBuilder(const GrGLContextInfo& ctxInfo,
     , fFSOutputs(kMaxFSOutputs)
     , fCtxInfo(ctxInfo)
     , fUniformManager(uniformManager)
-    , fCurrentStageIdx(kNonStageIdx)
     , fFSFeaturesAddedMask(0)
 #if GR_GL_EXPERIMENTAL_GS
     , fUsesGS(desc.fExperimentalGS)
@@ -215,6 +214,21 @@ void GrGLShaderBuilder::addFSFeature(uint32_t featureBit, const char* extensionN
     }
 }
 
+void GrGLShaderBuilder::nameVariable(SkString* out, char prefix, const char* name) {
+    if ('\0' == prefix) {
+        *out = name;
+    } else {
+        out->printf("%c%s", prefix, name);
+    }
+    if (fCodeStage.inStageCode()) {

[robertphillips, 2013/05/08 19:56:33]

This '__' behavior should probably be mentioned in the comment

[bsalomon, 2013/05/08 20:38:54]

Two things: 1) It's a private function. 2) We don't want the caller to rely on
any particular behavior. I'm going to update the comment to be more vague rather
than promise a particular name-mangling.

+        if (out->endsWith('_')) {
+            // Names containing "__" are reserved.
+            out->append("x");
+        }
+        out->appendf("_Stage%d", fCodeStage.stageIndex());
+    }
+}
+
 const char* GrGLShaderBuilder::dstColor() {
     static const char kFBFetchColorName[] = "gl_LastFragData[0]";
     GrGLCaps::FBFetchType fetchType = fCtxInfo.caps()->fbFetchType();
@@ -364,12 +378,7 @@ GrGLUniformManager::UniformHandle GrGLShaderBuilder::addUniformArray(uint32_t vi
     GrAssert(h2 == h);
     uni.fVariable.setType(type);
     uni.fVariable.setTypeModifier(GrGLShaderVar::kUniform_TypeModifier);
-    SkString* uniName = uni.fVariable.accessName();
-    if (kNonStageIdx == fCurrentStageIdx) {
-        uniName->printf("u%s", name);
-    } else {
-        uniName->printf("u%s%d", name, fCurrentStageIdx);
-    }
+    this->nameVariable(uni.fVariable.accessName(), 'u', name);
     uni.fVariable.setArrayCount(count);
     uni.fVisibility = visibility;
 
@@ -415,11 +424,8 @@ void GrGLShaderBuilder::addVarying(GrSLType type,
     fVSOutputs.push_back();
     fVSOutputs.back().setType(type);
     fVSOutputs.back().setTypeModifier(GrGLShaderVar::kVaryingOut_TypeModifier);
-    if (kNonStageIdx == fCurrentStageIdx) {
-        fVSOutputs.back().accessName()->printf("v%s", name);
-    } else {
-        fVSOutputs.back().accessName()->printf("v%s%d", name, fCurrentStageIdx);
-    }
+    this->nameVariable(fVSOutputs.back().accessName(), 'v', name);
+
     if (vsOutName) {
         *vsOutName = fVSOutputs.back().getName().c_str();
     }
@@ -436,11 +442,7 @@ void GrGLShaderBuilder::addVarying(GrSLType type,
         fGSOutputs.push_back();
         fGSOutputs.back().setType(type);
         fGSOutputs.back().setTypeModifier(GrGLShaderVar::kVaryingOut_TypeModifier);

[robertphillips, 2013/05/08 19:56:33]

Did you mean to change this from 'g' to 'd'?

[bsalomon, 2013/05/08 20:38:54]

Done.

-        if (kNonStageIdx == fCurrentStageIdx) {
-            fGSOutputs.back().accessName()->printf("g%s", name);
-        } else {
-            fGSOutputs.back().accessName()->printf("g%s%d", name, fCurrentStageIdx);
-        }
+        this->nameVariable(fGSOutputs.back().accessName(), 'd', name);
         fsName = fGSOutputs.back().accessName();
     } else {
         fsName = fVSOutputs.back().accessName();
@@ -470,18 +472,16 @@ const char* GrGLShaderBuilder::fragmentPosition() {
     } else {
         static const char* kCoordName = "fragCoordYDown";
         if (!fSetupFragPosition) {
+            // temporarily change the stage index because we're inserting non-stage code.
+            CodeStage::AutoRestore csar(&fCodeStage, NULL);
+
             GrAssert(GrGLUniformManager::kInvalidUniformHandle == fRTHeightUniform);
             const char* rtHeightName;
 
-            // temporarily change the stage index because we're inserting a uniform whose name
-            // shouldn't be mangled to be stage-specific.
-            int oldStageIdx = fCurrentStageIdx;
-            fCurrentStageIdx = kNonStageIdx;
             fRTHeightUniform = this->addUniform(kFragment_ShaderType,
                                                 kFloat_GrSLType,
                                                 "RTHeight",
                                                 &rtHeightName);
-            fCurrentStageIdx = oldStageIdx;
 
             this->fFSCode.prependf("\tvec4 %s = vec4(gl_FragCoord.x, %s - gl_FragCoord.y, gl_FragCoord.zw);\n",
                                    kCoordName, rtHeightName);
@@ -514,11 +514,7 @@ void GrGLShaderBuilder::emitFunction(ShaderType shader,
                                      SkString* outName) {
     GrAssert(kFragment_ShaderType == shader);
     fFSFunctions.append(GrGLSLTypeString(returnType));
-    if (kNonStageIdx != fCurrentStageIdx) {
-        outName->printf("%s_%d", name, fCurrentStageIdx);
-    } else {
-        *outName = name;
-    }
+    this->nameVariable(outName, '\0', name);
     fFSFunctions.appendf(" %s", outName->c_str());
     fFSFunctions.append("(");
     for (int i = 0; i < argCnt; ++i) {
@@ -623,52 +619,86 @@ void GrGLShaderBuilder::finished(GrGLuint programID) {
     fUniformManager.getUniformLocations(programID, fUniforms);
 }
 
-GrGLEffect* GrGLShaderBuilder::createAndEmitGLEffect(
-                                const GrEffectStage& stage,
-                                GrGLEffect::EffectKey key,
-                                const char* fsInColor,
-                                const char* fsOutColor,
-                                SkTArray<GrGLUniformManager::UniformHandle, true>* samplerHandles) {
-    GrAssert(NULL != stage.getEffect());
-
-    const GrEffectRef& effect = *stage.getEffect();
-    int numTextures = effect->numTextures();
-    SkSTArray<8, GrGLShaderBuilder::TextureSampler> textureSamplers;
-    textureSamplers.push_back_n(numTextures);
-    for (int i = 0; i < numTextures; ++i) {
-        textureSamplers[i].init(this, &effect->textureAccess(i), i);
-        samplerHandles->push_back(textureSamplers[i].fSamplerUniform);
-    }
-    GrDrawEffect drawEffect(stage, this->hasExplicitLocalCoords());
-
-    int numAttributes = stage.getVertexAttribIndexCount();
-    const int* attributeIndices = stage.getVertexAttribIndices();
-    SkSTArray<GrEffect::kMaxVertexAttribs, SkString> attributeNames;
-    for (int i = 0; i < numAttributes; ++i) {
-        SkString attributeName("aAttr");
-        attributeName.appendS32(attributeIndices[i]);
-
-        if (this->addAttribute(effect->vertexAttribType(i), attributeName.c_str())) {
-            fEffectAttributes.push_back().set(attributeIndices[i], attributeName);
+void GrGLShaderBuilder::emitEffects(
+                        const GrEffectStage* effectStages[],
+                        const GrBackendEffectFactory::EffectKey effectKeys[],
+                        int effectCnt,

[robertphillips, 2013/05/08 19:56:33]

xtra space?

[bsalomon, 2013/05/08 20:38:54]

Done.

+                        SkString*  fsInOutColor,
+                        GrSLConstantVec* fsInOutColorKnownValue,
+                        SkTArray<GrGLUniformManager::UniformHandle, true>* effectSamplerHandles[],
+                        GrGLEffect* glEffects[]) {
+    bool effectEmitted = false;
+
+    SkString inColor = *fsInOutColor;
+    SkString outColor;
+
+    for (int e = 0; e < effectCnt; ++e) {
+        if (NULL == effectStages[e]) {
+            continue;
+        }
+
+        GrAssert(NULL != effectStages[e]->getEffect());
+        const GrEffectStage& stage = *effectStages[e];
+        const GrEffectRef& effect = *stage.getEffect();
+
+        CodeStage::AutoRestore csar(&fCodeStage, &stage);
+
+        int numTextures = effect->numTextures();
+        SkSTArray<8, GrGLShaderBuilder::TextureSampler> textureSamplers;
+        textureSamplers.push_back_n(numTextures);
+        for (int t = 0; t < numTextures; ++t) {
+            textureSamplers[t].init(this, &effect->textureAccess(t), t);
+            effectSamplerHandles[e]->push_back(textureSamplers[t].fSamplerUniform);
+        }
+        GrDrawEffect drawEffect(stage, this->hasExplicitLocalCoords());
+
+        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.
+            SkString attributeName("aAttr");
+            attributeName.appendS32(attributeIndices[a]);
+            if (this->addAttribute(effect->vertexAttribType(a), attributeName.c_str())) {
+                fEffectAttributes.push_back().set(attributeIndices[a], 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");
+            this->fsCodeAppendf("\tvec4 %s = %s;\n", inColor.c_str(), GrGLSLZerosVecf(4));
         }
-    }
 
-    GrGLEffect* glEffect = effect->getFactory().createGLInstance(drawEffect);
+        // create var to hold stage result
+        this->nameVariable(&outColor, '\0', "output");
+        this->fsCodeAppendf("\tvec4 %s;\n", outColor.c_str());
 
-    // Enclose custom code in a block to avoid namespace conflicts
-    this->fVSCode.appendf("\t{ // %s\n", glEffect->name());
-    this->fFSCode.appendf("\t{ // %s \n", glEffect->name());
+        // Enclose custom code in a block to avoid namespace conflicts
+        SkString openBrace;

[robertphillips, 2013/05/08 19:56:33]

extra space in "] ->name"

[bsalomon, 2013/05/08 20:38:54]

Done.

+        openBrace.printf("\t{ // Stage %d: %s\n", fCodeStage.stageIndex(), glEffects[e] ->name());
+        this->fVSCode.append(openBrace);
+        this->fFSCode.append(openBrace);
 
-    glEffect->emitCode(this,
-                       drawEffect,
-                       key,
-                       fsOutColor,
-                       fsInColor,
-                       textureSamplers);
-    this->fVSCode.appendf("\t}\n");
-    this->fFSCode.appendf("\t}\n");
+        glEffects[e]->emitCode(this,
+                               drawEffect,
+                               effectKeys[e],
+                               outColor.c_str(),
+                               inColor.isEmpty() ? NULL : inColor.c_str(),
+                               textureSamplers);
+        this->fVSCode.append("\t}\n");
+        this->fFSCode.append("\t}\n");
 
-    return glEffect;
+        inColor = outColor;
+        *fsInOutColorKnownValue = kNone_GrSLConstantVec;
+        effectEmitted = true;
+    }
+
+    if (effectEmitted) {
+        *fsInOutColor = outColor;
+    }
 }
 
 const SkString* GrGLShaderBuilder::getEffectAttributeName(int attributeIndex) const {