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

Unit tests added to check all color types in SkOneShotDiscardablePixelref

SkResourceCacheTest seems to have been allways using default Heap Allocator.
 Fixed so that it uses private (not global that is) SkDiscardableMemory.

BUG=skia:4355

Review URL: https://codereview.chromium.org/1426753006

3 files changed, 201 insertions, 101 deletions

diff --git a/src/core/SkResourceCache.cpp b/src/core/SkResourceCache.cpp
index 240d1977e5..6d8e1662d9 100644
--- a/src/core/SkResourceCache.cpp
+++ b/src/core/SkResourceCache.cpp
@@ -79,7 +79,9 @@ class SkOneShotDiscardablePixelRef : public SkPixelRef {
 public:
 
     // Ownership of the discardablememory is transfered to the pixelref
-    SkOneShotDiscardablePixelRef(const SkImageInfo&, SkDiscardableMemory*, size_t rowBytes);
+    // The pixelref will ref() the colortable (if not NULL), and unref() in destructor
+    SkOneShotDiscardablePixelRef(const SkImageInfo&, SkDiscardableMemory*, size_t rowBytes,
+                                 SkColorTable*);
     ~SkOneShotDiscardablePixelRef();
 
 protected:
@@ -93,22 +95,29 @@ private:
     SkDiscardableMemory* fDM;
     size_t               fRB;
     bool                 fFirstTime;
+    SkColorTable*        fCTable;
 
     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 +141,7 @@ bool SkOneShotDiscardablePixelRef::onNewLockPixels(LockRec* rec) {
 
 SUCCESS:
     rec->fPixels = fDM->data();
-    rec->fColorTable = nullptr;
+    rec->fColorTable = fCTable;
     rec->fRowBytes = fRB;
     return true;
 }
@@ -166,18 +175,22 @@ 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();
     bitmap->lockPixels();
     return bitmap->readyToDraw();
 }
diff --git a/tests/CachedDecodingPixelRefTest.cpp b/tests/CachedDecodingPixelRefTest.cpp
index 2813b42bf8..51af15fd39 100644
--- a/tests/CachedDecodingPixelRefTest.cpp
+++ b/tests/CachedDecodingPixelRefTest.cpp
@@ -163,17 +163,21 @@ public:
     };
     static int Width() { return 10; }
     static int Height() { return 10; }
