Draft change to start pulling uniform color into GP

src/gpu/GrGeometryProcessor.h

Index: src/gpu/GrGeometryProcessor.h
diff --git a/src/gpu/GrGeometryProcessor.h b/src/gpu/GrGeometryProcessor.h
index 50ecac48ba0c74a33d27dbf29bd1f203ab9f3051..e6cd59b01f60d2f890cde51e4533c7a4c3d3e02e 100644
--- a/src/gpu/GrGeometryProcessor.h
+++ b/src/gpu/GrGeometryProcessor.h
@@ -14,6 +14,30 @@
 #include "GrShaderVar.h"
 
 /*
+ * The GrPrimitiveProcessor represents some kind of geometric primitive.  This includes the shape

[bsalomon, 2014/12/15 15:31:13]

This comment is great!

Two minor suggestions:

maybe convey early in the second paragraph that the feedback loops are on the
CPU side and pertain to invariant portions of the GPU computation.

deferred geometry: maybe a quick sentence that system is evolving to do any
tessellation and mesh draws after a batching/reorder pass is applied (that is,
define "deferred geometry").

We need a better word than bundle, GrGeoData? GrPrimitive? ... doesn't need to
be resolved now.

+ * of the primitive and the inherent color of the primitive.  The GrPrimitiveProcessor is
+ * responsible for providing a color and coverage input into the Ganesh rendering pipeline.  Through
+ * optimization, Ganesh may decide a different color, no color, and / or no coverage are required
+ * from the GrPrimitiveProcessor, so the GrPrimitiveProcessor must be able to support this
+ * functionality.  We also use the GrPrimitiveProcessor to make batching decisions.
+ *
+ * There are two feedback loops between the GrFragmentProcessors, the GrXferProcessor, and the
+ * GrPrimitiveProcessor.  The GrPrimitiveProcessor seeds these loops, one with initial color and one
+ * with initial coverage, in its onComputeInvariantColor / Coverage calls.  These seed values are
+ * processed by the subsequent stages of the rendering pipeline and the output is then fed back
+ * into the GrPrimitiveProcessor in the initBatchTracker call, where the GrPrimitiveProcessor can
+ * then initialize the GrBatchTracker struct with the appropriate values.
+ *
+ * In a deferred geometry world, the GrPrimitiveProcessor can always 'batch'  To do this, each
+ * primitive type is associated with one GrPrimitiveProcessor, who has complete control of how
+ * it draws.  Each primitive draw will bundle all required data to perform the draw, and these
+ * bundles of data will be owned by an instance of the associated GrPrimitiveProcessor.  Bundles
+ * can be updated alongside the GrBatchTracker struct itself, ultimately allowing the
+ * GrPrimitiveProcessor complete control of how it gets data into the fragment shader as long as
+ * it emits the appropriate color, or none at all, as directed.
+ */
+
+/*
  * A struct for tracking batching decisions.  While this lives on GrOptState, it is managed
  * entirely by the derived classes of the GP.
  */
@@ -21,18 +45,18 @@ class GrBatchTracker {
 public:
     template <typename T> const T& cast() const {
         SkASSERT(sizeof(T) <= kMaxSize);
-        return *reinterpret_cast<const T*>(fData);
+        return *reinterpret_cast<const T*>(fData.get());
     }
 
     template <typename T> T* cast() {
         SkASSERT(sizeof(T) <= kMaxSize);
-        return reinterpret_cast<T*>(fData);
+        return reinterpret_cast<T*>(fData.get());
     }
 
     static const size_t kMaxSize = 32;
 
 private:
-    uint8_t fData[kMaxSize];
+    mutable SkAlignedSStorage<kMaxSize> fData;

[bsalomon, 2014/12/15 15:31:13]

still unclear to me why this is mutable and the thing is passed around as const

 };
 
 class GrGLCaps;
