Clip GPU blur against (inflated) clip bounds

Clip the size of the FBOs used for blurs to the bounds of the clipping region, inflated by the blur radius.  This is a little tricky, since we want the rect we use for downsampling to be an integer multiple of the scale factor, so we scale down the clip rect to downsampled space, inflate by the (scaled) kernel width, and intersect the rects there.

Also check the blur radius against 0 and abort early, to prevent the fuzzer blowing up on a missing uniform.

http://code.google.com/p/skia/issues/detail?id=328

[bsalomon, 2011-08-01 20:49:23]

http://codereview.appspot.com/4841043/diff/1/src/gpu/SkGpuDevice.cpp
File src/gpu/SkGpuDevice.cpp (right):

http://codereview.appspot.com/4841043/diff/1/src/gpu/SkGpuDevice.cpp#newcode848
src/gpu/SkGpuDevice.cpp:848: }
Won't this cause us to fall back to a SW draw? Maybe the calling code should
notice the 0 radius and call neither the SW nor GPU path.

It's probably a rare case but if some canvas app is setting the blur radius
dynamically we may see zero radius blurs.

[reed1, 2011-08-01 20:54:18]

I've recently updated the ceil and floor macros. In light of that, I've added
roundOut() to rect for you (already submitted) using the new versions.

[Stephen White, 2011-08-01 21:00:53]

http://codereview.appspot.com/4841043/diff/1/src/gpu/SkGpuDevice.cpp
File src/gpu/SkGpuDevice.cpp (right):

http://codereview.appspot.com/4841043/diff/1/src/gpu/SkGpuDevice.cpp#newcode848
src/gpu/SkGpuDevice.cpp:848: }
On 2011/08/01 20:49:23, bsalomon wrote:
> Won't this cause us to fall back to a SW draw? Maybe the calling code should
> notice the 0 radius and call neither the SW nor GPU path.

Yeah, it'll fall back to software draw, but software draw seems to bail on zero
radius also (see SkBlurMask.cpp:260).  

Note that this seems to happen only in the case where the matrix causes the
radius to go to zero (I think perhaps zero radius before matrix application is
handled upstairs?  not sure), where I'm guessing the matrix probably also causes
the primitive to be squished to nothingness.  Just a guess, though.

> It's probably a rare case but if some canvas app is setting the blur radius
> dynamically we may see zero radius blurs.

I think actual zero radius blurs are handled correctly; it's just the case of
where the matrix is squishing them to zero.    Unfortunately, we can't really
check at the call site without duplicating code, since the mask filter may not
even be a blur, so we can't just pass in the mapped radius.  Perhaps we need
some kind of "isNoOp()" on filters which takes a matrix to apply?

[bsalomon, 2011-08-01 21:03:53]

Ah I see... Yeah, the case of the radius being smashed to zero by the matrix
probably isn't worth spending any time to optimize :)

LGTM.

On Mon, Aug 1, 2011 at 5:00 PM, <senorblanco@chromium.org> wrote:

>
>
http://codereview.appspot.com/**4841043/diff/1/src/gpu/**SkGpuDevice.cpp<http...
> File src/gpu/SkGpuDevice.cpp (right):
>
> http://codereview.appspot.com/**4841043/diff/1/src/gpu/**
>
SkGpuDevice.cpp#newcode848<http://codereview.appspot.com/4841043/diff/1/src/gpu/SkGpuDevice.cpp#newcode848>
> src/gpu/SkGpuDevice.cpp:848: }
> On 2011/08/01 20:49:23, bsalomon wrote:
>
>> Won't this cause us to fall back to a SW draw? Maybe the calling code
>>
> should
>
>> notice the 0 radius and call neither the SW nor GPU path.
>>
>
> Yeah, it'll fall back to software draw, but software draw seems to bail
> on zero radius also (see SkBlurMask.cpp:260).
>
> Note that this seems to happen only in the case where the matrix causes
> the radius to go to zero (I think perhaps zero radius before matrix
> application is handled upstairs?  not sure), where I'm guessing the
> matrix probably also causes the primitive to be squished to nothingness.
>  Just a guess, though.
>
>
>  It's probably a rare case but if some canvas app is setting the blur
>>
> radius
>
>> dynamically we may see zero radius blurs.
>>
>
> I think actual zero radius blurs are handled correctly; it's just the
> case of where the matrix is squishing them to zero.    Unfortunately, we
> can't really check at the call site without duplicating code, since the
> mask filter may not even be a blur, so we can't just pass in the mapped
> radius.  Perhaps we need some kind of "isNoOp()" on filters which takes
> a matrix to apply?
>
>
>
http://codereview.appspot.com/**4841043/<http://codereview.appspot.com/4841043/>
>

[Stephen White, 2011-08-01 21:16:20]

Update to latest & remove SkRect changes

[reed1, 2011-08-01 21:19:32]

LGTM

[bsalomon, 2011-08-01 21:26:03]

Aye

On Mon, Aug 1, 2011 at 5:19 PM, <reed@google.com> wrote:

> LGTM
>
>
>
http://codereview.appspot.com/**4841043/<http://codereview.appspot.com/4841043/>
>

[Stephen White, 2011-08-31 14:01:03]

On 2011/08/01 21:26:03, bsalomon wrote:
> Aye
> 
> On Mon, Aug 1, 2011 at 5:19 PM, <mailto:reed@google.com> wrote:
> 
> > LGTM
> >
> >
> >
>
http://codereview.appspot.com/**4841043/%3Chttp://codereview.appspot.com/4841...>
> >

This was landed way back in the year r2020.  Closing.