Proactive SkPixelRef invalidation

src/core/SkPixelRef.cpp

Index: src/core/SkPixelRef.cpp
diff --git a/src/core/SkPixelRef.cpp b/src/core/SkPixelRef.cpp
index 08775f24de96f86a653da58f89bcf29e6561583f..0011f3f1c4d8464402431c116ae8ea5c232b1116 100644
--- a/src/core/SkPixelRef.cpp
+++ b/src/core/SkPixelRef.cpp
@@ -85,12 +85,12 @@ void SkPixelRef::setMutex(SkBaseMutex* mutex) {
 // just need a > 0 value, so pick a funny one to aid in debugging
 #define SKPIXELREF_PRELOCKED_LOCKCOUNT     123456789
 
-SkPixelRef::SkPixelRef(SkBaseMutex* mutex) : fPreLocked(false) {
+SkPixelRef::SkPixelRef(SkBaseMutex* mutex) {
     this->setMutex(mutex);
     fPixels = NULL;
     fColorTable = NULL; // we do not track ownership of this
     fLockCount = 0;
-    fGenerationID = 0;  // signal to rebuild
+    this->needsNewGenID();
     fIsImmutable = false;
     fPreLocked = false;
 }
@@ -103,9 +103,26 @@ SkPixelRef::SkPixelRef(SkFlattenableReadBuffer& buffer, SkBaseMutex* mutex)
     fLockCount = 0;
     fIsImmutable = buffer.readBool();
     fGenerationID = buffer.readUInt();
+    fUniqueGenerationID = false;  // Conservatively assuming the original still exists.
     fPreLocked = false;
 }
 
+SkPixelRef::~SkPixelRef() {
+    this->invalidate();
+}
+
+void SkPixelRef::needsNewGenID() {
+    fGenerationID = 0;
+    fUniqueGenerationID = false;
+}
+
+void SkPixelRef::cloneGenID(const SkPixelRef& that) {
+    // This is subtle.  We must call that.getGenerationID() to make sure it's genID isn't 0.

[scroggo, 2013/10/22 21:04:02]

its*

[mtklein, 2013/10/23 15:28:10]

Done.

+    this->fGenerationID = that.getGenerationID();
+    this->fUniqueGenerationID = false;
+    that.fUniqueGenerationID = false;
+}
+
 void SkPixelRef::setPreLocked(void* pixels, SkColorTable* ctable) {
 #ifndef SK_IGNORE_PIXELREF_SETPRELOCKED
     // only call me in your constructor, otherwise fLockCount tracking can get
@@ -129,6 +146,7 @@ void SkPixelRef::flatten(SkFlattenableWriteBuffer& buffer) const {
         buffer.writeUInt(0);
     } else {
         buffer.writeUInt(fGenerationID);
+        fUniqueGenerationID = false;  // Conservative, a copy is probably about to exist.
     }
 }
 
@@ -178,18 +196,39 @@ bool SkPixelRef::onDecodeInto(int pow2, SkBitmap* bitmap) {
 uint32_t SkPixelRef::getGenerationID() const {
     if (0 == fGenerationID) {
         fGenerationID = SkNextPixelRefGenerationID();
+        fUniqueGenerationID = true;  // The only time we can be sure of this!
     }
     return fGenerationID;
 }
 
+void SkPixelRef::addInvalidationListener(InvalidationListener* listener) {
+    if (NULL == listener || !fUniqueGenerationID) {
+        // No point in tracking this if we're not going to call it.
+        SkDELETE(listener);
+        return;
+    }
+    *fInvalidationListeners.append() = listener;
+}
+
+void SkPixelRef::invalidate() {
+    // We don't invalidate ourselves if we think another SkPixelRef is sharing our genID.
+    if (fUniqueGenerationID) {
+        for (int i = 0; i < fInvalidationListeners.count(); i++) {
+            fInvalidationListeners[i]->onInvalidate();
+        }
+    }
+    // Listeners get at most one shot, so whether these triggered or not, blow them away.

[scroggo, 2013/10/22 21:04:02]

Should these comments go in the header file so a future user knows that the
invalidation listeners may not get called?

[mtklein, 2013/10/23 15:28:10]

See if the header makes more sense now?  Leaving these here as a reminder.

[scroggo, 2013/10/23 15:39:19]

Yes, the header makes more sense now.

+    fInvalidationListeners.deleteAll();
+}
+
 void SkPixelRef::notifyPixelsChanged() {
 #ifdef SK_DEBUG
     if (fIsImmutable) {
         SkDebugf("========== notifyPixelsChanged called on immutable pixelref");
     }
 #endif
-    // this signals us to recompute this next time around
-    fGenerationID = 0;
+    this->invalidate();
+    this->needsNewGenID();
 }
 
 void SkPixelRef::setImmutable() {