Make atomic lists list for bitmaps and paths

Make the listner list for bitmas and paths use a simple atomic
stack. This stack implementation does not have the ABA problem
because you can only pop the entire stack at once
with exchange(nullptr).

BUG=skia:8324

Change-Id: I7b0438a42c473b36cd4b0cbf236bf1692c5afab3
Reviewed-on: https://skia-review.googlesource.com/154861
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Herb Derby <herb@google.com>
This commit is contained in:
Herb Derby 2018-09-17 12:07:02 -04:00 committed by Skia Commit-Bot
parent 90d2d9381e
commit 6c8d242b14
4 changed files with 32 additions and 12 deletions

View File

@ -86,6 +86,7 @@ public:
struct GenIDChangeListener { struct GenIDChangeListener {
virtual ~GenIDChangeListener() {} virtual ~GenIDChangeListener() {}
virtual void onChange() = 0; virtual void onChange() = 0;
GenIDChangeListener* next;
}; };
// Takes ownership of listener. // Takes ownership of listener.
@ -116,7 +117,7 @@ private:
const uint32_t fStableID; const uint32_t fStableID;
#endif #endif
SkTDArray<GenIDChangeListener*> fGenIDChangeListeners; // pointers are owned std::atomic<GenIDChangeListener*> fGenIDChangeListeners{nullptr}; // pointers are owned
// Set true by caches when they cache content that's derived from the current pixels. // Set true by caches when they cache content that's derived from the current pixels.
std::atomic<bool> fAddedToCache; std::atomic<bool> fAddedToCache;

View File

@ -8,6 +8,8 @@
#ifndef SkPathRef_DEFINED #ifndef SkPathRef_DEFINED
#define SkPathRef_DEFINED #define SkPathRef_DEFINED
#include <atomic>
#include "SkAtomics.h" #include "SkAtomics.h"
#include "SkMatrix.h" #include "SkMatrix.h"
#include "SkPoint.h" #include "SkPoint.h"
@ -311,6 +313,7 @@ public:
struct GenIDChangeListener : SkRefCnt { struct GenIDChangeListener : SkRefCnt {
virtual ~GenIDChangeListener() {} virtual ~GenIDChangeListener() {}
virtual void onChange() = 0; virtual void onChange() = 0;
GenIDChangeListener* next;
}; };
void addGenIDChangeListener(sk_sp<GenIDChangeListener>); void addGenIDChangeListener(sk_sp<GenIDChangeListener>);
@ -544,7 +547,7 @@ private:
mutable uint32_t fGenerationID; mutable uint32_t fGenerationID;
SkDEBUGCODE(int32_t fEditorsAttached;) // assert that only one editor in use at any time. SkDEBUGCODE(int32_t fEditorsAttached;) // assert that only one editor in use at any time.
SkTDArray<GenIDChangeListener*> fGenIDChangeListeners; // pointers are reffed std::atomic<GenIDChangeListener*> fGenIDChangeListeners{nullptr}; // pointers are reffed
mutable uint8_t fBoundsIsDirty; mutable uint8_t fBoundsIsDirty;
mutable bool fIsFinite; // only meaningful if bounds are valid mutable bool fIsFinite; // only meaningful if bounds are valid

View File

@ -700,20 +700,28 @@ uint32_t SkPathRef::genID() const {
} }
void SkPathRef::addGenIDChangeListener(sk_sp<GenIDChangeListener> listener) { void SkPathRef::addGenIDChangeListener(sk_sp<GenIDChangeListener> listener) {
if (nullptr == listener || this == gEmpty) { if (nullptr == listener || this == gEmpty) {
return; return;
} }
*fGenIDChangeListeners.append() = listener.release();
SkPathRef::GenIDChangeListener* ptr = listener.release();
ptr->next = fGenIDChangeListeners;
while (!fGenIDChangeListeners.compare_exchange_weak(ptr->next, ptr)) {}
} }
// we need to be called *before* the genID gets changed or zerod // we need to be called *before* the genID gets changed or zerod
void SkPathRef::callGenIDChangeListeners() { void SkPathRef::callGenIDChangeListeners() {
for (int i = 0; i < fGenIDChangeListeners.count(); i++) {
fGenIDChangeListeners[i]->onChange();
}
// Listeners get at most one shot, so whether these triggered or not, blow them away. SkPathRef::GenIDChangeListener* ptr = fGenIDChangeListeners.exchange(nullptr);
fGenIDChangeListeners.unrefAll();
while (ptr != nullptr) {
ptr->onChange();
auto next = ptr->next;
// Listeners get at most one shot, so whether these triggered or not, blow them away.
ptr->unref();
ptr = next;
}
} }
SkRRect SkPathRef::getRRect() const { SkRRect SkPathRef::getRRect() const {

View File

@ -92,15 +92,19 @@ void SkPixelRef::addGenIDChangeListener(GenIDChangeListener* listener) {
delete listener; delete listener;
return; return;
} }
*fGenIDChangeListeners.append() = listener;
listener->next = fGenIDChangeListeners;
while (!fGenIDChangeListeners.compare_exchange_weak(listener->next, listener)) {}
} }
// we need to be called *before* the genID gets changed or zerod // we need to be called *before* the genID gets changed or zerod
void SkPixelRef::callGenIDChangeListeners() { void SkPixelRef::callGenIDChangeListeners() {
// We don't invalidate ourselves if we think another SkPixelRef is sharing our genID. // We don't invalidate ourselves if we think another SkPixelRef is sharing our genID.
GenIDChangeListener* head = fGenIDChangeListeners.exchange(nullptr);
if (this->genIDIsUnique()) { if (this->genIDIsUnique()) {
for (int i = 0; i < fGenIDChangeListeners.count(); i++) { for (auto cursor = head; cursor != nullptr; cursor = cursor->next) {
fGenIDChangeListeners[i]->onChange(); cursor->onChange();
} }
// TODO: SkAtomic could add "old_value = atomic.xchg(new_value)" to make this clearer. // TODO: SkAtomic could add "old_value = atomic.xchg(new_value)" to make this clearer.
@ -110,7 +114,11 @@ void SkPixelRef::callGenIDChangeListeners() {
} }
} }
// Listeners get at most one shot, so whether these triggered or not, blow them away. // Listeners get at most one shot, so whether these triggered or not, blow them away.
fGenIDChangeListeners.deleteAll(); for (auto cursor = head; cursor != nullptr;) {
auto next = cursor->next;
delete cursor;
cursor = next;
}
} }
void SkPixelRef::notifyPixelsChanged() { void SkPixelRef::notifyPixelsChanged() {