Add GrProcOptInfo class to track various output information for color and coverage stages.

Add GrProcOptInfo class to track various output information for color and coverage stages.

BUG=skia:

Committed: https://skia.googlesource.com/skia/+/b6cbc38702dc4a1540a17181cbdab71060c92ce7

[egdaniel, 2014-11-12 19:27:45]

egdaniel@google.com changed reviewers:
+ bsalomon@google.com

[egdaniel, 2014-11-12 19:27:46]

https://codereview.chromium.org/719203002/diff/20001/src/gpu/GrProcessor.cpp
File src/gpu/GrProcessor.cpp (right):

https://codereview.chromium.org/719203002/diff/20001/src/gpu/GrProcessor.cpp#...
src/gpu/GrProcessor.cpp:133: this->onComputeInvariantOutput(inout);
In a follow up cl I will remove the virtual onComputeIO and simply make
computeIO the virtual call. I didn't want to clog up this cl with logs of simple
replacement changes

[bsalomon, 2014-11-12 22:09:40]

https://codereview.chromium.org/719203002/diff/40001/src/gpu/GrProcOptInfo.h
File src/gpu/GrProcOptInfo.h (right):

https://codereview.chromium.org/719203002/diff/40001/src/gpu/GrProcOptInfo.h#...
src/gpu/GrProcOptInfo.h:20: : fInout(0, static_cast<GrColorComponentFlags>(0),
false), fFirstEffectStageIndex(0),
can you put each var on its own line? This is kinda hard to read.

