Swap render target instead of creating a new gpu device for surface copy-on-write

Swap render target instead of creating a new gpu device for surface copy-on-write

Swap render target of the gpu device instead of creating a new gpu
device when making a copy-on-write upon surface modification.

This removes the SkCanvas::setRootDevice which contains problematic code
when trying to increase the use of SkImages internally in Skia.

BUG=skia:3388
Committed: https://skia.googlesource.com/skia/+/abcfab4d68d53900ef33320bb2622696c14d14b0

[Kimmo Kinnunen, 2015-02-16 14:21:19]

kkinnunen@nvidia.com changed reviewers:
+ bsalomon@google.com

[reed1, 2015-02-18 13:49:06]

reed@google.com changed reviewers:
+ reed@google.com

[reed1, 2015-02-18 13:49:07]

I hope this works :) 'cause I'd love to get rid of that method on canvas...

[bsalomon, 2015-02-18 14:29:37]

Agreed that if this works, it's a great change.

https://chromiumcodereview.appspot.com/925343002/diff/20001/src/gpu/SkGpuDevi...
File src/gpu/SkGpuDevice.h (right):

https://chromiumcodereview.appspot.com/925343002/diff/20001/src/gpu/SkGpuDevi...
src/gpu/SkGpuDevice.h:54: static GrRenderTarget* CreateRenderTarget(GrContext*
context, SkSurface::Budgeted budgeted,
Is it possible to just infer the parameters for the new RT from the old RT?
Maybe by making this method non-static? Or maybe have

GrRenderTarget* replaceRenderTarget(bool copyOldToNew);

that creates the new one (using budgeted status of the old one) and returns the
old one?

https://chromiumcodereview.appspot.com/925343002/diff/20001/src/image/SkSurfa...
File src/image/SkSurface_Gpu.cpp (right):

https://chromiumcodereview.appspot.com/925343002/diff/20001/src/image/SkSurfa...
src/image/SkSurface_Gpu.cpp:111: SkAutoTUnref<GrRenderTarget>
rt(SkGpuDevice::CreateRenderTarget(ctx, budgeted, info,
It seems a tad unfortunate to require this two step creation for the initial
SkGpuDevice.

[Kimmo Kinnunen, 2015-02-18 14:42:54]

https://chromiumcodereview.appspot.com/925343002/diff/20001/src/gpu/SkGpuDevi...
File src/gpu/SkGpuDevice.h (right):

https://chromiumcodereview.appspot.com/925343002/diff/20001/src/gpu/SkGpuDevi...
src/gpu/SkGpuDevice.h:54: static GrRenderTarget* CreateRenderTarget(GrContext*
context, SkSurface::Budgeted budgeted,
On 2015/02/18 14:29:36, bsalomon wrote:
> Is it possible to just infer the parameters for the new RT from the old RT?
> Maybe by making this method non-static? Or maybe have
> 
> GrRenderTarget* replaceRenderTarget(bool copyOldToNew);
> 
> that creates the new one (using budgeted status of the old one) and returns
the
> old one?

Yeah, this is just a refactoring. I did not hone it, since the problem you
address may be going away (the can be seen as "inferred") in:
https://chromiumcodereview.appspot.com/926843003/

[Kimmo Kinnunen, 2015-02-18 15:06:46]

https://chromiumcodereview.appspot.com/925343002/diff/20001/src/image/SkSurfa...
File src/image/SkSurface_Gpu.cpp (right):

https://chromiumcodereview.appspot.com/925343002/diff/20001/src/image/SkSurfa...
src/image/SkSurface_Gpu.cpp:111: SkAutoTUnref<GrRenderTarget>
rt(SkGpuDevice::CreateRenderTarget(ctx, budgeted, info,
On 2015/02/18 14:29:37, bsalomon wrote:
> It seems a tad unfortunate to require this two step creation for the initial
> SkGpuDevice.

Yeah.. Once the texture creation succeeds, the SkGpuDevice creation can not
fail. I'd prefer constructing the SkGpuDevice inside SkSurface_Gpu constructor,
since surface constructor is where the device is created. I did not do this,
since it would imply SkGpuDevice::Create should be removed. I did not remove it,
because I was afraid of changing the API would cause the patch review/merge to
stall.

[Kimmo Kinnunen, 2015-02-19 15:52:19]

https://codereview.chromium.org/925343002/diff/20001/src/image/SkSurface_Gpu.cpp
File src/image/SkSurface_Gpu.cpp (right):

https://codereview.chromium.org/925343002/diff/20001/src/image/SkSurface_Gpu....
src/image/SkSurface_Gpu.cpp:111: SkAutoTUnref<GrRenderTarget>
rt(SkGpuDevice::CreateRenderTarget(ctx, budgeted, info,
On 2015/02/18 14:29:37, bsalomon wrote:
> It seems a tad unfortunate to require this two step creation for the initial
> SkGpuDevice.

Done.

[bsalomon, 2015-02-19 22:48:35]

Does the non-static SkGpuDevice::replaceRenderTarget(bool) idea not work?

[Kimmo Kinnunen, 2015-02-20 12:34:41]

On 2015/02/19 22:48:35, bsalomon wrote:
> Does the non-static SkGpuDevice::replaceRenderTarget(bool) idea not work?

Done.

[bsalomon, 2015-02-20 17:33:03]

Awesome, lgtm

[bsalomon, 2015-02-20 17:33:04]

Awesome, lgtm

[Kimmo Kinnunen, 2015-02-23 06:46:25]

The CQ bit was checked by kkinnunen@nvidia.com

[commit-bot: I haz the power, 2015-02-23 06:47:09]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/925343002/120001

[commit-bot: I haz the power, 2015-02-23 06:53:49]

Committed patchset #7 (id:120001) as
https://skia.googlesource.com/skia/+/abcfab4d68d53900ef33320bb2622696c14d14b0