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

src/image/SkSurface_Gpu.cpp

Index: src/image/SkSurface_Gpu.cpp
diff --git a/src/image/SkSurface_Gpu.cpp b/src/image/SkSurface_Gpu.cpp
index a1c31f31379ad022735e69dd5a64636f6149fca4..5fb4b718a27b3a664a00cf1d055166ddf4dd70c2 100644
--- a/src/image/SkSurface_Gpu.cpp
+++ b/src/image/SkSurface_Gpu.cpp
@@ -24,7 +24,7 @@ SkSurface_Gpu::SkSurface_Gpu(SkGpuDevice* device)
 }
 
 SkSurface_Gpu::~SkSurface_Gpu() {
-    SkSafeUnref(fDevice);
+    fDevice->unref();
 }
 
 SkCanvas* SkSurface_Gpu::onNewCanvas() {
@@ -59,8 +59,8 @@ void SkSurface_Gpu::onDraw(SkCanvas* canvas, SkScalar x, SkScalar y,
     canvas->drawBitmap(fDevice->accessBitmap(false), x, y, paint);
 }
 
-// Create a new SkGpuDevice and, if necessary, copy the contents of the old
-// device into it. Note that this flushes the SkGpuDevice but
+// Create a new render target and, if necessary, copy the contents of the old
+// render target into it. Note that this flushes the SkGpuDevice but
 // doesn't force an OpenGL flush.
 void SkSurface_Gpu::onCopyOnWrite(ContentChangeMode mode) {
     GrRenderTarget* rt = fDevice->accessRenderTarget();
@@ -71,19 +71,19 @@ void SkSurface_Gpu::onCopyOnWrite(ContentChangeMode mode) {
     if (rt->asTexture() == SkTextureImageGetTexture(image)) {
         GrRenderTarget* oldRT = this->fDevice->accessRenderTarget();
         SkSurface::Budgeted budgeted = oldRT->resourcePriv().isBudgeted() ? kYes_Budgeted :
-                                                                            kNo_Budgeted;
-        SkAutoTUnref<SkGpuDevice> newDevice(
-            SkGpuDevice::Create(oldRT->getContext(), budgeted, fDevice->imageInfo(),
-                                oldRT->numSamples(), &this->props(), 0));
-        if (kRetain_ContentChangeMode == mode && !oldRT->wasDestroyed() && newDevice) {
-            oldRT->getContext()->copySurface(newDevice->accessRenderTarget(), oldRT);
+                kNo_Budgeted;
+        SkAutoTUnref<GrRenderTarget> newRT(
+            SkGpuDevice::CreateRenderTarget(oldRT->getContext(), budgeted, fDevice->imageInfo(),
+                                            oldRT->numSamples()));
+
+        if (kRetain_ContentChangeMode == mode && !oldRT->wasDestroyed() && newRT) {
+            oldRT->getContext()->copySurface(newRT, oldRT);
         }
 
         SkASSERT(this->getCachedCanvas());
         SkASSERT(this->getCachedCanvas()->getDevice() == fDevice);
 
-        this->getCachedCanvas()->setRootDevice(newDevice);
-        SkRefCnt_SafeAssign(fDevice, newDevice.get());
+        this->fDevice->swapRenderTarget(newRT);
 
         SkTextureImageApplyBudgetedDecision(image);
     } else if (kDiscard_ContentChangeMode == mode) {
@@ -95,6 +95,7 @@ void SkSurface_Gpu::onDiscard() {
     fDevice->accessRenderTarget()->discard();
 }
 
+
 ///////////////////////////////////////////////////////////////////////////////
 
 SkSurface* SkSurface::NewRenderTargetDirect(GrRenderTarget* target, const SkSurfaceProps* props) {
@@ -107,9 +108,13 @@ SkSurface* SkSurface::NewRenderTargetDirect(GrRenderTarget* target, const SkSurf
 
 SkSurface* SkSurface::NewRenderTarget(GrContext* ctx, Budgeted budgeted, const SkImageInfo& info,
                                       int sampleCount, const SkSurfaceProps* props) {
-    SkAutoTUnref<SkGpuDevice> device(SkGpuDevice::Create(ctx, budgeted, info, sampleCount, props,
-                                                         SkGpuDevice::kNeedClear_Flag));
-    if (!device) {
+    SkAutoTUnref<GrRenderTarget> rt(SkGpuDevice::CreateRenderTarget(ctx, budgeted, info,

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

It seems a tad unfortunate to require this two step creation for the initial
SkGpuDevice.

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

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:18]

Done.

+                                                                    sampleCount));
+    if (NULL == rt) {
+        return NULL;
+    }
+    SkAutoTUnref<SkGpuDevice> device(SkGpuDevice::Create(rt, props, SkGpuDevice::kNeedClear_Flag));
+    if (NULL == device) {
         return NULL;
     }
     return SkNEW_ARGS(SkSurface_Gpu, (device));