Implement computeFastBounds() for SkImageFilter. This allows for the correct culling of primitives …

Give correct bounds to SkCanvas's AutoDrawLooper use for filter draws.

In conjuction with https://codereview.chromium.org/137423005/, this allows the canvas to tighten the bounds passed to saveLayer() only the affected region.

R=reed@google.com
BUG=100703
Committed: https://code.google.com/p/skia/source/detail?r=13221

[Stephen White, 2014-01-27 15:45:53]

reed@: PTAL. Thanks!

[reed1, 2014-01-27 16:32:26]

Looks like this is two interesting CLs

1. add bounds to AutoLooper
2. add computeFastBounds to imagefilter

The first seems complex enough, with possible perf implications (both ways) that
I think we could try to land it separately. Does that make sense?

https://codereview.chromium.org/141433017/diff/30001/include/core/SkImageFilt...
File include/core/SkImageFilter.h (right):

https://codereview.chromium.org/141433017/diff/30001/include/core/SkImageFilt...
include/core/SkImageFilter.h:147: virtual void computeFastBounds(const SkRect&,
SkRect*);
This seems to be const in other places. Can it be const here?

[Stephen White, 2014-01-27 16:43:58]

On 2014/01/27 16:32:26, reed1 wrote:
> Looks like this is two interesting CLs
> 
> 1. add bounds to AutoLooper
> 2. add computeFastBounds to imagefilter
> 
> The first seems complex enough, with possible perf implications (both ways)
that
> I think we could try to land it separately. Does that make sense?

OK. I think I'll have to land #2 first, or #1 will start culling filtered
primitives incorrectly.

>
https://codereview.chromium.org/141433017/diff/30001/include/core/SkImageFilt...
> File include/core/SkImageFilter.h (right):
> 
>
https://codereview.chromium.org/141433017/diff/30001/include/core/SkImageFilt...
> include/core/SkImageFilter.h:147: virtual void computeFastBounds(const
SkRect&,
> SkRect*);
> This seems to be const in other places. Can it be const here?

Done.

[Stephen White, 2014-01-27 21:56:42]

Uploaded a new patch set containing only the changes to the SkCanvas
AutoDrawLooper, as the remainder was landed as r13207.

[reed1, 2014-01-28 18:37:13]

lgtm

[Stephen White, 2014-01-28 18:58:00]

Thanks for your review.

One thing I wanted to point out: this code is a little ugly because it has to
convert a const-ref return value to a const-ptr (in order to allow NULL). If we
changed the signature of computeFastBounds and friends to return a const-ptr
instead, it would be cleaner. I'll upload a patch and you decide which one you
like.

[Stephen White, 2014-01-28 19:06:08]

On 2014/01/28 18:58:00, Stephen White wrote:
> Thanks for your review.
> 
> One thing I wanted to point out: this code is a little ugly because it has to
> convert a const-ref return value to a const-ptr (in order to allow NULL). If
we
> changed the signature of computeFastBounds and friends to return a const-ptr
> instead, it would be cleaner. I'll upload a patch and you decide which one you
> like.

Alternate version (w/const-ptr computeFastBounds) uploaded as patch set 4.

[Stephen White, 2014-01-28 19:06:29]

On 2014/01/28 19:06:08, Stephen White wrote:
> On 2014/01/28 18:58:00, Stephen White wrote:
> > Thanks for your review.
> > 
> > One thing I wanted to point out: this code is a little ugly because it has
to
> > convert a const-ref return value to a const-ptr (in order to allow NULL). If
> we
> > changed the signature of computeFastBounds and friends to return a const-ptr
> > instead, it would be cleaner. I'll upload a patch and you decide which one
you
> > like.
> 
> Alternate version (w/const-ptr computeFastBounds) uploaded as patch set 4.

.. and by 4, I mean 5.

[reed1, 2014-01-28 19:11:17]

I like the newer version too (w/ the * instead of &)

[Stephen White, 2014-01-28 19:17:52]

On 2014/01/28 19:11:17, reed1 wrote:
> I like the newer version too (w/ the * instead of &)

Argh, sorry for the tease. I didn't notice that computeFastBounds() is *public*,
and it's called by Blink, so patch set 5 would break compile on landing in
Chrome. :(

I think I'll land patch set 4 for now, and we can look at cleaning this up
later.

[Stephen White, 2014-01-28 19:22:40]

Committed patchset #6 manually as r13221 (presubmit successful).