SkPaintFilterCanvas skip-draw support

SkPaintFilterCanvas skip-draw support

At the time SkPaintFilterCanvas was introduced as a SkDrawFilter replacement,
no clients were relying on the draw veto logic.  Now Chromium does.

To facilitate migrating off SkDrawFilter, let's augment SkPaintFilterCanvas
with skip-draw semantics.

A side effect of the CL is that now we call the filter virtual even for null
paints.

BUG=skia:4782
R=reed@google.com,robertphillips@google.com
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1577933002

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

[f(malita), 2016-01-11 18:40:56]

Description was changed from

==========
SkPaintFilterCanvas skip-draw support

BUG=skia:4782
==========

to

==========
SkPaintFilterCanvas skip-draw support

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

[f(malita), 2016-01-11 18:54:57]

Description was changed from

==========
SkPaintFilterCanvas skip-draw support

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

to

==========
SkPaintFilterCanvas skip-draw support

At the time SkPaintFilterCanvas was introduced as a SkDrawFilter replacement,
no clients were relying on the draw veto logic.  Now Chromium does.

To facilitate migrating off SkDrawFilter, let's augment SkPaintFilterCanvas
with skip-draw semantics.

A side effect of the CL is that now we call the filter virtual even for null
paints.

BUG=skia:4782
R=reed@google.com,robertphillips@google.com
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[f(malita), 2016-01-11 18:54:57]

fmalita@chromium.org changed reviewers:
+ reed@google.com, robertphillips@google.com

[f(malita), 2016-01-11 19:29:45]

Not crazy about piling more stuff onto SkPaintFilterCanvas, but the alternative
is ~6 SkCanvas method overrides in Chromium - which seems fragile.

https://codereview.chromium.org/1577933002/diff/20001/include/utils/SkPaintFi...
File include/utils/SkPaintFilterCanvas.h (right):

https://codereview.chromium.org/1577933002/diff/20001/include/utils/SkPaintFi...
include/utils/SkPaintFilterCanvas.h:61: if (paint) {
Legacy/transient semantics.  To be converted to a pure virtual after Chromium
gets updated.

[f(malita), 2016-01-11 19:29:49]

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

[commit-bot: I haz the power, 2016-01-11 19:29:59]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1577933002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1577933002/20001

[commit-bot: I haz the power, 2016-01-11 19:37:48]

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

[commit-bot: I haz the power, 2016-01-11 19:37:48]

Dry run: This issue passed the CQ dry run.

[reed1, 2016-01-11 19:38:57]

note:

Chrome's use always treats the paint as RO, while this (and SkDrawFilter) first
copy the dang paint and then pass it to the client.

Wonder if there is a simplicity/perf reason to offer just a skip version of the
virtual/callback, where no one has to copy the paint...

[f(malita), 2016-01-11 19:52:17]

On 2016/01/11 19:38:57, reed1 wrote:
> note:
> 
> Chrome's use always treats the paint as RO, while this (and SkDrawFilter)
first
> copy the dang paint and then pass it to the client.
> 
> Wonder if there is a simplicity/perf reason to offer just a skip version of
the
> virtual/callback, where no one has to copy the paint...

Good point.  Let me see if I can find a way to achieve that without cloning
SkPaintFilterCanvas.

[reed1, 2016-01-11 20:12:23]

that said, TODAY we have the same copy, and it seems ok, so I'm not actually
complaining, just noticing.

[f(malita), 2016-01-11 21:02:37]

On 2016/01/11 20:12:23, reed1 wrote:
> that said, TODAY we have the same copy, and it seems ok, so I'm not actually
> complaining, just noticing.

True, but it'd still be nice to handle this case as it would work better for
the upcoming Chromium conversion.

One way is to use an SkTLazy<SkPaint>* outparam, and defer the actual paint copy
to overriders - then implementations can skip it when not needed.

(A nice side effect of using a separate output param is that clients can now
override even null paints)

PTAL at PS#3.

[f(malita), 2016-01-11 21:02:44]

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

[commit-bot: I haz the power, 2016-01-11 21:02:55]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1577933002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1577933002/40001

[reed1, 2016-01-11 21:07:33]

1. Is the idea that chrome and android will both use this (modified) subclass?

2. can we change the first parameter to const SkPaint& ?

[f(malita), 2016-01-11 21:11:49]

On 2016/01/11 21:07:33, reed1 wrote:
> 1. Is the idea that chrome and android will both use this (modified) subclass?

Yes.  Chromium already does (see OpacityFilterCanvas), but we need the skip
semantics for one more site.  I think we can convert Android to it too, but I
haven't checked yet.


> 2. can we change the first parameter to const SkPaint& ?

Type-based filtering makes sense even with null paints (which are valid args for
some draw ops).

[reed1, 2016-01-11 21:14:58]

On 2016/01/11 21:11:49, f(malita) wrote:
> On 2016/01/11 21:07:33, reed1 wrote:
> > 1. Is the idea that chrome and android will both use this (modified)
subclass?
> 
> Yes.  Chromium already does (see OpacityFilterCanvas), but we need the skip
> semantics for one more site.  I think we can convert Android to it too, but I
> haven't checked yet.
> 
> 
> > 2. can we change the first parameter to const SkPaint& ?
> 
> Type-based filtering makes sense even with null paints (which are valid args
for
> some draw ops).

Ah, right. n/m

[reed1, 2016-01-11 21:15:46]

reed@google.com changed reviewers:
+ djsollen@google.com, scroggo@google.com

[reed1, 2016-01-11 21:15:46]

lgtm

would be really great if this helps us remove android's dependency on
DrawFilters too.

[commit-bot: I haz the power, 2016-01-11 21:16:45]

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

[commit-bot: I haz the power, 2016-01-11 21:16:46]

Dry run: This issue passed the CQ dry run.

[f(malita), 2016-01-11 21:57:45]

The CQ bit was checked by fmalita@chromium.org

[commit-bot: I haz the power, 2016-01-11 21:57:51]

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

[commit-bot: I haz the power, 2016-01-11 21:58:31]

Description was changed from

==========
SkPaintFilterCanvas skip-draw support

At the time SkPaintFilterCanvas was introduced as a SkDrawFilter replacement,
no clients were relying on the draw veto logic.  Now Chromium does.

To facilitate migrating off SkDrawFilter, let's augment SkPaintFilterCanvas
with skip-draw semantics.

A side effect of the CL is that now we call the filter virtual even for null
paints.

BUG=skia:4782
R=reed@google.com,robertphillips@google.com
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
SkPaintFilterCanvas skip-draw support

At the time SkPaintFilterCanvas was introduced as a SkDrawFilter replacement,
no clients were relying on the draw veto logic.  Now Chromium does.

To facilitate migrating off SkDrawFilter, let's augment SkPaintFilterCanvas
with skip-draw semantics.

A side effect of the CL is that now we call the filter virtual even for null
paints.

BUG=skia:4782
R=reed@google.com,robertphillips@google.com
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

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

[commit-bot: I haz the power, 2016-01-11 21:58:32]

Committed patchset #3 (id:40001) as
https://skia.googlesource.com/skia/+/bad23dc9ed2e00f2a066db01ab88e4c4adcecfc6