https://codereview.chromium.org/719203002/diff/40001/src/gpu/GrProcOptInfo.h#...
src/gpu/GrProcOptInfo.h:23: void calcWithInitialValues(const GrFragmentStage*,
int stageCount, GrColor startColor,
Can we just call it calc()? Or do we expect variations?

https://codereview.chromium.org/719203002/diff/40001/src/gpu/GrProcOptInfo.h#...
src/gpu/GrProcOptInfo.h:33: int firstEffectiveStageIndex() const { return
fFirstEffectStageIndex; }
from here down, these could use some comments. e.g. what does removeVertexAttrib
mean?

https://codereview.chromium.org/719203002/diff/40001/src/gpu/GrProcOptInfo.h#...
src/gpu/GrProcOptInfo.h:40: GrInvariantOutput fInout;
fInOut ? Usually cap each word.

[egdaniel, 2014-11-13 14:58:29]

https://codereview.chromium.org/719203002/diff/40001/src/gpu/GrProcOptInfo.h
File src/gpu/GrProcOptInfo.h (right):

https://codereview.chromium.org/719203002/diff/40001/src/gpu/GrProcOptInfo.h#...
src/gpu/GrProcOptInfo.h:20: : fInout(0, static_cast<GrColorComponentFlags>(0),
false), fFirstEffectStageIndex(0),
On 2014/11/12 22:09:39, bsalomon wrote:
> can you put each var on its own line? This is kinda hard to read.

Done.

https://codereview.chromium.org/719203002/diff/40001/src/gpu/GrProcOptInfo.h#...
src/gpu/GrProcOptInfo.h:23: void calcWithInitialValues(const GrFragmentStage*,
int stageCount, GrColor startColor,
On 2014/11/12 22:09:39, bsalomon wrote:
> Can we just call it calc()? Or do we expect variations?

In the future we plan to also have a version calcWithGeomProcessor() which we
depend on a gp to supply initial color, flags, areCoverageStages instead of
explicitly passing them in.

https://codereview.chromium.org/719203002/diff/40001/src/gpu/GrProcOptInfo.h#...
src/gpu/GrProcOptInfo.h:33: int firstEffectiveStageIndex() const { return
fFirstEffectStageIndex; }
On 2014/11/12 22:09:40, bsalomon wrote:
> from here down, these could use some comments. e.g. what does
removeVertexAttrib
> mean?

Done.

https://codereview.chromium.org/719203002/diff/40001/src/gpu/GrProcOptInfo.h#...
src/gpu/GrProcOptInfo.h:40: GrInvariantOutput fInout;
On 2014/11/12 22:09:40, bsalomon wrote:
> fInOut ? Usually cap each word.

Done.

[bsalomon, 2014-11-13 15:09:19]

lgtm. Feel free to ignore my comment suggestions, was just trying to simplify
it.

https://codereview.chromium.org/719203002/diff/80001/src/gpu/GrProcOptInfo.cpp
File src/gpu/GrProcOptInfo.cpp (right):

https://codereview.chromium.org/719203002/diff/80001/src/gpu/GrProcOptInfo.cp...
src/gpu/GrProcOptInfo.cpp:40: fReadsDst = false; // Reset this since we don't
care if previous stages read dst
nice catch :)

https://codereview.chromium.org/719203002/diff/80001/src/gpu/GrProcOptInfo.h
File src/gpu/GrProcOptInfo.h (right):

https://codereview.chromium.org/719203002/diff/80001/src/gpu/GrProcOptInfo.h#...
src/gpu/GrProcOptInfo.h:17: class GrProcOptInfo {
GrProcOptInfo is gathers invariant data from a set of processor stages. It is
used to recognize optimizations related to eliminating stages and vertex
attributes that aren't necessary for a draw.

https://codereview.chromium.org/719203002/diff/80001/src/gpu/GrProcOptInfo.h#...
src/gpu/GrProcOptInfo.h:21: fFirstEffectStageIndex(0),
style is to 

: fPut()
, fThe()
, fCommas()
, fUnder()
, fTheColon() {
}

https://codereview.chromium.org/719203002/diff/80001/src/gpu/GrProcOptInfo.h#...
src/gpu/GrProcOptInfo.h:43: * If the firstEffectiveStageIndex is used,
corresponding values from inputColorIsUsed(),
"If stages before the first effective stage are removed,"
?

https://codereview.chromium.org/719203002/diff/80001/src/gpu/GrProcOptInfo.h#...
src/gpu/GrProcOptInfo.h:50: * This value returns whether or not an input will be
read by the starting stage. The starting
Maybe simplify to "True if the first effective stage reads its input, false
otherwise."?

https://codereview.chromium.org/719203002/diff/80001/src/gpu/GrProcOptInfo.h#...
src/gpu/GrProcOptInfo.h:56: * This returns the color that should be inputed into
the starting stage. This value is only
"If input color is used and per-vertex colors are not used, this is the input
color to the first effective stage"
?

Sorry for nit picking these comments, but its confusing stuff and making this
concise and clear may help us later.

https://codereview.chromium.org/719203002/diff/80001/src/gpu/GrProcOptInfo.h#...
src/gpu/GrProcOptInfo.h:69: */
"Given the set of optimizations determined by GrProcOptInfo, should the caller
remove a the color/coverage vertex attribute that was input to the first stage."

?

https://codereview.chromium.org/719203002/diff/80001/src/gpu/GrProcOptInfo.h#...
src/gpu/GrProcOptInfo.h:76: bool readsDst() const { return fReadsDst; }
"Returns true if any of the stages preserved by GrProcOptInfo read the dst
color."

?

[egdaniel, 2014-11-13 18:34:16]

https://codereview.chromium.org/719203002/diff/80001/src/gpu/GrProcOptInfo.h
File src/gpu/GrProcOptInfo.h (right):

https://codereview.chromium.org/719203002/diff/80001/src/gpu/GrProcOptInfo.h#...
src/gpu/GrProcOptInfo.h:17: class GrProcOptInfo {
On 2014/11/13 15:09:19, bsalomon wrote:
> GrProcOptInfo is gathers invariant data from a set of processor stages. It is
> used to recognize optimizations related to eliminating stages and vertex
> attributes that aren't necessary for a draw.

Done.

https://codereview.chromium.org/719203002/diff/80001/src/gpu/GrProcOptInfo.h#...
src/gpu/GrProcOptInfo.h:21: fFirstEffectStageIndex(0),
On 2014/11/13 15:09:19, bsalomon wrote:
> style is to 
> 
> : fPut()
> , fThe()
> , fCommas()
> , fUnder()
> , fTheColon() {
> }

Ahh I understand my error, just like TheColon I should merge my last two data
members into a single variable. Thanks for the advice!

https://codereview.chromium.org/719203002/diff/80001/src/gpu/GrProcOptInfo.h#...
src/gpu/GrProcOptInfo.h:43: * If the firstEffectiveStageIndex is used,
corresponding values from inputColorIsUsed(),
On 2014/11/13 15:09:19, bsalomon wrote:
> "If stages before the first effective stage are removed,"
> ?

Done.

https://codereview.chromium.org/719203002/diff/80001/src/gpu/GrProcOptInfo.h#...
src/gpu/GrProcOptInfo.h:50: * This value returns whether or not an input will be
read by the starting stage. The starting
On 2014/11/13 15:09:19, bsalomon wrote:
> Maybe simplify to "True if the first effective stage reads its input, false
> otherwise."?

Done.

https://codereview.chromium.org/719203002/diff/80001/src/gpu/GrProcOptInfo.h#...
src/gpu/GrProcOptInfo.h:56: * This returns the color that should be inputed into
the starting stage. This value is only
On 2014/11/13 15:09:19, bsalomon wrote:
> "If input color is used and per-vertex colors are not used, this is the input
> color to the first effective stage"
> ?
> 
> Sorry for nit picking these comments, but its confusing stuff and making this
> concise and clear may help us later.

Done.

https://codereview.chromium.org/719203002/diff/80001/src/gpu/GrProcOptInfo.h#...
src/gpu/GrProcOptInfo.h:69: */
On 2014/11/13 15:09:19, bsalomon wrote:
> "Given the set of optimizations determined by GrProcOptInfo, should the caller
> remove a the color/coverage vertex attribute that was input to the first
stage."
> 
> ?

Done.

https://codereview.chromium.org/719203002/diff/80001/src/gpu/GrProcOptInfo.h#...
src/gpu/GrProcOptInfo.h:76: bool readsDst() const { return fReadsDst; }
On 2014/11/13 15:09:19, bsalomon wrote:
> "Returns true if any of the stages preserved by GrProcOptInfo read the dst
> color."
> 
> ?

Done.

[egdaniel, 2014-11-13 18:38:27]

The CQ bit was checked by egdaniel@google.com

[commit-bot: I haz the power, 2014-11-13 18:39:15]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/719203002/120001

[commit-bot: I haz the power, 2014-11-13 18:39:24]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-11-13 18:39:25]

Failed to apply patch for src/gpu/GrDrawState.cpp:
While running git apply --index -3 -p1;
  error: patch failed: src/gpu/GrDrawState.cpp:7
  Falling back to three-way merge...
  <stdin>:239: new blank line at EOF.
  +
  Applied patch to 'src/gpu/GrDrawState.cpp' with conflicts.
  U src/gpu/GrDrawState.cpp
  warning: 1 line adds whitespace errors.

Patch:       src/gpu/GrDrawState.cpp
Index: src/gpu/GrDrawState.cpp
diff --git a/src/gpu/GrDrawState.cpp b/src/gpu/GrDrawState.cpp
index
23ebea6afcfd9cbbb4f476edea789b46e21118d4..c764e25cdb77a96b89b5a8fcc426869d2234dada
100644
--- a/src/gpu/GrDrawState.cpp
+++ b/src/gpu/GrDrawState.cpp
@@ -7,9 +7,9 @@
 
 #include "GrDrawState.h"
 
-#include "GrInvariantOutput.h"
 #include "GrOptDrawState.h"
 #include "GrPaint.h"
+#include "GrProcOptInfo.h"
 
 //////////////////////////////////////////////////////////////////////////////s
 
@@ -150,6 +150,15 @@ GrDrawState& GrDrawState::operator=(const GrDrawState&
that) {
 
     fHints = that.fHints;
 
+    fColorProcInfoValid = that.fColorProcInfoValid;
+    fCoverageProcInfoValid = that.fCoverageProcInfoValid;
+    if (fColorProcInfoValid) {
+        fColorProcInfo = that.fColorProcInfo;
+    }
+    if (fCoverageProcInfoValid) {
+        fCoverageProcInfo = that.fCoverageProcInfo;
+    }
+
     memcpy(fFixedFunctionVertexAttribIndices,
             that.fFixedFunctionVertexAttribIndices,
             sizeof(fFixedFunctionVertexAttribIndices));
@@ -183,6 +192,9 @@ void GrDrawState::onReset(const SkMatrix* initialViewMatrix)
{
     fDrawFace = kBoth_DrawFace;
 
     fHints = 0;
+
+    fColorProcInfoValid = false;
+    fCoverageProcInfoValid = false;
 }
 
 bool GrDrawState::setIdentityViewMatrix()  {
@@ -238,6 +250,8 @@ void GrDrawState::setFromPaint(const GrPaint& paint, const
SkMatrix& vm, GrRende
 
     this->setBlendFunc(paint.getSrcBlendCoeff(), paint.getDstBlendCoeff());
     this->setCoverage(0xFF);
+    fColorProcInfoValid = false;
+    fCoverageProcInfoValid = false;
 }
 

////////////////////////////////////////////////////////////////////////////////
@@ -335,6 +349,8 @@ void GrDrawState::internalSetVertexAttribs(const
GrVertexAttrib* attribs, int co
         overlapCheck |= (mask << offsetShift);
 #endif
     }
+    fColorProcInfoValid = false;
+    fCoverageProcInfoValid = false;
     // Positions must be specified.
     SkASSERT(-1 !=
fFixedFunctionVertexAttribIndices[kPosition_GrVertexAttribBinding]);
 }
@@ -354,6 +370,8 @@ void GrDrawState::setDefaultVertexAttribs() {
            0xff,
            sizeof(fFixedFunctionVertexAttribIndices));
     fFixedFunctionVertexAttribIndices[kPosition_GrVertexAttribBinding] = 0;
+    fColorProcInfoValid = false;
+    fCoverageProcInfoValid = false;
 }
 

////////////////////////////////////////////////////////////////////////////////
@@ -383,24 +401,8 @@ bool GrDrawState::hasSolidCoverage() const {
         return false;
     }
 
-    GrColor color;
-    GrColorComponentFlags flags;
-    // Initialize to an unknown starting coverage if per-vertex coverage is
specified.
-    if (this->hasCoverageVertexAttribute()) {
-        color = 0;
-        flags = static_cast<GrColorComponentFlags>(0);
-    } else {
-        color = this->getCoverageColor();
-        flags = kRGBA_GrColorComponentFlags;
-    }
-    GrInvariantOutput inout(color, flags, true);
-
-    // check the coverage output from the GP
-    if (this->hasGeometryProcessor()) {
-        fGeometryProcessor->computeInvariantOutput(&inout);
-    }
-
-    return inout.isSolidWhite();
+    this->calcCoverageInvariantOutput();
+    return fCoverageProcInfo.isSolidWhite();
 }
 
 //////////////////////////////////////////////////////////////////////////////
@@ -418,18 +420,13 @@
GrDrawState::AutoVertexAttribRestore::AutoVertexAttribRestore(GrDrawState* drawS
 
 bool GrDrawState::willEffectReadDstColor() const {
     if (!this->isColorWriteDisabled()) {
-        for (int s = 0; s < this->numColorStages(); ++s) {
-            if (this->getColorStage(s).getProcessor()->willReadDstColor()) {
-                return true;
-            }
-        }
-    }
-    for (int s = 0; s < this->numCoverageStages(); ++s) {
-        if (this->getCoverageStage(s).getProcessor()->willReadDstColor()) {
+        this->calcColorInvariantOutput();
+        if (fColorProcInfo.readsDst()) {
             return true;
         }
     }
-    return false;
+    this->calcCoverageInvariantOutput();
+    return fCoverageProcInfo.readsDst();
 }
 
 void GrDrawState::AutoRestoreEffects::set(GrDrawState* ds) {
@@ -450,6 +447,10 @@ void GrDrawState::AutoRestoreEffects::set(GrDrawState* ds)
{
         int n = fDrawState->numCoverageStages() - fCoverageEffectCnt;
         SkASSERT(n >= 0);
         fDrawState->fCoverageStages.pop_back_n(n);
+        if (m + n > 0) {
+            fDrawState->fColorProcInfoValid = false;
+            fDrawState->fCoverageProcInfoValid = false;
+        }
         SkDEBUGCODE(--fDrawState->fBlockEffectRemovalCnt;)
     }
     fDrawState = ds;
@@ -688,60 +689,13 @@ GrDrawState::BlendOptFlags GrDrawState::getBlendOpts(bool
forceCoverage,
     return kNone_BlendOpt;
 }
 
-
 bool GrDrawState::srcAlphaWillBeOne() const {
-    GrColor color;
-    GrColorComponentFlags flags;
-    // Check if per-vertex or constant color may have partial alpha
-    if (this->hasColorVertexAttribute()) {
-        if (fHints & kVertexColorsAreOpaque_Hint) {
-            flags = kA_GrColorComponentFlag;
-            color = 0xFF << GrColor_SHIFT_A;
-        } else {
-            flags = static_cast<GrColorComponentFlags>(0);
-            color = 0;
-        }
-    } else {
-        flags = kRGBA_GrColorComponentFlags;
-        color = this->getColor();
-    }
-    GrInvariantOutput inoutColor(color, flags, false);
-
-    // Run through the color stages
-    for (int s = 0; s < this->numColorStages(); ++s) {
-        const GrProcessor* processor = this->getColorStage(s).getProcessor();
-        processor->computeInvariantOutput(&inoutColor);
-    }
-
-    // Check whether coverage is treated as color. If so we run through the
coverage computation.
+    this->calcColorInvariantOutput();
     if (this->isCoverageDrawing()) {
-        // The shader generated for coverage drawing runs the full coverage
computation and then
-        // makes the shader output be the multiplication of color and coverage.
We mirror that here.
-        if (this->hasCoverageVertexAttribute()) {
-            flags = static_cast<GrColorComponentFlags>(0);
-            color = 0;
-        } else {
-            flags = kRGBA_GrColorComponentFlags;
-            color = this->getCoverageColor();
-        }
-        GrInvariantOutput inoutCoverage(color, flags, true);
-
-        if (this->hasGeometryProcessor()) {
-            fGeometryProcessor->computeInvariantOutput(&inoutCoverage);
-        }
-
-        // Run through the coverage stages
-        for (int s = 0; s < this->numCoverageStages(); ++s) {
-            const GrProcessor* processor =
this->getCoverageStage(s).getProcessor();
-            processor->computeInvariantOutput(&inoutCoverage);
-        }
-
-        // Since the shader will multiply coverage and color, the only way the
final A==1 is if
-        // coverage and color both have A==1.
-        return (inoutColor.isOpaque() && inoutCoverage.isOpaque());
+        this->calcCoverageInvariantOutput();
+        return (fColorProcInfo.isOpaque() && fCoverageProcInfo.isOpaque());
     }
-
-    return inoutColor.isOpaque();
+    return fColorProcInfo.isOpaque();
 }
 
 bool GrDrawState::willBlendWithDst() const {
@@ -764,3 +718,43 @@ bool GrDrawState::willBlendWithDst() const {
     return false;
 }
 
+void GrDrawState::calcColorInvariantOutput() const {
+    if (!fColorProcInfoValid) {
+        GrColor color;
+        GrColorComponentFlags flags;
+        if (this->hasColorVertexAttribute()) {
+            if (fHints & kVertexColorsAreOpaque_Hint) {
+                flags = kA_GrColorComponentFlag;
+                color = 0xFF << GrColor_SHIFT_A;
+            } else {
+                flags = static_cast<GrColorComponentFlags>(0);
+                color = 0;
+            }
+        } else {
+            flags = kRGBA_GrColorComponentFlags;
+            color = this->getColor();
+        }
+        fColorProcInfo.calcWithInitialValues(fColorStages.begin(),
this->numColorStages(),
+                                             color, flags, false);
+        fColorProcInfoValid = true;
+    }
+}
+
+void GrDrawState::calcCoverageInvariantOutput() const {
+    if (!fCoverageProcInfoValid) {
+        GrColor color;
+        GrColorComponentFlags flags;
+        // Check if per-vertex or constant color may have partial alpha
+        if (this->hasCoverageVertexAttribute()) {
+            flags = static_cast<GrColorComponentFlags>(0);
+            color = 0;
+        } else {
+            flags = kRGBA_GrColorComponentFlags;
+            color = this->getCoverageColor();
+        }
+        fCoverageProcInfo.calcWithInitialValues(fCoverageStages.begin(),
this->numCoverageStages(),
+                                                color, flags, true,
fGeometryProcessor.get());
+        fCoverageProcInfoValid = true;
+    }
+}
+

[egdaniel, 2014-11-13 18:44:06]

The CQ bit was checked by egdaniel@google.com

[commit-bot: I haz the power, 2014-11-13 18:44:13]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/719203002/140001

[commit-bot: I haz the power, 2014-11-13 18:46:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-11-13 18:46:14]

Try jobs failed on following builders:
  Build-Ubuntu13.10-GCC4.8-x86_64-Release-Trybot on client.skia.compile
(http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu13.10-GC...)

[egdaniel, 2014-11-13 18:52:02]

The CQ bit was checked by egdaniel@google.com

[commit-bot: I haz the power, 2014-11-13 18:52:13]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/719203002/160001

[bsalomon, 2014-11-13 18:53:19]

On 2014/11/13 18:34:16, egdaniel wrote:
> 
>
https://codereview.chromium.org/719203002/diff/80001/src/gpu/GrProcOptInfo.h#...
> src/gpu/GrProcOptInfo.h:21: fFirstEffectStageIndex(0),
> On 2014/11/13 15:09:19, bsalomon wrote:
> > style is to 
> > 
> > : fPut()
> > , fThe()
> > , fCommas()
> > , fUnder()
> > , fTheColon() {
> > }
> 
> Ahh I understand my error, just like TheColon I should merge my last two data
> members into a single variable. Thanks for the advice!

No! Only combine variables that wouldn't be capitalized in a book title!

[commit-bot: I haz the power, 2014-11-13 19:00:40]

Committed patchset #9 (id:160001) as
https://skia.googlesource.com/skia/+/b6cbc38702dc4a1540a17181cbdab71060c92ce7