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..caae23f17147e4720177ca91594b222559a8484a 100644
--- a/src/core/SkPixelRef.cpp
+++ b/src/core/SkPixelRef.cpp
@@ -75,6 +75,16 @@ int32_t SkNextPixelRefGenerationID() {
 
 ///////////////////////////////////////////////////////////////////////////////
 
+static void mark_invalid(SkImageInfo* info) {
+    info->fWidth = -1;
+}
+
+static bool is_invalid(const SkImageInfo& info) {
+    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,49 @@ void SkPixelRef::flatten(SkFlattenableWriteBuffer& buffer) const {
     }
 }
 
-void SkPixelRef::lockPixels() {
-    SkASSERT(!fPreLocked || SKPIXELREF_PRELOCKED_LOCKCOUNT == fLockCount);
+bool SkPixelRef::getInfo(SkImageInfo* info) {
+#ifdef SK_DEBUG
+    if (fPreLocked) {
+        SkASSERT(!is_invalid(fInfo));

[scroggo, 2013/12/04 22:47:38]

This seems redundant.

[reed1, 2013/12/05 13:32:39]

I don't think we assert elsewhere that prelocked means the info is valid.

[scroggo, 2013/12/05 13:45:34]

Not directly, but just below this we assert that invalid means the pixel ref is
not prelocked. In what situation would we hit this assert, but would not have
hit the assert below?

+    }
+#endif
+
+    if (is_invalid(fInfo)) {
+        SkASSERT(!fPreLocked);
 
-    if (!fPreLocked) {
         SkAutoMutexAcquire  ac(*fMutex);
+        
+        SkImageInfo tmpInfo;

[scroggo, 2013/12/04 22:47:38]

This one is just in case the implementation of onGetInfo does not honor the
contract and modifies the passed in info? I think that will still only be a
problem if it sets it to something valid and returns false.

[reed1, 2013/12/05 13:32:39]

Exactly.

Also, we are experimenting with making this yet-more-thread-safe, and I will
need to separate out calling onGetInfo from writing the answer into fInfo, for
memory barrier persnicketiness

+        if (!this->onGetInfo(&tmpInfo)) {
+            return false;
+        }
+        fInfo = tmpInfo;
+    }
+    *info = fInfo;
+    return true;
+}
 
+bool SkPixelRef::lockPixels(LockRec* rec) {
+    SkASSERT(!fPreLocked || SKPIXELREF_PRELOCKED_LOCKCOUNT == fLockCount);
+    
+    if (!fPreLocked) {
+        SkAutoMutexAcquire  ac(*fMutex);
+        
         if (1 == ++fLockCount) {
-            fPixels = this->onLockPixels(&fColorTable);
+            LockRec rec;
+            if (!this->onNewLockPixels(&rec)) {
+                return false;
+            }
+            fRec = rec;
         }
     }
+    *rec = fRec;
+    return true;
+}
+
+void SkPixelRef::lockPixels() {
+    LockRec rec;
+    (void)this->lockPixels(&rec);
 }
 
 void SkPixelRef::unlockPixels() {
@@ -171,8 +217,7 @@ void SkPixelRef::unlockPixels() {
         SkASSERT(fLockCount > 0);
         if (0 == --fLockCount) {
             this->onUnlockPixels();
-            fPixels = NULL;
-            fColorTable = NULL;
+            fRec.zero();
         }
     }
 }
@@ -249,6 +294,33 @@ SkData* SkPixelRef::onRefEncodedData() {
 
 ///////////////////////////////////////////////////////////////////////////////
 
+#ifdef SK_SUPPORT_LEGACY_ONLOCKPIXELS
+
+void* SkPixelRef::onLockPixels(SkColorTable** ctable) {
+    return NULL;
+}
+
+bool SkPixelRef::onGetInfo(SkImageInfo* info) {
+    return false;
+}
+
+bool SkPixelRef::onNewLockPixels(LockRec* rec) {
+    SkColorTable* ctable;
+    void* pixels = this->onLockPixels(&ctable);
+    if (!pixels) {
+        return false;
+    }
+
+    rec->fPixels = pixels;
+    rec->fColorTable = ctable;
+    rec->fRowBytes = 0; // callers don't currently need this (thank goodness)
+    return true;
+}
+
+#endif
+
+///////////////////////////////////////////////////////////////////////////////
+
 #ifdef SK_BUILD_FOR_ANDROID
 void SkPixelRef::globalRef(void* data) {
     this->ref();