@@ -42,13 +66,23 @@ class GrOptDrawState;
 struct GrInitInvariantOutput;
 
 /*
- * GrGeometryProcessors and GrPathProcessors may effect invariantColor
+ * GrPrimitiveProcessor defines an interface which all subclasses must implement.  All
+ * GrPrimitiveProcessors must proivide seed color and coverage for the Ganesh color / coverage
+ * pipelines, and they must provide some notion of equality
  */
 class GrPrimitiveProcessor : public GrProcessor {
 public:
-    // TODO GPs and PPs have to provide an initial coverage because the coverage invariant code is
-    // broken right now
-    virtual uint8_t coverage() const = 0;
+    virtual bool canMakeEqual(const GrBatchTracker& mine,
+                              const GrPrimitiveProcessor& that,
+                              const GrBatchTracker& theirs) const = 0;
+
+    /*
+     * We always call canMakeEqual before makeEqual so there is no need to do any kind of equality
+     * testing here
+     * TODO make this pure virtual when primProcs can actually use it
+     */
+    virtual void makeEqual(GrBatchTracker*, const GrBatchTracker&) const {}
+
     virtual void getInvariantOutputColor(GrInitInvariantOutput* out) const = 0;
     virtual void getInvariantOutputCoverage(GrInitInvariantOutput* out) const = 0;
 
@@ -56,28 +90,34 @@ private:
     typedef GrProcessor INHERITED;
 };
 
