Fix crash with SkDeferredCanvas+SkSurface integration with in order draw buffer.

The fImmediateDevice member of DeferredDevice (SkDeferredCanvas.cpp) was becoming invalid after a fork of the backingstore in SkSurface_Gpu cause the device to be substituted.

New unit test code was to exercise SkSurface copy on write with draws that are deferred in GrInOrderDrawBuffer.  The bad pointer was causing the test to crash.

TEST=skia unit test DeferredCanvas, subtest TestDeferredCanvasSurface
Review URL: https://codereview.chromium.org/14263015

git-svn-id: http://skia.googlecode.com/svn/trunk@8686 2bbb7eff-a529-9590-31e7-b0007b416f81

diff --git a/src/utils/SkDeferredCanvas.cpp b/src/utils/SkDeferredCanvas.cpp
index af70fd2..42e9537 100644 (file)
--- a/src/utils/SkDeferredCanvas.cpp
+++ b/src/utils/SkDeferredCanvas.cpp
@@ -146,7 +146,7 @@ public:
     void setNotificationClient(SkDeferredCanvas::NotificationClient* notificationClient);
     SkCanvas* recordingCanvas();
     SkCanvas* immediateCanvas() const {return fImmediateCanvas;}
-    SkDevice* immediateDevice() const {return fImmediateDevice;}
+    SkDevice* immediateDevice() const {return fImmediateCanvas->getTopDevice();}
     SkImage* newImageSnapshot();
     bool isFreshFrame();
     bool hasPendingCommands();
@@ -243,7 +243,6 @@ private:
 
     DeferredPipeController fPipeController;
     SkGPipeWriter  fPipeWriter;
-    SkDevice* fImmediateDevice;
     SkCanvas* fImmediateCanvas;
     SkCanvas* fRecordingCanvas;
     SkSurface* fSurface;
@@ -260,8 +259,7 @@ DeferredDevice::DeferredDevice(SkDevice* immediateDevice)
                immediateDevice->isOpaque(),
                immediateDevice->getDeviceProperties()) {
     fSurface = NULL;
-    fImmediateDevice = immediateDevice;  // ref counted via fImmediateCanvas
-    fImmediateCanvas = SkNEW_ARGS(SkCanvas, (fImmediateDevice));
+    fImmediateCanvas = SkNEW_ARGS(SkCanvas, (immediateDevice));
     this->init();
 }
 
@@ -277,7 +275,6 @@ DeferredDevice::DeferredDevice(SkSurface* surface)
     SkSafeRef(fImmediateCanvas);
     fSurface = surface;
     SkSafeRef(fSurface);
-    fImmediateDevice = fImmediateCanvas->getDevice();  // ref counted via fImmediateCanvas
     this->init();
 }
 
@@ -306,7 +303,7 @@ void DeferredDevice::setMaxRecordingStorage(size_t maxStorage) {
 void DeferredDevice::beginRecording() {
     SkASSERT(NULL == fRecordingCanvas);
     fRecordingCanvas = fPipeWriter.startRecording(&fPipeController, 0,
-        fImmediateDevice->width(), fImmediateDevice->height());
+        immediateDevice()->width(), immediateDevice()->height());
 }
 
 void DeferredDevice::setNotificationClient(
@@ -406,20 +403,20 @@ SkImage* DeferredDevice::newImageSnapshot() {
 }
 
 uint32_t DeferredDevice::getDeviceCapabilities() {
-    return fImmediateDevice->getDeviceCapabilities();
+    return immediateDevice()->getDeviceCapabilities();
 }
 
 int DeferredDevice::width() const {
-    return fImmediateDevice->width();
+    return immediateDevice()->width();
 }
 
 int DeferredDevice::height() const {
-    return fImmediateDevice->height();
+    return immediateDevice()->height();
 }
 
 SkGpuRenderTarget* DeferredDevice::accessRenderTarget() {
     this->flushPendingCommands(kNormal_PlaybackMode);
-    return fImmediateDevice->accessRenderTarget();
+    return immediateDevice()->accessRenderTarget();
 }
 
 void DeferredDevice::writePixels(const SkBitmap& bitmap,
@@ -435,7 +432,7 @@ void DeferredDevice::writePixels(const SkBitmap& bitmap,
         kPMColorAlias != config8888) {
         //Special case config: no deferral
         this->flushPendingCommands(kNormal_PlaybackMode);
-        fImmediateDevice->writePixels(bitmap, x, y, config8888);
+        immediateDevice()->writePixels(bitmap, x, y, config8888);
         return;
     }
 
@@ -453,7 +450,7 @@ void DeferredDevice::writePixels(const SkBitmap& bitmap,
 
 const SkBitmap& DeferredDevice::onAccessBitmap(SkBitmap*) {
     this->flushPendingCommands(kNormal_PlaybackMode);
-    return fImmediateDevice->accessBitmap(false);
+    return immediateDevice()->accessBitmap(false);
 }
 
 SkDevice* DeferredDevice::onCreateCompatibleDevice(
@@ -464,7 +461,7 @@ SkDevice* DeferredDevice::onCreateCompatibleDevice(
     SkASSERT(usage != kSaveLayer_Usage);
     // Create a compatible non-deferred device.
     SkAutoTUnref<SkDevice> compatibleDevice
-        (fImmediateDevice->createCompatibleDevice(config, width, height,
+        (immediateDevice()->createCompatibleDevice(config, width, height,
             isOpaque));
     DeferredDevice* device = SkNEW_ARGS(DeferredDevice, (compatibleDevice));
     device->setNotificationClient(fNotificationClient);

diff --git a/tests/DeferredCanvasTest.cpp b/tests/DeferredCanvasTest.cpp
index 2926ea9..3e73393 100644 (file)
--- a/tests/DeferredCanvasTest.cpp
+++ b/tests/DeferredCanvasTest.cpp
@@ -527,16 +527,25 @@ static void TestDeferredCanvasSurface(skiatest::Reporter* reporter, GrContextFac
     // write
     PixelPtr pixels2 = getSurfacePixelPtr(surface, useGpu);
     REPORTER_ASSERT(reporter, pixels1 != pixels2);
-    canvas.clear(SK_ColorWHITE);
-    canvas.flush();
+    // Verify copy-on write with a draw operation that gets deferred by
+    // the in order draw buffer.
+    SkPaint paint;
+    canvas.drawPaint(paint);
+    SkImage* image4 = canvas.newImageSnapshot();  // implicit flush
+    SkAutoTUnref<SkImage> aur_i4(image4);
+    REPORTER_ASSERT(reporter, image4->uniqueID() != image3->uniqueID());
     PixelPtr pixels3 = getSurfacePixelPtr(surface, useGpu);
+    REPORTER_ASSERT(reporter, pixels2 != pixels3);
     // Verify that a direct canvas flush with a pending draw does not trigger
     // a copy on write when the surface is not sharing its buffer with an
     // SkImage.
-    canvas.clear(SK_ColorBLACK);
+    canvas.clear(SK_ColorWHITE);
     canvas.flush();
     PixelPtr pixels4 = getSurfacePixelPtr(surface, useGpu);
-    REPORTER_ASSERT(reporter, pixels3 == pixels4);
+    canvas.drawPaint(paint);
+    canvas.flush();
+    PixelPtr pixels5 = getSurfacePixelPtr(surface, useGpu);
+    REPORTER_ASSERT(reporter, pixels4 == pixels5);
 }
 
 static void TestDeferredCanvas(skiatest::Reporter* reporter, GrContextFactory* factory) {