Make the GPU dashing effect MSAA-friendly.

Make the GPU dashing effect MSAA-friendly.

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

[Stephen White, 2015-04-17 20:11:38]

senorblanco@chromium.org changed reviewers:
+ egdaniel@chromium.org

[Stephen White, 2015-04-17 20:11:38]

Greg: PTAL. Thanks!

I'm thinking an enum might be clearer than an edge type + MSAA bool. Let me know
what you think.

[Stephen White, 2015-04-17 20:38:24]

On 2015/04/17 20:11:38, Stephen White wrote:
> Greg: PTAL. Thanks!
> 
> I'm thinking an enum might be clearer than an edge type + MSAA bool. Let me
know
> what you think.

Patch set 3 is a version with an enum instead of the edge type + bool.

[Stephen White, 2015-04-17 21:41:09]

The CQ bit was checked by senorblanco@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-04-17 21:41:32]

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

[commit-bot: I haz the power, 2015-04-17 21:47:44]

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

[commit-bot: I haz the power, 2015-04-17 21:47:45]

Dry run: This issue passed the CQ dry run.

[egdaniel, 2015-04-20 19:55:13]

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

[egdaniel, 2015-04-20 19:55:13]

https://codereview.chromium.org/1092793006/diff/80001/src/gpu/effects/GrDashi...
File src/gpu/effects/GrDashingEffect.cpp (right):

https://codereview.chromium.org/1092793006/diff/80001/src/gpu/effects/GrDashi...
src/gpu/effects/GrDashingEffect.cpp:873: // xy, refer to circle radius - 0.5, z
refers to cicles center x coord
silly nit not from you, but do you mind changing this comment to be x refers to
radius - 0.5, y refers to circle's center. There is no z component

https://codereview.chromium.org/1092793006/diff/80001/src/gpu/effects/GrDashi...
src/gpu/effects/GrDashingEffect.cpp:901: fsBuilder->codeAppendf("float alpha =
1.0;");
since we fall into the else here for both msaa and bw, can we end up in a case
with msaa where the top/bottom points of the circles have some coverage but the
sides do not?

https://codereview.chromium.org/1092793006/diff/80001/src/gpu/effects/GrDashi...
src/gpu/effects/GrDashingEffect.cpp:1154: // Now compute coverage in x and y and
multiply them to get the fraction of the pixel
not doing y anymore here.

[Stephen White, 2015-04-20 20:20:32]

Thanks for your review.

https://codereview.chromium.org/1092793006/diff/80001/src/gpu/effects/GrDashi...
File src/gpu/effects/GrDashingEffect.cpp (right):

https://codereview.chromium.org/1092793006/diff/80001/src/gpu/effects/GrDashi...
src/gpu/effects/GrDashingEffect.cpp:873: // xy, refer to circle radius - 0.5, z
refers to cicles center x coord
On 2015/04/20 19:55:13, egdaniel wrote:
> silly nit not from you, but do you mind changing this comment to be x refers
to
> radius - 0.5, y refers to circle's center. There is no z component

Done.

https://codereview.chromium.org/1092793006/diff/80001/src/gpu/effects/GrDashi...
src/gpu/effects/GrDashingEffect.cpp:901: fsBuilder->codeAppendf("float alpha =
1.0;");
On 2015/04/20 19:55:13, egdaniel wrote:
> since we fall into the else here for both msaa and bw, can we end up in a case
> with msaa where the top/bottom points of the circles have some coverage but
the
> sides do not?

Yeah, I wasn't sure about this. Since we're not bloating the edges anymore for
MSAA, I don't *think* it will introduce artifacts, but I'll definitely keep an
eye on the bot results and circle back to this if need be.

https://codereview.chromium.org/1092793006/diff/80001/src/gpu/effects/GrDashi...
src/gpu/effects/GrDashingEffect.cpp:1154: // Now compute coverage in x and y and
multiply them to get the fraction of the pixel
On 2015/04/20 19:55:13, egdaniel wrote:
> not doing y anymore here.

Done.

[Stephen White, 2015-04-20 20:58:52]

OK, new patch up bloating in Y only for MSAA w/round caps

[egdaniel, 2015-04-20 21:07:41]

lgtm

[Stephen White, 2015-04-20 21:39:05]

The CQ bit was checked by senorblanco@chromium.org

[commit-bot: I haz the power, 2015-04-20 21:39:20]

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

[commit-bot: I haz the power, 2015-04-20 21:44:30]

Committed patchset #9 (id:30002) as
https://skia.googlesource.com/skia/+/f3c2c46cf818fe408d0ec295338002fc01c64926