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

tests/CachedDecodingPixelRefTest.cpp

 /*
  * Copyright 2013 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkBitmap.h"
 #include "SkCanvas.h"
 #include "SkData.h"
@@ skipping 145 matching lines @@
 
 class TestImageGenerator : public SkImageGenerator {
 public:
     enum TestType {
         kFailGetPixels_TestType,
         kSucceedGetPixels_TestType,
         kLast_TestType = kSucceedGetPixels_TestType
     };
     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) {
+    static uint32_t Color() { return 0xff10345a; } // value choosen so that there is no loss when converting to to RGB565 and back

[reed1, 2015/11/09 14:14:19]

I know the old code used uint32_t for this type, but w/ your extensions to the
test, it is getting very confusion what this type is, specifically, is it
SkColor or SkPMColor.

Since your modifications at the bottom of this file now always call
bm.getColor(x,y), I think we should declare this to be SkColor. To do that
cleanly, we'll need to convert it to SkPMColor when we use it to write into our
image buffer (both for kN32 and for the colortable entry.

[reed1, 2015/11/09 14:14:19]

nits: skia limits itself to 100 columns

[aleksandar.stojiljkovic, 2015/11/09 15:28:00]

Acknowledged.

[aleksandar.stojiljkovic, 2015/11/09 15:28:00]

Acknowledged.

+    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(),
+    static SkImageInfo GetMyInfo(SkColorType colorType) {
+        return SkImageInfo::Make(TestImageGenerator::Width(), TestImageGenerator::Height(), colorType,

[reed1, 2015/11/09 14:14:19]

100 cols

[aleksandar.stojiljkovic, 2015/11/09 15:28:00]

Acknowledged.

                                     kOpaque_SkAlphaType);
     }
 
     bool onGetPixels(const SkImageInfo& info, void* pixels, size_t rowBytes,
                      SkPMColor ctable[], int* ctableCount) override {
         REPORTER_ASSERT(fReporter, pixels != nullptr);
         REPORTER_ASSERT(fReporter, rowBytes >= info.minRowBytes());
         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(reinterpret_cast<SkColor*>(bytePtr),
+                    TestImageGenerator::Color(), info.width());
+                bytePtr += rowBytes;
+            }
+            break;
+        case kIndex_8_SkColorType:
+            *ctableCount = 1;
+            ctable[0] = TestImageGenerator::Color();
+            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,
+                    SkDitherPixel32ToPixel16(TestImageGenerator::Color()), info.width());

[reed1, 2015/11/09 14:14:19]

why the dither conversion? Since you're repeating the same value for all pixels,
it is a little confusing/unnecessary it seems.

[aleksandar.stojiljkovic, 2015/11/09 15:28:00]

I just didn't know better:
SkDitherPixel32ToPixel16 (though called diether) doesn't take surrounding pixels
into account, but only does RGB888-RGB565 quantization and packing into 16 bits:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/skia/i...

I wanted to call directly SkPackRGB16, but there is assert if components don't
fit 5/6 bits. 

[reed1, 2015/11/09 15:32:47]

That dither call is the compliment to SkPixel32ToPixel16(). and *always* packs
with a different bias applied, so it can pair up with SkPixel32ToPixel16
(calling each alternatively).

I think your code will read simpler if you just call SkPixel32ToPixel16().

+                bytePtr += rowBytes;
+            }
+            break;
+        default:
+            return false;
         }
         return true;
     }
 
 private:
     const TestType fType;
     skiatest::Reporter* const fReporter;
 
     typedef SkImageGenerator INHERITED;
 };
 
 static void check_test_image_generator_bitmap(skiatest::Reporter* reporter,
                                               const SkBitmap& bm) {
     REPORTER_ASSERT(reporter, TestImageGenerator::Width() == bm.width());
     REPORTER_ASSERT(reporter, TestImageGenerator::Height() == bm.height());
     SkAutoLockPixels autoLockPixels(bm);
     REPORTER_ASSERT(reporter, bm.getPixels());
     if (nullptr == bm.getPixels()) {
         return;
     }
     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;
             }
         }
     }
     REPORTER_ASSERT(reporter, 0 == errors);
 }
 
 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);
 
     REPORTER_ASSERT(reporter, success);
     if (TestImageGenerator::kSucceedGetPixels_TestType == type) {
         check_test_image_generator_bitmap(reporter, lazy);
     } else if (TestImageGenerator::kFailGetPixels_TestType == type) {
         SkAutoLockPixels autoLockPixels(lazy);
         REPORTER_ASSERT(reporter, nullptr == lazy.getPixels());
     }
 }
 
 /**
  *  This tests the basic functionality of SkDiscardablePixelRef with a
  *  basic SkImageGenerator implementation and several
  *  SkDiscardableMemory::Factory choices.
  */
 DEF_TEST(DiscardableAndCachingPixelRef, reporter) {
-    check_pixelref(TestImageGenerator::kFailGetPixels_TestType, reporter, nullptr);
-    check_pixelref(TestImageGenerator::kSucceedGetPixels_TestType, reporter, nullptr);
+    SkColorType testColorTypes[] = {

[reed1, 2015/11/09 14:14:19]

nit: const

+        kN32_SkColorType,
+        kIndex_8_SkColorType,
+        kRGB_565_SkColorType
+    };
+    for (size_t i = 0; i < SK_ARRAY_COUNT(testColorTypes); ++i) {
+        check_pixelref(TestImageGenerator::kFailGetPixels_TestType, reporter, nullptr, testColorTypes[i]);

[reed1, 2015/11/09 14:14:19]

100 cols

+        check_pixelref(TestImageGenerator::kSucceedGetPixels_TestType, reporter, nullptr, testColorTypes[i]);
 
-    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, testColorTypes[i]);
+        REPORTER_ASSERT(reporter, 0 == pool->getRAMUsed());
+        check_pixelref(TestImageGenerator::kSucceedGetPixels_TestType, reporter, pool, testColorTypes[i]);
+        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, testColorTypes[i]);
+        check_pixelref(TestImageGenerator::kSucceedGetPixels_TestType, reporter, globalPool, testColorTypes[i]);
+    }
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 
 DEF_TEST(Image_NewFromGenerator, r) {
     TestImageGenerator::TestType testTypes[] = {
         TestImageGenerator::kFailGetPixels_TestType,
         TestImageGenerator::kSucceedGetPixels_TestType,
     };
+    SkColorType testColorTypes[] = {

[reed1, 2015/11/09 14:14:19]

const

+        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 (size_t j = 0; j < SK_ARRAY_COUNT(testColorTypes); ++j) {
+            SkImageGenerator* gen = new TestImageGenerator(test, r, testColorTypes[j]);
+            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));
+            }
         }
     }
 }