SkResourceCache::GetAllocator() index8 and other color types handling

src/core/SkResourceCache.cpp

Index: src/core/SkResourceCache.cpp
diff --git a/src/core/SkResourceCache.cpp b/src/core/SkResourceCache.cpp
index 240d1977e53968d417de449d84ff42c8e34f405e..43155470dbc9ed208e9479349dd6d18e81a9a911 100644
--- a/src/core/SkResourceCache.cpp
+++ b/src/core/SkResourceCache.cpp
@@ -79,7 +79,7 @@ class SkOneShotDiscardablePixelRef : public SkPixelRef {
 public:
 
     // Ownership of the discardablememory is transfered to the pixelref
-    SkOneShotDiscardablePixelRef(const SkImageInfo&, SkDiscardableMemory*, size_t rowBytes);
+    SkOneShotDiscardablePixelRef(const SkImageInfo&, SkDiscardableMemory*, size_t rowBytes, SkColorTable* ctable);

[scroggo, 2015/11/09 20:18:09]

nit: over 100 chars.

Also, the name "ctable" adds no extra information, so I think this is just fine
as "SkColorTable*"

     ~SkOneShotDiscardablePixelRef();
 
 protected:
@@ -93,22 +93,29 @@ private:
     SkDiscardableMemory* fDM;
     size_t               fRB;
     bool                 fFirstTime;
+    SkColorTable*        fCTable;

[scroggo, 2015/11/09 20:18:09]

Can you add a comment about ownership of the color table? (Sometimes we do not
take a reference as in SkPixmap, but here you do.)

 
     typedef SkPixelRef INHERITED;
 };
 
 SkOneShotDiscardablePixelRef::SkOneShotDiscardablePixelRef(const SkImageInfo& info,
                                              SkDiscardableMemory* dm,
-                                             size_t rowBytes)
+                                             size_t rowBytes,
+                                             SkColorTable* ctable)
     : INHERITED(info)
     , fDM(dm)
     , fRB(rowBytes)
+    , fCTable(ctable)
 {
     SkASSERT(dm->data());
     fFirstTime = true;
+    SkSafeRef(ctable);
 }
 
-SkOneShotDiscardablePixelRef::~SkOneShotDiscardablePixelRef() { delete fDM; }
+SkOneShotDiscardablePixelRef::~SkOneShotDiscardablePixelRef() {
+    delete fDM;
+    SkSafeUnref(fCTable);
+}
 
 bool SkOneShotDiscardablePixelRef::onNewLockPixels(LockRec* rec) {
     if (fFirstTime) {
@@ -132,7 +139,7 @@ bool SkOneShotDiscardablePixelRef::onNewLockPixels(LockRec* rec) {
 
 SUCCESS:
     rec->fPixels = fDM->data();
-    rec->fColorTable = nullptr;
+    rec->fColorTable = fCTable;
     rec->fRowBytes = fRB;
     return true;
 }
@@ -166,18 +173,21 @@ bool SkResourceCacheDiscardableAllocator::allocPixelRef(SkBitmap* bitmap, SkColo
         return false;
     }
 
-    SkDiscardableMemory* dm = fFactory(size);
-    if (nullptr == dm) {
-        return false;
+    if (kIndex_8_SkColorType == bitmap->colorType()) {
+        if (!ctable) {
+            return false;
+        }
+    } else {
+        ctable = nullptr;
     }
 
-    // can we relax this?
-    if (kN32_SkColorType != bitmap->colorType()) {
+    SkDiscardableMemory* dm = fFactory(size);
+    if (nullptr == dm) {
         return false;
     }
 
     SkImageInfo info = bitmap->info();
-    bitmap->setPixelRef(new SkOneShotDiscardablePixelRef(info, dm, bitmap->rowBytes()))->unref();
+    bitmap->setPixelRef(new SkOneShotDiscardablePixelRef(info, dm, bitmap->rowBytes(), ctable))->unref();

[scroggo, 2015/11/09 20:18:09]

nit: over 100 chars

     bitmap->lockPixels();
     return bitmap->readyToDraw();
 }