Add gpu backend for SkPixelXorXfermode

Add gpu backend for SkPixelXorXfermode
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1631873002

Committed: https://skia.googlesource.com/skia/+/727b7d27afd01ab76771ba0b3279208cff728d1a

[robertphillips, 2016-01-25 18:04:58]

Description was changed from

==========
Add gpu backend for SkPixelXorXfermode
==========

to

==========
Add gpu backend for SkPixelXorXfermode
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[robertphillips, 2016-01-25 21:19:01]

Description was changed from

==========
Add gpu backend for SkPixelXorXfermode
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
Add gpu backend for SkPixelXorXfermode
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[robertphillips, 2016-01-25 21:19:01]

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

[bsalomon, 2016-01-25 21:52:44]

https://codereview.chromium.org/1631873002/diff/20001/src/effects/SkPixelXorX...
File src/effects/SkPixelXorXfermode.cpp (right):

https://codereview.chromium.org/1631873002/diff/20001/src/effects/SkPixelXorX...
src/effects/SkPixelXorXfermode.cpp:68:
fragBuilder->emitFunction(kFloat_GrSLType,
Should we have the int version, too? Seems a lot simpler than what you have
here.

https://codereview.chromium.org/1631873002/diff/20001/src/effects/SkPixelXorX...
src/effects/SkPixelXorXfermode.cpp:115: class GrPixelXorFP : public
GrFragmentProcessor {
Don't really need Gr prefix here

https://codereview.chromium.org/1631873002/diff/20001/src/effects/SkPixelXorX...
src/effects/SkPixelXorXfermode.cpp:190: SkColorGetR(pixXor.opColor())/255.0f,
Is it correct to upload unpremul components?

https://codereview.chromium.org/1631873002/diff/20001/src/gpu/glsl/GrGLSLXfer...
File src/gpu/glsl/GrGLSLXferProcessor.h (right):

https://codereview.chromium.org/1631873002/diff/20001/src/gpu/glsl/GrGLSLXfer...
src/gpu/glsl/GrGLSLXferProcessor.h:72: static void
DefaultCoverageModulation(GrGLSLXPFragmentBuilder* fragBuilder,
I thought this already existed somewhere... was it reverted with the PLS change?

[egdaniel, 2016-01-25 21:54:17]

https://codereview.chromium.org/1631873002/diff/20001/src/effects/SkPixelXorX...
File src/effects/SkPixelXorXfermode.cpp (right):

https://codereview.chromium.org/1631873002/diff/20001/src/effects/SkPixelXorX...
src/effects/SkPixelXorXfermode.cpp:61: GrGLSLShaderVar("f1", kFloat_GrSLType, 0,
kHigh_GrSLPrecision),
highp?

https://codereview.chromium.org/1631873002/diff/20001/src/effects/SkPixelXorX...
src/effects/SkPixelXorXfermode.cpp:64: GrGLSLShaderVar("fNum", kFloat_GrSLType,
0, kHigh_GrSLPrecision),
rename fPower2Something or any other name that is more descriptive

https://codereview.chromium.org/1631873002/diff/20001/src/effects/SkPixelXorX...
src/effects/SkPixelXorXfermode.cpp:68:
fragBuilder->emitFunction(kFloat_GrSLType,
comment describing what this function does

[robertphillips, 2016-01-25 22:51:12]

https://codereview.chromium.org/1631873002/diff/20001/src/effects/SkPixelXorX...
File src/effects/SkPixelXorXfermode.cpp (right):

https://codereview.chromium.org/1631873002/diff/20001/src/effects/SkPixelXorX...
src/effects/SkPixelXorXfermode.cpp:61: GrGLSLShaderVar("f1", kFloat_GrSLType, 0,
kHigh_GrSLPrecision),
On 2016/01/25 21:54:17, egdaniel wrote:
> highp?

Done.

https://codereview.chromium.org/1631873002/diff/20001/src/effects/SkPixelXorX...
src/effects/SkPixelXorXfermode.cpp:64: GrGLSLShaderVar("fNum", kFloat_GrSLType,
0, kHigh_GrSLPrecision),
On 2016/01/25 21:54:17, egdaniel wrote:
> rename fPower2Something or any other name that is more descriptive

Done - fPowerOf2Divisor.

https://codereview.chromium.org/1631873002/diff/20001/src/effects/SkPixelXorX...
src/effects/SkPixelXorXfermode.cpp:68:
fragBuilder->emitFunction(kFloat_GrSLType,
On 2016/01/25 21:54:17, egdaniel wrote:
> comment describing what this function does

Done.

https://codereview.chromium.org/1631873002/diff/20001/src/effects/SkPixelXorX...
src/effects/SkPixelXorXfermode.cpp:68:
fragBuilder->emitFunction(kFloat_GrSLType,
On 2016/01/25 21:52:43, bsalomon wrote:
> Should we have the int version, too? Seems a lot simpler than what you have
> here.

Can we chat about this tomorrow? I'm not sure it will buy us anything.

https://codereview.chromium.org/1631873002/diff/20001/src/effects/SkPixelXorX...
src/effects/SkPixelXorXfermode.cpp:115: class GrPixelXorFP : public
GrFragmentProcessor {
On 2016/01/25 21:52:43, bsalomon wrote:
> Don't really need Gr prefix here

Done.

https://codereview.chromium.org/1631873002/diff/20001/src/effects/SkPixelXorX...
src/effects/SkPixelXorXfermode.cpp:190: SkColorGetR(pixXor.opColor())/255.0f,
On 2016/01/25 21:52:43, bsalomon wrote:
> Is it correct to upload unpremul components?

I have updated the GM to demonstrate this class' behavior. The opColor is never
converted to a PMColor so passing in 0xf7FF0000 is the same as passing in
0xFFFF0000.

https://codereview.chromium.org/1631873002/diff/20001/src/gpu/glsl/GrGLSLXfer...
File src/gpu/glsl/GrGLSLXferProcessor.h (right):

https://codereview.chromium.org/1631873002/diff/20001/src/gpu/glsl/GrGLSLXfer...
src/gpu/glsl/GrGLSLXferProcessor.h:72: static void
DefaultCoverageModulation(GrGLSLXPFragmentBuilder* fragBuilder,
On 2016/01/25 21:52:43, bsalomon wrote:
> I thought this already existed somewhere... was it reverted with the PLS
change?

Yeah - it got reverted in the OverdrawMode CL
(https://codereview.chromium.org/1607253002/). I will just update that one to
ToT.

[bsalomon, 2016-01-26 17:11:24]

lgtm

https://codereview.chromium.org/1631873002/diff/20001/src/effects/SkPixelXorX...
File src/effects/SkPixelXorXfermode.cpp (right):

https://codereview.chromium.org/1631873002/diff/20001/src/effects/SkPixelXorX...
src/effects/SkPixelXorXfermode.cpp:190: SkColorGetR(pixXor.opColor())/255.0f,
On 2016/01/25 22:51:11, robertphillips wrote:
> On 2016/01/25 21:52:43, bsalomon wrote:
> > Is it correct to upload unpremul components?
> 
> I have updated the GM to demonstrate this class' behavior. The opColor is
never
> converted to a PMColor so passing in 0xf7FF0000 is the same as passing in
> 0xFFFF0000.

Weird, ok

[robertphillips, 2016-01-26 17:52:24]

The CQ bit was checked by robertphillips@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-01-26 17:52:30]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1631873002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1631873002/40001

[commit-bot: I haz the power, 2016-01-26 17:57:00]

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

[commit-bot: I haz the power, 2016-01-26 17:57:01]

Dry run: Try jobs failed on following builders:
  Test-Ubuntu-GCC-ShuttleA-GPU-GTX660-x86_64-Release-Trybot on client.skia
(JOB_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-ShuttleA-GPU...)

[robertphillips, 2016-01-26 18:13:21]

The CQ bit was checked by robertphillips@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-01-26 18:13:29]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1631873002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1631873002/60001

[commit-bot: I haz the power, 2016-01-26 18:16:59]

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

[commit-bot: I haz the power, 2016-01-26 18:17:00]

Dry run: Try jobs failed on following builders:
  Test-Ubuntu-GCC-ShuttleA-GPU-GTX660-x86_64-Release-Trybot on client.skia
(JOB_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-ShuttleA-GPU...)

[robertphillips, 2016-01-26 18:51:06]

The CQ bit was checked by robertphillips@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-01-26 18:51:15]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1631873002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1631873002/80001

[commit-bot: I haz the power, 2016-01-26 19:02:52]

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

[commit-bot: I haz the power, 2016-01-26 19:02:52]

Dry run: This issue passed the CQ dry run.

[robertphillips, 2016-01-26 19:26:34]

The CQ bit was checked by robertphillips@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-01-26 19:26:42]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1631873002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1631873002/100001

[commit-bot: I haz the power, 2016-01-26 19:37:17]

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

[commit-bot: I haz the power, 2016-01-26 19:37:18]

Dry run: This issue passed the CQ dry run.

[robertphillips, 2016-01-26 20:06:32]

The CQ bit was checked by robertphillips@google.com

[robertphillips, 2016-01-26 20:06:32]

The patchset sent to the CQ was uploaded after l-g-t-m from bsalomon@google.com
Link to the patchset: https://codereview.chromium.org/1631873002/#ps100001
(title: "And fix case where the srcColor is null")

[commit-bot: I haz the power, 2016-01-26 20:06:39]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1631873002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1631873002/100001

[commit-bot: I haz the power, 2016-01-26 20:07:16]

Description was changed from

==========
Add gpu backend for SkPixelXorXfermode
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
Add gpu backend for SkPixelXorXfermode
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

Committed:
https://skia.googlesource.com/skia/+/727b7d27afd01ab76771ba0b3279208cff728d1a
==========

[commit-bot: I haz the power, 2016-01-26 20:07:17]

Committed patchset #6 (id:100001) as
https://skia.googlesource.com/skia/+/727b7d27afd01ab76771ba0b3279208cff728d1a