-    static uint32_t Color() { return 0xff123456; }
-    TestImageGenerator(TestType type, skiatest::Reporter* reporter)
-    : INHERITED(GetMyInfo()), fType(type), fReporter(reporter) {
+    // value choosen so that there is no loss when converting to to RGB565 and back
+    static SkColor Color() { return 0xff10345a; }
+    static SkPMColor PMColor() { return SkPreMultiplyColor(Color()); }
+
+    TestImageGenerator(TestType type, skiatest::Reporter* reporter,
+                       SkColorType colorType = kN32_SkColorType)
+    : INHERITED(GetMyInfo(colorType)), fType(type), fReporter(reporter) {
         SkASSERT((fType <= kLast_TestType) && (fType >= 0));
     }
     virtual ~TestImageGenerator() { }
 
 protected:
-    static SkImageInfo GetMyInfo() {
-        return SkImageInfo::MakeN32(TestImageGenerator::Width(), TestImageGenerator::Height(),
-                                    kOpaque_SkAlphaType);
+    static SkImageInfo GetMyInfo(SkColorType colorType) {
+        return SkImageInfo::Make(TestImageGenerator::Width(), TestImageGenerator::Height(),
+                                 colorType, kOpaque_SkAlphaType);
     }
 
     bool onGetPixels(const SkImageInfo& info, void* pixels, size_t rowBytes,
@@ -183,14 +187,35 @@ protected:
         if (fType != kSucceedGetPixels_TestType) {
             return false;
         }
-        if (info.colorType() != kN32_SkColorType) {
+        if (info.colorType() != kN32_SkColorType && info.colorType() != getInfo().colorType()) {
             return false;
         }
         char* bytePtr = static_cast<char*>(pixels);
-        for (int y = 0; y < info.height(); ++y) {
-            sk_memset32(reinterpret_cast<SkColor*>(bytePtr),
-                        TestImageGenerator::Color(), info.width());
-            bytePtr += rowBytes;
+        switch (info.colorType()) {
+            case kN32_SkColorType:
+                for (int y = 0; y < info.height(); ++y) {
+                    sk_memset32((uint32_t*)bytePtr,
+                                TestImageGenerator::PMColor(), info.width());
+                    bytePtr += rowBytes;
+                }
+                break;
+            case kIndex_8_SkColorType:
+                *ctableCount = 1;
+                ctable[0] = TestImageGenerator::PMColor();
+                for (int y = 0; y < info.height(); ++y) {
+                    memset(bytePtr, 0, info.width());
+                    bytePtr += rowBytes;
+                }
+                break;
+            case kRGB_565_SkColorType:
+                for (int y = 0; y < info.height(); ++y) {
+                    sk_memset16((uint16_t*)bytePtr,
+                        SkPixel32ToPixel16(TestImageGenerator::PMColor()), info.width());
+                    bytePtr += rowBytes;
+                }
+                break;
+            default:
+                return false;
         }
         return true;
     }
@@ -214,7 +239,7 @@ static void check_test_image_generator_bitmap(skiatest::Reporter* reporter,
     int errors = 0;
     for (int y = 0; y < bm.height(); ++y) {
         for (int x = 0; x < bm.width(); ++x) {
-            if (TestImageGenerator::Color() != *bm.getAddr32(x, y)) {
+            if (TestImageGenerator::Color() != bm.getColor(x, y)) {
                 ++errors;
             }
         }
@@ -224,8 +249,9 @@ static void check_test_image_generator_bitmap(skiatest::Reporter* reporter,
 
 static void check_pixelref(TestImageGenerator::TestType type,
                            skiatest::Reporter* reporter,
-                           SkDiscardableMemory::Factory* factory) {
-    SkAutoTDelete<SkImageGenerator> gen(new TestImageGenerator(type, reporter));
+                           SkDiscardableMemory::Factory* factory,
+                           SkColorType colorType) {
+    SkAutoTDelete<SkImageGenerator> gen(new TestImageGenerator(type, reporter, colorType));
     REPORTER_ASSERT(reporter, gen.get() != nullptr);
     SkBitmap lazy;
     bool success = SkDEPRECATED_InstallDiscardablePixelRef(gen.detach(), nullptr, &lazy, factory);
@@ -245,56 +271,77 @@ static void check_pixelref(TestImageGenerator::TestType type,
  *  SkDiscardableMemory::Factory choices.
  */
 DEF_TEST(DiscardableAndCachingPixelRef, reporter) {
-    check_pixelref(TestImageGenerator::kFailGetPixels_TestType, reporter, nullptr);
-    check_pixelref(TestImageGenerator::kSucceedGetPixels_TestType, reporter, nullptr);
+    const SkColorType testColorTypes[] = {
+        kN32_SkColorType,
+        kIndex_8_SkColorType,
+        kRGB_565_SkColorType
+    };
+    for (const SkColorType testColorType : testColorTypes) {
+        check_pixelref(TestImageGenerator::kFailGetPixels_TestType, reporter, nullptr,
+                       testColorType);
+        check_pixelref(TestImageGenerator::kSucceedGetPixels_TestType, reporter, nullptr,
+                       testColorType);
 
-    SkAutoTUnref<SkDiscardableMemoryPool> pool(
-        SkDiscardableMemoryPool::Create(1, nullptr));
-    REPORTER_ASSERT(reporter, 0 == pool->getRAMUsed());
-    check_pixelref(TestImageGenerator::kFailGetPixels_TestType, reporter, pool);
-    REPORTER_ASSERT(reporter, 0 == pool->getRAMUsed());
-    check_pixelref(TestImageGenerator::kSucceedGetPixels_TestType, reporter, pool);
-    REPORTER_ASSERT(reporter, 0 == pool->getRAMUsed());
+        SkAutoTUnref<SkDiscardableMemoryPool> pool(
+            SkDiscardableMemoryPool::Create(1, nullptr));
+        REPORTER_ASSERT(reporter, 0 == pool->getRAMUsed());
+        check_pixelref(TestImageGenerator::kFailGetPixels_TestType, reporter, pool,
+                       testColorType);
+        REPORTER_ASSERT(reporter, 0 == pool->getRAMUsed());
+        check_pixelref(TestImageGenerator::kSucceedGetPixels_TestType, reporter, pool,
+                       testColorType);
+        REPORTER_ASSERT(reporter, 0 == pool->getRAMUsed());
 
-    SkDiscardableMemoryPool* globalPool = SkGetGlobalDiscardableMemoryPool();
-    // Only acts differently from nullptr on a platform that has a
-    // default discardable memory implementation that differs from the
-    // global DM pool.
-    check_pixelref(TestImageGenerator::kFailGetPixels_TestType, reporter, globalPool);
-    check_pixelref(TestImageGenerator::kSucceedGetPixels_TestType, reporter, globalPool);
+        SkDiscardableMemoryPool* globalPool = SkGetGlobalDiscardableMemoryPool();
+        // Only acts differently from nullptr on a platform that has a
+        // default discardable memory implementation that differs from the
+        // global DM pool.
+        check_pixelref(TestImageGenerator::kFailGetPixels_TestType, reporter, globalPool,
+                       testColorType);
+        check_pixelref(TestImageGenerator::kSucceedGetPixels_TestType, reporter, globalPool,
+                       testColorType);
+    }
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 
 DEF_TEST(Image_NewFromGenerator, r) {
-    TestImageGenerator::TestType testTypes[] = {
+    const TestImageGenerator::TestType testTypes[] = {
         TestImageGenerator::kFailGetPixels_TestType,
         TestImageGenerator::kSucceedGetPixels_TestType,
     };
+    const SkColorType testColorTypes[] = {
+        kN32_SkColorType,
+        kIndex_8_SkColorType,
+        kRGB_565_SkColorType
+    };
     for (size_t i = 0; i < SK_ARRAY_COUNT(testTypes); ++i) {
         TestImageGenerator::TestType test = testTypes[i];
-        SkImageGenerator* gen = new TestImageGenerator(test, r);
-        SkAutoTUnref<SkImage> image(SkImage::NewFromGenerator(gen));
-        if (nullptr == image.get()) {
-            ERRORF(r, "SkImage::NewFromGenerator unexpecedly failed ["
-                   SK_SIZE_T_SPECIFIER "]", i);
-            continue;
-        }
-        REPORTER_ASSERT(r, TestImageGenerator::Width() == image->width());
-        REPORTER_ASSERT(r, TestImageGenerator::Height() == image->height());
-        REPORTER_ASSERT(r, image->isLazyGenerated());
+        for (const SkColorType testColorType : testColorTypes) {
+            SkImageGenerator* gen = new TestImageGenerator(test, r, testColorType);
+            SkAutoTUnref<SkImage> image(SkImage::NewFromGenerator(gen));
+            if (nullptr == image.get()) {
+                ERRORF(r, "SkImage::NewFromGenerator unexpecedly failed ["
+                    SK_SIZE_T_SPECIFIER "]", i);
+                continue;
+            }
+            REPORTER_ASSERT(r, TestImageGenerator::Width() == image->width());
+            REPORTER_ASSERT(r, TestImageGenerator::Height() == image->height());
+            REPORTER_ASSERT(r, image->isLazyGenerated());
 
-        SkBitmap bitmap;
-        bitmap.allocN32Pixels(TestImageGenerator::Width(), TestImageGenerator::Height());
-        SkCanvas canvas(bitmap);
-        const SkColor kDefaultColor = 0xffabcdef;
-        canvas.clear(kDefaultColor);
-        canvas.drawImage(image, 0, 0, nullptr);
-        if (TestImageGenerator::kSucceedGetPixels_TestType == test) {
-            REPORTER_ASSERT(
-                    r, TestImageGenerator::Color() == *bitmap.getAddr32(0, 0));
-        } else {
-            REPORTER_ASSERT(r, kDefaultColor == bitmap.getColor(0,0));
+            SkBitmap bitmap;
+            bitmap.allocN32Pixels(TestImageGenerator::Width(), TestImageGenerator::Height());
+            SkCanvas canvas(bitmap);
+            const SkColor kDefaultColor = 0xffabcdef;
+            canvas.clear(kDefaultColor);
+            canvas.drawImage(image, 0, 0, nullptr);
+            if (TestImageGenerator::kSucceedGetPixels_TestType == test) {
+                REPORTER_ASSERT(
+                    r, TestImageGenerator::Color() == bitmap.getColor(0, 0));
+            }
+            else {
+                REPORTER_ASSERT(r, kDefaultColor == bitmap.getColor(0, 0));
+            }
         }
     }
 }
diff --git a/tests/SkResourceCacheTest.cpp b/tests/SkResourceCacheTest.cpp
index 6e8ae40986..f9a0e6d505 100644
--- a/tests/SkResourceCacheTest.cpp
+++ b/tests/SkResourceCacheTest.cpp
@@ -14,11 +14,18 @@
 #include "SkPictureRecorder.h"
 #include "SkResourceCache.h"
 #include "SkSurface.h"
+#include "SkTypes.h"
 
 ////////////////////////////////////////////////////////////////////////////////////////
 
 static void make_bitmap(SkBitmap* bitmap, const SkImageInfo& info, SkBitmap::Allocator* allocator) {
-    if (allocator) {
+    if (info.colorType() == kIndex_8_SkColorType) {
+        bitmap->setInfo(info);
+        SkPMColor ctStorage[256];
+        memset(ctStorage, 0xFF, sizeof(ctStorage)); // init with opaque-white for the moment
+        SkAutoTUnref<SkColorTable> ctable(new SkColorTable(ctStorage, 256));
+        bitmap->allocPixels(allocator, ctable);
+    } else if (allocator) {
         bitmap->setInfo(info);
         allocator->allocPixelRef(bitmap, 0);
     } else {
@@ -173,55 +180,88 @@ static void test_bitmap_notify(skiatest::Reporter* reporter, SkResourceCache* ca
     }
 }
 
-DEF_TEST(BitmapCache_discarded_bitmap, reporter) {
-    SkResourceCache::DiscardableFactory factory = SkResourceCache::GetDiscardableFactory();
-    SkBitmap::Allocator* allocator = SkBitmapCache::GetAllocator();
-    
-    SkAutoTDelete<SkResourceCache> cache;
-    if (factory) {
-        cache.reset(new SkResourceCache(factory));
-    } else {
-        const size_t byteLimit = 100 * 1024;
-        cache.reset(new SkResourceCache(byteLimit));
-    }
-    SkBitmap cachedBitmap;
-    make_bitmap(&cachedBitmap, SkImageInfo::MakeN32Premul(5, 5), allocator);
-    cachedBitmap.setImmutable();
-    cachedBitmap.unlockPixels();
-
-    SkBitmap bm;
-    SkIRect rect = SkIRect::MakeWH(5, 5);
+#include "SkDiscardableMemoryPool.h"
 
-    // Add a bitmap to the cache.
-    REPORTER_ASSERT(reporter, SkBitmapCache::Add(cachedBitmap.pixelRef(), rect, cachedBitmap, cache));
-    REPORTER_ASSERT(reporter, SkBitmapCache::Find(cachedBitmap.getGenerationID(), rect, &bm, cache));
+static SkDiscardableMemoryPool* gPool = 0;
+static SkDiscardableMemory* pool_factory(size_t bytes) {
+    SkASSERT(gPool);
+    return gPool->create(bytes);
+}
 
-    // Finding more than once works fine.
-    REPORTER_ASSERT(reporter, SkBitmapCache::Find(cachedBitmap.getGenerationID(), rect, &bm, cache));
-    bm.unlockPixels();
+static void testBitmapCache_discarded_bitmap(skiatest::Reporter* reporter, SkResourceCache* cache,
+                                             SkResourceCache::DiscardableFactory factory) {
+    SkBitmap::Allocator* allocator = cache->allocator();
+    const SkColorType testTypes[] = {
+        kAlpha_8_SkColorType,
+        kRGB_565_SkColorType,
+        kRGBA_8888_SkColorType,
+        kBGRA_8888_SkColorType,
+        kIndex_8_SkColorType,
+        kGray_8_SkColorType
+    };
+    for (const SkColorType testType : testTypes) {
+        SkBitmap cachedBitmap;
+        make_bitmap(&cachedBitmap, SkImageInfo::Make(5, 5, testType, kPremul_SkAlphaType),
+                    allocator);
+        cachedBitmap.setImmutable();
+        cachedBitmap.unlockPixels();
+
+        SkBitmap bm;
+        SkIRect rect = SkIRect::MakeWH(5, 5);
+
+        // Add a bitmap to the cache.
+        REPORTER_ASSERT(reporter, SkBitmapCache::Add(cachedBitmap.pixelRef(), rect, cachedBitmap,
+                                                     cache));
+        REPORTER_ASSERT(reporter, SkBitmapCache::Find(cachedBitmap.getGenerationID(), rect, &bm,
+                                                      cache));
+
+        // Finding more than once works fine.
+        REPORTER_ASSERT(reporter, SkBitmapCache::Find(cachedBitmap.getGenerationID(), rect, &bm,
+                                                      cache));
+        bm.unlockPixels();
+
+        // Drop the pixels in the bitmap.
+        if (factory) {
+            REPORTER_ASSERT(reporter, gPool->getRAMUsed() > 0);
+            gPool->dumpPool();
+
+            // The bitmap is not in the cache since it has been dropped.
+            REPORTER_ASSERT(reporter, !SkBitmapCache::Find(cachedBitmap.getGenerationID(), rect,
+                                                           &bm, cache));
+        }
 
-    // Drop the pixels in the bitmap.
-    if (factory) {
-        REPORTER_ASSERT(reporter, SkGetGlobalDiscardableMemoryPool()->getRAMUsed() > 0);
-        SkGetGlobalDiscardableMemoryPool()->dumpPool();
+        make_bitmap(&cachedBitmap, SkImageInfo::Make(5, 5, testType, kPremul_SkAlphaType),
+                    allocator);
+        cachedBitmap.setImmutable();
+        cachedBitmap.unlockPixels();
 
-        // The bitmap is not in the cache since it has been dropped.
-        REPORTER_ASSERT(reporter, !SkBitmapCache::Find(cachedBitmap.getGenerationID(), rect, &bm, cache));
+        // We can add the bitmap back to the cache and find it again.
+        REPORTER_ASSERT(reporter, SkBitmapCache::Add(cachedBitmap.pixelRef(), rect, cachedBitmap,
+                                                     cache));
+        REPORTER_ASSERT(reporter, SkBitmapCache::Find(cachedBitmap.getGenerationID(), rect, &bm,
+                                                      cache));
     }
-
-    make_bitmap(&cachedBitmap, SkImageInfo::MakeN32Premul(5, 5), allocator);
-    cachedBitmap.setImmutable();
-    cachedBitmap.unlockPixels();
-
-    // We can add the bitmap back to the cache and find it again.
-    REPORTER_ASSERT(reporter, SkBitmapCache::Add(cachedBitmap.pixelRef(), rect, cachedBitmap, cache));
-    REPORTER_ASSERT(reporter, SkBitmapCache::Find(cachedBitmap.getGenerationID(), rect, &bm, cache));
-
     test_mipmapcache(reporter, cache);
     test_bitmap_notify(reporter, cache);
     test_mipmap_notify(reporter, cache);
 }
 
+DEF_TEST(BitmapCache_discarded_bitmap, reporter) {
+    const size_t byteLimit = 100 * 1024;
+    {
+        SkResourceCache cache(byteLimit);
+        testBitmapCache_discarded_bitmap(reporter, &cache, nullptr);
+    }
+    {
+        SkAutoTUnref<SkDiscardableMemoryPool> pool(
+            SkDiscardableMemoryPool::Create(byteLimit, nullptr));
+        gPool = pool.get();
+        SkResourceCache::DiscardableFactory factory = pool_factory;
+        SkResourceCache cache(factory);
+        testBitmapCache_discarded_bitmap(reporter, &cache, factory);
+    }
+}
+
 static void test_discarded_image(skiatest::Reporter* reporter, const SkMatrix& transform,
                                  SkImage* (*buildImage)()) {
     SkAutoTUnref<SkSurface> surface(SkSurface::NewRasterN32Premul(10, 10));