diff options
author | reed <reed@chromium.org> | 2015-05-30 13:41:15 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-05-30 13:41:15 -0700 |
commit | db74f62c7a464c215b95f563ae4f7a991c4fd058 (patch) | |
tree | 87506df7081ebf1db9d3c0342f8fbed5c185130c /src/core/SkPixelRef.cpp | |
parent | 11e833d91b5ba2193ed593cb74900dddbec24b6f (diff) |
Revert[6] of add asserts around results from requestLock
This reverts commit 11e833d91b5ba2193ed593cb74900dddbec24b6f.
BUG=skia:
TBR=
Review URL: https://codereview.chromium.org/1159183003
Diffstat (limited to 'src/core/SkPixelRef.cpp')
-rw-r--r-- | src/core/SkPixelRef.cpp | 68 |
1 files changed, 42 insertions, 26 deletions
diff --git a/src/core/SkPixelRef.cpp b/src/core/SkPixelRef.cpp index 90e2a40ea4..c5aa3d130d 100644 --- a/src/core/SkPixelRef.cpp +++ b/src/core/SkPixelRef.cpp @@ -160,8 +160,22 @@ void SkPixelRef::cloneGenID(const SkPixelRef& that) { 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; @@ -172,38 +186,33 @@ void SkPixelRef::setPreLocked(void* pixels, size_t rowBytes, SkColorTable* ctabl #endif } -bool SkPixelRef::lockPixelsInsideMutex(LockRec* rec) { +// Increments fLockCount only on success +bool SkPixelRef::lockPixelsInsideMutex() { 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()); - - LockRec rec; - if (!this->onNewLockPixels(&rec)) { + if (!this->onNewLockPixels(&fRec)) { + fRec.zero(); fLockCount -= 1; // we return fLockCount unchanged if we fail. return false; } - SkASSERT(!rec.isZero()); // else why did onNewLock return true? - fRec = rec; } - *rec = fRec; + validate_pixels_ctable(fInfo, fRec.fPixels, fRec.fColorTable); return true; } -bool SkPixelRef::lockPixels(LockRec* rec) { +// 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() { SkASSERT(!fPreLocked || SKPIXELREF_PRELOCKED_LOCKCOUNT == fLockCount); - if (fPreLocked) { - *rec = fRec; - return true; - } else { + if (!fPreLocked) { TRACE_EVENT_BEGIN0("skia", "SkPixelRef::lockPixelsMutex"); SkAutoMutexAcquire ac(*fMutex); TRACE_EVENT_END0("skia", "SkPixelRef::lockPixelsMutex"); SkDEBUGCODE(int oldCount = fLockCount;) - bool success = this->lockPixelsInsideMutex(rec); + bool success = this->lockPixelsInsideMutex(); // lockPixelsInsideMutex only increments the count if it succeeds. SkASSERT(oldCount + (int)success == fLockCount); @@ -211,14 +220,19 @@ bool SkPixelRef::lockPixels(LockRec* rec) { // 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; } - return success; } + validate_pixels_ctable(fInfo, fRec.fPixels, fRec.fColorTable); + return true; } -bool SkPixelRef::lockPixels() { - LockRec rec; - return this->lockPixels(&rec); +bool SkPixelRef::lockPixels(LockRec* rec) { + if (this->lockPixels()) { + *rec = fRec; + return true; + } + return false; } void SkPixelRef::unlockPixels() { @@ -253,11 +267,14 @@ bool SkPixelRef::requestLock(const LockRequest& request, LockResult* result) { result->fPixels = fRec.fPixels; result->fRowBytes = fRec.fRowBytes; result->fSize.set(fInfo.width(), fInfo.height()); - return true; } else { SkAutoMutexAcquire ac(*fMutex); - return this->onRequestLock(request, result); + if (!this->onRequestLock(request, result)) { + return false; + } } + validate_pixels_ctable(fInfo, result->fPixels, result->fCTable); + return true; } bool SkPixelRef::lockPixelsAreWritable() const { @@ -358,16 +375,15 @@ static void unlock_legacy_result(void* ctx) { } bool SkPixelRef::onRequestLock(const LockRequest& request, LockResult* result) { - LockRec rec; - if (!this->lockPixelsInsideMutex(&rec)) { + if (!this->lockPixelsInsideMutex()) { return false; } result->fUnlockProc = unlock_legacy_result; result->fUnlockContext = SkRef(this); // this is balanced in our fUnlockProc - result->fCTable = rec.fColorTable; - result->fPixels = rec.fPixels; - result->fRowBytes = rec.fRowBytes; + result->fCTable = fRec.fColorTable; + result->fPixels = fRec.fPixels; + result->fRowBytes = fRec.fRowBytes; result->fSize.set(fInfo.width(), fInfo.height()); return true; } |