Move convolution from code in GrGLProgram to new GrConvolutionEffect

class. This is the first test of the new Ganesh shader pipeline.

Also includes some cleanup of the gpu.gyp file: added src/gpu, allowing
us to remove ../ from many #include directives.

http://codereview.appspot.com/6199053/



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

gyp/gpu.gyp

@@ -164,6 +164,7 @@
         '../include/config',
         '../include/gpu',
         '../src/core', # SkRasterClip.h
+        '../src/gpu'
       ],
       'dependencies': [
         'libtess.gyp:libtess',
@@ -248,6 +249,8 @@
         '../src/gpu/GrPathUtils.cpp',
         '../src/gpu/GrPathUtils.h',
         '../src/gpu/GrPlotMgr.h',
+        '../src/gpu/GrProgramStageFactory.cpp',
+        '../src/gpu/GrProgramStageFactory.h',
         '../src/gpu/GrRandom.h',
         '../src/gpu/GrRectanizer.cpp',
         '../src/gpu/GrRectanizer.h',
@@ -276,6 +279,9 @@
         '../src/gpu/GrVertexBuffer.h',
         '../src/gpu/gr_unittests.cpp',
 
+        '../src/gpu/effects/GrConvolutionEffect.cpp',
+        '../src/gpu/effects/GrConvolutionEffect.h',
+
         '../src/gpu/gl/GrGLCaps.cpp',
         '../src/gpu/gl/GrGLCaps.h',
         '../src/gpu/gl/GrGLContextInfo.cpp',

include/gpu/GrCustomStage.h

@@ -11,7 +11,7 @@
 #include "GrRefCnt.h"
 
 class GrContext;
-class GrGLProgramStageFactory;
+class GrProgramStageFactory;
 
 /** Provides custom vertex shader, fragment shader, uniform data for a
     particular stage of the Ganesh shading pipeline.
@@ -23,6 +23,10 @@ public:
     GrCustomStage();
     virtual ~GrCustomStage();
 
+    /** Human-meaningful string to identify this effect; may be embedded
+        in generated shader code. */
+    virtual const char* name() const = 0;
+
     /** If given an input texture that is/is not opaque, is this
         stage guaranteed to produce an opaque output? */
     virtual bool isOpaque(bool inputTextureIsOpaque) const;
@@ -30,14 +34,20 @@ public:
     /** This pointer, besides creating back-end-specific helper
         objects, is used for run-time-type-identification. Every
         subclass must return a consistent unique value for it. */
-    virtual GrGLProgramStageFactory* getGLFactory() const = 0;
+    virtual GrProgramStageFactory* getFactory() const = 0;
 
     /** Returns true if the other custom stage will generate
-        a compatible/equivalent shader. Must only be called if
+        equal output.
-        the two are already known to be of the same type (i.e.
+        Must only be called if the two are already known to be of the
-        they return the same value from getGLFactory()). */
+        same type (i.e.  they return the same value from getFactory()).
-    virtual bool isEquivalent(const GrCustomStage *) const = 0;
+        For equivalence (that they will generate the same
+        shader, but perhaps have different uniforms), check equality
+        of the stageKey produced by the GrProgramStageFactory. */
+    virtual bool isEqual(const GrCustomStage *) const = 0;
 
+private:
+
+    typedef GrRefCnt INHERITED;
 };
 
 #endif

include/gpu/GrSamplerState.h

@@ -133,9 +133,9 @@ public:
         return !memcmp(this, &s, bitwiseRegion) &&
                ((fCustomStage == s.fCustomStage) ||
                 (fCustomStage && s.fCustomStage &&
-                 (fCustomStage->getGLFactory() ==
+                 (fCustomStage->getFactory() ==
-                     s.fCustomStage->getGLFactory()) &&
+                     s.fCustomStage->getFactory()) &&
-                 fCustomStage->isEquivalent(s.fCustomStage)));
+                 fCustomStage->isEqual(s.fCustomStage)));
     }
     bool operator !=(const GrSamplerState& s) const { return !(*this == s); }
 
@@ -289,10 +289,6 @@ private:
     uint8_t             fKernelWidth;
     float               fKernel[MAX_KERNEL_WIDTH];
 
-    /// BUG! Ganesh only works correctly so long as fCustomStage is  
-    /// NULL; we need to have a complex ID system here so that we can
-    /// have an equality-like comparison to determine whether two
-    /// fCustomStages are equal.
     GrCustomStage*      fCustomStage;
 };
 

src/gpu/GrContext.cpp

@@ -11,6 +11,7 @@
 
 #include "GrBufferAllocPool.h"
 #include "GrClipIterator.h"
+#include "effects/GrConvolutionEffect.h"
 #include "GrGpu.h"
 #include "GrIndexBuffer.h"
 #include "GrInOrderDrawBuffer.h"
@@ -296,8 +297,8 @@ void convolve(GrGpu* gpu,
     drawState->sampler(0)->reset(GrSamplerState::kClamp_WrapMode,
                                  GrSamplerState::kConvolution_Filter,
                                  sampleM);
-    drawState->sampler(0)->setConvolutionParams(kernelWidth, kernel);
+    drawState->sampler(0)->setCustomStage(
-    drawState->sampler(0)->setFilterDirection(direction);
+        new GrConvolutionEffect(direction, kernelWidth, kernel));
     drawState->setTexture(0, texture);
     gpu->drawSimpleRect(rect, NULL, 1 << 0);
 }

src/gpu/GrProgramStageFactory.cpp

