Add all remaining separable xfer modes to GPU backend.

Add all remaining separable xfer modes to GPU backend.
Committed: https://code.google.com/p/skia/source/detail?r=8768

[bsalomon, 2013-04-19 13:59:21]

https://codereview.chromium.org/14189024/diff/9001/src/core/SkXfermode.cpp
File src/core/SkXfermode.cpp (right):

https://codereview.chromium.org/14189024/diff/9001/src/core/SkXfermode.cpp#ne...
src/core/SkXfermode.cpp:1089: builder->fsCodeAppendf("\t\t%s.a = 1.0 - (1.0 -
%s.a) * (1.0 - %s.a);\n", final, dst, src);
I can wrap these, but I'm not sure that it helps anything :)

[jvanverth1, 2013-04-19 14:16:15]

https://codereview.chromium.org/14189024/diff/9001/src/core/SkXfermode.cpp
File src/core/SkXfermode.cpp (right):

https://codereview.chromium.org/14189024/diff/9001/src/core/SkXfermode.cpp#ne...
src/core/SkXfermode.cpp:1030: ColorDodgeComponent(builder, outputColor,
inputColor, dstColor, 'r');
Is there a reason these (and following) are separated by component? I don't know
enough about the xfermodes to know why it's done this way.

https://codereview.chromium.org/14189024/diff/9001/src/core/SkXfermode.cpp#ne...
src/core/SkXfermode.cpp:1089: builder->fsCodeAppendf("\t\t%s.a = 1.0 - (1.0 -
%s.a) * (1.0 - %s.a);\n", final, dst, src);
On 2013/04/19 13:59:21, bsalomon wrote:
> I can wrap these, but I'm not sure that it helps anything :)

It'd probably help to put the varargs on the following line, at least.

https://codereview.chromium.org/14189024/diff/9001/src/core/SkXfermode.cpp#ne...
src/core/SkXfermode.cpp:1097: static void ColorDodgeComponent(GrGLShaderBuilder*
builder, const char* final, const char* src, const char* dst, const char
component) {
Wrap function decls at least?

[robertphillips, 2013-04-19 14:16:32]

lgtm + 1 suggestion

https://codereview.chromium.org/14189024/diff/9001/src/core/SkXfermode.cpp
File src/core/SkXfermode.cpp (right):

https://codereview.chromium.org/14189024/diff/9001/src/core/SkXfermode.cpp#ne...
src/core/SkXfermode.cpp:957: static bool IsSupportedMode(SkXfermode::Mode mode)
{
return(!<expression>)?

[bsalomon, 2013-04-19 14:24:28]

https://codereview.chromium.org/14189024/diff/9001/src/core/SkXfermode.cpp
File src/core/SkXfermode.cpp (right):

https://codereview.chromium.org/14189024/diff/9001/src/core/SkXfermode.cpp#ne...
src/core/SkXfermode.cpp:957: static bool IsSupportedMode(SkXfermode::Mode mode)
{
On 2013/04/19 14:16:32, robertphillips wrote:
> return(!<expression>)?

will do

https://codereview.chromium.org/14189024/diff/9001/src/core/SkXfermode.cpp#ne...
src/core/SkXfermode.cpp:1030: ColorDodgeComponent(builder, outputColor,
inputColor, dstColor, 'r');
On 2013/04/19 14:16:15, JimVV wrote:
> Is there a reason these (and following) are separated by component? I don't
know
> enough about the xfermodes to know why it's done this way.

Just that they couldn't be easily be done vectored and using a function meant
not duplicating the same code for r,g, and b

https://codereview.chromium.org/14189024/diff/9001/src/core/SkXfermode.cpp#ne...
src/core/SkXfermode.cpp:1089: builder->fsCodeAppendf("\t\t%s.a = 1.0 - (1.0 -
%s.a) * (1.0 - %s.a);\n", final, dst, src);
On 2013/04/19 14:16:15, JimVV wrote:
> On 2013/04/19 13:59:21, bsalomon wrote:
> > I can wrap these, but I'm not sure that it helps anything :)
> 
> It'd probably help to put the varargs on the following line, at least.

will do

https://codereview.chromium.org/14189024/diff/9001/src/core/SkXfermode.cpp#ne...
src/core/SkXfermode.cpp:1097: static void ColorDodgeComponent(GrGLShaderBuilder*
builder, const char* final, const char* src, const char* dst, const char
component) {
On 2013/04/19 14:16:15, JimVV wrote:
> Wrap function decls at least?

will do

[bsalomon, 2013-04-19 14:42:38]

Addressed comments

[jvanverth1, 2013-04-19 15:01:24]

lgtm

[bsalomon, 2013-04-19 15:03:26]

Committed patchset #5 manually as r8768 (presubmit successful).