Add caching of the snapshot image form a surface
authorreed@google.com <reed@google.com@2bbb7eff-a529-9590-31e7-b0007b416f81>
Tue, 28 Aug 2012 12:19:02 +0000 (12:19 +0000)
committerreed@google.com <reed@google.com@2bbb7eff-a529-9590-31e7-b0007b416f81>
Tue, 28 Aug 2012 12:19:02 +0000 (12:19 +0000)
Notify the surface when the canvas draws into it, so it can invalidate the
cached image, and (if needed) perform a copy-on-write on the surface if it
was being shared with the image.
Review URL: https://codereview.appspot.com/6441115

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

18 files changed:
gm/image.cpp
gyp/core.gyp
gyp/core.gypi
gyp/gmslides.gypi
include/core/SkCanvas.h
include/core/SkDevice.h
include/core/SkImage.h
include/core/SkSurface.h
src/core/SkCanvas.cpp
src/core/SkDevice.cpp
src/core/SkPictureRecord.h
src/image/SkImage.cpp
src/image/SkImagePriv.cpp
src/image/SkImagePriv.h
src/image/SkImage_Raster.cpp
src/image/SkSurface.cpp
src/image/SkSurface_Base.h
src/image/SkSurface_Raster.cpp

index 4ee90a4..21286db 100644 (file)
@@ -11,6 +11,8 @@
 #include "SkStream.h"
 #include "SkData.h"
 
