Fleshed out GPU portion of image/surface feature

This works in SampleApp but there is still some funkiness in GM proper. In GM the context doesn't exist when the raster backend is in use. For this reason the raster GM images don't contain the GPU column.

[reed1, 2012-10-30 19:55:59]

https://codereview.appspot.com/6813055/diff/1/include/core/SkImage.h
File include/core/SkImage.h (right):

https://codereview.appspot.com/6813055/diff/1/include/core/SkImage.h#newcode18
include/core/SkImage.h:18: class GrContext;
Do we need this guy now?

https://codereview.appspot.com/6813055/diff/1/src/image/SkImage_Gpu.cpp
File src/image/SkImage_Gpu.cpp (right):

https://codereview.appspot.com/6813055/diff/1/src/image/SkImage_Gpu.cpp#newco...
src/image/SkImage_Gpu.cpp:58: void SkImage_Gpu::setTexture(GrTexture* texture) {
perhaps check that the two textures are diff?

https://codereview.appspot.com/6813055/diff/1/src/image/SkSurface_Picture.cpp
File src/image/SkSurface_Picture.cpp (right):

https://codereview.appspot.com/6813055/diff/1/src/image/SkSurface_Picture.cpp...
src/image/SkSurface_Picture.cpp:80: 
// We always already spawn a copy of the recording picture when we are asked for
a snapshot, so we never need to do anything here.

[robertphillips, 2012-10-30 20:03:38]

https://codereview.appspot.com/6813055/diff/1/include/core/SkImage.h
File include/core/SkImage.h (right):

https://codereview.appspot.com/6813055/diff/1/include/core/SkImage.h#newcode18
include/core/SkImage.h:18: class GrContext;
On 2012/10/30 19:55:59, reed1 wrote:
> Do we need this guy now?

Done.

https://codereview.appspot.com/6813055/diff/1/src/image/SkImage_Gpu.cpp
File src/image/SkImage_Gpu.cpp (right):

https://codereview.appspot.com/6813055/diff/1/src/image/SkImage_Gpu.cpp#newco...
src/image/SkImage_Gpu.cpp:58: void SkImage_Gpu::setTexture(GrTexture* texture) {
On 2012/10/30 19:55:59, reed1 wrote:
> perhaps check that the two textures are diff?

Done.

https://codereview.appspot.com/6813055/diff/1/src/image/SkSurface_Picture.cpp
File src/image/SkSurface_Picture.cpp (right):

https://codereview.appspot.com/6813055/diff/1/src/image/SkSurface_Picture.cpp...
src/image/SkSurface_Picture.cpp:80: 
On 2012/10/30 19:55:59, reed1 wrote:
> // We always already spawn a copy of the recording picture when we are asked
for
> a snapshot, so we never need to do anything here.

Done.

[bsalomon, 2012-10-30 20:05:17]

https://codereview.appspot.com/6813055/diff/1/src/gpu/SkGpuDevice.cpp
File src/gpu/SkGpuDevice.cpp (right):

https://codereview.appspot.com/6813055/diff/1/src/gpu/SkGpuDevice.cpp#newcode208
src/gpu/SkGpuDevice.cpp:208: int sampleCount)
Not directly related to this CL, but we really should turn this into a factory
since it can fail.

https://codereview.appspot.com/6813055/diff/1/src/image/SkSurface_Gpu.cpp
File src/image/SkSurface_Gpu.cpp (right):

https://codereview.appspot.com/6813055/diff/1/src/image/SkSurface_Gpu.cpp#new...
src/image/SkSurface_Gpu.cpp:40: bool SkSurface_Gpu::Valid(GrContext* ctx, const
SkImage::Info& info,
For similar reasons to the Image/Texture case I don't like doing this validation
here. It should be up to GrContext to decide if the params are valid or not.
Also we can have failures on valid inputs (e.g. out of memory).

I propose asking GrContext to create a GrRenderTarget and if it succeeds wrap a
SkGpuDevice/SkSurface_Gpu around it. If we need additional validation let's add
it there where it can be shared with other render-target users.

https://codereview.appspot.com/6813055/diff/1/src/image/SkSurface_Gpu.cpp#new...
src/image/SkSurface_Gpu.cpp:116: fDevice->flush();
Do we need this? I think copyTexture() should handle it for us (or it needs to
be fixed). Also, we may add lazier flushing internally in the future and
probably won't remember to update this callsite.

[robertphillips, 2012-10-31 14:26:11]

https://codereview.appspot.com/6813055/diff/1/src/gpu/SkGpuDevice.cpp
File src/gpu/SkGpuDevice.cpp (right):

https://codereview.appspot.com/6813055/diff/1/src/gpu/SkGpuDevice.cpp#newcode208
src/gpu/SkGpuDevice.cpp:208: int sampleCount)
Converted to TODO

https://codereview.appspot.com/6813055/diff/1/src/image/SkSurface_Gpu.cpp
File src/image/SkSurface_Gpu.cpp (right):

https://codereview.appspot.com/6813055/diff/1/src/image/SkSurface_Gpu.cpp#new...
src/image/SkSurface_Gpu.cpp:40: bool SkSurface_Gpu::Valid(GrContext* ctx, const
SkImage::Info& info,
On 2012/10/30 20:05:17, bsalomon wrote:
> For similar reasons to the Image/Texture case I don't like doing this
validation
> here. It should be up to GrContext to decide if the params are valid or not.
> Also we can have failures on valid inputs (e.g. out of memory).
> 
> I propose asking GrContext to create a GrRenderTarget and if it succeeds wrap
a
> SkGpuDevice/SkSurface_Gpu around it. If we need additional validation let's
add
> it there where it can be shared with other render-target users.

Done.

https://codereview.appspot.com/6813055/diff/1/src/image/SkSurface_Gpu.cpp#new...
src/image/SkSurface_Gpu.cpp:116: fDevice->flush();
On 2012/10/30 20:05:17, bsalomon wrote:
> Do we need this? I think copyTexture() should handle it for us (or it needs to
> be fixed). Also, we may add lazier flushing internally in the future and
> probably won't remember to update this callsite.

Done.

[bsalomon, 2012-10-31 14:31:35]

LGTM

https://codereview.appspot.com/6813055/diff/2003/src/image/SkImage_Gpu.cpp
File src/image/SkImage_Gpu.cpp (right):

https://codereview.appspot.com/6813055/diff/2003/src/image/SkImage_Gpu.cpp#ne...
src/image/SkImage_Gpu.cpp:75: return SkNEW_ARGS(SkImage_Gpu, (texture));
Doesn't this mean that SkImage_Gpu() never gets NULL (and doesnt need to assign
fTexture to NULL and then do a safe assign)?

[robertphillips, 2012-10-31 14:49:07]

Committed as r6214

https://codereview.appspot.com/6813055/diff/2003/src/image/SkImage_Gpu.cpp
File src/image/SkImage_Gpu.cpp (right):

https://codereview.appspot.com/6813055/diff/2003/src/image/SkImage_Gpu.cpp#ne...
src/image/SkImage_Gpu.cpp:75: return SkNEW_ARGS(SkImage_Gpu, (texture));
On 2012/10/31 14:31:36, bsalomon wrote:
> Doesn't this mean that SkImage_Gpu() never gets NULL (and doesnt need to
assign
> fTexture to NULL and then do a safe assign)?

Done.