Cleanup: Removing unnecessary args/complexity in SkSurface_Base and friends

src/image/SkSurface.cpp

 /*
  * Copyright 2012 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkSurface_Base.h"
 #include "SkImagePriv.h"
 #include "SkCanvas.h"
@@ skipping 41 matching lines @@
 }
 
 SkImage* SkSurface_Base::getCachedImage() {
     if (NULL == fCachedImage) {
         fCachedImage = this->onNewImageSnapshot();
         this->installIntoCanvasForDirtyNotification();
     }
     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 == fCachedImage) {
+        return;
     }
 
-    if (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);
-        }
+    SkASSERT(fCachedCanvas);  // Can't have a cached image without a cached canvas.

[reed1, 2013/04/16 15:31:23]

Why not? I'm trying trace the code to see why this must be true. And even if it
happens to be true today, is that a real invariant from this method's
perspective?

At least with today's code, if I do this, I don't have a cached-canvas when I
get to aboutToDraw.

            SkSurface* sur = SkSurface::NewRaster(info);
            sur->notifyContentChanged();

+    SkASSERT(fCachedCanvas->getSurfaceBase() == this);
+    fCachedCanvas->setSurfaceBase(NULL);
 
-        // regardless of copy-on-write, we must drop our cached image now, so
-        // that the next request will get our new contents.
-        fCachedImage->unref();
-        fCachedImage = NULL;
+    // 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();
     }
+
+    // regardless of copy-on-write, we must drop our cached image now, so
+    // that the next request will get our new contents.
+    fCachedImage->unref();
+    fCachedImage = NULL;
 }
 
 uint32_t SkSurface_Base::newGenerationID() {
     this->installIntoCanvasForDirtyNotification();
 
     static int32_t gID;
     return sk_atomic_inc(&gID) + 1;
 }
 
 static SkSurface_Base* asSB(SkSurface* surface) {
     return static_cast<SkSurface_Base*>(surface);
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 
 SkSurface::SkSurface(int width, int height) : fWidth(width), fHeight(height) {
     SkASSERT(width >= 0);
     SkASSERT(height >= 0);
     fGenerationID = 0;
 }
 
 uint32_t SkSurface::generationID() {
     if (0 == fGenerationID) {
         fGenerationID = asSB(this)->newGenerationID();
     }
     return fGenerationID;
 }
 
 void SkSurface::notifyContentChanged() {
-    asSB(this)->aboutToDraw(NULL);
+    asSB(this)->aboutToDraw();
 }
 
 SkCanvas* SkSurface::getCanvas() {
     return asSB(this)->getCachedCanvas();
 }
 
 SkImage* SkSurface::newImageSnapshot() {
     SkImage* image = asSB(this)->getCachedImage();
     SkSafeRef(image);   // the caller will call unref() to balance this
     return image;
 }
 
 SkSurface* SkSurface::newSurface(const SkImage::Info& info) {
     return asSB(this)->onNewSurface(info);
 }
 
 void SkSurface::draw(SkCanvas* canvas, SkScalar x, SkScalar y,
                      const SkPaint* paint) {
     return asSB(this)->onDraw(canvas, x, y, paint);
 }