Solo gp

src/gpu/gl/builders/GrGLFragmentShaderBuilder.h

Index: src/gpu/gl/builders/GrGLFragmentShaderBuilder.h
diff --git a/src/gpu/gl/builders/GrGLFragmentShaderBuilder.h b/src/gpu/gl/builders/GrGLFragmentShaderBuilder.h
index b3e0ab0405610ed15a119744439ff071b925c09c..2dd416203f621617ebdd923996d195a83b5046e2 100644
--- a/src/gpu/gl/builders/GrGLFragmentShaderBuilder.h
+++ b/src/gpu/gl/builders/GrGLFragmentShaderBuilder.h
@@ -11,27 +11,10 @@
 
 class GrGLProgramBuilder;
 
-class GrGLFragmentShaderBuilder : public GrGLShaderBuilder {
+class GrGLBaseFragmentShaderBuilder : public GrGLShaderBuilder {

[bsalomon, 2014/09/22 15:35:24]

The name's are starting to get a bit confusing, I think.

What about something like:

GrGLFragmentShaderBuilder with subclasses GrGLFragmentProccesorBuilder and
GrGLGeometryProcessorFragmentBuilder? This might be off the mark because I'm not
sure I fully get it. We might need a nice comment somewhere describing the
ShaderBuilder hierarchy and what the different classes are used for.

[joshua.litt, 2014/09/22 18:14:44]

 I will use these names for now.  Immediately after this patch I will break out
the user interface from the backend building of shaders which should really
improve these files.

On 2014/09/22 15:35:24, bsalomon wrote:

 public:
-    typedef uint8_t DstReadKey;
-    typedef uint8_t FragPosKey;
-
-    /**  Returns a key for adding code to read the copy-of-dst color in service of effects that
-        require reading the dst. It must not return 0 because 0 indicates that there is no dst
-        copy read at all (in which case this function should not be called). */
-    static DstReadKey KeyForDstRead(const GrTexture* dstCopy, const GrGLCaps&);
-
-    /** Returns a key for reading the fragment location. This should only be called if there is an
-       effect that will requires the fragment position. If the fragment position is not required,
-       the key is 0. */
-    static FragPosKey KeyForFragmentPosition(const GrRenderTarget* dst, const GrGLCaps&);
-
-    GrGLFragmentShaderBuilder(GrGLProgramBuilder* program, const GrGLProgramDesc& desc);
-
-    /** 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();
-
+    GrGLBaseFragmentShaderBuilder(GrGLProgramBuilder* program) : INHERITED(program) {}
+    virtual ~GrGLBaseFragmentShaderBuilder() {}
     /**
      * Use of these features may require a GLSL extension to be enabled. Shaders may not compile
      * if code is added that uses one of these features without calling enableFeature()
@@ -45,19 +28,61 @@ public:
      * If the feature is supported then true is returned and any necessary #extension declarations
      * are added to the shaders. If the feature is not supported then false will be returned.
      */
-    bool enableFeature(GLSLFeature);
+    virtual bool enableFeature(GLSLFeature) = 0;
 
     /**
      * This returns a variable name to access the 2D, perspective correct version of the coords in
      * the fragment shader. If the coordinates at index are 3-dimensional, it immediately emits a
      * perspective divide into the fragment shader (xy / z) to convert them to 2D.
      */
-    SkString ensureFSCoords2D(const TransformedCoordsArray& coords, int index);
+    virtual SkString ensureFSCoords2D(const GrGLProcessor::TransformedCoordsArray& coords,
+                                      int index) = 0;
 
 
     /** Returns a variable name that represents the position of the fragment in the FS. The position
         is in device space (e.g. 0,0 is the top left and pixel centers are at half-integers). */
-    const char* fragmentPosition();
+    virtual const char* fragmentPosition() = 0;
+
+private:
+    typedef GrGLShaderBuilder INHERITED;
+};
+
+class GrGLFullFragmentShaderBuilder : public GrGLBaseFragmentShaderBuilder {
+public:
+    GrGLFullFragmentShaderBuilder(GrGLProgramBuilder* program) : INHERITED(program) {}
+    /** 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. */
+    virtual const char* dstColor() = 0;
+
+private:
+    typedef GrGLBaseFragmentShaderBuilder INHERITED;
+};
+
+class GrGLFragmentShaderBuilder : public GrGLFullFragmentShaderBuilder {
+public:
+    typedef uint8_t DstReadKey;
+    typedef uint8_t FragPosKey;
+
+    /**  Returns a key for adding code to read the copy-of-dst color in service of effects that
+        require reading the dst. It must not return 0 because 0 indicates that there is no dst
+        copy read at all (in which case this function should not be called). */
+    static DstReadKey KeyForDstRead(const GrTexture* dstCopy, const GrGLCaps&);
+
+    /** Returns a key for reading the fragment location. This should only be called if there is an
+       effect that will requires the fragment position. If the fragment position is not required,
+       the key is 0. */
+    static FragPosKey KeyForFragmentPosition(const GrRenderTarget* dst, const GrGLCaps&);
+
+    GrGLFragmentShaderBuilder(GrGLProgramBuilder* program, const GrGLProgramDesc& desc);
+
+    virtual const char* dstColor() SK_OVERRIDE;
+
+    virtual bool enableFeature(GLSLFeature) SK_OVERRIDE;
+
+    virtual SkString ensureFSCoords2D(const GrGLProcessor::TransformedCoordsArray& coords,
+                                      int index) SK_OVERRIDE;
+
+    virtual const char* fragmentPosition() SK_OVERRIDE;
 
 private:
     /*
@@ -113,7 +138,7 @@ private:
     friend class GrGLProgramBuilder;
     friend class GrGLFullProgramBuilder;
 
-    typedef GrGLShaderBuilder INHERITED;
+    typedef GrGLFullFragmentShaderBuilder INHERITED;
 };
 
 #endif