Expand pixelref to return SkImageInfo and rowbytes

src/core/SkPixelRef.cpp

Index: src/core/SkPixelRef.cpp
diff --git a/src/core/SkPixelRef.cpp b/src/core/SkPixelRef.cpp
index 972474ccc3357f301bda62da187def8f8b7c5db9..04c4c391f5a567cb2ba9350d456273e803ef27ec 100644
--- a/src/core/SkPixelRef.cpp
+++ b/src/core/SkPixelRef.cpp
@@ -75,6 +75,16 @@ int32_t SkNextPixelRefGenerationID() {
 
 ///////////////////////////////////////////////////////////////////////////////
 
+static void mark_invalid(SkImageInfo* info) {

[scroggo, 2013/12/04 18:14:03]

Any reason to put this here instead of on SkImageInfo?

[reed1, 2013/12/04 21:12:54]

ImageInfo's valid is possibly what I should use, but it was originally written
just for the valid-deserialization-security code. Perhaps it should be made
private to that logic for now, since it doesn't really check for all possible
"logical" invalid combinations, just checking for out-of-range values.

I'm just doing it here to avoid having a bool field, since I am "caching" the
ImageInfo.

+    info->fWidth = -1;
+}
+
+static bool is_invalid(const SkImageInfo& info) {

[scroggo, 2013/12/04 18:14:03]

What's the reason for calling this instead of !SkImageInfo::isValid()?

[reed1, 2013/12/04 21:12:54]

see above

+    return info.fWidth < 0;
+}
+
+///////////////////////////////////////////////////////////////////////////////
+
 void SkPixelRef::setMutex(SkBaseMutex* mutex) {
     if (NULL == mutex) {
         mutex = get_default_mutex();
@@ -87,8 +97,8 @@ void SkPixelRef::setMutex(SkBaseMutex* mutex) {
 
 SkPixelRef::SkPixelRef(SkBaseMutex* mutex) {
     this->setMutex(mutex);
-    fPixels = NULL;
-    fColorTable = NULL; // we do not track ownership of this
+    mark_invalid(&fInfo);
+    fRec.zero();
     fLockCount = 0;
     this->needsNewGenID();
     fIsImmutable = false;
@@ -98,8 +108,8 @@ SkPixelRef::SkPixelRef(SkBaseMutex* mutex) {
 SkPixelRef::SkPixelRef(SkFlattenableReadBuffer& buffer, SkBaseMutex* mutex)
         : INHERITED(buffer) {
     this->setMutex(mutex);
-    fPixels = NULL;
-    fColorTable = NULL; // we do not track ownership of this
+    mark_invalid(&fInfo);
+    fRec.zero();
     fLockCount = 0;
     fIsImmutable = buffer.readBool();
     fGenerationID = buffer.readUInt();
@@ -123,12 +133,15 @@ void SkPixelRef::cloneGenID(const SkPixelRef& that) {
     that.fUniqueGenerationID = false;
 }
 
-void SkPixelRef::setPreLocked(void* pixels, SkColorTable* ctable) {
+void SkPixelRef::setPreLocked(const SkImageInfo& info, void* pixels,
+                              size_t rowBytes, SkColorTable* ctable) {
 #ifndef SK_IGNORE_PIXELREF_SETPRELOCKED
     // only call me in your constructor, otherwise fLockCount tracking can get
     // out of sync.
-    fPixels = pixels;
-    fColorTable = ctable;
+    fInfo = info;
+    fRec.fPixels = pixels;
+    fRec.fColorTable = ctable;
+    fRec.fRowBytes = rowBytes;
     fLockCount = SKPIXELREF_PRELOCKED_LOCKCOUNT;
     fPreLocked = true;
 #endif
@@ -150,16 +163,43 @@ void SkPixelRef::flatten(SkFlattenableWriteBuffer& buffer) const {
     }
 }
 
-void SkPixelRef::lockPixels() {
-    SkASSERT(!fPreLocked || SKPIXELREF_PRELOCKED_LOCKCOUNT == fLockCount);
-
+bool SkPixelRef::getInfo(SkImageInfo* info) {
     if (!fPreLocked) {
         SkAutoMutexAcquire  ac(*fMutex);
+        
+        if (is_invalid(fInfo)) {
+            if (!this->onGetInfo(&fInfo)) {
+                mark_invalid(&fInfo);
+                return false;
+            }
+        }
+    }
+    *info = fInfo;
+    return true;
+}
 
-        if (1 == ++fLockCount) {
-            fPixels = this->onLockPixels(&fColorTable);
+bool SkPixelRef::lockPixels(LockRec* rec) {
+    SkASSERT(!fPreLocked || SKPIXELREF_PRELOCKED_LOCKCOUNT == fLockCount);
+    
+    if (!fPreLocked) {
+        SkAutoMutexAcquire  ac(*fMutex);
+        
+        if (0 == fLockCount) {
+            fRec.zero();
+            if (!this->onNewLockPixels(&fRec)) {
+                fRec.zero();
+                return false;
+            }
         }
+        fLockCount += 1;
     }
+    *rec = fRec;
+    return true;
+}
+
+void SkPixelRef::lockPixels() {
+    LockRec rec;
+    this->lockPixels(&rec);
 }
 
 void SkPixelRef::unlockPixels() {
@@ -171,8 +211,7 @@ void SkPixelRef::unlockPixels() {
         SkASSERT(fLockCount > 0);
         if (0 == --fLockCount) {
             this->onUnlockPixels();
-            fPixels = NULL;
-            fColorTable = NULL;
+            fRec.zero();
         }
     }
 }