Insert clip fragment processor outside GrCMM

Insert clip fragment processor outside GrCMM

Committed: https://skia.googlesource.com/skia/+/0ba8c2401e7b9b0dc68a434347a32c733045a57b

[bsalomon, 2015-10-06 18:51:33]

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

[robertphillips, 2015-10-07 13:06:44]

looks good to me but I would like to take another look.

https://codereview.chromium.org/1393553002/diff/20001/src/gpu/GrClipMaskManag...
File src/gpu/GrClipMaskManager.cpp (right):

https://codereview.chromium.org/1393553002/diff/20001/src/gpu/GrClipMaskManag...
src/gpu/GrClipMaskManager.cpp:121: SkASSERT(elements.count() <=
kMaxAnalyticElements);
Init this ('fps') to all 0s here and rm the "fps[fpCnt] = nullptr;' line ?

https://codereview.chromium.org/1393553002/diff/20001/src/gpu/GrClipMaskManag...
File src/gpu/GrClipMaskManager.h (right):

https://codereview.chromium.org/1393553002/diff/20001/src/gpu/GrClipMaskManag...
src/gpu/GrClipMaskManager.h:26: 
// informative comment ?

[bsalomon, 2015-10-07 13:38:24]

https://codereview.chromium.org/1393553002/diff/20001/src/gpu/GrClipMaskManag...
File src/gpu/GrClipMaskManager.cpp (right):

https://codereview.chromium.org/1393553002/diff/20001/src/gpu/GrClipMaskManag...
src/gpu/GrClipMaskManager.cpp:121: SkASSERT(elements.count() <=
kMaxAnalyticElements);
On 2015/10/07 13:06:43, robertphillips wrote:
> Init this ('fps') to all 0s here and rm the "fps[fpCnt] = nullptr;' line ?

Done, but I don't think it improved safety as it is still the case that fpCnt
must agree with the number of valid ptrs in the array in order for the unref()
loop to be correct.

[robertphillips, 2015-10-07 15:42:42]

lgtm

https://codereview.chromium.org/1393553002/diff/20001/src/gpu/GrClipMaskManag...
File src/gpu/GrClipMaskManager.cpp (right):

https://codereview.chromium.org/1393553002/diff/20001/src/gpu/GrClipMaskManag...
src/gpu/GrClipMaskManager.cpp:121: SkASSERT(elements.count() <=
kMaxAnalyticElements);
Yeah - I know it's just a sop but the fragility of this method is what was
giving me pause. I don't think we want to add a lot of guards though (e.g.,
SkAutoTUnref<GrFragmentProcessor> fps[kMaxAnalyticElemtents]).

https://codereview.chromium.org/1393553002/diff/40001/src/gpu/GrClipMaskManag...
File src/gpu/GrClipMaskManager.h (right):

https://codereview.chromium.org/1393553002/diff/40001/src/gpu/GrClipMaskManag...
src/gpu/GrClipMaskManager.h:28: * Produced by GrClipMaskManager. It provides a
set of modifications to the drawing state that
creat -> create ?

[bsalomon, 2015-10-07 16:11:21]

The CQ bit was checked by bsalomon@google.com

[bsalomon, 2015-10-07 16:11:22]

The patchset sent to the CQ was uploaded after l-g-t-m from
robertphillips@google.com
Link to the patchset: https://codereview.chromium.org/1393553002/#ps60001
(title: "update")

[commit-bot: I haz the power, 2015-10-07 16:11:34]

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

[bsalomon, 2015-10-07 16:11:37]

https://codereview.chromium.org/1393553002/diff/40001/src/gpu/GrClipMaskManag...
File src/gpu/GrClipMaskManager.h (right):

https://codereview.chromium.org/1393553002/diff/40001/src/gpu/GrClipMaskManag...
src/gpu/GrClipMaskManager.h:28: * Produced by GrClipMaskManager. It provides a
set of modifications to the drawing state that
On 2015/10/07 15:42:42, robertphillips wrote:
> creat -> create ?

Done.

[commit-bot: I haz the power, 2015-10-07 16:12:16]

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

[commit-bot: I haz the power, 2015-10-07 16:12:17]

Try jobs failed on following builders:
  Build-Ubuntu-GCC-Arm64-Debug-Android-Trybot on client.skia.compile
(JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-Arm...)
  Build-Ubuntu-GCC-x86_64-Release-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-x86...)

[bsalomon, 2015-10-07 16:13:18]

The CQ bit was checked by bsalomon@google.com

[bsalomon, 2015-10-07 16:13:18]

The patchset sent to the CQ was uploaded after l-g-t-m from
robertphillips@google.com
Link to the patchset: https://codereview.chromium.org/1393553002/#ps80001
(title: "add class to friends")

[commit-bot: I haz the power, 2015-10-07 16:13:24]

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

[commit-bot: I haz the power, 2015-10-07 16:20:33]

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