+extern GrContext* GetGr();
+
 static SkData* fileToData(const char path[]) {
     SkFILEStream stream(path);
     if (!stream.isValid()) {
@@ -57,9 +59,18 @@ static void test_surface(SkCanvas* canvas, SkSurface* surf) {
     drawContents(surf, SK_ColorRED);
     SkImage* imgR = surf->newImageShapshot();
 
+    if (true) {
+        SkImage* imgR2 = surf->newImageShapshot();
+        SkASSERT(imgR == imgR2);
+        imgR2->unref();
+    }
+
     drawContents(surf, SK_ColorGREEN);
     SkImage* imgG = surf->newImageShapshot();
 
+    // since we've drawn after we snapped imgR, imgG will be a different obj
+    SkASSERT(imgR != imgG);
+
     drawContents(surf, SK_ColorBLUE);
 
     SkPaint paint;
@@ -130,6 +141,10 @@ protected:
         test_surface(canvas, surf2);
     }
 
+    virtual uint32_t onGetFlags() const SK_OVERRIDE {
+        return GM::kSkipPicture_Flag;
+    }
+
 private:
     typedef skiagm::GM INHERITED;
 };
index 806626a..e23572a 100644 (file)
@@ -17,6 +17,7 @@
         '../include/ports',
         '../include/xml',
         '../src/core',
+        '../src/image',
       ],
       'msvs_disabled_warnings': [4244, 4267,4345, 4390, 4554, 4800],
       'conditions': [
index 4595b6a..99227a3 100644 (file)
         '<(skia_src_path)/core/SkUtils.cpp',
         '<(skia_src_path)/core/SkWriter32.cpp',
         '<(skia_src_path)/core/SkXfermode.cpp',
+
+        '<(skia_src_path)/image/SkDataPixelRef.cpp',
+        '<(skia_src_path)/image/SkImage.cpp',
+        '<(skia_src_path)/image/SkImagePriv.cpp',
+        '<(skia_src_path)/image/SkImage_Codec.cpp',
+#        '<(skia_src_path)/image/SkImage_Gpu.cpp',
+        '<(skia_src_path)/image/SkImage_Picture.cpp',
+        '<(skia_src_path)/image/SkImage_Raster.cpp',
+        '<(skia_src_path)/image/SkSurface.cpp',
+#        '<(skia_src_path)/image/SkSurface_Gpu.cpp',
+        '<(skia_src_path)/image/SkSurface_Picture.cpp',
+        '<(skia_src_path)/image/SkSurface_Raster.cpp',
+
         '<(skia_src_path)/pipe/SkGPipeRead.cpp',
         '<(skia_src_path)/pipe/SkGPipeWrite.cpp',
 
index e36abf5..d80c123 100644 (file)
@@ -39,6 +39,7 @@
     '../gm/imageblur.cpp',
     '../gm/imagemagnifier.cpp',
     '../gm/lighting.cpp',
+    '../gm/image.cpp',
     '../gm/imagefiltersbase.cpp',
     '../gm/imagefiltersgraph.cpp',
     '../gm/lcdtext.cpp',
index 01e0c75..1f29e1c 100644 (file)
@@ -26,6 +26,7 @@ class SkDevice;
 class SkDraw;
 class SkDrawFilter;
 class SkPicture;
+class SkSurface_Base;
 
 /** \class SkCanvas
 
@@ -950,6 +951,10 @@ protected:
     bool clipRectBounds(const SkRect* bounds, SaveFlags flags,
                         SkIRect* intersection);
 
+    // notify our surface (if we have one) that we are about to draw, so it
+    // can perform copy-on-write or invalidate any cached images
+    void predrawNotify();
+
 private:
     class MCRec;
 
@@ -964,6 +969,13 @@ private:
     SkDevice*   fLastDeviceToGainFocus;
     int         fSaveLayerCount;    // number of successful saveLayer calls
 
+    SkSurface_Base*  fSurfaceBase;
+    SkSurface_Base* getSurfaceBase() const { return fSurfaceBase; }
+    void setSurfaceBase(SkSurface_Base* sb) {
+        fSurfaceBase = sb;
+    }
+    friend class SkSurface_Base;
+
     void prepareForDeviceDraw(SkDevice*, const SkMatrix&, const SkRegion&);
 
     bool fDeviceCMDirty;            // cleared by updateDeviceCMCache()
index 326115c..41e316c 100644 (file)
@@ -380,6 +380,12 @@ private:
     friend class SkDeviceFilteredPaint;
     friend class DeviceImageFilterProxy;
 
+    friend class SkSurface_Raster;
+    // used to change the backend's pixels (and possibly config/rowbytes)
+    // but cannot change the width/height, so there should be no change to
+    // any clip information.
+    void replaceBitmapBackendForRasterSurface(const SkBitmap&);
+
     // just called by SkCanvas when built as a layer
     void setOrigin(int x, int y) { fOrigin.set(x, y); }
     // just called by SkCanvas for saveLayer
index 985a3bb..b0071e2 100644 (file)
@@ -37,7 +37,7 @@ class SkColorSpace;
  */
 class SkImage : public SkRefCnt {
 public:
-    SK_DECLARE_INST_COUNT(SkImage)
+//    SK_DECLARE_INST_COUNT(SkImage)
 
     enum ColorType {
         kAlpha_8_ColorType,
index 8e407b9..9f8908a 100644 (file)
@@ -26,7 +26,7 @@ class GrRenderTarget;
  */
 class SkSurface : public SkRefCnt {
 public:
-    SK_DECLARE_INST_COUNT(SkSurface)
+//    SK_DECLARE_INST_COUNT(SkSurface)
 
     /**
      *  Create a new surface, using the specified pixels/rowbytes as its
@@ -78,7 +78,7 @@ public:
      *  If this surface is empty (i.e. has a zero-dimention), this will return
      *  0.
      */
-    uint32_t generationID() const;
+    uint32_t generationID();
 
     /**
      *  Call this if the contents have changed. This will (lazily) force a new
@@ -128,10 +128,15 @@ public:
 protected:
     SkSurface(int width, int height);
 
+    // called by subclass if their contents have changed
+    void dirtyGenerationID() {
+        fGenerationID = 0;
+    }
+
 private:
-    const int           fWidth;
-    const int           fHeight;
-    mutable uint32_t    fGenerationID;
+    const int   fWidth;
+    const int   fHeight;
+    uint32_t    fGenerationID;
 };
 
 #endif
index b7c06c3..f5c6bc2 100644 (file)
@@ -16,6 +16,7 @@
 #include "SkPicture.h"
 #include "SkRasterClip.h"
 #include "SkScalarCompare.h"
+#include "SkSurface_Base.h"
 #include "SkTemplates.h"
 #include "SkTextFormatParams.h"
 #include "SkTLazy.h"
@@ -53,6 +54,12 @@ SK_DEFINE_INST_COUNT(SkDrawFilter)
 
 typedef SkTLazy<SkPaint> SkLazyPaint;
 
+void SkCanvas::predrawNotify() {
+    if (fSurfaceBase) {
+        fSurfaceBase->aboutToDraw(this);
+    }
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 
 /*  This is the record we keep for each SkDevice that the user installs.
@@ -425,6 +432,7 @@ private:
 
 #define LOOPER_BEGIN_DRAWDEVICE(paint, type)                        \
 /*    AutoValidator   validator(fMCRec->fTopLayer->fDevice); */     \
+    this->predrawNotify();                                          \
     AutoDrawLooper  looper(this, paint, true);                      \
     while (looper.next(type)) {                                     \
         SkAutoBounderCommit ac(fBounder);                           \
@@ -432,6 +440,7 @@ private:
 
 #define LOOPER_BEGIN(paint, type)                                   \
 /*    AutoValidator   validator(fMCRec->fTopLayer->fDevice); */     \
+    this->predrawNotify();                                          \
     AutoDrawLooper  looper(this, paint);                            \
     while (looper.next(type)) {                                     \
         SkAutoBounderCommit ac(fBounder);                           \
@@ -459,6 +468,8 @@ SkDevice* SkCanvas::init(SkDevice* device) {
     fExternalMatrix.reset();
     fExternalInverse.reset();
     fUseExternalMatrix = false;
+    
+    fSurfaceBase = NULL;
 
     return this->setDevice(device);
 }
@@ -2072,3 +2083,5 @@ int SkCanvas::LayerIter::y() const { return fImpl->getY(); }
 ///////////////////////////////////////////////////////////////////////////////
 
 SkCanvas::ClipVisitor::~ClipVisitor() { }
+
+
index 6146fd6..f8bed65 100644 (file)
@@ -50,6 +50,13 @@ SkDevice::~SkDevice() {
     delete fMetaData;
 }
 
+void SkDevice::replaceBitmapBackendForRasterSurface(const SkBitmap& bm) {
+    SkASSERT(bm.width() == fBitmap.width());
+    SkASSERT(bm.height() == fBitmap.height());
+    fBitmap = bm;   // intent is to use bm's pixelRef (and rowbytes/config)
+    fBitmap.lockPixels();
+}
+
 SkDevice* SkDevice::createCompatibleDevice(SkBitmap::Config config,
                                            int width, int height,
                                            bool isOpaque) {
index 50cf4db..a19a084 100644 (file)
@@ -96,6 +96,8 @@ private:
     };
 
     void addDraw(DrawType drawType) {
+        this->predrawNotify();
+
 #ifdef SK_DEBUG_TRACE
         SkDebugf("add %s\n", DrawTypeToString(drawType));
 #endif
index b6fc489..251721f 100644 (file)
@@ -10,7 +10,7 @@
 #include "SkBitmap.h"
 #include "SkCanvas.h"
 
-SK_DEFINE_INST_COUNT(SkImage)
+//SK_DEFINE_INST_COUNT(SkImage)
 
 static SkImage_Base* asIB(SkImage* image) {
     return static_cast<SkImage_Base*>(image);
index a020eaf..e7a244a 100644 (file)
@@ -96,14 +96,14 @@ bool SkBitmapToImageInfo(const SkBitmap& bm, SkImage::Info* info) {
     return true;
 }
 
-SkImage* SkNewImageFromBitmap(const SkBitmap& bm) {
+SkImage* SkNewImageFromBitmap(const SkBitmap& bm, bool canSharePixelRef) {
     SkImage::Info info;
     if (!SkBitmapToImageInfo(bm, &info)) {
         return NULL;
     }
 
     SkImage* image = NULL;
-    if (bm.isImmutable()) {
+    if (canSharePixelRef || bm.isImmutable()) {
         image = SkNewImageFromPixelRef(info, bm.pixelRef(), bm.rowBytes());
     } else {
         bm.lockPixels();
index c0aff98..9332abc 100644 (file)
@@ -19,14 +19,22 @@ extern SkBitmap::Config SkImageInfoToBitmapConfig(const SkImage::Info&,
 extern int SkImageBytesPerPixel(SkImage::ColorType);
 
 extern bool SkBitmapToImageInfo(const SkBitmap&, SkImage::Info*);
+
+// Call this if you explicitly want to use/share this pixelRef in the image
 extern SkImage* SkNewImageFromPixelRef(const SkImage::Info&, SkPixelRef*,
                                        size_t rowBytes);
 
 /**
  *  Examines the bitmap to decide if it can share the existing pixelRef, or
- *  if it needs to make a deep-copy of the pixels
+ *  if it needs to make a deep-copy of the pixels. The bitmap's pixelref will
+ *  be shared if either the bitmap is marked as immutable, or canSharePixelRef
+ *  is true.
+ *
+ *  If the bitmap's config cannot be converted into a corresponding
+ *  SkImage::Info, or the bitmap's pixels cannot be accessed, this will return
+ *  NULL.
  */
-extern SkImage* SkNewImageFromBitmap(const SkBitmap&);
+extern SkImage* SkNewImageFromBitmap(const SkBitmap&, bool canSharePixelRef);
 
 extern void SkImagePrivDrawPicture(SkCanvas*, SkPicture*,
                                    SkScalar x, SkScalar y, const SkPaint*);
@@ -42,4 +50,9 @@ static inline size_t SkImageMinRowBytes(const SkImage::Info& info) {
     return SkAlign4(rb);
 }
 
+// Given an image created from SkNewImageFromBitmap, return its pixelref. This
+// may be called to see if the surface and the image share the same pixelref,
+// in which case the surface may need to perform a copy-on-write.
+extern SkPixelRef* SkBitmapImageGetPixelRef(SkImage* rasterImage);
+
 #endif
index 216f094..daed241 100644 (file)
@@ -59,6 +59,8 @@ public:
     // exposed for SkSurface_Raster via SkNewImageFromPixelRef
     SkImage_Raster(const SkImage::Info&, SkPixelRef*, size_t rowBytes);
 
+    SkPixelRef* getPixelRef() const { return fBitmap.pixelRef(); }
+
 private:
     SkImage_Raster() : INHERITED(0, 0) {}
 
@@ -93,8 +95,6 @@ SkImage_Raster::SkImage_Raster(const Info& info, SkColorSpace* cs,
 
 SkImage_Raster::SkImage_Raster(const Info& info, SkPixelRef* pr, size_t rowBytes)
 : INHERITED(info.fWidth, info.fHeight) {
-    SkASSERT(pr->isImmutable());
-
     bool isOpaque;
     SkBitmap::Config config = SkImageInfoToBitmapConfig(info, &isOpaque);
 
@@ -159,3 +159,7 @@ SkImage* SkNewImageFromPixelRef(const SkImage::Info& info, SkPixelRef* pr,
     return SkNEW_ARGS(SkImage_Raster, (info, pr, rowBytes));
 }
 
+SkPixelRef* SkBitmapImageGetPixelRef(SkImage* image) {
+    return ((SkImage_Raster*)image)->getPixelRef();
+}
+
index 299a8d2..7b82a34 100644 (file)
@@ -9,15 +9,28 @@
 #include "SkImagePriv.h"
 #include "SkCanvas.h"
 
-SK_DEFINE_INST_COUNT(SkSurface)
+//SK_DEFINE_INST_COUNT(SkSurface)
 
 ///////////////////////////////////////////////////////////////////////////////
 
+void SkSurface_Base::installIntoCanvasForDirtyNotification() {
+    if (fCachedCanvas) {
+        fCachedCanvas->setSurfaceBase(this);
+    }
+}
+
 SkSurface_Base::SkSurface_Base(int width, int height) : INHERITED(width, height) {
     fCachedCanvas = NULL;
+    fCachedImage = NULL;
 }
 
 SkSurface_Base::~SkSurface_Base() {
+    // in case the canvas outsurvives us, we null the callback
+    if (fCachedCanvas) {
+        fCachedCanvas->setSurfaceBase(NULL);
+    }
+
+    SkSafeUnref(fCachedImage);
     SkSafeUnref(fCachedCanvas);
 }
 
@@ -30,6 +43,55 @@ void SkSurface_Base::onDraw(SkCanvas* canvas, SkScalar x, SkScalar y,
     }
 }
 
+void SkSurface_Base::onCopyOnWrite(SkImage*, SkCanvas*) {}
+
+SkCanvas* SkSurface_Base::getCachedCanvas() {
+    if (NULL == fCachedCanvas) {
+        fCachedCanvas = this->onNewCanvas();
+        this->installIntoCanvasForDirtyNotification();
+    }
+    return fCachedCanvas;
+}
+
+SkImage* SkSurface_Base::getCachedImage() {
+    if (NULL == fCachedImage) {
+        fCachedImage = this->onNewImageShapshot();
+        this->installIntoCanvasForDirtyNotification();
+    }
+    return fCachedImage;
+}
+
+void SkSurface_Base::aboutToDraw(SkCanvas* canvas) {
+    this->dirtyGenerationID();
+
+    if (canvas) {
+        SkASSERT(canvas == fCachedCanvas);
+        SkASSERT(canvas->getSurfaceBase() == this);
+        canvas->setSurfaceBase(NULL);
+    }
+
+    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);
+        }
+
+        // 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);
 }
@@ -42,16 +104,29 @@ SkSurface::SkSurface(int width, int height) : fWidth(width), fHeight(height) {
     fGenerationID = 0;
 }
 
+uint32_t SkSurface::generationID() {
+    if (0 == fGenerationID) {
+        fGenerationID = asSB(this)->newGenerationID();
+    }
+    return fGenerationID;
+}
+
+void SkSurface::notifyContentChanged() {
+    asSB(this)->aboutToDraw(NULL);
+}
+
 SkCanvas* SkSurface::getCanvas() {
     return asSB(this)->getCachedCanvas();
 }
 
-SkSurface* SkSurface::newSurface(const SkImage::Info& info, SkColorSpace* cs) {
-    return asSB(this)->onNewSurface(info, cs);
+SkImage* SkSurface::newImageShapshot() {
+    SkImage* image = asSB(this)->getCachedImage();
+    SkSafeRef(image);   // the caller will call unref() to balance this
+    return image;
 }
 
-SkImage* SkSurface::newImageShapshot() {
-    return asSB(this)->onNewImageShapshot();
+SkSurface* SkSurface::newSurface(const SkImage::Info& info, SkColorSpace* cs) {
+    return asSB(this)->onNewSurface(info, cs);
 }
 
 void SkSurface::draw(SkCanvas* canvas, SkScalar x, SkScalar y,
index 8166c8c..52b494f 100644 (file)
@@ -45,18 +45,29 @@ public:
     virtual void onDraw(SkCanvas*, SkScalar x, SkScalar y, const SkPaint*);
 
     /**
-     *  Returns a the result of onNewCanvas(), but caches it so that only one
-     *  canvas never ever be created.
+     *  If the surface is about to change, we call this so that our subclass
+     *  can optionally fork their backend (copy-on-write) in case it was
+     *  being shared with the cachedImage.
+     *
+     *  The default implementation does nothing.
      */
-    SkCanvas* getCachedCanvas() {
-        if (NULL == fCachedCanvas) {
-            fCachedCanvas = this->onNewCanvas();
-        }
-        return fCachedCanvas;
-    }
+    virtual void onCopyOnWrite(SkImage* cachedImage, SkCanvas*);
+
+    inline SkCanvas* getCachedCanvas();
+    inline SkImage* getCachedImage();
+
+    // called by SkSurface to compute a new genID
+    uint32_t newGenerationID();
 
 private:
     SkCanvas*   fCachedCanvas;
+    SkImage*    fCachedImage;
+
+    void aboutToDraw(SkCanvas*);
+    friend class SkCanvas;
+    friend class SkSurface;
+
+    inline void installIntoCanvasForDirtyNotification();
 
     typedef SkSurface INHERITED;
 };
index c4ac959..8afab6b 100644 (file)
@@ -8,6 +8,7 @@
 #include "SkSurface_Base.h"
 #include "SkImagePriv.h"
 #include "SkCanvas.h"
+#include "SkDevice.h"
 #include "SkMallocPixelRef.h"
 
 static const size_t kIgnoreRowBytesValue = (size_t)~0;
@@ -24,6 +25,7 @@ public:
     virtual SkImage* onNewImageShapshot() SK_OVERRIDE;
     virtual void onDraw(SkCanvas*, SkScalar x, SkScalar y,
                         const SkPaint*) SK_OVERRIDE;
+    virtual void onCopyOnWrite(SkImage*, SkCanvas*) SK_OVERRIDE;
 
 private:
     SkBitmap    fBitmap;
@@ -89,7 +91,7 @@ SkSurface_Raster::SkSurface_Raster(const SkImage::Info& info, SkColorSpace* cs,
     fBitmap.setConfig(config, info.fWidth, info.fHeight, rb);
     fBitmap.setPixels(pixels);
     fBitmap.setIsOpaque(isOpaque);
-    fWeOwnThePixels = false;
+    fWeOwnThePixels = false;    // We are "Direct"
 }
 
 SkSurface_Raster::SkSurface_Raster(const SkImage::Info& info, SkColorSpace* cs,
@@ -117,18 +119,28 @@ SkSurface* SkSurface_Raster::onNewSurface(const SkImage::Info& info,
     return SkSurface::NewRaster(info, cs);
 }
 
-SkImage* SkSurface_Raster::onNewImageShapshot() {
-    // if we don't own the pixels, we need to make a deep-copy
-    // if we do, we need to perform a copy-on-write the next time
-    // we draw to this bitmap from our canvas...
-    return SkNewImageFromBitmap(fBitmap);
-}
-
 void SkSurface_Raster::onDraw(SkCanvas* canvas, SkScalar x, SkScalar y,
                               const SkPaint* paint) {
     canvas->drawBitmap(fBitmap, x, y, paint);
 }
 
+SkImage* SkSurface_Raster::onNewImageShapshot() {
+    return SkNewImageFromBitmap(fBitmap, fWeOwnThePixels);
+}
+
+void SkSurface_Raster::onCopyOnWrite(SkImage* image, SkCanvas* canvas) {
+    // are we sharing pixelrefs with the image?
+    if (SkBitmapImageGetPixelRef(image) == 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);
+    }
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 
 SkSurface* SkSurface::NewRasterDirect(const SkImage::Info& info,