remove separate color from coverage

remove separate color from coverage

BUG=skia:

Committed: https://skia.googlesource.com/skia/+/4dd99886420329fcacd72d87f2c46ff2555d0945

[joshualitt, 2014-11-10 16:23:12]

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

[egdaniel, 2014-11-10 16:28:06]

https://codereview.chromium.org/699023003/diff/1/src/gpu/GrDrawState.h
File src/gpu/GrDrawState.h (right):

https://codereview.chromium.org/699023003/diff/1/src/gpu/GrDrawState.h#newcod...
src/gpu/GrDrawState.h:254: return this->numColorStages() +
this->numCoverageStages() +
Should we just make this numFragmentStages() instead of color + coverage?

[joshualitt, 2014-11-10 16:30:48]

feedback incorporated

[bsalomon, 2014-11-10 18:50:12]

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

https://codereview.chromium.org/699023003/diff/20001/src/gpu/GrOptDrawState.c...
src/gpu/GrOptDrawState.cpp:141: if (!this->isCoverageDrawing() &&
Can we make GrOptDrawState have something like:

bool computesCoverageInFS() const

or some existing function to replace all this logic?

[joshualitt, 2014-11-10 19:23:39]

On 2014/11/10 18:50:12, bsalomon wrote:
>
https://codereview.chromium.org/699023003/diff/20001/src/gpu/GrOptDrawState.cpp
> File src/gpu/GrOptDrawState.cpp (right):
> 
>
https://codereview.chromium.org/699023003/diff/20001/src/gpu/GrOptDrawState.c...
> src/gpu/GrOptDrawState.cpp:141: if (!this->isCoverageDrawing() &&
> Can we make GrOptDrawState have something like:
> 
> bool computesCoverageInFS() const
> 
> or some existing function to replace all this logic?

Brian, I cleaned it up a bit.  If its still confusing I can create a function.

[bsalomon, 2014-11-10 20:13:41]

https://codereview.chromium.org/699023003/diff/50001/src/gpu/GrOptDrawState.cpp
File src/gpu/GrOptDrawState.cpp (right):

https://codereview.chromium.org/699023003/diff/50001/src/gpu/GrOptDrawState.c...
src/gpu/GrOptDrawState.cpp:145: if (!this->isCoverageDrawing() &&
(this->numCoverageStages() || !ds.hasSolidCoverage())) {
Do we need this->numCoverageStages()?

Seems like hasSolidCoverage is a strong enough test.

[joshualitt, 2014-11-10 20:17:16]

https://codereview.chromium.org/699023003/diff/50001/src/gpu/GrOptDrawState.cpp
File src/gpu/GrOptDrawState.cpp (right):

https://codereview.chromium.org/699023003/diff/50001/src/gpu/GrOptDrawState.c...
src/gpu/GrOptDrawState.cpp:145: if (!this->isCoverageDrawing() &&
(this->numCoverageStages() || !ds.hasSolidCoverage())) {
On 2014/11/10 20:13:41, bsalomon wrote:
> Do we need this->numCoverageStages()?
> 
> Seems like hasSolidCoverage is a strong enough test.
> 

We could actually JUST do that test, but its a bit of an optimization to skip
dsout if we are doing coverage drawing.  Also, its a bit of an optimization to
not loop over the coverage stages and compute inout for them.  Its your call
though.

[joshualitt, 2014-11-10 20:32:26]

feedback incorporated

[bsalomon, 2014-11-10 21:07:57]

lgtm

[joshualitt, 2014-11-11 14:42:44]

On 2014/11/10 21:07:57, bsalomon wrote:
> lgtm

Greg, please take one more look at this patch just to make sure.

[joshualitt, 2014-11-11 16:25:16]

The CQ bit was checked by joshualitt@google.com

[commit-bot: I haz the power, 2014-11-11 16:25:41]

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

[commit-bot: I haz the power, 2014-11-11 16:25:45]

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

[commit-bot: I haz the power, 2014-11-11 16:25:46]

Failed to apply patch for expectations/gm/ignored-tests.txt:
While running git apply --index -3 -p1;
  error: patch failed: expectations/gm/ignored-tests.txt:63
  Falling back to three-way merge...
  Applied patch to 'expectations/gm/ignored-tests.txt' with conflicts.
  U expectations/gm/ignored-tests.txt

Patch:       expectations/gm/ignored-tests.txt
Index: expectations/gm/ignored-tests.txt
diff --git a/expectations/gm/ignored-tests.txt
b/expectations/gm/ignored-tests.txt
index
893608987d39c23a39067cceaea57b9bfc28df4b..f268a0f9c085f3650f52b6cd342ebe6d21efe1fa
100644
--- a/expectations/gm/ignored-tests.txt
+++ b/expectations/gm/ignored-tests.txt
@@ -63,5 +63,6 @@ multipicturedraw_rrectclip_simple
 multipicturedraw_rrectclip_tiled
 multipicturedraw_sierpinski_simple
 multipicturedraw_sierpinski_tiled
+
 #joshualitt
-complexclip2_path_aa
+hairmodes

[joshualitt, 2014-11-11 16:43:01]

The CQ bit was checked by joshualitt@google.com

[commit-bot: I haz the power, 2014-11-11 16:43:51]

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

[commit-bot: I haz the power, 2014-11-11 16:51:34]

Committed patchset #11 (id:190001) as
https://skia.googlesource.com/skia/+/4dd99886420329fcacd72d87f2c46ff2555d0945