@@ -0,0 +1,17 @@
+/*
+ * Copyright 2012 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "GrProgramStageFactory.h"
+
+GrProgramStageFactory::~GrProgramStageFactory(void) {
+
+}
+
+uint16_t GrProgramStageFactory::stageKey(const GrCustomStage*) {
+    return 0;
+}
+

src/gpu/GrProgramStageFactory.h

@@ -0,0 +1,50 @@
+/*
+ * Copyright 2012 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#ifndef GrProgramStageFactory_DEFINED
+#define GrProgramStageFactory_DEFINED
+
+#include "GrTypes.h"
+
+class GrCustomStage;
+class GrGLProgramStage;
+
+/** Given a GrCustomStage of a particular type, creates the corresponding
+    graphics-backend-specific GrProgramStage. Also tracks equivalence
+    of shaders generated via stageKey().
+
+    TODO: most of this class' subclasses are boilerplate and ought to be
+    templateable?
+*/
+
+class GrProgramStageFactory {
+
+public:
+
+    virtual ~GrProgramStageFactory();
+
+    /** Returns a short unique identifier for this subclass x its
+        parameters. If the key differs, different shader code must
+        be generated; if the key matches, shader code can be reused.
+        0 == no custom stage. */
+    virtual uint16_t stageKey(const GrCustomStage*);
+
+    /** Returns a new instance of the appropriate *GL* implementation class
+        for the given GrCustomStage; caller is responsible for deleting
+        the object. */
+    virtual GrGLProgramStage* createGLInstance(GrCustomStage*) = 0;
+
+protected:
+
+    /** Disable default constructor - instances should be singletons
+        with static factory functions: our test examples are all stateless,
+        but we suspect that future implementations may want to cache data? */
+    GrProgramStageFactory() { }
+};
+
+
+#endif

src/gpu/effects/GrConvolutionEffect.cpp