+/*
+ * This enum is shared by GrPrimitiveProcessors and GrGLPrimitiveProcessors to coordinate shaders
+ * with vertex attributes / uniforms.
+ */
+enum GrGPInput {
+    kAllOnes_GrGPInput,
+    kAttribute_GrGPInput,
+    kUniform_GrGPInput,
+    kIgnored_GrGPInput,
+};
+
 /**
- * A GrGeometryProcessor is used to perform computation in the vertex shader and
- * add support for custom vertex attributes. A GrGemeotryProcessor is typically
- * tied to the code that does a specific type of high-level primitive rendering
- * (e.g. anti-aliased circle rendering). The GrGeometryProcessor used for a draw is
- * specified using GrDrawState. There can only be one geometry processor active for
- * a draw. The custom vertex attributes required by the geometry processor must be
- * added to the vertex attribute array specified on the GrDrawState.
- * GrGeometryProcessor subclasses should be immutable after construction.
+ * A GrGeometryProcessor is a flexible method for rendering a primitive.  The GrGeometryProcessor
+ * has complete control over vertex attributes and uniforms(aside from the render target) but it
+ * must obey the same contract as any GrPrimitiveProcessor, specifically it must emit a color and
+ * coverage into the fragment shader.  Where this color and coverage come from is completely the
+ * responsibility of the GrGeometryProcessor.

[bsalomon, 2014/12/15 15:31:13]

nice

  */
 class GrGeometryProcessor : public GrPrimitiveProcessor {
 public:
     // TODO the Hint can be handled in a much more clean way when we have deferred geometry or
     // atleast bundles
-    GrGeometryProcessor(GrColor color, bool opaqueVertexColors = false, uint8_t coverage = 0xff)
+    GrGeometryProcessor(GrColor color, bool opaqueVertexColors = false)
         : fVertexStride(0)
         , fColor(color)
-        , fCoverage(coverage)
         , fOpaqueVertexColors(opaqueVertexColors)
         , fWillUseGeoShader(false)
         , fHasVertexColor(false)
-        , fHasVertexCoverage(false)
         , fHasLocalCoords(false) {}
 
     virtual const char* name() const = 0;
@@ -123,56 +163,103 @@ public:
 
     bool willUseGeoShader() const { return fWillUseGeoShader; }
 
-    /** Returns true if this and other processor conservatively draw identically. It can only return
-        true when the two prcoessors are of the same subclass (i.e. they return the same object from
-        from getFactory()).
-        A return value of true from isEqual() should not be used to test whether the processors
-        would generate the same shader code. To test for identical code generation use the
-        processors' keys computed by the GrBackendEffectFactory. */
-    bool isEqual(const GrGeometryProcessor& that) const {
+    /*
+     * In an ideal world, two GrGeometryProcessors with the same class id and texture accesses
+     * would ALWAYS be able to batch together.  If two GrGeometryProcesosrs are the same then we
+     * will only keep one of them.  The remaining GrGeometryProcessor then updates its
+     * GrBatchTracker to incorporate the draw information from the GrGeometryProcessor we discard.
+     * Any bundles associated with the discarded GrGeometryProcessor will be attached to the
+     * remaining GrGeometryProcessor.
+     */
+    bool canMakeEqual(const GrBatchTracker& mine,
+                      const GrPrimitiveProcessor& that,
+                      const GrBatchTracker& theirs) const SK_OVERRIDE {
         if (this->classID() != that.classID() || !this->hasSameTextureAccesses(that)) {
             return false;
         }
 
         // TODO remove the hint
-        if (fHasVertexColor && fOpaqueVertexColors != that.fOpaqueVertexColors) {
-            return false;
-        }
-
-        if (!fHasVertexColor && this->color() != that.color()) {
+        const GrGeometryProcessor& other = that.cast<GrGeometryProcessor>();
+        if (fHasVertexColor && fOpaqueVertexColors != other.fOpaqueVertexColors) {
             return false;
         }
 
-        // TODO this is fragile, most gps set their coverage to 0xff so this is okay.  In the long
-        // term this should move to subclasses which set explicit coverage
-        if (!fHasVertexCoverage && this->coverage() != that.coverage()) {
+        if (!fHasVertexColor && this->color() != other.color()) {
             return false;
         }
-        return this->onIsEqual(that);
+        return this->onCanMakeEqual(mine, theirs);
     }
 
+    virtual bool onCanMakeEqual(const GrBatchTracker& mine, const GrBatchTracker& theirs) const = 0;

[bsalomon, 2014/12/15 15:31:13]

move to private?

+
+    /*
+     * This struct allows the optstate to communicate requirements to the GP.
+     */
     struct InitBT {
-        bool fOutputColor;
-        bool fOutputCoverage;
-        GrColor fColor;
-        GrColor fCoverage;
+        bool fColorIgnored;
+        bool fCoverageIgnored;
+        GrColor fOverrideColor;
     };
 
-    virtual void initBatchTracker(GrBatchTracker*, const InitBT&) const {}
-
+    virtual void initBatchTracker(GrBatchTracker*, const InitBT&) const = 0;
+    
+    // TODO we can remove color from the GrGeometryProcessor base class once we have bundles of
+    // primitive data
     GrColor color() const { return fColor; }
-    uint8_t coverage() const { return fCoverage; }
 
-    // TODO this is a total hack until the gp can own whether or not it uses uniform
-    // color / coverage
+    // TODO this is a total hack until the gp can do deferred geometry
     bool hasVertexColor() const { return fHasVertexColor; }
-    bool hasVertexCoverage() const { return fHasVertexCoverage; }
+
+    // TODO this is a total hack until gp can setup and manage local coords
     bool hasLocalCoords() const { return fHasLocalCoords; }
 
     void getInvariantOutputColor(GrInitInvariantOutput* out) const SK_OVERRIDE;
     void getInvariantOutputCoverage(GrInitInvariantOutput* out) const SK_OVERRIDE;
 
 protected:
+    /*
+     * A simple helper function to determine by what means the GrGeometryProcessor should use to

[bsalomon, 2014/12/15 15:31:13]

Can you throw in the word "optional" ? Just want to make sure it is clear that a
GrGP can implement color/cov however it wants so long as the override contract
is maintained.

+     * provide color.  If we are given an override color(ie the given overridecolor is NOT illegal)

[bsalomon, 2014/12/15 15:31:13]

can you say GrColor_ILLEGAL? I think it might be confusing to just say illegal.

+     * then we must always emit that color via a uniform.  Otherwise, if our color is ignored

[bsalomon, 2014/12/15 15:31:13]

Does it have to be via a uniform?  A GP could rewrite its bundles w/ the
override color and batch together bundles that have different overrides via
vertex attr (in the def. geo. world). I guess that's kind of the point of the
TODO below.

+     * then we should not emit a color.  Lastly, if we don't have vertex colors then we must emit
+     * a color via uniform
+     * TODO this function changes quite a bit with deferred geometry.  There the GrGeometryProcessor
+     * can upload a new color via attribute if needed.
+     */
+    static GrGPInput GetColorInputType(GrColor* color, GrColor primitiveColor, const InitBT& init,
+                                       bool hasVertexColor) {
+        if (GrColor_ILLEGAL != init.fOverrideColor) {
+            *color = init.fOverrideColor;
+            return kUniform_GrGPInput;
+        } else if (init.fColorIgnored) {
+            *color = GrColor_ILLEGAL;
+            return kIgnored_GrGPInput;
+        }
+
+        *color = primitiveColor;
+        if (hasVertexColor) {
+            return kAttribute_GrGPInput;
+        } else {
+            return kUniform_GrGPInput;
+        }
+    }
+
+    /*
+     * CanCombineOutput will return true if two draws are 'batchable' from a color perspective.
+     * TODO remove this when GPs can upgrade to attribute color
+     */
+    static bool CanCombineOutput(GrGPInput left, GrColor lColor, GrGPInput right, GrColor rColor) {
+        if (left != right) {
+            return false;
+        }
+
+        if (kUniform_GrGPInput == left && lColor != rColor) {
+            return false;
+        }
+
+        return true;
+    }
+
     /**
      * Subclasses call this from their constructor to register vertex attributes.  Attributes
      * will be padded to the nearest 4 bytes for performance reasons.
@@ -188,7 +275,6 @@ protected:
 
     // TODO hack see above
     void setHasVertexColor() { fHasVertexColor = true; }
-    void setHasVertexCoverage() { fHasVertexCoverage = true; }
     void setHasLocalCoords() { fHasLocalCoords = true; }
 
     virtual void onGetInvariantOutputColor(GrInitInvariantOutput*) const {}
@@ -200,11 +286,9 @@ private:
     SkSTArray<kMaxVertexAttribs, GrAttribute, true> fAttribs;
     size_t fVertexStride;
     GrColor fColor;
-    uint8_t fCoverage;
     bool fOpaqueVertexColors;
     bool fWillUseGeoShader;
     bool fHasVertexColor;
-    bool fHasVertexCoverage;
     bool fHasLocalCoords;
 
     typedef GrProcessor INHERITED;
@@ -220,13 +304,23 @@ public:
         return SkNEW_ARGS(GrPathProcessor, (color));
     }
 
+    bool canMakeEqual(const GrBatchTracker& mine,
+                      const GrPrimitiveProcessor& that,
+                      const GrBatchTracker& theirs) const SK_OVERRIDE {
+        if (this->classID() != that.classID() || !this->hasSameTextureAccesses(that)) {

[bsalomon, 2014/12/15 15:31:13]

Seems a little weird that path procs can even have texture accesses. Maybe we
can think about hiding the ability to add them and expose it only on GrProc
subclasses that need it (FP, GP, XP)

+            return false;
+        }
+        const GrPathProcessor& other = that.cast<GrPathProcessor>();
+        return other.color() == this->color();

[bsalomon, 2014/12/15 15:31:13]

So if there is an override these two colors would reflect that right? And they'd
both be illegal if the GP color wasn't needed, right? Just double checking my
own understanding of things.

+    }
+
     const char* name() const SK_OVERRIDE { return "PathProcessor"; }
-    uint8_t coverage() const SK_OVERRIDE { return 0xff; }
+    GrColor color() const { return fColor; }
     void getInvariantOutputColor(GrInitInvariantOutput* out) const SK_OVERRIDE;
     void getInvariantOutputCoverage(GrInitInvariantOutput* out) const SK_OVERRIDE;
 
 private:
-    GrPathProcessor(GrColor color) : fColor(color) {}
+    GrPathProcessor(GrColor color);
     GrColor fColor;
 
     typedef GrProcessor INHERITED;