Replace fWillReadFragmentPosition with a bitfield

include/gpu/GrProcessor.h

Index: include/gpu/GrProcessor.h
diff --git a/include/gpu/GrProcessor.h b/include/gpu/GrProcessor.h
index 9b5db5e033912e36be0070babb40468b23563a89..e23c2489274e7b5f5a5c5d64b4e63b686f27d268 100644
--- a/include/gpu/GrProcessor.h
+++ b/include/gpu/GrProcessor.h
@@ -79,8 +79,18 @@ public:
     /** Shortcut for textureAccess(index).texture(); */
     GrTexture* texture(int index) const { return this->textureAccess(index).getTexture(); }
 
-    /** Will this processor read the fragment position? */
-    bool willReadFragmentPosition() const { return fWillReadFragmentPosition; }
+    /**
+     * Platform specific built-in state that a processor can include in the fragment shader. A
+     * processor must enable this state before using it.
+     */
+    enum BuiltInState {
+        kNone_BuiltInState              = 0,
+        kFragmentPosition_BuiltInState  = 1
+    };
+
+    GR_DECL_BITFIELD_OPS_FRIENDS(BuiltInState);
+
+    BuiltInState builtInState() const { return fBuiltInState; }
 
     void* operator new(size_t size);
     void operator delete(void* target);
@@ -100,7 +110,7 @@ public:
     uint32_t classID() const { SkASSERT(kIllegalProcessorClassID != fClassID); return fClassID; }
 
 protected:
-    GrProcessor() : fClassID(kIllegalProcessorClassID), fWillReadFragmentPosition(false) {}
+    GrProcessor() : fClassID(kIllegalProcessorClassID), fBuiltInState(kNone_BuiltInState) {}
 
     /**
      * Subclasses call this from their constructor to register GrTextureAccesses. The processor
@@ -112,12 +122,7 @@ protected:
 
     bool hasSameTextureAccesses(const GrProcessor&) const;
 
-    /**
-     * If the prcoessor will generate a backend-specific processor that will read the fragment
-     * position in the FS then it must call this method from its constructor. Otherwise, the
-     * request to access the fragment position will be denied.
-     */
-    void setWillReadFragmentPosition() { fWillReadFragmentPosition = true; }
+    void enableBuiltInState(BuiltInState state) { fBuiltInState |= state; }

[joshualitt, 2016/02/25 19:21:29]

This is more a question for Brian / Robert, but do we want to hide the fact that
the implementation is a bit field? I kind of like setWillReadFragmentPosition /
willReadFragmentPosition, though I also like that compose shader can just grab
the whole bitfield of its children.  Maybe retain the old willRead/setWillRead
calls, but also allow access to the BuiltInState enum?  I don't really have a
strong preference.

[Chris Dalton, 2016/02/25 19:45:30]

So, willRead isn't very needed anymore on this class. GrPipeline in this change
still keeps "willReadFragPosition", and that's where everybody else asks this
question AFAIK.

[Chris Dalton, 2016/02/25 20:38:37]

Also, FWIW, this feels pretty similar to "fsBuilder->enableFeature(kMy_Feature)"

[bsalomon, 2016/02/25 21:11:03]

I find the setWillFoo() a bit more readable than enableFeature(kFoo). It doesn't
feel like these are inherently related, just happen to be sharing a bitfield for
implementation.

WRT children FPs, I really hope we replace the current mess of pulling state
into the parent with a system that iterates over the DAG externally.

[Chris Dalton, 2016/02/25 22:24:24]

Ok, brought back "setWillFoo()".

For children FPs, I added a combineBuiltInState() method. We can gladly remove
it once child FPs change.

 
     template <typename PROC_SUBCLASS> void initClassID() {
          static uint32_t kClassID = GenClassID();
@@ -145,9 +150,11 @@ private:
     };
     static int32_t gCurrProcessorClassID;
 
-    bool                                         fWillReadFragmentPosition;
+    BuiltInState fBuiltInState;
 
     typedef GrProgramElement INHERITED;
 };
 
+GR_MAKE_BITFIELD_OPS(GrProcessor::BuiltInState);
+
 #endif