From 97af1a64ae6bdddd346d8babfd9f188279dd6644 Mon Sep 17 00:00:00 2001 From: "reed@google.com" Date: Tue, 28 Aug 2012 12:19:02 +0000 Subject: [PATCH] Add caching of the snapshot image form a surface 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 --- gm/image.cpp | 15 ++++++ gyp/core.gyp | 1 + gyp/core.gypi | 13 ++++++ gyp/gmslides.gypi | 1 + include/core/SkCanvas.h | 12 +++++ include/core/SkDevice.h | 6 +++ include/core/SkImage.h | 2 +- include/core/SkSurface.h | 15 ++++-- src/core/SkCanvas.cpp | 13 ++++++ src/core/SkDevice.cpp | 7 +++ src/core/SkPictureRecord.h | 2 + src/image/SkImage.cpp | 2 +- src/image/SkImagePriv.cpp | 4 +- src/image/SkImagePriv.h | 17 ++++++- src/image/SkImage_Raster.cpp | 8 +++- src/image/SkSurface.cpp | 85 ++++++++++++++++++++++++++++++++-- src/image/SkSurface_Base.h | 27 +++++++---- src/image/SkSurface_Raster.cpp | 28 +++++++---- 18 files changed, 224 insertions(+), 34 deletions(-) diff --git a/gm/image.cpp b/gm/image.cpp index 4ee90a4fc6..21286db731 100644 --- a/gm/image.cpp +++ b/gm/image.cpp @@ -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; }; diff --git a/gyp/core.gyp b/gyp/core.gyp index 806626a53d..e23572a387 100644 --- a/gyp/core.gyp +++ b/gyp/core.gyp @@ -17,6 +17,7 @@ '../include/ports', '../include/xml', '../src/core', + '../src/image', ], 'msvs_disabled_warnings': [4244, 4267,4345, 4390, 4554, 4800], 'conditions': [ diff --git a/gyp/core.gypi b/gyp/core.gypi index 4595b6ae1c..99227a3fcd 100644 --- a/gyp/core.gypi +++ b/gyp/core.gypi @@ -160,6 +160,19 @@ '<(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', diff --git a/gyp/gmslides.gypi b/gyp/gmslides.gypi index e36abf56ae..d80c123610 100644 --- a/gyp/gmslides.gypi +++ b/gyp/gmslides.gypi @@ -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', diff --git a/include/core/SkCanvas.h b/include/core/SkCanvas.h index 01e0c75a24..1f29e1c00a 100644 --- a/include/core/SkCanvas.h +++ b/include/core/SkCanvas.h @@ -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() diff --git a/include/core/SkDevice.h b/include/core/SkDevice.h index 326115ca31..41e316c9cc 100644 --- a/include/core/SkDevice.h +++ b/include/core/SkDevice.h @@ -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 diff --git a/include/core/SkImage.h b/include/core/SkImage.h index 985a3bbbbb..b0071e28d8 100644 --- a/include/core/SkImage.h +++ b/include/core/SkImage.h @@ -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, diff --git a/include/core/SkSurface.h b/include/core/SkSurface.h index 8e407b92f6..9f8908a3bd 100644 --- a/include/core/SkSurface.h +++ b/include/core/SkSurface.h @@ -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 diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp index b7c06c3351..f5c6bc2a65 100644 --- a/src/core/SkCanvas.cpp +++ b/src/core/SkCanvas.cpp @@ -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 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() { } + + diff --git a/src/core/SkDevice.cpp b/src/core/SkDevice.cpp index 6146fd6c71..f8bed65e42 100644 --- a/src/core/SkDevice.cpp +++ b/src/core/SkDevice.cpp @@ -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) { diff --git a/src/core/SkPictureRecord.h b/src/core/SkPictureRecord.h index 50cf4db099..a19a084534 100644 --- a/src/core/SkPictureRecord.h +++ b/src/core/SkPictureRecord.h @@ -96,6 +96,8 @@ private: }; void addDraw(DrawType drawType) { + this->predrawNotify(); + #ifdef SK_DEBUG_TRACE SkDebugf("add %s\n", DrawTypeToString(drawType)); #endif diff --git a/src/image/SkImage.cpp b/src/image/SkImage.cpp index b6fc4899da..251721f00d 100644 --- a/src/image/SkImage.cpp +++ b/src/image/SkImage.cpp @@ -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(image); diff --git a/src/image/SkImagePriv.cpp b/src/image/SkImagePriv.cpp index a020eaff14..e7a244ab54 100644 --- a/src/image/SkImagePriv.cpp +++ b/src/image/SkImagePriv.cpp @@ -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(); diff --git a/src/image/SkImagePriv.h b/src/image/SkImagePriv.h index c0aff983a2..9332abc7af 100644 --- a/src/image/SkImagePriv.h +++ b/src/image/SkImagePriv.h @@ -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 diff --git a/src/image/SkImage_Raster.cpp b/src/image/SkImage_Raster.cpp index 216f094569..daed241360 100644 --- a/src/image/SkImage_Raster.cpp +++ b/src/image/SkImage_Raster.cpp @@ -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(); +} + diff --git a/src/image/SkSurface.cpp b/src/image/SkSurface.cpp index 299a8d22e0..7b82a340da 100644 --- a/src/image/SkSurface.cpp +++ b/src/image/SkSurface.cpp @@ -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(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, diff --git a/src/image/SkSurface_Base.h b/src/image/SkSurface_Base.h index 8166c8c435..52b494f237 100644 --- a/src/image/SkSurface_Base.h +++ b/src/image/SkSurface_Base.h @@ -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; }; diff --git a/src/image/SkSurface_Raster.cpp b/src/image/SkSurface_Raster.cpp index c4ac959730..8afab6b01c 100644 --- a/src/image/SkSurface_Raster.cpp +++ b/src/image/SkSurface_Raster.cpp @@ -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,