@@ -0,0 +1,257 @@
+/*
+ * Copyright 2012 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "GrConvolutionEffect.h"
+#include "gl/GrGLProgramStage.h"
+#include "gl/GrGLSL.h"
+#include "gl/GrGLTexture.h"
+#include "GrProgramStageFactory.h"
+
+/////////////////////////////////////////////////////////////////////
+
+class GrGLConvolutionEffect : public GrGLProgramStage {
+
+public:
+
+    GrGLConvolutionEffect(GrConvolutionEffect* data);
+    virtual const char* name() const SK_OVERRIDE;
+    virtual void setupVSUnis(VarArray* vsUnis, int stage) SK_OVERRIDE;
+    virtual void setupFSUnis(VarArray* fsUnis, int stage) SK_OVERRIDE;
+    virtual void emitVS(GrStringBuilder* code,
+                        const char* vertexCoords) SK_OVERRIDE;
+    virtual void emitFS(GrStringBuilder* code,
+                        const char* outputColor,
+                        const char* inputColor,
+                        const char* samplerName,
+                        const char* sampleCoords) SK_OVERRIDE;
+    virtual void initUniforms(const GrGLInterface*, int programID) SK_OVERRIDE;
+
+    virtual void setData(const GrGLInterface*, GrCustomStage*,
+                         const GrGLTexture*) SK_OVERRIDE;
+
+protected:
+
+    GrConvolutionEffect* fData;
+ 
+    GrGLShaderVar* fKernelVar;
+    GrGLShaderVar* fImageIncrementVar;
+ 
+    GrGLint fKernelLocation;
+    GrGLint fImageIncrementLocation;
+
+private:
+
+    typedef GrGLProgramStage INHERITED;
+};
+
+GrGLConvolutionEffect::GrGLConvolutionEffect(GrConvolutionEffect* data)
+    : fData(data)
+    , fKernelVar(NULL)
+    , fImageIncrementVar(NULL)
+    , fKernelLocation(0)
+    , fImageIncrementLocation(0)
+{
+
+}
+
+const char* GrGLConvolutionEffect::name() const SK_OVERRIDE {
+    return fData->name();
+}
+
+void GrGLConvolutionEffect::setupVSUnis(VarArray* vsUnis,
+                                        int stage) SK_OVERRIDE {
+    fImageIncrementVar = &vsUnis->push_back();
+    fImageIncrementVar->setType(kVec2f_GrSLType);
+    fImageIncrementVar->setTypeModifier(
+        GrGLShaderVar::kUniform_TypeModifier);
+    (*fImageIncrementVar->accessName()) = "uImageIncrement";
+    fImageIncrementVar->accessName()->appendS32(stage);
+    fImageIncrementVar->setEmitPrecision(true);
+
+    fImageIncrementLocation = kUseUniform;
+}
+
+void GrGLConvolutionEffect::setupFSUnis(VarArray* fsUnis,
+                                        int stage) SK_OVERRIDE {
+    fKernelVar = &fsUnis->push_back();
+    fKernelVar->setType(kFloat_GrSLType);
+    fKernelVar->setTypeModifier(
+        GrGLShaderVar::kUniform_TypeModifier);
+    fKernelVar->setArrayCount(fData->fKernelWidth);
+    (*fKernelVar->accessName()) = "uKernel";
+    fKernelVar->accessName()->appendS32(stage);
+
+    fKernelLocation = kUseUniform;
+
+    // Image increment is used in both vertex & fragment shaders.
+    fsUnis->push_back(*fImageIncrementVar).setEmitPrecision(false);
+}
+
+void GrGLConvolutionEffect::emitVS(GrStringBuilder* code,
+                        const char* vertexCoords) SK_OVERRIDE {
+    float scale = (fData->fKernelWidth - 1) * 0.5f;
+    code->appendf("\t\t%s -= vec2(%g, %g) * %s;\n",
+                  vertexCoords, scale, scale,
+                  fImageIncrementVar->getName().c_str());
+
+}
+
+void GrGLConvolutionEffect::emitFS(GrStringBuilder* code,
+                        const char* outputColor,
+                        const char* inputColor,
+                        const char* samplerName,
+                        const char* sampleCoords) SK_OVERRIDE {
+    const char* texFunc = "texture2D";
+    bool complexCoord = false;
+
+    GrStringBuilder modulate;
+    if (NULL != inputColor) {
+        modulate.printf(" * %s", inputColor);
+    }
+
+    // Creates the string "kernel[i]" with workarounds for
+    // possible driver bugs
+    GrStringBuilder kernelIndex;
+    fKernelVar->appendArrayAccess("i", &kernelIndex);
+
+    code->appendf("\t\tvec4 sum = vec4(0, 0, 0, 0);\n");
+    code->appendf("\t\tvec2 coord = %s;\n", sampleCoords);
+    code->appendf("\t\tfor (int i = 0; i < %d; i++) {\n",
+                  fData->fKernelWidth);
+
+    code->appendf("\t\t\tsum += ");
+    this->emitTextureLookup(code, samplerName, "coord");
+    code->appendf(" * %s;\n", kernelIndex.c_str());
+
+    code->appendf("\t\t\tcoord += %s;\n",
+                  fImageIncrementVar->getName().c_str());
+    code->appendf("\t\t}\n");
+    code->appendf("\t\t%s = sum%s;\n", outputColor, modulate.c_str());
+}
+
+void GrGLConvolutionEffect::initUniforms(const GrGLInterface* gl,
+                                         int programID) SK_OVERRIDE {
+    GR_GL_CALL_RET(gl, fKernelLocation,
+        GetUniformLocation(programID, fKernelVar->getName().c_str()));
+    GR_GL_CALL_RET(gl, fImageIncrementLocation,
+        GetUniformLocation(programID,
+            fImageIncrementVar->getName().c_str()));
+}
+
+void GrGLConvolutionEffect::setData(const GrGLInterface* gl,
+                                    GrCustomStage* data,
+                                    const GrGLTexture* texture) SK_OVERRIDE {
+    fData = static_cast<GrConvolutionEffect*>(data);
+    GR_GL_CALL(gl, Uniform1fv(fKernelLocation,
+                              fData->fKernelWidth,
+                              fData->fKernel));
+    float imageIncrement[2] = { 0 };
+    switch (fData->fDirection) {
+        case GrSamplerState::kX_FilterDirection:
+            imageIncrement[0] = 1.0f / texture->width();
+            break;
+        case GrSamplerState::kY_FilterDirection:
+            imageIncrement[1] = 1.0f / texture->width();
+            break;
+        default:
+            GrCrash("Unknown filter direction.");
+    }
+    GR_GL_CALL(gl, Uniform2fv(fImageIncrementLocation, 1, imageIncrement));
+}
+
+/////////////////////////////////////////////////////////////////////
+// TODO: stageKey() and sEffectId are the only non-boilerplate in
+// this class; we ought to be able to templatize?
+
+class GrConvolutionEffectFactory : public GrProgramStageFactory {
+
+public:
+
+    virtual ~GrConvolutionEffectFactory();
+
+    virtual uint16_t stageKey(const GrCustomStage* s) SK_OVERRIDE;
+    virtual GrGLProgramStage* createGLInstance(GrCustomStage* s) SK_OVERRIDE;
+
+    static GrConvolutionEffectFactory* getInstance();
+
+protected:
+
+    GrConvolutionEffectFactory();
+
+    // TODO: find a more reliable installation than hand-coding
+    // id values like '1'. 
+    static const int sEffectId = 1;
+
+private:
+
+    typedef GrProgramStageFactory INHERITED;
+};
+
+GrConvolutionEffectFactory::~GrConvolutionEffectFactory() {
+
+}
+
+uint16_t GrConvolutionEffectFactory::stageKey(const GrCustomStage* s)
+    SK_OVERRIDE {
+    const GrConvolutionEffect* c =
+        static_cast<const GrConvolutionEffect*>(s);
+    GrAssert(c->width() < 256);
+    return (sEffectId << 8) | (c->width() & 0xff);
+}
+
+GrGLProgramStage* GrConvolutionEffectFactory::createGLInstance(
+    GrCustomStage* s) SK_OVERRIDE {
+    return new GrGLConvolutionEffect(static_cast<GrConvolutionEffect*>(s));
+}
+
+GrConvolutionEffectFactory* GrConvolutionEffectFactory::getInstance() {
+    static GrConvolutionEffectFactory* instance =
+        new GrConvolutionEffectFactory;
+    return instance;
+}
+
+GrConvolutionEffectFactory::GrConvolutionEffectFactory() {
+
+}
+
+/////////////////////////////////////////////////////////////////////
+
+GrConvolutionEffect::GrConvolutionEffect(
+        GrSamplerState::FilterDirection direction,
+        unsigned int kernelWidth,
+        const float* kernel)
+    : fDirection (direction)
+    , fKernelWidth (kernelWidth) {
+    GrAssert(kernelWidth <= MAX_KERNEL_WIDTH);
+    for (unsigned int i = 0; i < kernelWidth; i++) {
+        fKernel[i] = kernel[i];
+    }
+}
+
+GrConvolutionEffect::~GrConvolutionEffect() {
+
+}
+
+const char* GrConvolutionEffect::name() const {
+    return "Convolution";
+}
+
+GrProgramStageFactory* GrConvolutionEffect::getFactory() const {
+    return GrConvolutionEffectFactory::getInstance();
+}
+
+bool GrConvolutionEffect::isEqual(const GrCustomStage * sBase) const {
+    const GrConvolutionEffect* s =
+        static_cast<const GrConvolutionEffect*>(sBase);
+
+    return (fKernelWidth == s->fKernelWidth &&
+            fDirection == s->fDirection &&
+            0 == memcmp(fKernel, s->fKernel, fKernelWidth * sizeof(float)));
+}
+
+
+

src/gpu/effects/GrConvolutionEffect.h

@@ -0,0 +1,41 @@
+/*
+ * Copyright 2012 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#ifndef GrConvolutionEffect_DEFINED
+#define GrConvolutionEffect_DEFINED
+
+#include "GrCustomStage.h"
+#include "GrSamplerState.h" // for MAX_KENEL_WIDTH, FilterDirection
+
+class GrConvolutionEffect : public GrCustomStage {
+
+public:
+
+    GrConvolutionEffect(GrSamplerState::FilterDirection direction,
+                        unsigned int kernelWidth, const float* kernel);
+    virtual ~GrConvolutionEffect();
+
+    virtual const char* name() const SK_OVERRIDE;
+    virtual GrProgramStageFactory* getFactory() const SK_OVERRIDE;
+    virtual bool isEqual(const GrCustomStage *) const SK_OVERRIDE;
+
+    unsigned int width() const { return fKernelWidth; }
+
+protected:
+
+    GrSamplerState::FilterDirection fDirection;
+    unsigned int fKernelWidth;
+    float fKernel[MAX_KERNEL_WIDTH];
+
+    friend class GrGLConvolutionEffect;
+
+private:
+
+    typedef GrCustomStage INHERITED;
+};
+
+#endif

src/gpu/gl/GrGLCaps.cpp

@@ -1,4 +1,3 @@
-
 /*
  * Copyright 2012 Google Inc.
  *

src/gpu/gl/GrGLCaps.h

@@ -1,4 +1,3 @@
-
 /*
  * Copyright 2012 Google Inc.
  *

src/gpu/gl/GrGLContextInfo.cpp

@@ -1,4 +1,3 @@
-
 /*
  * Copyright 2012 Google Inc.
  *

src/gpu/gl/GrGLCreateNativeInterface_none.cpp

@@ -1,4 +1,3 @@
-
 /*
  * Copyright 2011 Google Inc.
  *

src/gpu/gl/GrGLCreateNullInterface.cpp

@@ -1,4 +1,3 @@
-
 /*
  * Copyright 2011 Google Inc.
  *
@@ -8,7 +7,7 @@
 
 
 #include "gl/GrGLInterface.h"
-#include "../GrTDArray.h"
+#include "GrTDArray.h"
 #include "GrGLDefines.h"
 
 GrGLvoid GR_GL_FUNCTION_TYPE nullGLActiveTexture(GrGLenum texture) {}

src/gpu/gl/GrGLDefaultInterface_native.cpp

@@ -1,4 +1,3 @@
-
 /*
  * Copyright 2011 Google Inc.
  *

src/gpu/gl/GrGLDefaultInterface_none.cpp

@@ -1,4 +1,3 @@
-
 /*
  * Copyright 2011 Google Inc.
  *

src/gpu/gl/GrGLDefines.h

@@ -1,4 +1,3 @@
-
 /*
  * Copyright 2011 Google Inc.
  *

src/gpu/gl/GrGLIRect.h

@@ -1,4 +1,3 @@
-
 /*
  * Copyright 2011 Google Inc.
  *

src/gpu/gl/GrGLIndexBuffer.cpp

@@ -1,4 +1,3 @@
-
 /*
  * Copyright 2011 Google Inc.
  *

src/gpu/gl/GrGLIndexBuffer.h

@@ -1,4 +1,3 @@
-
 /*
  * Copyright 2011 Google Inc.
  *
@@ -11,7 +10,7 @@
 #ifndef GrGLIndexBuffer_DEFINED
 #define GrGLIndexBuffer_DEFINED
 
-#include "../GrIndexBuffer.h"
+#include "GrIndexBuffer.h"
 #include "gl/GrGLInterface.h"
 
 class GrGpuGL;

src/gpu/gl/GrGLInterface.cpp

@@ -1,4 +1,3 @@
-
 /*
  * Copyright 2011 Google Inc.
  *

src/gpu/gl/GrGLProgram.cpp

@@ -1,4 +1,3 @@
-
 /*
  * Copyright 2011 Google Inc.
  *
@@ -6,13 +5,13 @@
  * found in the LICENSE file.
  */
 
