added GrMSAAPathRenderer

added GrMSAAPathRenderer
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1834133003

Committed: https://skia.googlesource.com/skia/+/47a2dc8e229e93e1bcf7405747320920da1ab742

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

Committed: https://skia.googlesource.com/skia/+/6536ae590e3873694cb02cf25dab8001a71f269e

[ethannicholas, 2016-03-28 20:06:33]

Description was changed from

==========
added GrMSAAPathRenderer
==========

to

==========
added GrMSAAPathRenderer
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[ethannicholas, 2016-03-29 15:55:25]

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

[egdaniel, 2016-03-29 20:57:39]

https://codereview.chromium.org/1834133003/diff/60001/src/gpu/batches/GrMSAAP...
File src/gpu/batches/GrMSAAPathRenderer.cpp (right):

https://codereview.chromium.org/1834133003/diff/60001/src/gpu/batches/GrMSAAP...
src/gpu/batches/GrMSAAPathRenderer.cpp:2: * Copyright 2011 Google Inc.
2016

https://codereview.chromium.org/1834133003/diff/60001/src/gpu/batches/GrMSAAP...
src/gpu/batches/GrMSAAPathRenderer.cpp:29: // Stencil rules for paths
Can we share all these with default path renderer in some way since they're
identical?

https://codereview.chromium.org/1834133003/diff/60001/src/gpu/batches/GrMSAAP...
src/gpu/batches/GrMSAAPathRenderer.cpp:143: uint16_t edgeV0Idx,
align these

https://codereview.chromium.org/1834133003/diff/60001/src/gpu/batches/GrMSAAP...
src/gpu/batches/GrMSAAPathRenderer.cpp:235: key |=
qp.viewMatrix().hasPerspective() ? 0x1 : 0x0;
something feels wrong with having to check perspective and identity here. Any
ways that those might affect the shader seem hidden to this GP. If we need this
to be part of the key shouldn't it be added by whoever actually is affected by
them?

https://codereview.chromium.org/1834133003/diff/60001/src/gpu/batches/GrMSAAP...
src/gpu/batches/GrMSAAPathRenderer.cpp:266: GrColor       fColor;
why are you storing fColor on the GLSLProcessor?

[bsalomon, 2016-03-29 21:43:22]

I think most of my comments pertain to cargo cult code that is infecting this
via the default path renderer.

https://codereview.chromium.org/1834133003/diff/60001/src/gpu/batches/GrMSAAP...
File src/gpu/batches/GrMSAAPathRenderer.cpp (right):

https://codereview.chromium.org/1834133003/diff/60001/src/gpu/batches/GrMSAAP...
src/gpu/batches/GrMSAAPathRenderer.cpp:595: 
why not use vertex colors?

https://codereview.chromium.org/1834133003/diff/60001/src/gpu/batches/GrMSAAP...
src/gpu/batches/GrMSAAPathRenderer.cpp:684: uint8_t coverage() const { return
fBatch.fCoverage; }
Why do you have a coverage?

