Use SkXfermode as public facing enum for GrPorterDuffXP

Use SkXfermode as public facing enum for GrPorterDuffXP

BUG=skia:

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

[egdaniel, 2015-02-13 20:32:34]

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

[egdaniel, 2015-02-13 20:32:35]

ptal

[egdaniel, 2015-02-13 22:21:17]

updated to use less porter duff xp's throughout code and remove need for
pdXPF::Create(coeff, coeff)

[bsalomon, 2015-02-14 20:45:59]

lgtm

https://codereview.chromium.org/926593005/diff/20001/include/gpu/GrXferProces...
File include/gpu/GrXferProcessor.h (right):

https://codereview.chromium.org/926593005/diff/20001/include/gpu/GrXferProces...
include/gpu/GrXferProcessor.h:42: kS2C_GrBlendCoeff,
if any of these need comments its the S2 guys :)

https://codereview.chromium.org/926593005/diff/60001/include/gpu/GrXferProces...
File include/gpu/GrXferProcessor.h (right):

https://codereview.chromium.org/926593005/diff/60001/include/gpu/GrXferProces...
include/gpu/GrXferProcessor.h:47: kTotalGrBlendCoeffCount
Do we still need this total value?

https://codereview.chromium.org/926593005/diff/60001/include/gpu/effects/GrCo...
File include/gpu/effects/GrCoverageSetOpXP.h (right):

https://codereview.chromium.org/926593005/diff/60001/include/gpu/effects/GrCo...
include/gpu/effects/GrCoverageSetOpXP.h:21: * */
extra space?

https://codereview.chromium.org/926593005/diff/60001/src/gpu/SkGpuDevice.cpp
File src/gpu/SkGpuDevice.cpp (right):

https://codereview.chromium.org/926593005/diff/60001/src/gpu/SkGpuDevice.cpp#...
src/gpu/SkGpuDevice.cpp:681:
tempPaint.setCoverageSetOpXPFactory(SkRegion::kUnion_Op);
I know we spent time figuring out that this is union, but now I realize that the
old code was trying to draw with replace, but to avoid triggering dsb/dst-copy
it was using union. Since the dst was cleared to zero union is equivalent to
replace. Might as well use replace.

[bsalomon, 2015-02-17 13:58:14]

https://codereview.chromium.org/926593005/diff/60001/src/gpu/SkGpuDevice.cpp
File src/gpu/SkGpuDevice.cpp (right):

https://codereview.chromium.org/926593005/diff/60001/src/gpu/SkGpuDevice.cpp#...
src/gpu/SkGpuDevice.cpp:681:
tempPaint.setCoverageSetOpXPFactory(SkRegion::kUnion_Op);
On 2015/02/14 20:45:59, bsalomon wrote:
> I know we spent time figuring out that this is union, but now I realize that
the
> old code was trying to draw with replace, but to avoid triggering dsb/dst-copy
> it was using union. Since the dst was cleared to zero union is equivalent to
> replace. Might as well use replace.

Also, we should install this XP unconditionally. It was guarded by doAA only
because this code had to try to be too smart in the pre-XP days by knowing that
aa could trigger coverage stages and therefore present blending challenges in
src mode.

[egdaniel, 2015-02-17 14:43:01]

https://codereview.chromium.org/926593005/diff/60001/src/gpu/SkGpuDevice.cpp
File src/gpu/SkGpuDevice.cpp (right):

https://codereview.chromium.org/926593005/diff/60001/src/gpu/SkGpuDevice.cpp#...
src/gpu/SkGpuDevice.cpp:681:
tempPaint.setCoverageSetOpXPFactory(SkRegion::kUnion_Op);
On 2015/02/17 13:58:13, bsalomon wrote:
> On 2015/02/14 20:45:59, bsalomon wrote:
> > I know we spent time figuring out that this is union, but now I realize that
> the
> > old code was trying to draw with replace, but to avoid triggering
dsb/dst-copy
> > it was using union. Since the dst was cleared to zero union is equivalent to
> > replace. Might as well use replace.
> 
> Also, we should install this XP unconditionally. It was guarded by doAA only
> because this code had to try to be too smart in the pre-XP days by knowing
that
> aa could trigger coverage stages and therefore present blending challenges in
> src mode.

Do we still want to set aa on the paint? Should it be tempPaint.setAA(doAA)?

[bsalomon, 2015-02-17 15:09:33]

On 2015/02/17 14:43:01, egdaniel wrote:
> https://codereview.chromium.org/926593005/diff/60001/src/gpu/SkGpuDevice.cpp
> File src/gpu/SkGpuDevice.cpp (right):
> 
>
https://codereview.chromium.org/926593005/diff/60001/src/gpu/SkGpuDevice.cpp#...
> src/gpu/SkGpuDevice.cpp:681:
> tempPaint.setCoverageSetOpXPFactory(SkRegion::kUnion_Op);
> On 2015/02/17 13:58:13, bsalomon wrote:
> > On 2015/02/14 20:45:59, bsalomon wrote:
> > > I know we spent time figuring out that this is union, but now I realize
that
> > the
> > > old code was trying to draw with replace, but to avoid triggering
> dsb/dst-copy
> > > it was using union. Since the dst was cleared to zero union is equivalent
to
> > > replace. Might as well use replace.
> > 
> > Also, we should install this XP unconditionally. It was guarded by doAA only
> > because this code had to try to be too smart in the pre-XP days by knowing
> that
> > aa could trigger coverage stages and therefore present blending challenges
in
> > src mode.
> 
> Do we still want to set aa on the paint? Should it be tempPaint.setAA(doAA)?

yeah, exactly

[egdaniel, 2015-02-17 15:18:52]

https://codereview.chromium.org/926593005/diff/60001/include/gpu/GrXferProces...
File include/gpu/GrXferProcessor.h (right):

https://codereview.chromium.org/926593005/diff/60001/include/gpu/GrXferProces...
include/gpu/GrXferProcessor.h:47: kTotalGrBlendCoeffCount
On 2015/02/14 20:45:59, bsalomon wrote:
> Do we still need this total value?

we actually still use it for some Asserts like GrBlendCoeff.count ==
GL_BLEND_ENUM.count. I think there is still value to having it. We could also
remove/change the asserts.

[bsalomon, 2015-02-17 15:25:39]

On 2015/02/17 15:18:52, egdaniel wrote:
>
https://codereview.chromium.org/926593005/diff/60001/include/gpu/GrXferProces...
> File include/gpu/GrXferProcessor.h (right):
> 
>
https://codereview.chromium.org/926593005/diff/60001/include/gpu/GrXferProces...
> include/gpu/GrXferProcessor.h:47: kTotalGrBlendCoeffCount
> On 2015/02/14 20:45:59, bsalomon wrote:
> > Do we still need this total value?
> 
> we actually still use it for some Asserts like GrBlendCoeff.count ==
> GL_BLEND_ENUM.count. I think there is still value to having it. We could also
> remove/change the asserts.

ok, just checking.

[egdaniel, 2015-02-17 15:26:51]

New patchsets have been uploaded after l-g-t-m from bsalomon@google.com

[egdaniel, 2015-02-17 15:28:08]

The CQ bit was checked by egdaniel@google.com

[commit-bot: I haz the power, 2015-02-17 15:28:26]

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

[commit-bot: I haz the power, 2015-02-17 15:34:49]

Committed patchset #5 (id:80001) as
https://skia.googlesource.com/skia/+/b197b8ff31b73ccb20423023e03592df8ae78ea6