Edge antialiasing for convex shapes in Ganesh

This patch implements edge antialiasing for convex shapes, using the fragment shader to compare against the edge equations for each triangle.  Currently, it only works for flat shaded primitives (i.e., it was not integrated into the "active stages" path).  The skia.gyp changes cause this code to be compiled into SampleApp, but do not enable the tesselated path by default.

Notes:  the SkOSWindow_Unix.cpp change is to silence a valgrind warning about memcpy() with overlapping regions.  The GrBinHashKey change is to avoid running a two-pass hash (GrProgramDesc is now 52 bytes or so, exceeding the 32 byte default size).

[Stephen White, 2011-05-12 14:22:39]

Remove the code that forces blending on

[junov1, 2011-05-12 14:46:58]

Awesome change!

http://codereview.appspot.com/4519054/diff/10001/gpu/src/GrGLProgram.cpp
File gpu/src/GrGLProgram.cpp (right):

http://codereview.appspot.com/4519054/diff/10001/gpu/src/GrGLProgram.cpp#newc...
gpu/src/GrGLProgram.cpp:611: GrAssert(-1 !=
programData->fUniLocations.fEdgesUni);
-1 => GrGLProgram::kUnusedUniform

http://codereview.appspot.com/4519054/diff/10001/gpu/src/GrGLProgram.cpp#newc...
gpu/src/GrGLProgram.cpp:613: programData->fUniLocations.fEdgesUni = -1;
-1 => GrGLProgram::kUnusedUniform

http://codereview.appspot.com/4519054/diff/10001/gpu/src/GrGpuGLShaders.cpp
File gpu/src/GrGpuGLShaders.cpp (right):

