C API: add sk_xfermode.h, impl, test

C API: add sk_xfermode.h, impl, test

Committed: https://skia.googlesource.com/skia/+/7568d0b15e9d66387533d397b5aaba60491e1624

[hal.canary, 2015-07-22 15:43:29]

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

[commit-bot: I haz the power, 2015-07-22 15:43:35]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1242403005/1

[hal.canary, 2015-07-22 15:48:00]

halcanary@google.com changed reviewers:
+ reed@google.com

[hal.canary, 2015-07-22 15:48:01]

please take a look

[commit-bot: I haz the power, 2015-07-22 16:03:17]

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

[commit-bot: I haz the power, 2015-07-22 16:03:18]

Dry run: This issue passed the CQ dry run.

[hal.canary, 2015-07-28 21:17:56]

On second thought, let's not expose SkXfermode.  We'll only expose
SkXfermode::Mode for now.

[hal.canary, 2015-07-28 21:21:41]

Patchset #2 (id:20001) has been deleted

[reed1, 2015-07-29 18:43:42]

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

[reed1, 2015-07-29 18:43:42]

mulling over this. wondering if we want to just have enum-in, enum-out on the
c-paint, and not introduce an object at all. This is coming up (potentially) as
part of Brian's effort to remove xfermodes that are not in the enum

[hal.canary, 2015-07-29 19:47:38]

On 2015/07/29 18:43:42, reed1 wrote:
> mulling over this. wondering if we want to just have enum-in, enum-out on the
> c-paint, and not introduce an object at all. This is coming up (potentially)
as
> part of Brian's effort to remove xfermodes that are not in the enum

Maybe paints don't need getters at all.

[hal.canary, 2015-07-30 21:27:05]

One more try.

[reed1, 2015-07-31 15:29:27]

How about this as a defensive approach:

1. don't introduce UNKNOWN as a mode
2. don't offer a getter for xfermode_mode (at least for now)

This way we can start to use modes, and can in the future either
- introduce an alternate setter if we support full-on objects
- don't need to retro-explain what an enum-getter means if we have full objects
- discover in-the-field if clients really want a getter for this

[hal.canary, 2015-07-31 15:31:46]

On 2015/07/31 15:29:27, reed1 wrote:
> How about this as a defensive approach:
> 
> 1. don't introduce UNKNOWN as a mode
> 2. don't offer a getter for xfermode_mode (at least for now)
> 
> This way we can start to use modes, and can in the future either
> - introduce an alternate setter if we support full-on objects
> - don't need to retro-explain what an enum-getter means if we have full
objects
> - discover in-the-field if clients really want a getter for this

Patch Set 2, then?

[reed1, 2015-07-31 15:32:45]

doh! -- yes

[reed1, 2015-07-31 15:33:42]

https://codereview.chromium.org/1242403005/diff/40001/src/c/sk_paint.cpp
File src/c/sk_paint.cpp (right):

https://codereview.chromium.org/1242403005/diff/40001/src/c/sk_paint.cpp#newc...
src/c/sk_paint.cpp:166: default:                          return
SkXfermode::kSrcOver_Mode;
Why do we have default?

[hal.canary, 2015-07-31 15:41:59]

https://codereview.chromium.org/1242403005/diff/40001/src/c/sk_paint.cpp
File src/c/sk_paint.cpp (right):

https://codereview.chromium.org/1242403005/diff/40001/src/c/sk_paint.cpp#newc...
src/c/sk_paint.cpp:166: default:                          return
SkXfermode::kSrcOver_Mode;
On 2015/07/31 15:33:42, reed1 wrote:
> Why do we have default?

In case the user passes in garbage  

    sk_paint_set_xfermode_mode(p, (sk_xfermode_mode_t)199);

[hal.canary, 2015-07-31 15:42:10]

Patchset #3 (id:60001) has been deleted

[reed1, 2015-07-31 15:47:19]

I think we should:

- crash
- ignore it completely
- be undefined

I don't think we should claim to do something sensible like fall back on
SRC_OVER

[hal.canary, 2015-07-31 17:33:53]

On 2015/07/31 15:47:19, reed1 wrote:
> I think we should:
> 
> - crash
> - ignore it completely
> - be undefined
> 
> I don't think we should claim to do something sensible like fall back on
> SRC_OVER

Done (ignore it completely)

[hal.canary, 2015-07-31 17:56:04]

as discussed

[hal.canary, 2015-07-31 18:11:56]

On 2015/07/31 17:56:04, Hal Canary wrote:
> as discussed

once more

[reed1, 2015-07-31 18:15:31]

lgtm

https://codereview.chromium.org/1242403005/diff/100001/src/c/sk_paint.cpp
File src/c/sk_paint.cpp (right):

https://codereview.chromium.org/1242403005/diff/100001/src/c/sk_paint.cpp#new...
src/c/sk_paint.cpp:136: SkASSERT(paint);
don't need this, as the setXfermode will reliably crash

[hal.canary, 2015-07-31 19:58:23]

The CQ bit was checked by halcanary@google.com

[commit-bot: I haz the power, 2015-07-31 19:58:35]

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

[commit-bot: I haz the power, 2015-07-31 20:38:10]

Committed patchset #5 (id:120001) as
https://skia.googlesource.com/skia/+/7568d0b15e9d66387533d397b5aaba60491e1624