Add GrSurface parent class above GrTexture & GrRenderTarget

This change improves the generality of Skia's GrTexture & GrRenderTarget handling (through the magic of polymorphism). Follow on changes will fuse SkGrTexturePixelRef & SkGrRenderTargetPixelRef and propagate use of the new GrSurface class through out the code.

[robertphillips, 2012-06-15 20:53:19]

http://codereview.appspot.com/6297092/diff/1/include/gpu/GrSurface.h
File include/gpu/GrSurface.h (right):

http://codereview.appspot.com/6297092/diff/1/include/gpu/GrSurface.h#newcode43
include/gpu/GrSurface.h:43: 
I'm on the fence about moving "isMultisampled" & "numSamples" here or leaving
them in the render target. Thoughts?

http://codereview.appspot.com/6297092/diff/1/include/gpu/GrSurface.h#newcode62
include/gpu/GrSurface.h:62: virtual GrTexture* asTexture() = 0;
The const version of "asTexture" is all new. I added it for parallelism w/ the
"asRenderTarget" calls.

http://codereview.appspot.com/6297092/diff/1/src/gpu/gl/GrGLRenderTarget.cpp
File src/gpu/gl/GrGLRenderTarget.cpp (right):

http://codereview.appspot.com/6297092/diff/1/src/gpu/gl/GrGLRenderTarget.cpp#...
src/gpu/gl/GrGLRenderTarget.cpp:34: texture,
This is a bit ugly but propagating the GrTextureDesc back up the call stack (and
removing the viewport parameter) becomes problematic very quickly. One of the
issues is that the platform descriptor doesn't derive from GrTextureDesc so
entry points using the platform descriptor would need to perform some gymnastics
to pass down a GrTextureDesc.

[bsalomon, 2012-06-15 21:06:53]

http://codereview.appspot.com/6297092/diff/1/include/gpu/GrSurface.h
File include/gpu/GrSurface.h (right):

http://codereview.appspot.com/6297092/diff/1/include/gpu/GrSurface.h#newcode43
include/gpu/GrSurface.h:43: 
On 2012/06/15 20:53:19, robertphillips wrote:
> I'm on the fence about moving "isMultisampled" & "numSamples" here or leaving
> them in the render target. Thoughts?

I would say leave them in RT, at least for now. The texture is never really
multisampled. If the RT is multisampled it is downsampled into the texture.

http://codereview.appspot.com/6297092/diff/1/include/gpu/GrSurface.h#newcode62
include/gpu/GrSurface.h:62: virtual GrTexture* asTexture() = 0;
On 2012/06/15 20:53:19, robertphillips wrote:
> The const version of "asTexture" is all new. I added it for parallelism w/ the
> "asRenderTarget" calls.
SGTM

http://codereview.appspot.com/6297092/diff/1/include/gpu/GrTypes.h
File include/gpu/GrTypes.h (right):

http://codereview.appspot.com/6297092/diff/1/include/gpu/GrTypes.h#newcode480
include/gpu/GrTypes.h:480: GrTextureDesc(GrTextureFlags flags,
This makes me a bit wary simply because changing the signature to add a field
may be tricky if WK uses it.

http://codereview.appspot.com/6297092/diff/1/src/gpu/gl/GrGLRenderTarget.cpp
File src/gpu/gl/GrGLRenderTarget.cpp (right):

http://codereview.appspot.com/6297092/diff/1/src/gpu/gl/GrGLRenderTarget.cpp#...
src/gpu/gl/GrGLRenderTarget.cpp:34: texture,
On 2012/06/15 20:53:19, robertphillips wrote:
> This is a bit ugly but propagating the GrTextureDesc back up the call stack
(and
> removing the viewport parameter) becomes problematic very quickly. One of the
> issues is that the platform descriptor doesn't derive from GrTextureDesc so
> entry points using the platform descriptor would need to perform some
gymnastics
> to pass down a GrTextureDesc.

I see now why you added the cons to GrTextureDesc. Why don't we make a local
function here that returns a GrTextureDesc (even if it has to return by value).

For this cons can we just pass texture's desc (if there's a getter)?

[robertphillips, 2012-06-18 12:31:01]

Removed exhaustive ctor in GrTextureDesc & moved MSAA methods back to
GrRenderTarget (from GrSurface).

http://codereview.appspot.com/6297092/diff/1/include/gpu/GrSurface.h
File include/gpu/GrSurface.h (right):

http://codereview.appspot.com/6297092/diff/1/include/gpu/GrSurface.h#newcode43
include/gpu/GrSurface.h:43: 
On 2012/06/15 21:06:53, bsalomon wrote:
> On 2012/06/15 20:53:19, robertphillips wrote:
> > I'm on the fence about moving "isMultisampled" & "numSamples" here or
leaving
> > them in the render target. Thoughts?
> 
> I would say leave them in RT, at least for now. The texture is never really
> multisampled. If the RT is multisampled it is downsampled into the texture.
> 

Done.

http://codereview.appspot.com/6297092/diff/1/include/gpu/GrTypes.h
File include/gpu/GrTypes.h (right):

http://codereview.appspot.com/6297092/diff/1/include/gpu/GrTypes.h#newcode480
include/gpu/GrTypes.h:480: GrTextureDesc(GrTextureFlags flags,
On 2012/06/15 21:06:53, bsalomon wrote:
> This makes me a bit wary simply because changing the signature to add a field
> may be tricky if WK uses it.

Done.

http://codereview.appspot.com/6297092/diff/1/src/gpu/gl/GrGLRenderTarget.cpp
File src/gpu/gl/GrGLRenderTarget.cpp (right):

http://codereview.appspot.com/6297092/diff/1/src/gpu/gl/GrGLRenderTarget.cpp#...
src/gpu/gl/GrGLRenderTarget.cpp:34: texture,
Done. There is a getter but it isn't that simple. I would like to get to the
point where we don't need the "desc" and "viewport" parameters and just copy the
"texture"'s desc into the render target but some work needs to be done upstream
before that can happen.

[bsalomon, 2012-06-18 13:02:32]

LGTM

http://codereview.appspot.com/6297092/diff/1/src/gpu/gl/GrGLRenderTarget.cpp
File src/gpu/gl/GrGLRenderTarget.cpp (right):

http://codereview.appspot.com/6297092/diff/1/src/gpu/gl/GrGLRenderTarget.cpp#...
src/gpu/gl/GrGLRenderTarget.cpp:34: texture,
On 2012/06/18 12:31:01, robertphillips wrote:
> Done. There is a getter but it isn't that simple. I would like to get to the
> point where we don't need the "desc" and "viewport" parameters and just copy
the
> "texture"'s desc into the render target but some work needs to be done
upstream
> before that can happen.

For texture atlasing we may want both texture and RT to have a subrect/viewport
in the desc.

[robertphillips, 2012-06-21 21:09:34]

committed as r4292