Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(653)

Side by Side Diff: src/core/SkPixelRef.cpp

Issue 955043002: Steal a bit from the gen ID instead of managing two atomic values. (Closed) Base URL: https://skia.googlesource.com/skia.git@master
Patch Set: comment raciness Created 5 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
« no previous file with comments | « include/core/SkPixelRef.h ('k') | no next file » | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 /* 1 /*
2 * Copyright 2011 Google Inc. 2 * Copyright 2011 Google Inc.
3 * 3 *
4 * Use of this source code is governed by a BSD-style license that can be 4 * Use of this source code is governed by a BSD-style license that can be
5 * found in the LICENSE file. 5 * found in the LICENSE file.
6 */ 6 */
7 7
8 #include "SkBitmapCache.h" 8 #include "SkBitmapCache.h"
9 #include "SkPixelRef.h" 9 #include "SkPixelRef.h"
10 #include "SkThread.h" 10 #include "SkThread.h"
(...skipping 38 matching lines...) Expand 10 before | Expand all | Expand 10 after
49 SkASSERT(SkIsPow2(PIXELREF_MUTEX_RING_COUNT)); 49 SkASSERT(SkIsPow2(PIXELREF_MUTEX_RING_COUNT));
50 50
51 // atomic_inc might be overkill here. It may be fine if once in a while 51 // atomic_inc might be overkill here. It may be fine if once in a while
52 // we hit a race-condition and two subsequent calls get the same index... 52 // we hit a race-condition and two subsequent calls get the same index...
53 int index = sk_atomic_inc(&gPixelRefMutexRingIndex); 53 int index = sk_atomic_inc(&gPixelRefMutexRingIndex);
54 return &gPixelRefMutexRing[index & (PIXELREF_MUTEX_RING_COUNT - 1)]; 54 return &gPixelRefMutexRing[index & (PIXELREF_MUTEX_RING_COUNT - 1)];
55 } 55 }
56 56
57 /////////////////////////////////////////////////////////////////////////////// 57 ///////////////////////////////////////////////////////////////////////////////
58 58
59 int32_t SkNextPixelRefGenerationID(); 59 static uint32_t next_gen_id() {
60 60 static uint32_t gNextGenID = 0;
61 int32_t SkNextPixelRefGenerationID() { 61 uint32_t genID;
62 static int32_t gPixelRefGenerationID; 62 // Loop in case our global wraps around, as we never want to return a 0.
63 // do a loop in case our global wraps around, as we never want to
64 // return a 0
65 int32_t genID;
66 do { 63 do {
67 genID = sk_atomic_inc(&gPixelRefGenerationID) + 1; 64 genID = sk_atomic_fetch_add(&gNextGenID, 2u) + 2; // Never set the low bit.
68 } while (0 == genID); 65 } while (0 == genID);
69 return genID; 66 return genID;
70 } 67 }
71 68
72 /////////////////////////////////////////////////////////////////////////////// 69 ///////////////////////////////////////////////////////////////////////////////
73 70
74 void SkPixelRef::setMutex(SkBaseMutex* mutex) { 71 void SkPixelRef::setMutex(SkBaseMutex* mutex) {
75 if (NULL == mutex) { 72 if (NULL == mutex) {
76 mutex = get_default_mutex(); 73 mutex = get_default_mutex();
77 } 74 }
78 fMutex = mutex; 75 fMutex = mutex;
79 } 76 }
80 77
81 // just need a > 0 value, so pick a funny one to aid in debugging 78 // just need a > 0 value, so pick a funny one to aid in debugging
82 #define SKPIXELREF_PRELOCKED_LOCKCOUNT 123456789 79 #define SKPIXELREF_PRELOCKED_LOCKCOUNT 123456789
83 80
84 static SkImageInfo validate_info(const SkImageInfo& info) { 81 static SkImageInfo validate_info(const SkImageInfo& info) {
85 SkAlphaType newAlphaType = info.alphaType(); 82 SkAlphaType newAlphaType = info.alphaType();
86 SkAssertResult(SkColorTypeValidateAlphaType(info.colorType(), info.alphaType (), &newAlphaType)); 83 SkAssertResult(SkColorTypeValidateAlphaType(info.colorType(), info.alphaType (), &newAlphaType));
87 return info.makeAlphaType(newAlphaType); 84 return info.makeAlphaType(newAlphaType);
88 } 85 }
89 86
90 SkPixelRef::SkPixelRef(const SkImageInfo& info) 87 SkPixelRef::SkPixelRef(const SkImageInfo& info)
91 : fInfo(validate_info(info)) 88 : fInfo(validate_info(info))
92 #ifdef SK_BUILD_FOR_ANDROID_FRAMEWORK 89 #ifdef SK_BUILD_FOR_ANDROID_FRAMEWORK
93 , fStableID(SkNextPixelRefGenerationID()) 90 , fStableID(next_gen_id())
94 #endif 91 #endif
95 92
96 { 93 {
97 this->setMutex(NULL); 94 this->setMutex(NULL);
98 fRec.zero(); 95 fRec.zero();
99 fLockCount = 0; 96 fLockCount = 0;
100 this->needsNewGenID(); 97 this->needsNewGenID();
101 fIsImmutable = false; 98 fIsImmutable = false;
102 fPreLocked = false; 99 fPreLocked = false;
103 fAddedToCache.store(false); 100 fAddedToCache.store(false);
104 } 101 }
105 102
106 103
107 SkPixelRef::SkPixelRef(const SkImageInfo& info, SkBaseMutex* mutex) 104 SkPixelRef::SkPixelRef(const SkImageInfo& info, SkBaseMutex* mutex)
108 : fInfo(validate_info(info)) 105 : fInfo(validate_info(info))
109 #ifdef SK_BUILD_FOR_ANDROID_FRAMEWORK 106 #ifdef SK_BUILD_FOR_ANDROID_FRAMEWORK
110 , fStableID(SkNextPixelRefGenerationID()) 107 , fStableID(next_gen_id())
111 #endif 108 #endif
112 { 109 {
113 this->setMutex(mutex); 110 this->setMutex(mutex);
114 fRec.zero(); 111 fRec.zero();
115 fLockCount = 0; 112 fLockCount = 0;
116 this->needsNewGenID(); 113 this->needsNewGenID();
117 fIsImmutable = false; 114 fIsImmutable = false;
118 fPreLocked = false; 115 fPreLocked = false;
119 fAddedToCache.store(false); 116 fAddedToCache.store(false);
120 } 117 }
121 118
122 SkPixelRef::~SkPixelRef() { 119 SkPixelRef::~SkPixelRef() {
123 this->callGenIDChangeListeners(); 120 this->callGenIDChangeListeners();
124 } 121 }
125 122
126 void SkPixelRef::needsNewGenID() { 123 void SkPixelRef::needsNewGenID() {
127 fGenerationID.store(0); 124 fTaggedGenID.store(0);
128 fUniqueGenerationID.store(false); 125 SkASSERT(!this->genIDIsUnique()); // This method isn't threadsafe, so the as sert should be fine.
129 } 126 }
130 127
131 void SkPixelRef::cloneGenID(const SkPixelRef& that) { 128 void SkPixelRef::cloneGenID(const SkPixelRef& that) {
132 // This is subtle. We must call that.getGenerationID() to make sure its gen ID isn't 0. 129 // This is subtle. We must call that.getGenerationID() to make sure its gen ID isn't 0.
133 this->fGenerationID.store(that.getGenerationID()); 130 uint32_t genID = that.getGenerationID();
134 this->fUniqueGenerationID.store(false); 131
135 that.fUniqueGenerationID.store(false); 132 // Neither ID is unique any more.
133 // (These & ~1u are actually redundant. that.getGenerationID() just did it for us.)
134 this->fTaggedGenID.store(genID & ~1u);
135 that. fTaggedGenID.store(genID & ~1u);
136
137 // This method isn't threadsafe, so these asserts should be fine.
138 SkASSERT(!this->genIDIsUnique());
139 SkASSERT(!that. genIDIsUnique());
136 } 140 }
137 141
138 void SkPixelRef::setPreLocked(void* pixels, size_t rowBytes, SkColorTable* ctabl e) { 142 void SkPixelRef::setPreLocked(void* pixels, size_t rowBytes, SkColorTable* ctabl e) {
139 #ifndef SK_IGNORE_PIXELREF_SETPRELOCKED 143 #ifndef SK_IGNORE_PIXELREF_SETPRELOCKED
140 // only call me in your constructor, otherwise fLockCount tracking can get 144 // only call me in your constructor, otherwise fLockCount tracking can get
141 // out of sync. 145 // out of sync.
142 fRec.fPixels = pixels; 146 fRec.fPixels = pixels;
143 fRec.fColorTable = ctable; 147 fRec.fColorTable = ctable;
144 fRec.fRowBytes = rowBytes; 148 fRec.fRowBytes = rowBytes;
145 fLockCount = SKPIXELREF_PRELOCKED_LOCKCOUNT; 149 fLockCount = SKPIXELREF_PRELOCKED_LOCKCOUNT;
(...skipping 48 matching lines...) Expand 10 before | Expand all | Expand 10 after
194 198
195 bool SkPixelRef::lockPixelsAreWritable() const { 199 bool SkPixelRef::lockPixelsAreWritable() const {
196 return this->onLockPixelsAreWritable(); 200 return this->onLockPixelsAreWritable();
197 } 201 }
198 202
199 bool SkPixelRef::onLockPixelsAreWritable() const { 203 bool SkPixelRef::onLockPixelsAreWritable() const {
200 return true; 204 return true;
201 } 205 }
202 206
203 uint32_t SkPixelRef::getGenerationID() const { 207 uint32_t SkPixelRef::getGenerationID() const {
204 uint32_t id = fGenerationID.load(); 208 uint32_t id = fTaggedGenID.load();
205 if (0 == id) { 209 if (0 == id) {
206 id = SkNextPixelRefGenerationID(); 210 id = next_gen_id();
207 fGenerationID.store(id); 211 fTaggedGenID.store(id | 1u); // TODO(mtklein): should become a compare- and-swap
208 fUniqueGenerationID.store(true); // The only time we can be sure of thi s! 212 SkASSERT(this->genIDIsUnique());
209 } 213 }
210 return id; 214 return id & ~1u; // Mask off bottom unique bit.
211 } 215 }
212 216
213 void SkPixelRef::addGenIDChangeListener(GenIDChangeListener* listener) { 217 void SkPixelRef::addGenIDChangeListener(GenIDChangeListener* listener) {
214 if (NULL == listener || !fUniqueGenerationID.load()) { 218 if (NULL == listener || !this->genIDIsUnique()) {
215 // No point in tracking this if we're not going to call it. 219 // No point in tracking this if we're not going to call it.
216 SkDELETE(listener); 220 SkDELETE(listener);
217 return; 221 return;
218 } 222 }
219 *fGenIDChangeListeners.append() = listener; 223 *fGenIDChangeListeners.append() = listener;
220 } 224 }
221 225
222 // we need to be called *before* the genID gets changed or zerod 226 // we need to be called *before* the genID gets changed or zerod
223 void SkPixelRef::callGenIDChangeListeners() { 227 void SkPixelRef::callGenIDChangeListeners() {
224 // We don't invalidate ourselves if we think another SkPixelRef is sharing o ur genID. 228 // We don't invalidate ourselves if we think another SkPixelRef is sharing o ur genID.
225 if (fUniqueGenerationID.load()) { 229 if (this->genIDIsUnique()) {
226 for (int i = 0; i < fGenIDChangeListeners.count(); i++) { 230 for (int i = 0; i < fGenIDChangeListeners.count(); i++) {
227 fGenIDChangeListeners[i]->onChange(); 231 fGenIDChangeListeners[i]->onChange();
228 } 232 }
229 233
230 // TODO: SkAtomic could add "old_value = atomic.xchg(new_value)" to make this clearer. 234 // TODO: SkAtomic could add "old_value = atomic.xchg(new_value)" to make this clearer.
231 if (fAddedToCache.load()) { 235 if (fAddedToCache.load()) {
232 SkNotifyBitmapGenIDIsStale(this->getGenerationID()); 236 SkNotifyBitmapGenIDIsStale(this->getGenerationID());
233 fAddedToCache.store(false); 237 fAddedToCache.store(false);
234 } 238 }
235 } 239 }
(...skipping 33 matching lines...) Expand 10 before | Expand all | Expand 10 after
269 273
270 bool SkPixelRef::onGetYUV8Planes(SkISize sizes[3], void* planes[3], size_t rowBy tes[3], 274 bool SkPixelRef::onGetYUV8Planes(SkISize sizes[3], void* planes[3], size_t rowBy tes[3],
271 SkYUVColorSpace* colorSpace) { 275 SkYUVColorSpace* colorSpace) {
272 return false; 276 return false;
273 } 277 }
274 278
275 size_t SkPixelRef::getAllocatedSizeInBytes() const { 279 size_t SkPixelRef::getAllocatedSizeInBytes() const {
276 return 0; 280 return 0;
277 } 281 }
278 282
OLDNEW
« no previous file with comments | « include/core/SkPixelRef.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698