-
 #include "GrGLProgram.h"
 
-#include "../GrAllocator.h"
+#include "GrAllocator.h"
 #include "GrCustomStage.h"
 #include "GrGLProgramStage.h"
 #include "GrGLShaderVar.h"
+#include "GrProgramStageFactory.h"
 #include "SkTrace.h"
 #include "SkXfermode.h"
 
@@ -647,7 +646,6 @@ GrGLProgram::CachedData::~CachedData() {
 bool GrGLProgram::genProgram(const GrGLContextInfo& gl,
                              GrCustomStage** customStages,
                              GrGLProgram::CachedData* programData) const {
-
     ShaderCodeSegments segments;
     const uint32_t& layout = fProgramDesc.fVertexLayout;
 
@@ -757,19 +755,13 @@ bool GrGLProgram::genProgram(const GrGLContextInfo& gl,
     }
 
     ///////////////////////////////////////////////////////////////////////////
-    // Convert generic effect representation to GL-specific backend so they
+    // We need to convert generic effect representations to GL-specific
-    // can be accesseed in genStageCode() and in subsequent uses of
+    // backends so they can be accesseed in genStageCode() and in subsequent,
-    // programData.
+    // uses of programData, but it's safest to do so below when we're *sure*
+    // we need them.
     for (int s = 0; s < GrDrawState::kNumStages; ++s) {
-        GrCustomStage* customStage = customStages[s];
-        if (NULL != customStage) {
-            GrGLProgramStageFactory* factory = customStage->getGLFactory();
-            programData->fCustomStage[s] =
-                factory->createGLInstance(customStage);
-        } else {
         programData->fCustomStage[s] = NULL;
     }
-    }
 
     ///////////////////////////////////////////////////////////////////////////
     // compute the final color
@@ -798,6 +790,12 @@ bool GrGLProgram::genProgram(const GrGLContextInfo& gl,
                     inCoords = texCoordAttrs[tcIdx].c_str();
                 }
 
+                if (NULL != customStages[s]) {
+                    GrProgramStageFactory* factory =
+                        customStages[s]->getFactory();
+                    programData->fCustomStage[s] =
+                        factory->createGLInstance(customStages[s]);
+                }
                 this->genStageCode(gl,
                                    s,
                                    fProgramDesc.fStages[s],
@@ -918,6 +916,12 @@ bool GrGLProgram::genProgram(const GrGLContextInfo& gl,
                         inCoords = texCoordAttrs[tcIdx].c_str();
                     }
 
+                    if (NULL != customStages[s]) {
+                        GrProgramStageFactory* factory =
+                            customStages[s]->getFactory();
+                        programData->fCustomStage[s] =
+                            factory->createGLInstance(customStages[s]);
+                    }
                     this->genStageCode(gl, s,
                         fProgramDesc.fStages[s],
                         inCoverage.size() ? inCoverage.c_str() : NULL,
@@ -1618,76 +1622,6 @@ void gen2x2FS(int stageNum,
 
 }
 
-void genConvolutionVS(int stageNum,
-                      const StageDesc& desc,
-                      ShaderCodeSegments* segments,
-                      GrGLProgram::StageUniLocations* locations,
-                      GrGLShaderVar** kernel,
-                      const char** imageIncrementName,
-                      const char* varyingVSName) {
-    //GrGLShaderVar* kernel = &segments->fFSUnis.push_back();
-    *kernel = &segments->fFSUnis.push_back();
-    (*kernel)->setType(kFloat_GrSLType);
-    (*kernel)->setTypeModifier(GrGLShaderVar::kUniform_TypeModifier);
-    (*kernel)->setArrayCount(desc.fKernelWidth);
-    GrGLShaderVar* imgInc = &segments->fFSUnis.push_back();
-    imgInc->setType(kVec2f_GrSLType);
-    imgInc->setTypeModifier(GrGLShaderVar::kUniform_TypeModifier);
-
-    convolve_param_names(stageNum,
-                         (*kernel)->accessName(),
-                         imgInc->accessName());
-    *imageIncrementName = imgInc->getName().c_str();
-
-    // need image increment in both VS and FS
-    segments->fVSUnis.push_back(*imgInc).setEmitPrecision(true);
-
-    locations->fKernelUni = kUseUniform;
-    locations->fImageIncrementUni = kUseUniform;
-    float scale = (desc.fKernelWidth - 1) * 0.5f;
-    segments->fVSCode.appendf("\t%s -= vec2(%g, %g) * %s;\n",
-                                  varyingVSName, scale, scale,
-                                  *imageIncrementName);
-}
-
-void genConvolutionFS(int stageNum,
-                      const StageDesc& desc,
-                      ShaderCodeSegments* segments,
-                      const char* samplerName,
-                      GrGLShaderVar* kernel,
-                      const char* swizzle,
-                      const char* imageIncrementName,
-                      const char* fsOutColor,
-                      GrStringBuilder& sampleCoords,
-                      GrStringBuilder& texFunc,
-                      GrStringBuilder& modulate) {
-    GrStringBuilder sumVar("sum");
-    sumVar.appendS32(stageNum);
-    GrStringBuilder coordVar("coord");
-    coordVar.appendS32(stageNum);
-
-    GrStringBuilder kernelIndex;
-    kernel->appendArrayAccess("i", &kernelIndex);
-
-    segments->fFSCode.appendf("\tvec4 %s = vec4(0, 0, 0, 0);\n",
-                              sumVar.c_str());
-    segments->fFSCode.appendf("\tvec2 %s = %s;\n", 
-                              coordVar.c_str(),
-                              sampleCoords.c_str());
-    segments->fFSCode.appendf("\tfor (int i = 0; i < %d; i++) {\n",
-                              desc.fKernelWidth);
-    segments->fFSCode.appendf("\t\t%s += %s(%s, %s)%s * %s;\n",
-                              sumVar.c_str(), texFunc.c_str(),
-                              samplerName, coordVar.c_str(), swizzle,
-                              kernelIndex.c_str());
-    segments->fFSCode.appendf("\t\t%s += %s;\n",
-                              coordVar.c_str(),
-                              imageIncrementName);
-    segments->fFSCode.append("\t}\n");
-    segments->fFSCode.appendf("\t%s = %s%s;\n", fsOutColor,
-                              sumVar.c_str(), modulate.c_str());
-}
- 
 void genMorphologyVS(int stageNum,
                      const StageDesc& desc,
                      ShaderCodeSegments* segments,
@@ -1775,7 +1709,7 @@ void GrGLProgram::genStageCode(const GrGLContextInfo& gl,
     /// Vertex Shader Stuff
 
     if (NULL != customStage) {
-        customStage->setupVSUnis(segments->fVSUnis, stageNum);
+        customStage->setupVSUnis(&segments->fVSUnis, stageNum);
     }
 
     // decide whether we need a matrix to transform texture coords
@@ -1852,10 +1786,7 @@ void GrGLProgram::genStageCode(const GrGLContextInfo& gl,
 
     GrGLShaderVar* kernel = NULL;
     const char* imageIncrementName = NULL;
-    if (StageDesc::kConvolution_FetchMode == desc.fFetchMode) {
+    if (StageDesc::kDilate_FetchMode == desc.fFetchMode ||
-        genConvolutionVS(stageNum, desc, segments, locations,
-                         &kernel, &imageIncrementName, varyingVSName);
-    } else if (StageDesc::kDilate_FetchMode == desc.fFetchMode ||
                StageDesc::kErode_FetchMode == desc.fFetchMode) {
         genMorphologyVS(stageNum, desc, segments, locations,
                         &imageIncrementName, varyingVSName);
@@ -1864,15 +1795,16 @@ void GrGLProgram::genStageCode(const GrGLContextInfo& gl,
     if (NULL != customStage) {
         GrStringBuilder vertexShader;
         customStage->emitVS(&vertexShader, varyingVSName);
-        segments->fVSCode.appendf("{\n");
+        segments->fVSCode.appendf("\t{ // stage %d %s\n",
+                                  stageNum, customStage->name());
         segments->fVSCode.append(vertexShader);
-        segments->fVSCode.appendf("}\n");
+        segments->fVSCode.appendf("\t}\n");
     }
 
     /// Fragment Shader Stuff
 
     if (NULL != customStage) {
-        customStage->setupFSUnis(segments->fFSUnis, stageNum);
+        customStage->setupFSUnis(&segments->fFSUnis, stageNum);
     }
 
     GrStringBuilder fsCoordName;
@@ -1985,9 +1917,6 @@ void GrGLProgram::genStageCode(const GrGLContextInfo& gl,
         break;
     case StageDesc::kConvolution_FetchMode:
         GrAssert(!(desc.fInConfigFlags & kMulByAlphaMask));
-        genConvolutionFS(stageNum, desc, segments,
-            samplerName, kernel, swizzle, imageIncrementName, fsOutColor,
-            sampleCoords, texFunc, modulate);
         break;
     case StageDesc::kDilate_FetchMode:
     case StageDesc::kErode_FetchMode:
@@ -2040,15 +1969,16 @@ void GrGLProgram::genStageCode(const GrGLContextInfo& gl,
 
         GrStringBuilder fragmentShader;
         fsCoordName = customStage->emitTextureSetup(
-                          &fragmentShader, varyingFSName,
+                          &fragmentShader, sampleCoords.c_str(),
                           stageNum, coordDims, varyingDims);
         customStage->emitFS(&fragmentShader, fsOutColor, fsInColor,
                             samplerName, fsCoordName.c_str());
       
         // Enclose custom code in a block to avoid namespace conflicts
-        segments->fFSCode.appendf("{\n");
+        segments->fFSCode.appendf("\t{ // stage %d %s \n",
+                                  stageNum, customStage->name());
         segments->fFSCode.append(fragmentShader);
-        segments->fFSCode.appendf("}\n");
+        segments->fFSCode.appendf("\t}\n");
     }
 }
 

src/gpu/gl/GrGLProgram.h

@@ -1,4 +1,3 @@
-
 /*
  * Copyright 2011 Google Inc.
  *
@@ -10,11 +9,11 @@
 #ifndef GrGLProgram_DEFINED
 #define GrGLProgram_DEFINED
 
-#include "../GrDrawState.h"
+#include "GrDrawState.h"
 #include "GrGLContextInfo.h"
 #include "GrGLSL.h"
-#include "../GrStringBuilder.h"
+#include "GrStringBuilder.h"
-#include "../GrGpu.h"
+#include "GrGpu.h"
 
 #include "SkXfermode.h"
 
@@ -108,6 +107,7 @@ public:
                 kCustomTextureDomain_OptFlagBit = 1 << 2,
                 kIsEnabled_OptFlagBit           = 1 << 7
             };
+            // Convolution is obsolete; left in for testing only
             enum FetchMode {
                 kSingle_FetchMode,
                 k2x2_FetchMode,
@@ -320,6 +320,9 @@ public:
 
         void copyAndTakeOwnership(CachedData& other) {
             memcpy(this, &other, sizeof(*this));
+            for (int i = 0; i < GrDrawState::kNumStages; ++i) {
+                other.fCustomStage[i] = NULL;
+            }
         }
 
     public:

src/gpu/gl/GrGLProgramStage.cpp

@@ -13,19 +13,11 @@ GrGLProgramStage::~GrGLProgramStage() {
 
 ///////////////////////////////////////////////////////////////////////////////
 
-GrGLProgramStageFactory::~GrGLProgramStageFactory(void) {
+void GrGLProgramStage::setupVSUnis(VarArray* vsUnis, int stage) {
 
 }
 
-uint16_t GrGLProgramStageFactory::stageKey(const GrCustomStage*) {
+void GrGLProgramStage::setupFSUnis(VarArray* fsUnis, int stage) {
-    return 0;
-}
-
-void GrGLProgramStage::setupVSUnis(VarArray& vsUnis, int stage) {
-
-}
-
-void GrGLProgramStage::setupFSUnis(VarArray& fsUnis, int stage) {
 
 }
 
@@ -33,7 +25,8 @@ void GrGLProgramStage::initUniforms(const GrGLInterface*, int progID) {
 
 }
 
-void GrGLProgramStage::setData(const GrGLInterface*, GrCustomStage*) {
+void GrGLProgramStage::setData(const GrGLInterface*, GrCustomStage*,
+                               const GrGLTexture*) {
 
 }
 

src/gpu/gl/GrGLProgramStage.h

@@ -8,13 +8,14 @@
 #ifndef GrGLCustomStage_DEFINED
 #define GrGLCustomStage_DEFINED
 
-#include "../GrAllocator.h"
+#include "GrAllocator.h"
 #include "GrGLShaderVar.h"
 #include "GrGLSL.h"
-#include "../GrStringBuilder.h"
+#include "GrStringBuilder.h"
 
 class GrCustomStage;
 struct GrGLInterface;
+class GrGLTexture;
 
 /** @file
     This file contains specializations for OpenGL of the shader stages
@@ -39,17 +40,19 @@ public:
     
     virtual ~GrGLProgramStage();
 
+    virtual const char* name() const = 0;
+
     /** Creates any uniform variables the vertex shader requires
         and appends them to vsUnis;
         must guarantee they are unique (typically done by
         appending the stage number). */
-    virtual void setupVSUnis(VarArray& vsUnis, int stage);
+    virtual void setupVSUnis(VarArray* vsUnis, int stage);
 
     /** Creates any uniform variables the fragment shader requires
         and appends them to fsUnis;
         must guarantee they are unique (typically done by
         appending the stage number). */
-    virtual void setupFSUnis(VarArray& fsUnis, int stage);
+    virtual void setupFSUnis(VarArray* fsUnis, int stage);
 
     /** Given an empty GrStringBuilder and the names of variables;
         must write shader code into that GrStringBuilder.
@@ -86,7 +89,8 @@ public:
         are to be read.
         TODO: since we don't have a factory, we can't assert to enforce
         this. Shouldn't we? */
-    virtual void setData(const GrGLInterface*, GrCustomStage*);
+    virtual void setData(const GrGLInterface*, GrCustomStage*,
+                         const GrGLTexture*);
 
     // TODO: needs a better name
     enum SamplerMode {
@@ -95,7 +99,7 @@ public:
         kExplicitDivide_SamplerMode  // must do an explicit divide
     };
 
-    void setSamplerMode(SamplerMode shaderMode) { fSamplerMode = shaderMode; }
+    void setSamplerMode(SamplerMode samplerMode) { fSamplerMode = samplerMode; }
 
     /** Returns the *effective* coord name after any perspective divide
         or other transform. */
@@ -118,33 +122,4 @@ protected:
 
 };
 
-
-/// Every GrGLProgramStage subclass needs a GrGLProgramStageFactory subclass
-/// to manage its creation.
-
-class GrGLProgramStageFactory {
-
-public:
-
-    virtual ~GrGLProgramStageFactory();
-
-    /** Returns a short unique identifier for this subclass x its
-        parameters. If the key differs, different shader code must
-        be generated; if the key matches, shader code can be reused.
-        0 == no custom stage. */
-    virtual uint16_t stageKey(const GrCustomStage*);
-
-    /** Returns a new instance of the appropriate implementation class
-        for the given GrCustomStage; caller is responsible for deleting
-        the object. */
-    virtual GrGLProgramStage* createGLInstance(GrCustomStage*) = 0;
-
-protected:
-
-    /** Disable default constructor - instances should be singletons
-        with static factory functions: our test examples are all stateless,
-        but we suspect that future implementations may want to cache data? */
-    GrGLProgramStageFactory() { }
-};
-
 #endif

src/gpu/gl/GrGLRenderTarget.cpp

@@ -1,4 +1,3 @@
-
 /*
  * Copyright 2011 Google Inc.
  *
@@ -6,7 +5,6 @@
  * found in the LICENSE file.
  */
 
-
 #include "GrGLRenderTarget.h"
 
 #include "GrGpuGL.h"

src/gpu/gl/GrGLRenderTarget.h

@@ -1,4 +1,3 @@
-
 /*
  * Copyright 2011 Google Inc.
  *

src/gpu/gl/GrGLShaderVar.h

@@ -1,4 +1,3 @@
-
 /*
  * Copyright 2011 Google Inc.
  *
@@ -11,7 +10,7 @@
 
 #include "GrGLContextInfo.h"
 #include "GrGLSL.h"
-#include "../GrStringBuilder.h"
+#include "GrStringBuilder.h"
 
 #define USE_UNIFORM_FLOAT_ARRAYS true
 

src/gpu/gl/GrGLStencilBuffer.cpp

@@ -1,4 +1,3 @@
-
 /*
  * Copyright 2011 Google Inc.
  *

src/gpu/gl/GrGLStencilBuffer.h

@@ -1,4 +1,3 @@
-
 /*
  * Copyright 2011 Google Inc.
  *
@@ -11,7 +10,7 @@
 #define GrGLStencilBuffer_DEFINED
 
 #include "gl/GrGLInterface.h"
-#include "../GrStencilBuffer.h"
+#include "GrStencilBuffer.h"
 
 class GrGLStencilBuffer : public GrStencilBuffer {
 public:

src/gpu/gl/GrGLTexture.cpp

@@ -1,4 +1,3 @@
-
 /*
  * Copyright 2011 Google Inc.
  *

src/gpu/gl/GrGLTexture.h

@@ -1,4 +1,3 @@
-
 /*
  * Copyright 2011 Google Inc.
  *
@@ -10,7 +9,7 @@
 #ifndef GrGLTexture_DEFINED
 #define GrGLTexture_DEFINED
 
-#include "../GrGpu.h"
+#include "GrGpu.h"
 #include "GrGLRenderTarget.h"
 
 /**

src/gpu/gl/GrGLUtil.cpp

@@ -1,4 +1,3 @@
-
 /*
  * Copyright 2011 Google Inc.
  *

src/gpu/gl/GrGLVertexBuffer.cpp

@@ -1,4 +1,3 @@
-
 /*
  * Copyright 2011 Google Inc.
  *

src/gpu/gl/GrGLVertexBuffer.h

@@ -1,4 +1,3 @@
-
 /*
  * Copyright 2011 Google Inc.
  *
@@ -11,7 +10,7 @@
 #ifndef GrGLVertexBuffer_DEFINED
 #define GrGLVertexBuffer_DEFINED
 
-#include "../GrVertexBuffer.h"
+#include "GrVertexBuffer.h"
 #include "gl/GrGLInterface.h"
 
 class GrGpuGL;

src/gpu/gl/GrGpuGL.cpp

@@ -1,4 +1,3 @@
-
 /*
  * Copyright 2011 Google Inc.
  *

src/gpu/gl/GrGpuGL.h

@@ -1,4 +1,3 @@
-
 /*
  * Copyright 2011 Google Inc.
  *
@@ -11,8 +10,8 @@
 #ifndef GrGpuGL_DEFINED
 #define GrGpuGL_DEFINED
 
-#include "../GrDrawState.h"
+#include "GrDrawState.h"
-#include "../GrGpu.h"
+#include "GrGpu.h"
 #include "GrGLContextInfo.h"
 #include "GrGLIndexBuffer.h"
 #include "GrGLIRect.h"

src/gpu/gl/GrGpuGLShaders.cpp

@@ -1,4 +1,3 @@
-
 /*
  * Copyright 2011 Google Inc.
  *
@@ -7,16 +6,18 @@
  */
 
 
-#include "../GrBinHashKey.h"
+#include "GrBinHashKey.h"
+#include "effects/GrConvolutionEffect.h"
 #include "GrCustomStage.h"
 #include "GrGLProgram.h"
 #include "GrGLProgramStage.h"
 #include "GrGLSL.h"
 #include "GrGpuGLShaders.h"
-#include "../GrGpuVertex.h"
+#include "GrGpuVertex.h"
 #include "GrNoncopyable.h"
-#include "../GrStringBuilder.h"
+#include "GrProgramStageFactory.h"
-#include "../GrRandom.h"
+#include "GrRandom.h"
+#include "GrStringBuilder.h"
 
 #define SKIP_CACHE_CHECK    true
 #define GR_UINT32_MAX   static_cast<uint32_t>(-1)
@@ -263,6 +264,10 @@ bool GrGpuGLShaders::programUnitTest() {
                 pdesc.fVertexLayout |= kTextFormat_VertexLayoutBit;
             }
             StageDesc& stage = pdesc.fStages[s];
+
+            stage.fCustomStageKey = 0;
+            customStages[s] = NULL;
+
             stage.fOptFlags = STAGE_OPTS[random_int(&random, GR_ARRAY_COUNT(STAGE_OPTS))];
             stage.fInConfigFlags = IN_CONFIG_FLAGS[random_int(&random, GR_ARRAY_COUNT(IN_CONFIG_FLAGS))];
             stage.fCoordMapping =  random_int(&random, StageDesc::kCoordMappingCnt);
@@ -294,8 +299,19 @@ bool GrGpuGLShaders::programUnitTest() {
                     break;
             }
 
-            stage.fCustomStageKey = 0;
+            // TODO: is there a more elegant way to express this?
-            customStages[s] = NULL;
+            if (stage.fFetchMode == StageDesc::kConvolution_FetchMode) {
+                int direction = random_int(&random, 2);
+                float kernel[stage.fKernelWidth];
+                for (int i = 0; i < stage.fKernelWidth; i++) {
+                    kernel[i] = random.nextF();
+                }
+                customStages[s] = new GrConvolutionEffect(
+                    (GrSamplerState::FilterDirection)direction,
+                    stage.fKernelWidth, kernel);
+                stage.fCustomStageKey =
+                    customStages[s]->getFactory()->stageKey(customStages[s]);
+            }
         }
         CachedData cachedData;
         if (!program.genProgram(this->glContextInfo(), customStages,
@@ -829,8 +845,11 @@ bool GrGpuGLShaders::flushGraphicsState(GrPrimitiveType type) {
             if (NULL != fProgramData->fCustomStage[s]) {
                 const GrSamplerState& sampler =
                     this->getDrawState().getSampler(s);
+                const GrGLTexture* texture =
+                    static_cast<const GrGLTexture*>(
+                        this->getDrawState().getTexture(s));
                 fProgramData->fCustomStage[s]->setData(
-                    this->glInterface(), sampler.getCustomStage());
+                    this->glInterface(), sampler.getCustomStage(), texture);
             }
         }
     }
@@ -1002,7 +1021,7 @@ void setup_custom_stage(GrGLProgram::ProgramDesc::StageDesc* stage,
                         GrGLProgram* program, int index) {
     GrCustomStage* customStage = sampler.getCustomStage();
     if (customStage) {
-        GrGLProgramStageFactory* factory = customStage->getGLFactory();
+        GrProgramStageFactory* factory = customStage->getFactory();
         stage->fCustomStageKey = factory->stageKey(customStage);
         customStages[index] = customStage;
     } else {
@@ -1215,8 +1234,7 @@ void GrGpuGLShaders::buildProgram(GrPrimitiveType type,
                 }
             }
 
-            if (sampler.getFilter() == GrSamplerState::kConvolution_Filter ||
+            if (sampler.getFilter() == GrSamplerState::kDilate_Filter ||
-                sampler.getFilter() == GrSamplerState::kDilate_Filter ||
                 sampler.getFilter() == GrSamplerState::kErode_Filter) {
                 stage.fKernelWidth = sampler.getKernelWidth();
             } else {

src/gpu/gl/GrGpuGLShaders.h

@@ -1,4 +1,3 @@
-
 /*
  * Copyright 2011 Google Inc.
  *