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

Unified Diff: src/core/SkPixelRef.cpp

Issue 1151063005: Revert of Revert[4] of add asserts around results from requestLock (Closed) Base URL: https://skia.googlesource.com/skia.git@master
Patch Set: Created 5 years, 7 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 side-by-side diff with in-line comments
Download patch
« no previous file with comments | « src/core/SkBitmap.cpp ('k') | tests/PixelRefTest.cpp » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/core/SkPixelRef.cpp
diff --git a/src/core/SkPixelRef.cpp b/src/core/SkPixelRef.cpp
index c5aa3d130d4bb49d55e0f85160ee32f85927af64..90e2a40ea4fb92513d4ba351a366ec330ba2b207 100644
--- a/src/core/SkPixelRef.cpp
+++ b/src/core/SkPixelRef.cpp
@@ -160,22 +160,8 @@
SkASSERT(!that. genIDIsUnique());
}
-static void validate_pixels_ctable(const SkImageInfo& info, const void* pixels,
- const SkColorTable* ctable) {
- if (info.isEmpty()) {
- return; // can't require pixels if the dimensions are empty
- }
- SkASSERT(pixels);
- if (kIndex_8_SkColorType == info.colorType()) {
- SkASSERT(ctable);
- } else {
- SkASSERT(NULL == ctable);
- }
-}
-
void SkPixelRef::setPreLocked(void* pixels, size_t rowBytes, SkColorTable* ctable) {
#ifndef SK_IGNORE_PIXELREF_SETPRELOCKED
- validate_pixels_ctable(fInfo, pixels, ctable);
// only call me in your constructor, otherwise fLockCount tracking can get
// out of sync.
fRec.fPixels = pixels;
@@ -186,33 +172,38 @@
#endif
}
-// Increments fLockCount only on success
-bool SkPixelRef::lockPixelsInsideMutex() {
+bool SkPixelRef::lockPixelsInsideMutex(LockRec* rec) {
fMutex->assertHeld();
+ // For historical reasons, we always inc fLockCount, even if we return false.
+ // It would be nice to change this (it seems), and only inc if we actually succeed...
if (1 == ++fLockCount) {
SkASSERT(fRec.isZero());
- if (!this->onNewLockPixels(&fRec)) {
- fRec.zero();
+
+ LockRec rec;
+ if (!this->onNewLockPixels(&rec)) {
fLockCount -= 1; // we return fLockCount unchanged if we fail.
return false;
}
- }
- validate_pixels_ctable(fInfo, fRec.fPixels, fRec.fColorTable);
+ SkASSERT(!rec.isZero()); // else why did onNewLock return true?
+ fRec = rec;
+ }
+ *rec = fRec;
return true;
}
-// For historical reasons, we always inc fLockCount, even if we return false.
-// It would be nice to change this (it seems), and only inc if we actually succeed...
-bool SkPixelRef::lockPixels() {
+bool SkPixelRef::lockPixels(LockRec* rec) {
SkASSERT(!fPreLocked || SKPIXELREF_PRELOCKED_LOCKCOUNT == fLockCount);
- if (!fPreLocked) {
+ if (fPreLocked) {
+ *rec = fRec;
+ return true;
+ } else {
TRACE_EVENT_BEGIN0("skia", "SkPixelRef::lockPixelsMutex");
SkAutoMutexAcquire ac(*fMutex);
TRACE_EVENT_END0("skia", "SkPixelRef::lockPixelsMutex");
SkDEBUGCODE(int oldCount = fLockCount;)
- bool success = this->lockPixelsInsideMutex();
+ bool success = this->lockPixelsInsideMutex(rec);
// lockPixelsInsideMutex only increments the count if it succeeds.
SkASSERT(oldCount + (int)success == fLockCount);
@@ -220,19 +211,14 @@
// For compatibility with SkBitmap calling lockPixels, we still want to increment
// fLockCount even if we failed. If we updated SkBitmap we could remove this oddity.
fLockCount += 1;
- return false;
- }
- }
- validate_pixels_ctable(fInfo, fRec.fPixels, fRec.fColorTable);
- return true;
-}
-
-bool SkPixelRef::lockPixels(LockRec* rec) {
- if (this->lockPixels()) {
- *rec = fRec;
- return true;
- }
- return false;
+ }
+ return success;
+ }
+}
+
+bool SkPixelRef::lockPixels() {
+ LockRec rec;
+ return this->lockPixels(&rec);
}
void SkPixelRef::unlockPixels() {
@@ -267,14 +253,11 @@
result->fPixels = fRec.fPixels;
result->fRowBytes = fRec.fRowBytes;
result->fSize.set(fInfo.width(), fInfo.height());
+ return true;
} else {
SkAutoMutexAcquire ac(*fMutex);
- if (!this->onRequestLock(request, result)) {
- return false;
- }
- }
- validate_pixels_ctable(fInfo, result->fPixels, result->fCTable);
- return true;
+ return this->onRequestLock(request, result);
+ }
}
bool SkPixelRef::lockPixelsAreWritable() const {
@@ -375,15 +358,16 @@
}
bool SkPixelRef::onRequestLock(const LockRequest& request, LockResult* result) {
- if (!this->lockPixelsInsideMutex()) {
+ LockRec rec;
+ if (!this->lockPixelsInsideMutex(&rec)) {
return false;
}
result->fUnlockProc = unlock_legacy_result;
result->fUnlockContext = SkRef(this); // this is balanced in our fUnlockProc
- result->fCTable = fRec.fColorTable;
- result->fPixels = fRec.fPixels;
- result->fRowBytes = fRec.fRowBytes;
+ result->fCTable = rec.fColorTable;
+ result->fPixels = rec.fPixels;
+ result->fRowBytes = rec.fRowBytes;
result->fSize.set(fInfo.width(), fInfo.height());
return true;
}
« no previous file with comments | « src/core/SkBitmap.cpp ('k') | tests/PixelRefTest.cpp » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698