diff --git a/include/private/SkPathRef.h b/include/private/SkPathRef.h index 92568d8ac383f..d250fc37dd3e2 100644 --- a/include/private/SkPathRef.h +++ b/include/private/SkPathRef.h @@ -309,9 +309,24 @@ class SK_API SkPathRef final : public SkNVRefCnt { */ uint32_t genID() const; - struct GenIDChangeListener : SkRefCnt { + class GenIDChangeListener : public SkRefCnt { + public: + GenIDChangeListener() : fShouldUnregisterFromPath(false) {} virtual ~GenIDChangeListener() {} + virtual void onChange() = 0; + + // The caller can use this method to notify the path that it no longer needs to listen. Once + // called, the path will remove this listener from the list at some future point. + void markShouldUnregisterFromPath() { + fShouldUnregisterFromPath.store(true, std::memory_order_relaxed); + } + bool shouldUnregisterFromPath() { + return fShouldUnregisterFromPath.load(std::memory_order_acquire); + } + + private: + std::atomic fShouldUnregisterFromPath; }; void addGenIDChangeListener(sk_sp); // Threadsafe. diff --git a/src/core/SkPathRef.cpp b/src/core/SkPathRef.cpp index f767323e5c8ea..9d49e3215cf41 100644 --- a/src/core/SkPathRef.cpp +++ b/src/core/SkPathRef.cpp @@ -93,6 +93,7 @@ SkPathRef::~SkPathRef() { // Deliberately don't validate() this path ref, otherwise there's no way // to read one that's not valid and then free its memory without asserting. this->callGenIDChangeListeners(); + SkASSERT(fGenIDChangeListeners.empty()); // These are raw ptrs. sk_free(fPoints); SkDEBUGCODE(fPoints = nullptr;) @@ -707,19 +708,33 @@ void SkPathRef::addGenIDChangeListener(sk_sp listener) { if (nullptr == listener || this == gEmpty) { return; } + SkAutoMutexAcquire lock(fGenIDChangeListenersMutex); + + // Clean out any stale listeners before we append the new one. + for (int i = 0; i < fGenIDChangeListeners.count(); ++i) { + if (fGenIDChangeListeners[i]->shouldUnregisterFromPath()) { + fGenIDChangeListeners[i]->unref(); + fGenIDChangeListeners.removeShuffle(i--); // No need to preserve the order after i. + } + } + + SkASSERT(!listener->shouldUnregisterFromPath()); *fGenIDChangeListeners.append() = listener.release(); } // we need to be called *before* the genID gets changed or zerod void SkPathRef::callGenIDChangeListeners() { SkAutoMutexAcquire lock(fGenIDChangeListenersMutex); - for (int i = 0; i < fGenIDChangeListeners.count(); i++) { - fGenIDChangeListeners[i]->onChange(); + for (GenIDChangeListener* listener : fGenIDChangeListeners) { + if (!listener->shouldUnregisterFromPath()) { + listener->onChange(); + } + // Listeners get at most one shot, so whether these triggered or not, blow them away. + listener->unref(); } - // Listeners get at most one shot, so whether these triggered or not, blow them away. - fGenIDChangeListeners.unrefAll(); + fGenIDChangeListeners.reset(); } SkRRect SkPathRef::getRRect() const { diff --git a/src/gpu/ccpr/GrCCPathCache.cpp b/src/gpu/ccpr/GrCCPathCache.cpp index 383275daa0837..839cba4cfec4d 100644 --- a/src/gpu/ccpr/GrCCPathCache.cpp +++ b/src/gpu/ccpr/GrCCPathCache.cpp @@ -163,6 +163,7 @@ inline void GrCCPathCache::HashNode::willExitHashTable() { SkASSERT(fPathCache->fLRU.isInList(fEntry.get())); fEntry->invalidateAtlas(); // Must happen on graphics thread. + fEntry->markShouldUnregisterFromPath(); fPathCache->fLRU.remove(fEntry.get()); } @@ -237,13 +238,13 @@ sk_sp GrCCPathCache::find(const GrShape& shape, const MaskTr } void GrCCPathCache::evict(GrCCPathCacheEntry* entry) { - SkASSERT(SkGetThreadID() == fGraphicsThreadID); + // Has the entry already been evicted? (We mark "shouldUnregisterFromPath" upon eviction.) + bool isInCache = !entry->shouldUnregisterFromPath(); + SkDEBUGCODE(HashNode* entryKeyNode = fHashTable.find(HashNode::GetKey(entry))); + SkASSERT((entryKeyNode && entryKeyNode->entry() == entry) == isInCache); + SkASSERT(fLRU.isInList(entry) == isInCache); - bool isInCache = entry->fNext || (fLRU.tail() == entry); - SkASSERT(isInCache == fLRU.isInList(entry)); if (isInCache) { - SkDEBUGCODE(HashNode* node = fHashTable.find(HashNode::GetKey(entry))); - SkASSERT(node && node->entry() == entry); fHashTable.remove(HashNode::GetKey(entry)); // HashNode::willExitHashTable() takes care of the rest. }