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

tests/SkResourceCacheTest.cpp

Index: tests/SkResourceCacheTest.cpp
diff --git a/tests/SkResourceCacheTest.cpp b/tests/SkResourceCacheTest.cpp
index 9faddd016da3f593882f0af86573217407a86c17..efc7091703174b93d091e975b70dc26404e79e83 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->tryAllocPixels(allocator, ctable);

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

I think this should be allocPixels, to match the others (so we'll throw instead
of silently failing).

+    } else if (allocator) {
         bitmap->setInfo(info);
         allocator->allocPixelRef(bitmap, 0);
     } else {
@@ -173,55 +180,91 @@ 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,
+        kARGB_4444_SkColorType,

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

Are we supporting 4444?

+        kRGBA_8888_SkColorType,
+        kBGRA_8888_SkColorType,
+        kIndex_8_SkColorType,
+        kGray_8_SkColorType
+    };
+    for (size_t i = 0; i < SK_ARRAY_COUNT(testTypes); ++i) {
+        SkBitmap cachedBitmap;
+        make_bitmap(&cachedBitmap, SkImageInfo::Make(5, 5, testTypes[i], 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, testTypes[i], 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;
+    {
+        SkAutoTDelete<SkResourceCache> cache;
+        cache.reset(new SkResourceCache(byteLimit));

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

nit: Why not just put this on the stack? (same down below)

[aleksandar.stojiljkovic, 2015/11/09 22:19:48]

Good point - refactored the purpose out. fixed.

+        testBitmapCache_discarded_bitmap(reporter, cache, nullptr);
+    }
+    {
+        SkAutoTDelete<SkResourceCache> cache;
+        SkAutoTUnref<SkDiscardableMemoryPool> pool(
+            SkDiscardableMemoryPool::Create(byteLimit, nullptr));
+        gPool = pool.get();
+        SkResourceCache::DiscardableFactory factory = pool_factory;
+        cache.reset(new SkResourceCache(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));