Debug GL Interface

Debug GL Interface

Initial version (v. -2.0) of the Debug GL Interface

[bsalomon, 2012-03-16 19:11:13]

Overall this is looking nice. I expect it'll be really useful. Once you get
something checked in let's get it running on the bots!

http://codereview.appspot.com/5846049/diff/1/src/gpu/gl/GrGLCreateDebugInterf...
File src/gpu/gl/GrGLCreateDebugInterface.cpp (right):

http://codereview.appspot.com/5846049/diff/1/src/gpu/gl/GrGLCreateDebugInterf...
src/gpu/gl/GrGLCreateDebugInterface.cpp:3: * Copyright 2011 Google Inc.
2012

http://codereview.appspot.com/5846049/diff/1/src/gpu/gl/GrGLCreateDebugInterf...
src/gpu/gl/GrGLCreateDebugInterface.cpp:36: void unRef() {
just for consistency with SkRefCnt can we make this unref()? (all lowercase)

http://codereview.appspot.com/5846049/diff/1/src/gpu/gl/GrGLCreateDebugInterf...
src/gpu/gl/GrGLCreateDebugInterface.cpp:58: virtual void DeleteAction() = 0;
minor style nit: lowercase member fn name.

http://codereview.appspot.com/5846049/diff/1/src/gpu/gl/GrGLCreateDebugInterf...
src/gpu/gl/GrGLCreateDebugInterface.cpp:60: protected:
not required (but I don't object)

http://codereview.appspot.com/5846049/diff/1/src/gpu/gl/GrGLCreateDebugInterf...
src/gpu/gl/GrGLCreateDebugInterface.cpp:83: delete [] fDataPtr;
minor style nit: delete[]

http://codereview.appspot.com/5846049/diff/1/src/gpu/gl/GrGLCreateDebugInterf...
src/gpu/gl/GrGLCreateDebugInterface.cpp:287: bool getInBeginEndBlock() const {
return fInBeginEndBlock; }
Not really necessary. ES doesn't support immediate mode drawing so I doubt we'll
ever use it.

http://codereview.appspot.com/5846049/diff/1/src/gpu/gl/GrGLCreateDebugInterf...
src/gpu/gl/GrGLCreateDebugInterface.cpp:317: static int fNextID;                
    // source for globally unique IDs
I like that IDs are unique across all objects and don't get recycled. It could
catch bugs that might accidentally work on a real GL.

http://codereview.appspot.com/5846049/diff/1/src/gpu/gl/GrGLCreateDebugInterf...
src/gpu/gl/GrGLCreateDebugInterface.cpp:737: case GR_GL_STENCIL_BITS:
Not necessary now but at some point we might want to get rid of some of these
that we no longer call. Gets are incredibly expensive in chrome because it
synchronizes the gpu process. So we've removed many gets.

[robertphillips, 2012-03-16 19:29:37]

http://codereview.appspot.com/5846049/diff/1/src/gpu/gl/GrGLCreateDebugInterf...
File src/gpu/gl/GrGLCreateDebugInterface.cpp (right):

http://codereview.appspot.com/5846049/diff/1/src/gpu/gl/GrGLCreateDebugInterf...
src/gpu/gl/GrGLCreateDebugInterface.cpp:3: * Copyright 2011 Google Inc.
On 2012/03/16 19:11:13, bsalomon wrote:
> 2012

Done.

http://codereview.appspot.com/5846049/diff/1/src/gpu/gl/GrGLCreateDebugInterf...
src/gpu/gl/GrGLCreateDebugInterface.cpp:36: void unRef() {
On 2012/03/16 19:11:13, bsalomon wrote:
> just for consistency with SkRefCnt can we make this unref()? (all lowercase)

Done.

http://codereview.appspot.com/5846049/diff/1/src/gpu/gl/GrGLCreateDebugInterf...
src/gpu/gl/GrGLCreateDebugInterface.cpp:58: virtual void DeleteAction() = 0;
On 2012/03/16 19:11:13, bsalomon wrote:
> minor style nit: lowercase member fn name.

Done.

http://codereview.appspot.com/5846049/diff/1/src/gpu/gl/GrGLCreateDebugInterf...
src/gpu/gl/GrGLCreateDebugInterface.cpp:60: protected:
On 2012/03/16 19:11:13, bsalomon wrote:
> not required (but I don't object)

Done.

http://codereview.appspot.com/5846049/diff/1/src/gpu/gl/GrGLCreateDebugInterf...
src/gpu/gl/GrGLCreateDebugInterface.cpp:83: delete [] fDataPtr;
On 2012/03/16 19:11:13, bsalomon wrote:
> minor style nit: delete[]

Done.

http://codereview.appspot.com/5846049/diff/1/src/gpu/gl/GrGLCreateDebugInterf...
src/gpu/gl/GrGLCreateDebugInterface.cpp:287: bool getInBeginEndBlock() const {
return fInBeginEndBlock; }
On 2012/03/16 19:11:13, bsalomon wrote:
> Not really necessary. ES doesn't support immediate mode drawing so I doubt
we'll
> ever use it.

Done.

http://codereview.appspot.com/5846049/diff/1/src/gpu/gl/GrGLCreateDebugInterf...
src/gpu/gl/GrGLCreateDebugInterface.cpp:317: static int fNextID;                
    // source for globally unique IDs
On 2012/03/16 19:11:13, bsalomon wrote:
> I like that IDs are unique across all objects and don't get recycled. It could
> catch bugs that might accidentally work on a real GL.

Done.

http://codereview.appspot.com/5846049/diff/1/src/gpu/gl/GrGLCreateDebugInterf...
src/gpu/gl/GrGLCreateDebugInterface.cpp:737: case GR_GL_STENCIL_BITS:
On 2012/03/16 19:11:13, bsalomon wrote:
> Not necessary now but at some point we might want to get rid of some of these
> that we no longer call. Gets are incredibly expensive in chrome because it
> synchronizes the gpu process. So we've removed many gets.

Done.

[robertphillips, 2012-03-19 12:33:48]

fixed tab in .gyp & now calling GrAlwaysAssert

[bsalomon, 2012-03-19 12:59:26]

On 2012/03/19 12:33:48, robertphillips wrote:
> fixed tab in .gyp & now calling GrAlwaysAssert

Great, LGTM. Should we make gm run through this as well?

[robertphillips, 2012-03-19 13:48:07]

On 2012/03/19 12:59:26, bsalomon wrote:
> On 2012/03/19 12:33:48, robertphillips wrote:
> > fixed tab in .gyp & now calling GrAlwaysAssert
> 
> Great, LGTM. Should we make gm run through this as well?

Hi Brian,
  I think it already does (at least, it certainly already runs in DEBUG). I will
check to make sure it is running for release gm.
 
   Rob

[robertphillips, 2012-03-19 14:44:44]

committed as r3426