Add createPlatformSurface to wrap external rendertarget-textures with MSAA support

[reed1, 2011-04-11 15:12:09]

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

http://codereview.appspot.com/4388049/diff/2001/gpu/include/GrTexture.h#newco...
gpu/include/GrTexture.h:72: 
is Resolve clear enough, or should we use MSAAResolve or MultiSampleResolve for
these apis?

http://codereview.appspot.com/4388049/diff/2001/gpu/include/GrTypes.h
File gpu/include/GrTypes.h (left):

http://codereview.appspot.com/4388049/diff/2001/gpu/include/GrTypes.h#oldcode156
gpu/include/GrTypes.h:156: 
Can this be moved into a private .cpp or header for now?

If not, perhaps we give it a clearer name (only because it took me a while to
grok it)? GrAutoTPtrValueRestore?

Can we use fPtr NULL or not instead of a separate fSaved field?

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

http://codereview.appspot.com/4388049/diff/2001/gpu/include/GrTypes.h#newcode440
gpu/include/GrTypes.h:440: enum GrPlatformRenderTargetFlags {
What happened to 0x02 and 0x04

http://codereview.appspot.com/4388049/diff/2001/gpu/include/GrTypes.h#newcode469
gpu/include/GrTypes.h:469: // Description of platform surface to create.
Can we add more space between the types and the fields :)

http://codereview.appspot.com/4388049/diff/2001/gpu/include/GrTypes.h#newcode508
gpu/include/GrTypes.h:508: 
Do we have example versions of this filled out for gl or dx? That might be
something useful in the initial block-comment.

http://codereview.appspot.com/4388049/diff/2001/src/gpu/SkGpuDevice.cpp
File src/gpu/SkGpuDevice.cpp (right):

http://codereview.appspot.com/4388049/diff/2001/src/gpu/SkGpuDevice.cpp#newco...
src/gpu/SkGpuDevice.cpp:1476: GrSafeUnref(fRootTexture);
Why do we call the "safe" version here?

[bsalomon, 2011-04-11 17:27:10]

On 2011/04/11 15:12:09, reed1 wrote:
> http://codereview.appspot.com/4388049/diff/2001/gpu/include/GrTexture.h
> File gpu/include/GrTexture.h (right):
> 
>
http://codereview.appspot.com/4388049/diff/2001/gpu/include/GrTexture.h#newco...
> gpu/include/GrTexture.h:72: 
> is Resolve clear enough, or should we use MSAAResolve or MultiSampleResolve
for
> these apis?

While not perfect, I think Resolve is clear enough... and we may use the same
API if we did our own super-sampling.

> 
> http://codereview.appspot.com/4388049/diff/2001/gpu/include/GrTypes.h
> File gpu/include/GrTypes.h (left):
> 
>
http://codereview.appspot.com/4388049/diff/2001/gpu/include/GrTypes.h#oldcode156
> gpu/include/GrTypes.h:156: 
> Can this be moved into a private .cpp or header for now?
> 
> If not, perhaps we give it a clearer name (only because it took me a while to
> grok it)? GrAutoTPtrValueRestore?
> 
> Can we use fPtr NULL or not instead of a separate fSaved field?

I kept it public but renamed it, removed fSaved, and added an example in the
comments

> 
> http://codereview.appspot.com/4388049/diff/2001/gpu/include/GrTypes.h
> File gpu/include/GrTypes.h (right):
> 
>
http://codereview.appspot.com/4388049/diff/2001/gpu/include/GrTypes.h#newcode440
> gpu/include/GrTypes.h:440: enum GrPlatformRenderTargetFlags {
> What happened to 0x02 and 0x04

Oops, fixed.


> 
>
http://codereview.appspot.com/4388049/diff/2001/gpu/include/GrTypes.h#newcode469
> gpu/include/GrTypes.h:469: // Description of platform surface to create.
> Can we add more space between the types and the fields :)

Done.

> 
>
http://codereview.appspot.com/4388049/diff/2001/gpu/include/GrTypes.h#newcode508
> gpu/include/GrTypes.h:508: 
> Do we have example versions of this filled out for gl or dx? That might be
> something useful in the initial block-comment.

I added a GL example. I put it afterwards just because it was pretty long. 


> http://codereview.appspot.com/4388049/diff/2001/src/gpu/SkGpuDevice.cpp
> File src/gpu/SkGpuDevice.cpp (right):
> 
>
http://codereview.appspot.com/4388049/diff/2001/src/gpu/SkGpuDevice.cpp#newco...
> src/gpu/SkGpuDevice.cpp:1476: GrSafeUnref(fRootTexture);
> Why do we call the "safe" version here?

The cons that takes GrRenderTarget* sets fRootTexture to NULL.

[bsalomon, 2011-04-12 15:43:26]

closed with r1102