https://codereview.chromium.org/1834133003/diff/60001/src/gpu/batches/GrMSAAP...
src/gpu/batches/GrMSAAPathRenderer.cpp:689: struct BatchTracker {
Older batches use this batch tracker struct, but these are just common
properties of all the path draws in the batch. I'm not sure why organizing them
in a struct is useful (even more so if we remove the ones that aren't really
necessary).

https://codereview.chromium.org/1834133003/diff/60001/src/gpu/batches/GrMSAAP...
src/gpu/batches/GrMSAAPathRenderer.cpp:694: bool fColorIgnored;
I think we should drop the color ignored stuff. It can't be worth it and just
adds complexity here.

https://codereview.chromium.org/1834133003/diff/60001/src/gpu/batches/GrMSAAP...
src/gpu/batches/GrMSAAPathRenderer.cpp:713: uint8_t newCoverage = 0xff;
?

https://codereview.chromium.org/1834133003/diff/60001/src/gpu/batches/GrMSAAP...
src/gpu/batches/GrMSAAPathRenderer.cpp:715: // Save the current xp on the draw
state so we can reset it if needed
draw state?

[ethannicholas, 2016-03-31 19:33:35]

I believe that addresses all of the feedback...

https://codereview.chromium.org/1834133003/diff/60001/src/gpu/batches/GrMSAAP...
File src/gpu/batches/GrMSAAPathRenderer.cpp (right):

https://codereview.chromium.org/1834133003/diff/60001/src/gpu/batches/GrMSAAP...
src/gpu/batches/GrMSAAPathRenderer.cpp:235: key |=
qp.viewMatrix().hasPerspective() ? 0x1 : 0x0;
On 2016/03/29 20:57:39, egdaniel wrote:
> something feels wrong with having to check perspective and identity here. Any
> ways that those might affect the shader seem hidden to this GP. If we need
this
> to be part of the key shouldn't it be added by whoever actually is affected by
> them?

GrGLSLGeometryProcessor::setupPosition generates different code based on these
flags, and other GenKey implementations I've looked at seem to be checking (at
least perspective) themselves, presumably for the same reason I am. I'm happy to
handle this better, but I don't know of a way to do it without significant
refactoring (which seems like it should be a separate change).

https://codereview.chromium.org/1834133003/diff/60001/src/gpu/batches/GrMSAAP...
src/gpu/batches/GrMSAAPathRenderer.cpp:266: GrColor       fColor;
On 2016/03/29 20:57:39, egdaniel wrote:
> why are you storing fColor on the GLSLProcessor?

Cared about it at one point, forgot to remove the field when that was no longer
the case.

[bsalomon, 2016-04-04 13:09:21]

lgtm w/ one request and a rebase pointer inline.

https://codereview.chromium.org/1834133003/diff/80001/src/gpu/batches/GrDefau...
File src/gpu/batches/GrDefaultPathRenderer.h (right):

https://codereview.chromium.org/1834133003/diff/80001/src/gpu/batches/GrDefau...
src/gpu/batches/GrDefaultPathRenderer.h:15: // Stencil rules for paths
Maybe put these in a separate shared header? GrPathStencilSettings.h or
something like that?

https://codereview.chromium.org/1834133003/diff/80001/src/gpu/batches/GrMSAAP...
File src/gpu/batches/GrMSAAPathRenderer.cpp (right):

https://codereview.chromium.org/1834133003/diff/80001/src/gpu/batches/GrMSAAP...
src/gpu/batches/GrMSAAPathRenderer.cpp:408: target->initDraw(lineGP);
Heads up, I recently removed this initDraw() funtion. You now just pass the gp
to target->draw(). You can probably defer creating the GPs until you're sure you
need them.

[ethannicholas, 2016-04-04 14:38:15]

The CQ bit was checked by ethannicholas@google.com

[ethannicholas, 2016-04-04 14:38:15]

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/1834133003/#ps120001
(title: "fixed import")

[commit-bot: I haz the power, 2016-04-04 14:38:23]

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

[commit-bot: I haz the power, 2016-04-04 14:41:13]

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

[commit-bot: I haz the power, 2016-04-04 14:41:14]

Try jobs failed on following builders:
  Build-Win-MSVC-x86_64-Debug-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Win-MSVC-x86_6...)

[ethannicholas, 2016-04-04 14:48:19]

The CQ bit was checked by ethannicholas@google.com

[ethannicholas, 2016-04-04 14:48:20]

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/1834133003/#ps140001
(title: "fixed Windows build errors")

[commit-bot: I haz the power, 2016-04-04 14:48:23]

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

[commit-bot: I haz the power, 2016-04-04 14:57:42]

Description was changed from

==========
added GrMSAAPathRenderer
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
added GrMSAAPathRenderer
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

Committed:
https://skia.googlesource.com/skia/+/47a2dc8e229e93e1bcf7405747320920da1ab742
==========

[commit-bot: I haz the power, 2016-04-04 14:57:43]

