Cleanup: Removing unnecessary args/complexity in SkSurface_Base and friends

Review URL: https://codereview.chromium.org/14263017

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

diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp
index 5b2e381aa64054a7154b69c3b4d1afdd099d13d2..2d8212de0fb0f73ad04e2a531778443efba99c34 100644 (file)
--- a/src/core/SkCanvas.cpp
+++ b/src/core/SkCanvas.cpp
@@ -122,7 +122,7 @@ typedef SkTLazy<SkPaint> SkLazyPaint;
 
 void SkCanvas::predrawNotify() {
     if (fSurfaceBase) {
-        fSurfaceBase->aboutToDraw(this);
+        fSurfaceBase->aboutToDraw();
     }
 }
 

diff --git a/src/image/SkSurface.cpp b/src/image/SkSurface.cpp
index 8cce965c51fc90ff88049ecc9f8d1e261b9566d6..e233dd0cb93f4244bb6209b8bcc185f602ca1b27 100644 (file)
--- a/src/image/SkSurface.cpp
+++ b/src/image/SkSurface.cpp
@@ -59,21 +59,21 @@ SkImage* SkSurface_Base::getCachedImage() {
     return fCachedImage;
 }
 
-void SkSurface_Base::aboutToDraw(SkCanvas* canvas) {
+void SkSurface_Base::aboutToDraw() {
     this->dirtyGenerationID();
 
-    if (canvas) {
-        SkASSERT(canvas == fCachedCanvas);
-        SkASSERT(canvas->getSurfaceBase() == this);
-        canvas->setSurfaceBase(NULL);
+    if (NULL != fCachedCanvas) {
+        SkASSERT(fCachedCanvas->getSurfaceBase() == this || \
+                 NULL == fCachedCanvas->getSurfaceBase());
+        fCachedCanvas->setSurfaceBase(NULL);
     }
 
-    if (fCachedImage) {
+    if (NULL != fCachedImage) {
         // the surface may need to fork its backend, if its sharing it with
         // the cached image. Note: we only call if there is an outstanding owner
         // on the image (besides us).
         if (fCachedImage->getRefCnt() > 1) {
-            this->onCopyOnWrite(fCachedImage, canvas);
+            this->onCopyOnWrite();
         }
 
         // regardless of copy-on-write, we must drop our cached image now, so
@@ -110,7 +110,7 @@ uint32_t SkSurface::generationID() {
 }
 
 void SkSurface::notifyContentChanged() {
-    asSB(this)->aboutToDraw(NULL);
+    asSB(this)->aboutToDraw();
 }
 
 SkCanvas* SkSurface::getCanvas() {

diff --git a/src/image/SkSurface_Base.h b/src/image/SkSurface_Base.h
index bda271bf1566f6de8d6bc8a902ce42108ac0af2d..d0590bc3ca64565c7ec001809dbf64d1382c1360 100644 (file)
--- a/src/image/SkSurface_Base.h
+++ b/src/image/SkSurface_Base.h
@@ -51,7 +51,7 @@ public:
      *
      *  The default implementation does nothing.
      */
-    virtual void onCopyOnWrite(SkImage* cachedImage, SkCanvas*) = 0;
+    virtual void onCopyOnWrite() = 0;
 
     inline SkCanvas* getCachedCanvas();
     inline SkImage* getCachedImage();
@@ -63,7 +63,7 @@ private:
     SkCanvas*   fCachedCanvas;
     SkImage*    fCachedImage;
 
-    void aboutToDraw(SkCanvas*);
+    void aboutToDraw();
     friend class SkCanvas;
     friend class SkSurface;
 

diff --git a/src/image/SkSurface_Gpu.cpp b/src/image/SkSurface_Gpu.cpp
index ec709ca50e2c51bc655a720a17692a4668bdbbd9..098c92b1b13cacf9e20ef44c46225a64d710d1c1 100644 (file)
--- a/src/image/SkSurface_Gpu.cpp
+++ b/src/image/SkSurface_Gpu.cpp
@@ -23,7 +23,7 @@ public:
     virtual SkImage* onNewImageSnapshot() SK_OVERRIDE;
     virtual void onDraw(SkCanvas*, SkScalar x, SkScalar y,
                         const SkPaint*) SK_OVERRIDE;
-    virtual void onCopyOnWrite(SkImage*, SkCanvas*) SK_OVERRIDE;
+    virtual void onCopyOnWrite() SK_OVERRIDE;
 
 private:
     SkGpuDevice* fDevice;
@@ -86,20 +86,21 @@ void SkSurface_Gpu::onDraw(SkCanvas* canvas, SkScalar x, SkScalar y,
 // Create a new SkGpuDevice and, if necessary, copy the contents of the old
 // device into it. Note that this flushes the SkGpuDevice but
 // doesn't force an OpenGL flush.
-void SkSurface_Gpu::onCopyOnWrite(SkImage* image, SkCanvas* canvas) {
+void SkSurface_Gpu::onCopyOnWrite() {
     GrRenderTarget* rt = (GrRenderTarget*) fDevice->accessRenderTarget();
 
     // are we sharing our render target with the image?
-    if (rt->asTexture() == SkTextureImageGetTexture(image)) {
+    SkASSERT(NULL != this->getCachedImage());
+    if (rt->asTexture() == SkTextureImageGetTexture(this->getCachedImage())) {
         SkGpuDevice* newDevice = static_cast<SkGpuDevice*>(
             fDevice->createCompatibleDevice(fDevice->config(), fDevice->width(),
             fDevice->height(), fDevice->isOpaque()));
         SkAutoTUnref<SkGpuDevice> aurd(newDevice);
         fDevice->context()->copyTexture(rt->asTexture(),
             (GrRenderTarget*)newDevice->accessRenderTarget());
-        SkASSERT(NULL != canvas);
-        SkASSERT(canvas->getDevice() == fDevice);
-        canvas->setDevice(newDevice);
+        SkASSERT(NULL != this->getCachedCanvas());
+        SkASSERT(this->getCachedCanvas()->getDevice() == fDevice);
+        this->getCachedCanvas()->setDevice(newDevice);
         SkRefCnt_SafeAssign(fDevice, newDevice);
     }
 }

diff --git a/src/image/SkSurface_Picture.cpp b/src/image/SkSurface_Picture.cpp
index d7b84ecaab6d275ff7a6ae3cb099379f6ebe0087..affa05c93344fe6b04a62a4ecfaf4dc984ea42fa 100644 (file)
--- a/src/image/SkSurface_Picture.cpp
+++ b/src/image/SkSurface_Picture.cpp
@@ -24,7 +24,7 @@ public:
     virtual SkImage* onNewImageSnapshot() SK_OVERRIDE;
     virtual void onDraw(SkCanvas*, SkScalar x, SkScalar y,
                         const SkPaint*) SK_OVERRIDE;
-    virtual void onCopyOnWrite(SkImage*, SkCanvas*) SK_OVERRIDE;
+    virtual void onCopyOnWrite() SK_OVERRIDE;
 
 private:
     SkPicture*  fPicture;
@@ -75,7 +75,7 @@ void SkSurface_Picture::onDraw(SkCanvas* canvas, SkScalar x, SkScalar y,
     SkImagePrivDrawPicture(canvas, fPicture, x, y, paint);
 }
 
-void SkSurface_Picture::onCopyOnWrite(SkImage* cachedImage, SkCanvas*) {
+void SkSurface_Picture::onCopyOnWrite() {
     // We always spawn a copy of the recording picture when we
     // are asked for a snapshot, so we never need to do anything here.
 }

diff --git a/src/image/SkSurface_Raster.cpp b/src/image/SkSurface_Raster.cpp
index b88d0e21ce3f0f7732a16ba02304244b4d59ea46..9a1f312252284973fba3a0ddb78d75dbb1b16924 100644 (file)
--- a/src/image/SkSurface_Raster.cpp
+++ b/src/image/SkSurface_Raster.cpp
@@ -25,7 +25,7 @@ public:
     virtual SkImage* onNewImageSnapshot() SK_OVERRIDE;
     virtual void onDraw(SkCanvas*, SkScalar x, SkScalar y,
                         const SkPaint*) SK_OVERRIDE;
-    virtual void onCopyOnWrite(SkImage*, SkCanvas*) SK_OVERRIDE;
+    virtual void onCopyOnWrite() SK_OVERRIDE;
 
 private:
     SkBitmap    fBitmap;
@@ -124,16 +124,18 @@ SkImage* SkSurface_Raster::onNewImageSnapshot() {
     return SkNewImageFromBitmap(fBitmap, fWeOwnThePixels);
 }
 
-void SkSurface_Raster::onCopyOnWrite(SkImage* image, SkCanvas* canvas) {
+void SkSurface_Raster::onCopyOnWrite() {
     // are we sharing pixelrefs with the image?
-    if (SkBitmapImageGetPixelRef(image) == fBitmap.pixelRef()) {
+    SkASSERT(NULL !=this->getCachedImage());
+    if (SkBitmapImageGetPixelRef(this->getCachedImage()) == fBitmap.pixelRef()) {
         SkASSERT(fWeOwnThePixels);
         SkBitmap prev(fBitmap);
         prev.deepCopyTo(&fBitmap, prev.config());
         // Now fBitmap is a deep copy of itself (and therefore different from
         // what is being used by the image. Next we update the canvas to use
         // this as its backend, so we can't modify the image's pixels anymore.
-        canvas->getDevice()->replaceBitmapBackendForRasterSurface(fBitmap);
+        SkASSERT(NULL != this->getCachedCanvas());
+        this->getCachedCanvas()->getDevice()->replaceBitmapBackendForRasterSurface(fBitmap);
     }
 }
 

diff --git a/tests/SurfaceTest.cpp b/tests/SurfaceTest.cpp
index 5f2dca64c622ade7faee02d63205d1fb87db9beb..27029a1f2d5ff18fcd534812e33302111db76b93 100644 (file)
--- a/tests/SurfaceTest.cpp
+++ b/tests/SurfaceTest.cpp
@@ -146,17 +146,49 @@ static void TestSurfaceWritableAfterSnapshotRelease(skiatest::Reporter* reporter
     surface->newImageSnapshot()->unref();  // Create and destroy SkImage
     canvas->clear(2);
 }
+static void TestSurfaceNoCanvas(skiatest::Reporter* reporter,
+                                          SurfaceType surfaceType,
+                                          GrContext* context) {
+    // Verifies the robustness of SkSurface for handling use cases where calls
+    // are made before a canvas is created.
+    {
+        // Test passes by not asserting
+        SkSurface* surface = createSurface(surfaceType, context);
+        SkAutoTUnref<SkSurface> aur_surface(surface);
+        surface->notifyContentChanged();
+        surface->validate();
+    }
+    {
+        SkSurface* surface = createSurface(surfaceType, context);
+        SkAutoTUnref<SkSurface> aur_surface(surface);
+        SkImage* image1 = surface->newImageSnapshot();
+        SkAutoTUnref<SkImage> aur_image1(image1);
+        image1->validate();
+        surface->validate();
+        surface->notifyContentChanged();
+        image1->validate();
+        surface->validate();
+        SkImage* image2 = surface->newImageSnapshot();
+        SkAutoTUnref<SkImage> aur_image2(image2);
+        image2->validate();
+        surface->validate();
+        REPORTER_ASSERT(reporter, image1 != image2);
+    }
+    
+}
 
 static void TestSurface(skiatest::Reporter* reporter, GrContextFactory* factory) {
     TestSurfaceCopyOnWrite(reporter, kRaster_SurfaceType, NULL);
     TestSurfaceCopyOnWrite(reporter, kPicture_SurfaceType, NULL);
     TestSurfaceWritableAfterSnapshotRelease(reporter, kRaster_SurfaceType, NULL);
     TestSurfaceWritableAfterSnapshotRelease(reporter, kPicture_SurfaceType, NULL);
+    TestSurfaceNoCanvas(reporter, kRaster_SurfaceType, NULL);
 #if SK_SUPPORT_GPU
     if (NULL != factory) {
         GrContext* context = factory->get(GrContextFactory::kNative_GLContextType);
         TestSurfaceCopyOnWrite(reporter, kGpu_SurfaceType, context);
         TestSurfaceWritableAfterSnapshotRelease(reporter, kGpu_SurfaceType, context);
+        TestSurfaceNoCanvas(reporter, kGpu_SurfaceType, context);
     }
 #endif
 }