Move texture descriptor into GrTexture

The two big questions here are:

  add copy operator to copy from platform -> GL (for onCreatePlatformTexture) and from normal -> GL (for onWriteTexturePixels & onCreateTexture)?

  proactively add conversion from platform -> normal flags?

[bsalomon, 2012-06-01 16:19:25]

Mostly LGTM.

I don't have a strong opinion about operator = between the GL, platform, and
regular descriptors. If we add operator =s then let's keep their decls/defs in
private .h & .cpp files, though.

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

http://codereview.appspot.com/6258068/diff/1/include/gpu/GrTexture.h#newcode127
include/gpu/GrTexture.h:127: validateDesc();
this->

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

http://codereview.appspot.com/6258068/diff/1/include/gpu/GrTypes.h#newcode597
include/gpu/GrTypes.h:597: * Note: These flags currently form a subset of
GrTexture's flags and
Maybe we should just make it so:

enum GrPlatformTextureFlags {
    /**
     * No flags enabled
     */
    kNone_GrPlatformTextureFlag         = kNone_GrTextureFlags,
    /**
     * Indicates that the texture is also a render target, and thus should have
     * a GrRenderTarget object.
     *
     * D3D (future): client must have created the texture with flags that allow
     * it to be used as a render target.
     */
    kRenderTarget_GrPlatformTextureFlag = kRenderTarget_GrTextureFlagBit,
};

(I'd like to remove the word "Bit" from the TextureFlag values but WebKit, blah,
blah).

I'd prefer not to talk about the impl details here, the comments here are really
meant to be read by a Ganesh client writer rather than a Skia developer. Perhaps
we just state that these are a subset of the GrTextureFlags.

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

http://codereview.appspot.com/6258068/diff/1/src/gpu/GrTexture.cpp#newcode77
src/gpu/GrTexture.cpp:77: GrAssert(0 == fDesc.fFlags &
kNoStencil_GrTextureFlagBit);
Don't we have to zero this and the sample count out in releaseRenderTarget()?

http://codereview.appspot.com/6258068/diff/1/src/gpu/gl/GrGLTexture.h
File src/gpu/gl/GrGLTexture.h (right):

