GPU blur cleanup

Clean up some issues raised in code review:

- convolveRect() is too low-level; made it private and exposed convolveInX() and convolveInY() instead
- added GrAutoTextureEntry to automatically unlock a texture entry
- the clipping and bounder checks were returning false from drawWithGPUMaskFilter(), causing the software blur to kick in; return true instead
- the Windows build was giving a spurious warning about reading an uninitialized variable; rearrange the code to fix it

[reed1, 2011-07-12 18:58:58]

http://codereview.appspot.com/4710042/diff/1/gpu/include/GrContext.h
File gpu/include/GrContext.h (left):

http://codereview.appspot.com/4710042/diff/1/gpu/include/GrContext.h#oldcode665
gpu/include/GrContext.h:665: 
Should have a verb in the name, like

GrAutoUnlockTextureEntry

so we know it only unlocks, and doesn't do both (whatever that might mean in
this context)

Also, simple comment, like:

/**
 * Unlocks entry when this goes out of scope. entry may be null.
 */

[Stephen White, 2011-07-12 19:15:03]

Rename autounlock class; add comments

[Stephen White, 2011-07-12 19:15:22]

http://codereview.appspot.com/4710042/diff/1/gpu/include/GrContext.h
File gpu/include/GrContext.h (left):

http://codereview.appspot.com/4710042/diff/1/gpu/include/GrContext.h#oldcode665
gpu/include/GrContext.h:665: 
On 2011/07/12 18:58:58, reed1 wrote:
> Should have a verb in the name, like
> 
> GrAutoUnlockTextureEntry
> 
> so we know it only unlocks, and doesn't do both (whatever that might mean in
> this context)
> 
> Also, simple comment, like:
> 
> /**
>  * Unlocks entry when this goes out of scope. entry may be null.
>  */

Done.

[bsalomon, 2011-07-12 19:24:34]

> convolveRect() is too low-level; made it private and exposed > convolveInX()
and
> convolveInY() instead

Hey Stephen, I was thinking that the logic related to performing the blur
(managing offscreens, reduction loop, convolving, etc) should be encapsulated in
an object that uses the GrDrawTarget interface and is used by GrContext in
drawPath, drawRect, etc. Something like:

GrBlur blur(drawTarget, radius, type, ...);
blur.setup(); // sets up surface to catch draws
// do some drawing here (path, rect, whatever)
blur.finish(); // performs downsamples, convolves, etc, and writes result to
original RT

I know moving the code Ganesh was already a lot of work and I can take on this
task. I plan to do something similar with the OSAA code.


The changes LGTM.

[reed1, 2011-07-12 19:28:40]

LGTM

[Stephen White, 2011-07-12 19:42:50]

On 2011/07/12 19:24:34, bsalomon wrote:
> Hey Stephen, I was thinking that the logic related to performing the blur
> (managing offscreens, reduction loop, convolving, etc) should be encapsulated
in
> an object that uses the GrDrawTarget interface and is used by GrContext in
> drawPath, drawRect, etc. Something like:
> 
> GrBlur blur(drawTarget, radius, type, ...);
> blur.setup(); // sets up surface to catch draws
> // do some drawing here (path, rect, whatever)
> blur.finish(); // performs downsamples, convolves, etc, and writes result to
> original RT

That's a really good idea.  (Actually, the implementation in WebKit was somewhat
similar).

> I know moving the code Ganesh was already a lot of work and I can take on this
> task. I plan to do something similar with the OSAA code.

I'd like to take it on once I've got the remaining bits cleaned up, since I'll
be messing around with this code a fair bit and we'd otherwise be touching a lot
of the same code I think.

> The changes LGTM.

[Stephen White, 2011-07-12 19:51:36]

Landed as r1482.  Closing.

[bsalomon, 2011-07-12 20:09:14]

On Tue, Jul 12, 2011 at 3:42 PM, <senorblanco@chromium.org> wrote:

> On 2011/07/12 19:24:34, bsalomon wrote:
>
>> I know moving the code Ganesh was already a lot of work and I can take
>>
> on this
>
>> task. I plan to do something similar with the OSAA code.
>>
>
> I'd like to take it on once I've got the remaining bits cleaned up,
> since I'll be messing around with this code a fair bit and we'd
> otherwise be touching a lot of the same code I think.
>
>
Awesome! :)