Remove Picture deletion listeners.

Looks like we can just have ~SkPicture put the message on the bus directly.

BUG=skia:3144

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

diff --git a/include/core/SkPicture.h b/include/core/SkPicture.h
index dedda3e..ebaeef0 100644 (file)
--- a/include/core/SkPicture.h
+++ b/include/core/SkPicture.h
@@ -180,14 +180,6 @@ public:
     bool suitableForGpuRasterization(GrContext*, const char ** = NULL) const;
 #endif
 
-    class DeletionListener : public SkRefCnt {
-    public:
-        virtual void onDeletion(uint32_t pictureID) = 0;
-    };
-
-    // Takes ref on listener.
-    void addDeletionListener(DeletionListener* listener) const;
-
     /** Return the approximate number of operations in this picture.  This
      *  number may be greater or less than the number of SkCanvas calls
      *  recorded: some calls may be recorded as more than one operation, or some
@@ -212,6 +204,9 @@ public:
         int fCount;
     };
 
+    // Sent via SkMessageBus from destructor.
+    struct DeletionMessage { int32_t fUniqueID; };
+
 private:
     // V2 : adds SkPixelRef's generation ID.
     // V3 : PictInfo tag at beginning, and EOF tag at the end
@@ -259,8 +254,6 @@ private:
     static const uint32_t MIN_PICTURE_VERSION = 19;
     static const uint32_t CURRENT_PICTURE_VERSION = 37;
 
-    void callDeletionListeners();
-
     void createHeader(SkPictInfo* info) const;
     static bool IsValidPictInfo(const SkPictInfo& info);
 
@@ -275,7 +268,6 @@ private:
     const uint32_t                        fUniqueID;
     const SkRect                          fCullRect;
     mutable SkAutoTUnref<const AccelData> fAccelData;
-    mutable SkTDArray<DeletionListener*> fDeletionListeners;  // pointers are refed
     SkAutoTDelete<const SkRecord>       fRecord;
     SkAutoTUnref<const SkBBoxHierarchy> fBBH;
     SkAutoTDelete<const SnapshotArray>  fDrawablePicts;

diff --git a/src/core/SkPicture.cpp b/src/core/SkPicture.cpp
index b24f719..b4c3063 100644 (file)
--- a/src/core/SkPicture.cpp
+++ b/src/core/SkPicture.cpp
@@ -17,6 +17,7 @@
 #include "SkCanvas.h"
 #include "SkChunkAlloc.h"
 #include "SkDrawPictureCallback.h"
+#include "SkMessageBus.h"
 #include "SkPaintPriv.h"
 #include "SkPathEffect.h"
 #include "SkPicture.h"
@@ -41,6 +42,8 @@
 #include "SkRecordOpts.h"
 #include "SkRecorder.h"
 
+DECLARE_SKMESSAGEBUS_MESSAGE(SkPicture::DeletionMessage);
+
 template <typename T> int SafeCount(const T* obj) {
     return obj ? obj->count() : 0;
 }
@@ -278,7 +281,9 @@ SkPicture const* const* SkPicture::drawablePicts() const {
 }
 
 SkPicture::~SkPicture() {
-    this->callDeletionListeners();
+    SkPicture::DeletionMessage msg;
+    msg.fUniqueID = this->uniqueID();
+    SkMessageBus<SkPicture::DeletionMessage>::Post(msg);
 }
 
 void SkPicture::EXPERIMENTAL_addAccelData(const SkPicture::AccelData* data) const {
@@ -526,21 +531,3 @@ SkPicture::SkPicture(const SkRect& cullRect, SkRecord* record, SnapshotArray* dr
     , fDrawablePicts(drawablePicts)
     , fAnalysis(*fRecord)
 {}
-
-// Note that we are assuming that this entry point will only be called from
-// one thread. Currently the only client of this method is
-// SkGpuDevice::EXPERIMENTAL_optimize which should be only called from a single
-// thread.
-void SkPicture::addDeletionListener(DeletionListener* listener) const {
-    SkASSERT(listener);
-
-    *fDeletionListeners.append() = SkRef(listener);
-}
-
-void SkPicture::callDeletionListeners() {
-    for (int i = 0; i < fDeletionListeners.count(); ++i) {
-        fDeletionListeners[i]->onDeletion(this->uniqueID());
-    }
-
-    fDeletionListeners.unrefAll();
-}

diff --git a/src/gpu/GrLayerCache.cpp b/src/gpu/GrLayerCache.cpp
index e4e086a..6b17675 100644 (file)
--- a/src/gpu/GrLayerCache.cpp
+++ b/src/gpu/GrLayerCache.cpp
@@ -10,8 +10,6 @@
 #include "GrLayerCache.h"
 #include "GrSurfacePriv.h"
 
-DECLARE_SKMESSAGEBUS_MESSAGE(GrPictureDeletedMessage);
-
 #ifdef SK_DEBUG
 void GrCachedLayer::validate(const GrTexture* backingTexture) const {
     SkASSERT(SK_InvalidGenID != fKey.pictureID());
@@ -56,7 +54,7 @@ void GrCachedLayer::validate(const GrTexture* backingTexture) const {
 
 class GrAutoValidateLayer : ::SkNoncopyable {
 public:
-    GrAutoValidateLayer(GrTexture* backingTexture, const GrCachedLayer* layer) 
+    GrAutoValidateLayer(GrTexture* backingTexture, const GrCachedLayer* layer)
         : fBackingTexture(backingTexture)
         , fLayer(layer) {
         if (fLayer) {
@@ -96,8 +94,8 @@ GrLayerCache::~GrLayerCache() {
 
     SkASSERT(0 == fPictureHash.count());
 
-    // The atlas only lets go of its texture when the atlas is deleted. 
-    fAtlas.free();    
+    // The atlas only lets go of its texture when the atlas is deleted.
+    fAtlas.free();
 }
 
 void GrLayerCache::initAtlas() {
@@ -120,12 +118,12 @@ void GrLayerCache::freeAll() {
     }
     fLayerHash.rewind();
 
-    // The atlas only lets go of its texture when the atlas is deleted. 
+    // The atlas only lets go of its texture when the atlas is deleted.
     fAtlas.free();
 }
 
-GrCachedLayer* GrLayerCache::createLayer(uint32_t pictureID, 
-                                         int start, int stop, 
+GrCachedLayer* GrLayerCache::createLayer(uint32_t pictureID,
+                                         int start, int stop,
                                          const SkIRect& bounds,
                                          const SkMatrix& ctm,
                                          const SkPaint* paint) {
@@ -137,7 +135,7 @@ GrCachedLayer* GrLayerCache::createLayer(uint32_t pictureID,
 }
 
 GrCachedLayer* GrLayerCache::findLayer(uint32_t pictureID,
-                                       int start, 
+                                       int start,
                                        const SkIRect& bounds,
                                        const SkMatrix& ctm) {
     SkASSERT(pictureID != SK_InvalidGenID && start > 0);
@@ -158,8 +156,8 @@ GrCachedLayer* GrLayerCache::findLayerOrCreate(uint32_t pictureID,
     return layer;
 }
 
-bool GrLayerCache::tryToAtlas(GrCachedLayer* layer, 
-                              const GrSurfaceDesc& desc, 
+bool GrLayerCache::tryToAtlas(GrCachedLayer* layer,
+                              const GrSurfaceDesc& desc,
                               bool* needsRendering) {
     SkDEBUGCODE(GrAutoValidateLayer avl(fAtlas ? fAtlas->getTexture() : NULL, layer);)
 
@@ -220,7 +218,7 @@ bool GrLayerCache::tryToAtlas(GrCachedLayer* layer,
                 return true;
             }
 
-            // The layer was rejected by the atlas (even though we know it is 
+            // The layer was rejected by the atlas (even though we know it is
             // plausibly atlas-able). See if a plot can be purged and try again.
             if (!this->purgePlot()) {
                 break;  // We weren't able to purge any plots
@@ -282,7 +280,7 @@ void GrLayerCache::unlock(GrCachedLayer* layer) {
                 SkDELETE(pictInfo);
             }
         }
-        
+
         layer->setPlot(NULL);
         layer->setTexture(NULL, GrIRect16::MakeEmpty());
 #endif
@@ -324,7 +322,7 @@ void GrLayerCache::validate() const {
             if (layer->locked()) {
                 plotLocks[layer->plot()->id()]++;
             }
-        } 
+        }
     }
 
     for (int i = 0; i < kNumPlotsX*kNumPlotsY; ++i) {
@@ -456,27 +454,12 @@ void GrLayerCache::purgeAll() {
 }
 #endif
 
-class GrPictureDeletionListener : public SkPicture::DeletionListener {
-    virtual void onDeletion(uint32_t pictureID) SK_OVERRIDE{
-        const GrPictureDeletedMessage message = { pictureID };
-        SkMessageBus<GrPictureDeletedMessage>::Post(message);
-    }
-};
-
-void GrLayerCache::trackPicture(const SkPicture* picture) {
-    if (NULL == fDeletionListener) {
-        fDeletionListener.reset(SkNEW(GrPictureDeletionListener));
-    }
-
-    picture->addDeletionListener(fDeletionListener);
-}
-
 void GrLayerCache::processDeletedPictures() {
-    SkTDArray<GrPictureDeletedMessage> deletedPictures;
+    SkTDArray<SkPicture::DeletionMessage> deletedPictures;
     fPictDeletionInbox.poll(&deletedPictures);
 
     for (int i = 0; i < deletedPictures.count(); i++) {
-        this->purge(deletedPictures[i].pictureID);
+        this->purge(deletedPictures[i].fUniqueID);
     }
 }
 

diff --git a/src/gpu/GrLayerCache.h b/src/gpu/GrLayerCache.h
index 8dec8e5..75e9130 100644 (file)
--- a/src/gpu/GrLayerCache.h
+++ b/src/gpu/GrLayerCache.h
@@ -19,12 +19,7 @@
 // Set to 0 to disable caching of hoisted layers
 #define GR_CACHE_HOISTED_LAYERS 0
 
-// The layer cache listens for these messages to purge picture-related resources.
-struct GrPictureDeletedMessage {
-    uint32_t pictureID;
-};
-
-// GrPictureInfo stores the atlas plots used by a single picture. A single 
+// GrPictureInfo stores the atlas plots used by a single picture. A single
 // plot may be used to store layers from multiple pictures.
 struct GrPictureInfo {
 public:
@@ -262,9 +257,6 @@ public:
         }
     }
 
-    // Setup to be notified when 'picture' is deleted
-    void trackPicture(const SkPicture* picture);
-
     // Cleanup after any SkPicture deletions
     void processDeletedPictures();
 
@@ -304,9 +296,7 @@ private:
 
     SkTDynamicHash<GrCachedLayer, GrCachedLayer::Key> fLayerHash;
 
-    SkMessageBus<GrPictureDeletedMessage>::Inbox fPictDeletionInbox;
-
-    SkAutoTUnref<SkPicture::DeletionListener> fDeletionListener;
+    SkMessageBus<SkPicture::DeletionMessage>::Inbox fPictDeletionInbox;
 
     // This implements a plot-centric locking mechanism (since the atlas
     // backing texture is always locked). Each layer that is locked (i.e.,

diff --git a/src/utils/SkPictureUtils.cpp b/src/utils/SkPictureUtils.cpp
index 73c61bb..7f9409a 100644 (file)
--- a/src/utils/SkPictureUtils.cpp
+++ b/src/utils/SkPictureUtils.cpp
@@ -232,10 +232,6 @@ size_t SkPictureUtils::ApproximateBytesUsed(const SkPicture* pict) {
     if (pict->fBBH.get()) {
         byteCount += pict->fBBH->bytesUsed();
     }
-    byteCount +=
-        pict->fDeletionListeners.reserved() * sizeof(SkPicture::DeletionListener*) +
-        pict->fDeletionListeners.count() * sizeof(SkPicture::DeletionListener);
-
     MeasureRecords visitor;
     for (unsigned curOp = 0; curOp < pict->fRecord->count(); curOp++) {
         byteCount += pict->fRecord->visit<size_t>(curOp, visitor);

diff --git a/tests/GpuLayerCacheTest.cpp b/tests/GpuLayerCacheTest.cpp
index 277590e..1043a20 100644 (file)
--- a/tests/GpuLayerCacheTest.cpp
+++ b/tests/GpuLayerCacheTest.cpp
@@ -53,8 +53,6 @@ static void create_layers(skiatest::Reporter* reporter,
         REPORTER_ASSERT(reporter, NULL == layer->paint());
         REPORTER_ASSERT(reporter, !layer->isAtlased());
     }
-
-    cache->trackPicture(&picture);
 }
 
 static void lock_layer(skiatest::Reporter* reporter,

diff --git a/tests/PictureTest.cpp b/tests/PictureTest.cpp
index ce10f56..77fed2f 100644 (file)
--- a/tests/PictureTest.cpp
+++ b/tests/PictureTest.cpp
@@ -1011,7 +1011,7 @@ static void test_savelayer_extraction(skiatest::Reporter* reporter) {
                                   kHeight/2.0 == info1.fBounds.height());
         REPORTER_ASSERT(reporter, info1.fLocalMat.isIdentity());
         REPORTER_ASSERT(reporter, info1.fPreMat.isIdentity());
-        REPORTER_ASSERT(reporter, kWidth/2.0 == info1.fBounds.fLeft && 
+        REPORTER_ASSERT(reporter, kWidth/2.0 == info1.fBounds.fLeft &&
                                   kHeight/2.0 == info1.fBounds.fTop);
         REPORTER_ASSERT(reporter, NULL == info1.fPaint);
         REPORTER_ASSERT(reporter, !info1.fIsNested &&
@@ -1733,7 +1733,7 @@ static void test_bytes_used(skiatest::Reporter* reporter) {
                               sizeof(SkPicture) + sizeof(SkRecord));
 
     // Protect against any unintentional bloat.
-    REPORTER_ASSERT(reporter, SkPictureUtils::ApproximateBytesUsed(empty.get()) <= 144);
+    REPORTER_ASSERT(reporter, SkPictureUtils::ApproximateBytesUsed(empty.get()) <= 128);
 
     // Sanity check of nested SkPictures.
     SkPictureRecorder r2;