Committed patchset #8 (id:140001) as
https://skia.googlesource.com/skia/+/47a2dc8e229e93e1bcf7405747320920da1ab742

[ethannicholas, 2016-04-04 15:26:57]

A revert of this CL (patchset #8 id:140001) has been created in
https://codereview.chromium.org/1860563002/ by ethannicholas@google.com.

The reason for reverting is: nanobench failure.

[ethannicholas, 2016-04-04 17:49:48]

Description was changed from

==========
added GrMSAAPathRenderer
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

Committed:
https://skia.googlesource.com/skia/+/47a2dc8e229e93e1bcf7405747320920da1ab742
==========

to

==========
added GrMSAAPathRenderer
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

Committed:
https://skia.googlesource.com/skia/+/47a2dc8e229e93e1bcf7405747320920da1ab742
==========

[ethannicholas, 2016-04-04 17:50:05]

The CQ bit was checked by ethannicholas@google.com

[ethannicholas, 2016-04-04 17:50:05]

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/1834133003/#ps160001
(title: "fix assertion failure")

[commit-bot: I haz the power, 2016-04-04 17:50:09]

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

[commit-bot: I haz the power, 2016-04-04 18:00:02]

Description was changed from

==========
added GrMSAAPathRenderer
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

Committed:
https://skia.googlesource.com/skia/+/47a2dc8e229e93e1bcf7405747320920da1ab742
==========

to

==========
added GrMSAAPathRenderer
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

Committed:
https://skia.googlesource.com/skia/+/47a2dc8e229e93e1bcf7405747320920da1ab742

Committed:
https://skia.googlesource.com/skia/+/b0fabd4ed55abcb6f5ed2f331c96c783f42bd37c
==========

[commit-bot: I haz the power, 2016-04-04 18:00:03]

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

[ethannicholas, 2016-04-04 18:41:17]

A revert of this CL (patchset #9 id:160001) has been created in
https://codereview.chromium.org/1853223003/ by ethannicholas@google.com.

The reason for reverting is: driver crash on Windows.

[ethannicholas, 2016-04-04 20:24:00]

Description was changed from

==========
added GrMSAAPathRenderer
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

Committed:
https://skia.googlesource.com/skia/+/47a2dc8e229e93e1bcf7405747320920da1ab742

Committed:
https://skia.googlesource.com/skia/+/b0fabd4ed55abcb6f5ed2f331c96c783f42bd37c
==========

to

==========
added GrMSAAPathRenderer
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

Committed:
https://skia.googlesource.com/skia/+/47a2dc8e229e93e1bcf7405747320920da1ab742

Committed:
https://skia.googlesource.com/skia/+/b0fabd4ed55abcb6f5ed2f331c96c783f42bd37c
==========

[ethannicholas, 2016-05-02 19:03:41]

The CQ bit was checked by ethannicholas@google.com

[ethannicholas, 2016-05-02 19:03:41]

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/1834133003/#ps200001
(title: "blacklisted Intel")

[commit-bot: I haz the power, 2016-05-02 19:03:50]

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

[commit-bot: I haz the power, 2016-05-02 19:16:55]

Description was changed from

==========
added GrMSAAPathRenderer
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

Committed:
https://skia.googlesource.com/skia/+/47a2dc8e229e93e1bcf7405747320920da1ab742

Committed:
https://skia.googlesource.com/skia/+/b0fabd4ed55abcb6f5ed2f331c96c783f42bd37c
==========

to

==========
added GrMSAAPathRenderer
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

Committed:
https://skia.googlesource.com/skia/+/47a2dc8e229e93e1bcf7405747320920da1ab742

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

Committed:
https://skia.googlesource.com/skia/+/6536ae590e3873694cb02cf25dab8001a71f269e
==========

[commit-bot: I haz the power, 2016-05-02 19:16:56]

Committed patchset #11 (id:200001) as
https://skia.googlesource.com/skia/+/6536ae590e3873694cb02cf25dab8001a71f269e