http://codereview.appspot.com/4519054/diff/10001/gpu/src/GrGpuGLShaders.cpp#n...
gpu/src/GrGpuGLShaders.cpp:401: if (-1 != uni) {
-1 => GrGLProgram::kUnusedUniform

[bsalomon, 2011-05-12 14:54:14]

Cool!

On 2011/05/12 14:22:39, Stephen White wrote:
> Remove the code that forces blending on
canDisableBlend() can return false when your new state bit is set. Leon had to
do the same thing for color filters.

Do you you it would be helpful to change the path renderer to track the current
path?

I was thinking we might be able to save some work in a sequence like this:

renderer->setPath(p)

if (renderer->canDrawPath(...) && renderer->supportsAA(...)) {
   renderer->drawPath(...)
}
renderer->setPath(NULL)

http://codereview.appspot.com/4519054/diff/10001/gpu/src/GrGLProgram.h
File gpu/src/GrGLProgram.h (right):

http://codereview.appspot.com/4519054/diff/10001/gpu/src/GrGLProgram.h#newcod...
gpu/src/GrGLProgram.h:171: fViewMatrixUni = kUnusedUniform;
We should init fEdgesUni to kUnusedUniform here

[Stephen White, 2011-05-12 15:25:47]

Incorporate reviewer comments

[junov1, 2011-05-12 15:28:04]

LGTM

[Stephen White, 2011-05-12 15:29:22]

Init the edges uniform as requested

[Stephen White, 2011-05-12 15:32:59]

http://codereview.appspot.com/4519054/diff/10001/gpu/src/GrGLProgram.cpp
File gpu/src/GrGLProgram.cpp (right):

http://codereview.appspot.com/4519054/diff/10001/gpu/src/GrGLProgram.cpp#newc...
gpu/src/GrGLProgram.cpp:611: GrAssert(-1 !=
programData->fUniLocations.fEdgesUni);
On 2011/05/12 14:46:58, junov1 wrote:
> -1 => GrGLProgram::kUnusedUniform

Done.

http://codereview.appspot.com/4519054/diff/10001/gpu/src/GrGLProgram.cpp#newc...
gpu/src/GrGLProgram.cpp:613: programData->fUniLocations.fEdgesUni = -1;
On 2011/05/12 14:46:58, junov1 wrote:
> -1 => GrGLProgram::kUnusedUniform

Done.

http://codereview.appspot.com/4519054/diff/10001/gpu/src/GrGLProgram.h
File gpu/src/GrGLProgram.h (right):

http://codereview.appspot.com/4519054/diff/10001/gpu/src/GrGLProgram.h#newcod...
gpu/src/GrGLProgram.h:171: fViewMatrixUni = kUnusedUniform;
On 2011/05/12 14:54:14, bsalomon wrote:
> We should init fEdgesUni to kUnusedUniform here

Done.

http://codereview.appspot.com/4519054/diff/10001/gpu/src/GrGpuGLShaders.cpp
File gpu/src/GrGpuGLShaders.cpp (right):

http://codereview.appspot.com/4519054/diff/10001/gpu/src/GrGpuGLShaders.cpp#n...
gpu/src/GrGpuGLShaders.cpp:401: if (-1 != uni) {
On 2011/05/12 14:46:58, junov1 wrote:
> -1 => GrGLProgram::kUnusedUniform

Done.

[bsalomon, 2011-05-12 15:38:05]

LGTM

On 2011/05/12 15:32:59, Stephen White wrote:
> http://codereview.appspot.com/4519054/diff/10001/gpu/src/GrGLProgram.cpp
> File gpu/src/GrGLProgram.cpp (right):
> 
>
http://codereview.appspot.com/4519054/diff/10001/gpu/src/GrGLProgram.cpp#newc...
> gpu/src/GrGLProgram.cpp:611: GrAssert(-1 !=
> programData->fUniLocations.fEdgesUni);
> On 2011/05/12 14:46:58, junov1 wrote:
> > -1 => GrGLProgram::kUnusedUniform
> 
> Done.
> 
>
http://codereview.appspot.com/4519054/diff/10001/gpu/src/GrGLProgram.cpp#newc...
> gpu/src/GrGLProgram.cpp:613: programData->fUniLocations.fEdgesUni = -1;
> On 2011/05/12 14:46:58, junov1 wrote:
> > -1 => GrGLProgram::kUnusedUniform
> 
> Done.
> 
> http://codereview.appspot.com/4519054/diff/10001/gpu/src/GrGLProgram.h
> File gpu/src/GrGLProgram.h (right):
> 
>
http://codereview.appspot.com/4519054/diff/10001/gpu/src/GrGLProgram.h#newcod...
> gpu/src/GrGLProgram.h:171: fViewMatrixUni = kUnusedUniform;
> On 2011/05/12 14:54:14, bsalomon wrote:
> > We should init fEdgesUni to kUnusedUniform here
> 
> Done.
> 
> http://codereview.appspot.com/4519054/diff/10001/gpu/src/GrGpuGLShaders.cpp
> File gpu/src/GrGpuGLShaders.cpp (right):
> 
>
http://codereview.appspot.com/4519054/diff/10001/gpu/src/GrGpuGLShaders.cpp#n...
> gpu/src/GrGpuGLShaders.cpp:401: if (-1 != uni) {
> On 2011/05/12 14:46:58, junov1 wrote:
> > -1 => GrGLProgram::kUnusedUniform
> 
> Done.

[Stephen White, 2011-05-12 15:39:46]

On 2011/05/12 14:54:14, bsalomon wrote:
> Cool!
> 
> On 2011/05/12 14:22:39, Stephen White wrote:
> > Remove the code that forces blending on
> canDisableBlend() can return false when your new state bit is set. Leon had to
> do the same thing for color filters.

Done.  I wasn't sure how color filters and AA interact, but I put my check first
for now.

> Do you you it would be helpful to change the path renderer to track the
current
> path?
> 
> I was thinking we might be able to save some work in a sequence like this:
> 
> renderer->setPath(p)
> 
> if (renderer->canDrawPath(...) && renderer->supportsAA(...)) {
>    renderer->drawPath(...)
> }
> renderer->setPath(NULL)

Well, I'm planning to implement the convex case next, so hopefully soon
supportsAA() will just return true.  But if it shows up as hot in profiling
before I get that done, we could definitely look at that.

[bsalomon, 2011-05-12 15:49:13]

On Thu, May 12, 2011 at 11:39 AM, <senorblanco@chromium.org> wrote:

> On 2011/05/12 14:54:14, bsalomon wrote:
>
>> On 2011/05/12 14:22:39, Stephen White wrote:
>
> > Remove the code that forces blending on
>> canDisableBlend() can return false when your new state bit is set.
>>
> Leon had to
>
>> do the same thing for color filters.
>>
>
> Done.  I wasn't sure how color filters and AA interact, but I put my
> check first for now.
>
> That's ok. Disabling the blend is an optimization. We can always make that
function more aggressive later.

[Stephen White, 2011-05-12 17:03:36]

Landed as http://code.google.com/p/skia/source/detail?r=1314; closing.