http://codereview.appspot.com/6258068/diff/1/src/gpu/gl/GrGLTexture.h#newcode61
src/gpu/gl/GrGLTexture.h:61: struct GrGLTextureDesc : public GrTextureDesc {
Can we keep the name Desc (or maybe GLDesc)? It's already in a class called
GrGLTexture. GrGLTexture::GrGLTextureDesc feels unnecessarily verbose.

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

http://codereview.appspot.com/6258068/diff/1/src/gpu/gl/GrGpuGL.cpp#newcode1052
src/gpu/gl/GrGpuGL.cpp:1052: glTexDesc.fFlags  = desc.fFlags;
glTexDesc = desc? or not allowed because no operator =?

[robertphillips, 2012-06-01 17:10:15]

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

http://codereview.appspot.com/6258068/diff/1/include/gpu/GrTexture.h#newcode127
include/gpu/GrTexture.h:127: validateDesc();
On 2012/06/01 16:19:25, bsalomon wrote:
> this->

Done.

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

http://codereview.appspot.com/6258068/diff/1/include/gpu/GrTypes.h#newcode597
include/gpu/GrTypes.h:597: * Note: These flags currently form a subset of
GrTexture's flags and
On 2012/06/01 16:19:25, bsalomon wrote:
> Maybe we should just make it so:
> 
> enum GrPlatformTextureFlags {
>     /**
>      * No flags enabled
>      */
>     kNone_GrPlatformTextureFlag         = kNone_GrTextureFlags,
>     /**
>      * Indicates that the texture is also a render target, and thus should
have
>      * a GrRenderTarget object.
>      *
>      * D3D (future): client must have created the texture with flags that
allow
>      * it to be used as a render target.
>      */
>     kRenderTarget_GrPlatformTextureFlag = kRenderTarget_GrTextureFlagBit,
> };
> 
> (I'd like to remove the word "Bit" from the TextureFlag values but WebKit,
blah,
> blah).
> 
> I'd prefer not to talk about the impl details here, the comments here are
really
> meant to be read by a Ganesh client writer rather than a Skia developer.
Perhaps
> we just state that these are a subset of the GrTextureFlags.

Done.

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

http://codereview.appspot.com/6258068/diff/1/src/gpu/GrTexture.cpp#newcode77
src/gpu/GrTexture.cpp:77: GrAssert(0 == fDesc.fFlags &
kNoStencil_GrTextureFlagBit);
On 2012/06/01 16:19:25, bsalomon wrote:
> Don't we have to zero this and the sample count out in releaseRenderTarget()?

Done.

http://codereview.appspot.com/6258068/diff/1/src/gpu/gl/GrGLTexture.h
File src/gpu/gl/GrGLTexture.h (right):

http://codereview.appspot.com/6258068/diff/1/src/gpu/gl/GrGLTexture.h#newcode61
src/gpu/gl/GrGLTexture.h:61: struct GrGLTextureDesc : public GrTextureDesc {
On 2012/06/01 16:19:25, bsalomon wrote:
> Can we keep the name Desc (or maybe GLDesc)? It's already in a class called
> GrGLTexture. GrGLTexture::GrGLTextureDesc feels unnecessarily verbose.

Done.

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

http://codereview.appspot.com/6258068/diff/1/src/gpu/gl/GrGpuGL.cpp#newcode1052
src/gpu/gl/GrGpuGL.cpp:1052: glTexDesc.fFlags  = desc.fFlags;
Not allowed unless we add a cross object operator =.

[bsalomon, 2012-06-01 17:43:07]

On 2012/06/01 17:10:15, robertphillips wrote:
> http://codereview.appspot.com/6258068/diff/1/include/gpu/GrTexture.h
> File include/gpu/GrTexture.h (right):
> 
>
http://codereview.appspot.com/6258068/diff/1/include/gpu/GrTexture.h#newcode127
> include/gpu/GrTexture.h:127: validateDesc();
> On 2012/06/01 16:19:25, bsalomon wrote:
> > this->
> 
> Done.
> 
> http://codereview.appspot.com/6258068/diff/1/include/gpu/GrTypes.h
> File include/gpu/GrTypes.h (right):
> 
> http://codereview.appspot.com/6258068/diff/1/include/gpu/GrTypes.h#newcode597
> include/gpu/GrTypes.h:597: * Note: These flags currently form a subset of
> GrTexture's flags and
> On 2012/06/01 16:19:25, bsalomon wrote:
> > Maybe we should just make it so:
> > 
> > enum GrPlatformTextureFlags {
> >     /**
> >      * No flags enabled
> >      */
> >     kNone_GrPlatformTextureFlag         = kNone_GrTextureFlags,
> >     /**
> >      * Indicates that the texture is also a render target, and thus should
> have
> >      * a GrRenderTarget object.
> >      *
> >      * D3D (future): client must have created the texture with flags that
> allow
> >      * it to be used as a render target.
> >      */
> >     kRenderTarget_GrPlatformTextureFlag = kRenderTarget_GrTextureFlagBit,
> > };
> > 
> > (I'd like to remove the word "Bit" from the TextureFlag values but WebKit,
> blah,
> > blah).
> > 
> > I'd prefer not to talk about the impl details here, the comments here are
> really
> > meant to be read by a Ganesh client writer rather than a Skia developer.
> Perhaps
> > we just state that these are a subset of the GrTextureFlags.
> 
> Done.
> 
> http://codereview.appspot.com/6258068/diff/1/src/gpu/GrTexture.cpp
> File src/gpu/GrTexture.cpp (right):
> 
> http://codereview.appspot.com/6258068/diff/1/src/gpu/GrTexture.cpp#newcode77
> src/gpu/GrTexture.cpp:77: GrAssert(0 == fDesc.fFlags &
> kNoStencil_GrTextureFlagBit);
> On 2012/06/01 16:19:25, bsalomon wrote:
> > Don't we have to zero this and the sample count out in
releaseRenderTarget()?
> 
> Done.
> 
> http://codereview.appspot.com/6258068/diff/1/src/gpu/gl/GrGLTexture.h
> File src/gpu/gl/GrGLTexture.h (right):
> 
>
http://codereview.appspot.com/6258068/diff/1/src/gpu/gl/GrGLTexture.h#newcode61
> src/gpu/gl/GrGLTexture.h:61: struct GrGLTextureDesc : public GrTextureDesc {
> On 2012/06/01 16:19:25, bsalomon wrote:
> > Can we keep the name Desc (or maybe GLDesc)? It's already in a class called
> > GrGLTexture. GrGLTexture::GrGLTextureDesc feels unnecessarily verbose.
> 
> Done.
> 
> http://codereview.appspot.com/6258068/diff/1/src/gpu/gl/GrGpuGL.cpp
> File src/gpu/gl/GrGpuGL.cpp (right):
> 
>
http://codereview.appspot.com/6258068/diff/1/src/gpu/gl/GrGpuGL.cpp#newcode1052
> src/gpu/gl/GrGpuGL.cpp:1052: glTexDesc.fFlags  = desc.fFlags;
> Not allowed unless we add a cross object operator =.

LGTM

[robertphillips, 2012-06-04 12:49:26]

committed as r4133