Incremental refactoring of GrGLProgram and GrGLShaderBuilder

R=robertphillips@google.com

Author: bsalomon@google.com

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

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

5 files changed, 134 insertions, 162 deletions

diff --git a/src/gpu/gl/GrGLProgram.cpp b/src/gpu/gl/GrGLProgram.cpp
index c63914893d..1d85b0a327 100644
--- a/src/gpu/gl/GrGLProgram.cpp
+++ b/src/gpu/gl/GrGLProgram.cpp
@@ -24,42 +24,33 @@ GrGLProgram* GrGLProgram::Create(GrGpuGL* gpu,
                                  const GrGLProgramDesc& desc,
                                  const GrEffectStage* colorStages[],
                                  const GrEffectStage* coverageStages[]) {
-    GrGLProgram* program = SkNEW_ARGS(GrGLProgram, (gpu, desc, colorStages, coverageStages));
-    if (!program->succeeded()) {
-        delete program;
-        program = NULL;
+    GrGLShaderBuilder::GenProgramOutput output;
+    SkAutoTUnref<GrGLUniformManager> uman(SkNEW_ARGS(GrGLUniformManager, (gpu)));
+    if (GrGLShaderBuilder::GenProgram(gpu, uman, desc, colorStages, coverageStages,
+                                      &output)) {
+        SkASSERT(0 != output.fProgramID);
+        return SkNEW_ARGS(GrGLProgram, (gpu, desc, uman, output));
     }
-    return program;
+    return NULL;
 }
 
 GrGLProgram::GrGLProgram(GrGpuGL* gpu,
                          const GrGLProgramDesc& desc,
-                         const GrEffectStage* colorStages[],
-                         const GrEffectStage* coverageStages[])
-: fGpu(gpu)
-, fUniformManager(gpu)
-, fHasVertexShader(false)
-, fNumTexCoordSets(0) {
-    fDesc = desc;
-    fProgramID = 0;
-
-    fDstCopyTexUnit = -1;
-
-    fColor = GrColor_ILLEGAL;
-
-    GrGLShaderBuilder::GenProgramOutput output;
-
-    if (GrGLShaderBuilder::GenProgram(gpu, fUniformManager, desc, colorStages, coverageStages,
-                                      &output)) {
-        fProgramID = output.fProgramID;
-        fUniformHandles = output.fUniformHandles;
-        fColorEffects.reset(output.fColorEffects);
-        fCoverageEffects.reset(output.fCoverageEffects);
-        fHasVertexShader = output.fHasVS;
-        fNumTexCoordSets = output.fNumTexCoordSets;
-        fGpu = gpu;
-        this->initSamplerUniforms();
-    }
+                         GrGLUniformManager* uman,
+                         const GrGLShaderBuilder::GenProgramOutput& builderOutput)
+    : fProgramID(builderOutput.fProgramID)
+    , fColor(GrColor_ILLEGAL)
+    , fCoverage(GrColor_ILLEGAL)
+    , fDstCopyTexUnit(-1)
+    , fColorEffects(builderOutput.fColorEffects)
+    , fCoverageEffects(builderOutput.fCoverageEffects)
+    , fDesc(desc)
+    , fGpu(gpu)
+    , fUniformManager(SkRef(uman))
+    , fUniformHandles(builderOutput.fUniformHandles)
+    , fHasVertexShader(builderOutput.fHasVS)
+    , fNumTexCoordSets(builderOutput.fNumTexCoordSets) {
+    this->initSamplerUniforms();
 }
 
 GrGLProgram::~GrGLProgram() {
@@ -98,11 +89,11 @@ void GrGLProgram::initSamplerUniforms() {
     GL_CALL(UseProgram(fProgramID));
     GrGLint texUnitIdx = 0;
     if (fUniformHandles.fDstCopySamplerUni.isValid()) {
-        fUniformManager.setSampler(fUniformHandles.fDstCopySamplerUni, texUnitIdx);
+        fUniformManager->setSampler(fUniformHandles.fDstCopySamplerUni, texUnitIdx);
         fDstCopyTexUnit = texUnitIdx++;
     }
-    fColorEffects->initSamplers(fUniformManager, &texUnitIdx);
-    fCoverageEffects->initSamplers(fUniformManager, &texUnitIdx);
+    fColorEffects->initSamplers(*fUniformManager, &texUnitIdx);
+    fCoverageEffects->initSamplers(*fUniformManager, &texUnitIdx);
 }
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -133,12 +124,12 @@ void GrGLProgram::setData(GrDrawState::BlendOptFlags blendOpts,
 
     if (NULL != dstCopy) {
         if (fUniformHandles.fDstCopyTopLeftUni.isValid()) {
-            fUniformManager.set2f(fUniformHandles.fDstCopyTopLeftUni,
-                                  static_cast<GrGLfloat>(dstCopy->offset().fX),
-                                  static_cast<GrGLfloat>(dstCopy->offset().fY));
-            fUniformManager.set2f(fUniformHandles.fDstCopyScaleUni,
-                                  1.f / dstCopy->texture()->width(),
-                                  1.f / dstCopy->texture()->height());
+            fUniformManager->set2f(fUniformHandles.fDstCopyTopLeftUni,
+                                   static_cast<GrGLfloat>(dstCopy->offset().fX),
+                                   static_cast<GrGLfloat>(dstCopy->offset().fY));
+            fUniformManager->set2f(fUniformHandles.fDstCopyScaleUni,
+                                   1.f / dstCopy->texture()->width(),
+                                   1.f / dstCopy->texture()->height());
             GrGLTexture* texture = static_cast<GrGLTexture*>(dstCopy->texture());
             static GrTextureParams kParams; // the default is clamp, nearest filtering.
             fGpu->bindTexture(fDstCopyTexUnit, kParams, texture);
@@ -152,8 +143,8 @@ void GrGLProgram::setData(GrDrawState::BlendOptFlags blendOpts,
         SkASSERT(!fUniformHandles.fDstCopySamplerUni.isValid());
     }
 
-    fColorEffects->setData(fGpu, fUniformManager, colorStages);
-    fCoverageEffects->setData(fGpu, fUniformManager, coverageStages);
+    fColorEffects->setData(fGpu, *fUniformManager, colorStages);
+    fCoverageEffects->setData(fGpu, *fUniformManager, coverageStages);
 
 
     // PathTexGen state applies to the the fixed function vertex shader. For
@@ -187,7 +178,7 @@ void GrGLProgram::setColor(const GrDrawState& drawState,
                     // OpenGL ES doesn't support unsigned byte varieties of glUniform
                     GrGLfloat c[4];
                     GrColorToRGBAFloat(color, c);
-                    fUniformManager.set4fv(fUniformHandles.fColorUni, 1, c);
+                    fUniformManager->set4fv(fUniformHandles.fColorUni, 1, c);
                     fColor = color;
                 }
                 sharedState->fConstAttribColorIndex = -1;
@@ -226,7 +217,7 @@ void GrGLProgram::setCoverage(const GrDrawState& drawState,
                     // OpenGL ES doesn't support unsigned byte varieties of glUniform
                     GrGLfloat c[4];
                     GrColorToRGBAFloat(coverage, c);
-                    fUniformManager.set4fv(fUniformHandles.fCoverageUni, 1, c);
+                    fUniformManager->set4fv(fUniformHandles.fCoverageUni, 1, c);
                     fCoverage = coverage;
                 }
                 sharedState->fConstAttribCoverageIndex = -1;
@@ -251,7 +242,7 @@ void GrGLProgram::setMatrixAndRenderTargetHeight(const GrDrawState& drawState) {
     // Load the RT height uniform if it is needed to y-flip gl_FragCoord.
     if (fUniformHandles.fRTHeightUni.isValid() &&
         fMatrixState.fRenderTargetSize.fHeight != size.fHeight) {
-        fUniformManager.set1f(fUniformHandles.fRTHeightUni, SkIntToScalar(size.fHeight));
+        fUniformManager->set1f(fUniformHandles.fRTHeightUni, SkIntToScalar(size.fHeight));
     }
 
     if (!fHasVertexShader) {
@@ -269,10 +260,10 @@ void GrGLProgram::setMatrixAndRenderTargetHeight(const GrDrawState& drawState) {
 
         GrGLfloat viewMatrix[3 * 3];
         fMatrixState.getGLMatrix<3>(viewMatrix);
-        fUniformManager.setMatrix3f(fUniformHandles.fViewMatrixUni, viewMatrix);
+        fUniformManager->setMatrix3f(fUniformHandles.fViewMatrixUni, viewMatrix);
 
         GrGLfloat rtAdjustmentVec[4];
         fMatrixState.getRTAdjustmentVec(rtAdjustmentVec);
-        fUniformManager.set4fv(fUniformHandles.fRTAdjustmentUni, 1, rtAdjustmentVec);
+        fUniformManager->set4fv(fUniformHandles.fRTAdjustmentUni, 1, rtAdjustmentVec);
     }
 }
diff --git a/src/gpu/gl/GrGLProgram.h b/src/gpu/gl/GrGLProgram.h
index a9a2da222e..4d5ce0f24d 100644
--- a/src/gpu/gl/GrGLProgram.h
+++ b/src/gpu/gl/GrGLProgram.h
@@ -166,18 +166,8 @@ private:
 
     GrGLProgram(GrGpuGL* gpu,
                 const GrGLProgramDesc& desc,
-                const GrEffectStage* colorStages[],
-                const GrEffectStage* coverageStages[]);
-
-    bool succeeded() const { return 0 != fProgramID; }
-
-    /**
-     * This is the heavy initialization routine for building a GLProgram. colorStages and
-     * coverageStages correspond to the output of GrGLProgramDesc::Build().
-     */
-    bool genProgram(GrGLShaderBuilder* builder,
-                    const GrEffectStage* colorStages[],
-                    const GrEffectStage* coverageStages[]);
+                GrGLUniformManager* uman,
+                const GrGLShaderBuilder::GenProgramOutput& builderOutput);
 
     // Sets the texture units for samplers
     void initSamplerUniforms();
@@ -194,25 +184,26 @@ private:
     void setMatrixAndRenderTargetHeight(const GrDrawState&);
 
     // GL program ID
-    GrGLuint                    fProgramID;
+    GrGLuint                            fProgramID;
 
     // these reflect the current values of uniforms (GL uniform values travel with program)
-    MatrixState                       fMatrixState;
-    GrColor                           fColor;
-    GrColor                           fCoverage;
-    int                               fDstCopyTexUnit;
-
-    SkAutoTDelete<GrGLProgramEffects> fColorEffects;
-    SkAutoTDelete<GrGLProgramEffects> fCoverageEffects;
-
-    GrGLProgramDesc                   fDesc;
-    GrGpuGL*                          fGpu;
-
-    GrGLUniformManager                fUniformManager;
-    GrGLShaderBuilder::UniformHandles fUniformHandles;
-
-    bool                              fHasVertexShader;
-    int                               fNumTexCoordSets;
+    MatrixState                         fMatrixState;
+    GrColor                             fColor;
+    GrColor                             fCoverage;
+    int                                 fDstCopyTexUnit;
+
+    SkAutoTDelete<GrGLProgramEffects>   fColorEffects;
+    SkAutoTDelete<GrGLProgramEffects>   fCoverageEffects;
+    
+    GrGLProgramDesc                     fDesc;
+    
+    GrGpuGL*                            fGpu;
+
+    SkAutoTUnref<GrGLUniformManager>    fUniformManager;
+    GrGLShaderBuilder::UniformHandles   fUniformHandles;
+
+    bool                                fHasVertexShader;
+    int                                 fNumTexCoordSets;
 
     typedef SkRefCnt INHERITED;
 };
diff --git a/src/gpu/gl/GrGLShaderBuilder.cpp b/src/gpu/gl/GrGLShaderBuilder.cpp
index 52644f82b8..79cb52fc8a 100644
--- a/src/gpu/gl/GrGLShaderBuilder.cpp
+++ b/src/gpu/gl/GrGLShaderBuilder.cpp
@@ -90,7 +90,7 @@ static const char kDstCopyColorName[] = "_dstColor";
 ///////////////////////////////////////////////////////////////////////////////
 
 bool GrGLShaderBuilder::GenProgram(GrGpuGL* gpu,
-                                   GrGLUniformManager& uman,
+                                   GrGLUniformManager* uman,
                                    const GrGLProgramDesc& desc,
                                    const GrEffectStage* inColorStages[],
                                    const GrEffectStage* inCoverageStages[],
@@ -174,12 +174,12 @@ bool GrGLShaderBuilder::genProgram(const GrEffectStage* colorStages[],
         return false;
     }
 
-    output->fUniformHandles.fRTHeightUni = this->getRTHeightUniform();
-    output->fUniformHandles.fDstCopyTopLeftUni = this->getDstCopyTopLeftUniform();
-    output->fUniformHandles.fDstCopyScaleUni = this->getDstCopyScaleUniform();
-    output->fUniformHandles.fColorUni = this->getColorUniform();
-    output->fUniformHandles.fCoverageUni = this->getCoverageUniform();
-    output->fUniformHandles.fDstCopySamplerUni = this->getDstCopySamplerUniform();
+    output->fUniformHandles.fRTHeightUni = fRTHeightUniform;
+    output->fUniformHandles.fColorUni = fColorUniform;
+    output->fUniformHandles.fCoverageUni = fCoverageUniform;
+    output->fUniformHandles.fDstCopyTopLeftUni = fDstCopyTopLeftUniform;
+    output->fUniformHandles.fDstCopyScaleUni = fDstCopyScaleUniform;
+    output->fUniformHandles.fDstCopySamplerUni = fDstCopySamplerUniform;
 
     return true;
 }
@@ -187,11 +187,11 @@ bool GrGLShaderBuilder::genProgram(const GrEffectStage* colorStages[],
 //////////////////////////////////////////////////////////////////////////////
 
 GrGLShaderBuilder::GrGLShaderBuilder(GrGpuGL* gpu,
-                                     GrGLUniformManager& uniformManager,
+                                     GrGLUniformManager* uniformManager,
                                      const GrGLProgramDesc& desc)
     : fDesc(desc)
     , fGpu(gpu)
-    , fUniformManager(uniformManager)
+    , fUniformManager(SkRef(uniformManager))
     , fFSFeaturesAddedMask(0)
     , fFSInputs(kVarsPerBlock)
     , fFSOutputs(kMaxFSOutputs)
@@ -453,7 +453,7 @@ GrGLUniformManager::UniformHandle GrGLShaderBuilder::addUniformArray(uint32_t vi
     BuilderUniform& uni = fUniforms.push_back();
     UniformHandle h = GrGLUniformManager::UniformHandle::CreateFromUniformIndex(fUniforms.count() - 1);
     SkDEBUGCODE(UniformHandle h2 =)
-    fUniformManager.appendUniform(type, count);
+    fUniformManager->appendUniform(type, count);
     // We expect the uniform manager to initially have no uniforms and that all uniforms are added
     // by this function. Therefore, the handles should match.
     SkASSERT(h2 == h);
@@ -682,8 +682,8 @@ bool GrGLShaderBuilder::finish(GrGLuint* outProgramId) {
     }
 
     this->bindProgramLocations(programId);
-    if (fUniformManager.isUsingBindUniform()) {
-      fUniformManager.getUniformLocations(programId, fUniforms);
+    if (fUniformManager->isUsingBindUniform()) {
+      fUniformManager->getUniformLocations(programId, fUniforms);
     }
 
     GL_CALL(LinkProgram(programId));
@@ -716,8 +716,8 @@ bool GrGLShaderBuilder::finish(GrGLuint* outProgramId) {
         }
     }
 
-    if (!fUniformManager.isUsingBindUniform()) {
-      fUniformManager.getUniformLocations(programId, fUniforms);
+    if (!fUniformManager->isUsingBindUniform()) {
+      fUniformManager->getUniformLocations(programId, fUniforms);
     }
 
     for (int i = 0; i < shadersToDelete.count(); ++i) {
@@ -830,7 +830,7 @@ const GrGLContextInfo& GrGLShaderBuilder::ctxInfo() const {
 ////////////////////////////////////////////////////////////////////////////////
 
 GrGLFullShaderBuilder::GrGLFullShaderBuilder(GrGpuGL* gpu,
-                                             GrGLUniformManager& uniformManager,
+                                             GrGLUniformManager* uniformManager,
                                              const GrGLProgramDesc& desc)
     : INHERITED(gpu, uniformManager, desc)
     , fVSAttrs(kVarsPerBlock)
@@ -1070,7 +1070,7 @@ void GrGLFullShaderBuilder::bindProgramLocations(GrGLuint programId) const {
 ////////////////////////////////////////////////////////////////////////////////
 
 GrGLFragmentOnlyShaderBuilder::GrGLFragmentOnlyShaderBuilder(GrGpuGL* gpu,
-                                                             GrGLUniformManager& uniformManager,
+                                                             GrGLUniformManager* uniformManager,
                                                              const GrGLProgramDesc& desc)
     : INHERITED(gpu, uniformManager, desc)
     , fNumTexCoordSets(0) {
diff --git a/src/gpu/gl/GrGLShaderBuilder.h b/src/gpu/gl/GrGLShaderBuilder.h
index 90f1a94ea3..f3a63af9ed 100644
--- a/src/gpu/gl/GrGLShaderBuilder.h
+++ b/src/gpu/gl/GrGLShaderBuilder.h
@@ -70,7 +70,7 @@ public:
     };
 
     static bool GenProgram(GrGpuGL* gpu,
-                           GrGLUniformManager& uman,
+                           GrGLUniformManager* uman,
                            const GrGLProgramDesc& desc,
                            const GrEffectStage* inColorStages[],
                            const GrEffectStage* inCoverageStages[],
@@ -174,7 +174,7 @@ public:
                                                       const char** outName = NULL);
 
     const GrGLShaderVar& getUniformVariable(GrGLUniformManager::UniformHandle u) const {
-        return fUniformManager.getBuilderUniform(fUniforms, u).fVariable;
+        return fUniformManager->getBuilderUniform(fUniforms, u).fVariable;
     }
 
     /**
@@ -195,51 +195,10 @@ public:
         is in device space (e.g. 0,0 is the top left and pixel centers are at half-integers). */
     const char* fragmentPosition();
 
-    /** Returns the color of the destination pixel. This may be NULL if no effect advertised
-        that it will read the destination. */
+    /** Returns the variable name that holds the color of the destination pixel. This may be NULL if
+        no effect advertised that it will read the destination. */
     const char* dstColor();
 
-    /**
-     * Interfaces used by GrGLProgram.
-     */
-    const GrGLSLExpr4& getInputColor() const {
-        return fInputColor;
-    }
-    const GrGLSLExpr4& getInputCoverage() const {
-        return fInputCoverage;
-    }
-
-    /**
-     * Adds code for effects and returns a GrGLProgramEffects* object. The caller is responsible for
-     * deleting it when finished. effectStages contains the effects to add. effectKeys[i] is the key
-     * generated from effectStages[i]. inOutFSColor specifies the input color to the first stage and
-     * is updated to be the output color of the last stage.
-     * The handles to texture samplers for effectStage[i] are added to
-     * effectSamplerHandles[i].
-     */
-    virtual GrGLProgramEffects* createAndEmitEffects(const GrEffectStage* effectStages[],
-                                                     const EffectKey effectKeys[],
-                                                     int effectCnt,
-                                                     GrGLSLExpr4* inOutFSColor) = 0;
-
-    const char* getColorOutputName() const;
-    const char* enableSecondaryOutput();
-
-    GrGLUniformManager::UniformHandle getRTHeightUniform() const { return fRTHeightUniform; }
-    GrGLUniformManager::UniformHandle getDstCopyTopLeftUniform() const {
-        return fDstCopyTopLeftUniform;
-    }
-    GrGLUniformManager::UniformHandle getDstCopyScaleUniform() const {
-        return fDstCopyScaleUniform;
-    }
-    GrGLUniformManager::UniformHandle getColorUniform() const { return fColorUniform; }
-    GrGLUniformManager::UniformHandle getCoverageUniform() const { return fCoverageUniform; }
-    GrGLUniformManager::UniformHandle getDstCopySamplerUniform() const {
-        return fDstCopySamplerUniform;
-    }
-
-    bool finish(GrGLuint* outProgramId);
-
     const GrGLContextInfo& ctxInfo() const;
 
     /**
@@ -262,7 +221,7 @@ public:
     };
 
 protected:
-    GrGLShaderBuilder(GrGpuGL*, GrGLUniformManager&, const GrGLProgramDesc&);
+    GrGLShaderBuilder(GrGpuGL*, GrGLUniformManager*, const GrGLProgramDesc&);
 
     GrGpuGL* gpu() const { return fGpu; }
 
@@ -274,11 +233,6 @@ protected:
     /** Add input/output variable declarations (i.e. 'varying') to the fragment shader. */
     GrGLShaderVar& fsInputAppend() { return fFSInputs.push_back(); }
 
-    // Generates a name for a variable. The generated string will be name prefixed by the prefix
-    // char (unless the prefix is '\0'). It also mangles the name to be stage-specific if we're
-    // generating stage code.
-    void nameVariable(SkString* out, char prefix, const char* name);
-
     // Helper for emitEffects().
     void createAndEmitEffects(GrGLProgramEffectsBuilder*,
                               const GrEffectStage* effectStages[],
@@ -286,7 +240,13 @@ protected:
                               int effectCnt,
                               GrGLSLExpr4* inOutFSColor);
 
+    // Generates a name for a variable. The generated string will be name prefixed by the prefix
+    // char (unless the prefix is '\0'). It also mangles the name to be stage-specific if we're
+    // generating stage code.
+    void nameVariable(SkString* out, char prefix, const char* name);
+
     virtual bool compileAndAttachShaders(GrGLuint programId, SkTDArray<GrGLuint>* shaderIds) const;
+
     virtual void bindProgramLocations(GrGLuint programId) const;
 
     void appendDecls(const VarArray&, SkString*) const;
@@ -349,6 +309,35 @@ private:
                     GenProgramOutput* output);
 
     /**
+    * Adds code for effects and returns a GrGLProgramEffects* object. The caller is responsible for
+    * deleting it when finished. effectStages contains the effects to add. effectKeys[i] is the key
+    * generated from effectStages[i]. inOutFSColor specifies the input color to the first stage and
+    * is updated to be the output color of the last stage.
+    * The handles to texture samplers for effectStage[i] are added to
+    * effectSamplerHandles[i].
+    */
+    virtual GrGLProgramEffects* createAndEmitEffects(const GrEffectStage* effectStages[],
+                                                     const EffectKey effectKeys[],
+                                                     int effectCnt,
+                                                     GrGLSLExpr4* inOutFSColor) = 0;
+
+    /** Enables using the secondary color output and returns the name of the var in which it is
+        to be stored */
+    const char* enableSecondaryOutput();
+    /** Gets the name of the primary color output. */
+    const char* getColorOutputName() const;
+
+    bool finish(GrGLuint* outProgramId);
+
+    const GrGLSLExpr4& getInputColor() const {
+        return fInputColor;
+    }
+
+    const GrGLSLExpr4& getInputCoverage() const {
+        return fInputCoverage;
+    }
+
+    /**
      * Features that should only be enabled by GrGLShaderBuilder itself.
      */
     enum GLSLPrivateFeature {
@@ -378,7 +367,7 @@ private:
 
     const GrGLProgramDesc&                  fDesc;
     GrGpuGL*                                fGpu;
-    GrGLUniformManager&                     fUniformManager;
+    SkAutoTUnref<GrGLUniformManager>        fUniformManager;
     uint32_t                                fFSFeaturesAddedMask;
     SkString                                fFSFunctions;
     SkString                                fFSExtensions;
@@ -410,7 +399,7 @@ private:
 
 class GrGLFullShaderBuilder : public GrGLShaderBuilder {
 public:
-    GrGLFullShaderBuilder(GrGpuGL*, GrGLUniformManager&, const GrGLProgramDesc&);
+    GrGLFullShaderBuilder(GrGpuGL*, GrGLUniformManager*, const GrGLProgramDesc&);
 
     /**
      * Called by GrGLEffects to add code to one of the shaders.
@@ -454,12 +443,6 @@ public:
     bool addEffectAttribute(int attributeIndex, GrSLType type, const SkString& name);
     const SkString* getEffectAttributeName(int attributeIndex) const;
 
-    virtual GrGLProgramEffects* createAndEmitEffects(
-                const GrEffectStage* effectStages[],
-                const EffectKey effectKeys[],
-                int effectCnt,
-                GrGLSLExpr4* inOutFSColor) SK_OVERRIDE;
-
     /**
      * The view matrix uniform is only valid in the VS. It is always mat33.
      */
@@ -471,11 +454,16 @@ public:
         return fRTAdustmentVecUniform;
     }
 
-protected:
-    virtual bool compileAndAttachShaders(GrGLuint programId, SkTDArray<GrGLuint>* shaderIds) const SK_OVERRIDE;
+private:
+    virtual GrGLProgramEffects* createAndEmitEffects(const GrEffectStage* effectStages[],
+                                                     const EffectKey effectKeys[],
+                                                     int effectCnt,
+                                                     GrGLSLExpr4* inOutFSColor) SK_OVERRIDE;
+    virtual bool compileAndAttachShaders(GrGLuint programId,
+                                         SkTDArray<GrGLuint>* shaderIds) const SK_OVERRIDE;
+
     virtual void bindProgramLocations(GrGLuint programId) const SK_OVERRIDE;
 
-private:
     VarArray                            fVSAttrs;
     VarArray                            fVSOutputs;
     VarArray                            fGSInputs;
@@ -504,18 +492,18 @@ private:
 
 class GrGLFragmentOnlyShaderBuilder : public GrGLShaderBuilder {
 public:
-    GrGLFragmentOnlyShaderBuilder(GrGpuGL*, GrGLUniformManager&, const GrGLProgramDesc&);
+    GrGLFragmentOnlyShaderBuilder(GrGpuGL*, GrGLUniformManager*, const GrGLProgramDesc&);
 
     int getNumTexCoordSets() const { return fNumTexCoordSets; }
     int addTexCoordSets(int count);
 
-    virtual GrGLProgramEffects* createAndEmitEffects(
-                const GrEffectStage* effectStages[],
-                const EffectKey effectKeys[],
-                int effectCnt,
-                GrGLSLExpr4* inOutFSColor) SK_OVERRIDE;
-
 private:
+
+    virtual GrGLProgramEffects* createAndEmitEffects(const GrEffectStage* effectStages[],
+                                                     const EffectKey effectKeys[],
+                                                     int effectCnt,
+                                                     GrGLSLExpr4* inOutFSColor) SK_OVERRIDE;
+
     int fNumTexCoordSets;
 
     typedef GrGLShaderBuilder INHERITED;
diff --git a/src/gpu/gl/GrGLUniformManager.h b/src/gpu/gl/GrGLUniformManager.h
index d7438f9ed7..6137bcd9cc 100644
--- a/src/gpu/gl/GrGLUniformManager.h
+++ b/src/gpu/gl/GrGLUniformManager.h
@@ -19,7 +19,7 @@ class SkMatrix;
 
 /** Manages a program's uniforms.
 */
-class GrGLUniformManager {
+class GrGLUniformManager : public SkRefCnt {
 public:
     // Opaque handle to a uniform
     class UniformHandle {
@@ -113,6 +113,8 @@ private:
     bool fUsingBindUniform;
     SkTArray<Uniform, true> fUniforms;
     GrGpuGL* fGpu;
+
+    typedef SkRefCnt INHERITED;
 };
 
 #endif