cacherator upscales colortables to unify caching

Bug: skia:
Change-Id: Ib63f96b83d696743bbe4335c998acd4d2ea8acdb
Reviewed-on: https://skia-review.googlesource.com/11787
Reviewed-by: Matt Sarett <msarett@google.com>
Commit-Queue: Mike Reed <reed@google.com>

diff --git a/gm/pictureimagegenerator.cpp b/gm/pictureimagegenerator.cpp
index 2154e52..a54fa64 100644 (file)
--- a/gm/pictureimagegenerator.cpp
+++ b/gm/pictureimagegenerator.cpp
@@ -162,7 +162,8 @@ protected:
             SkImageInfo bmInfo = gen->getInfo().makeColorSpace(canvas->imageInfo().refColorSpace());
 
             SkBitmap bm;
-            SkAssertResult(gen->tryGenerateBitmap(&bm, bmInfo, nullptr));
+            bm.allocPixels(bmInfo);
+            SkAssertResult(gen->getPixels(bm.info(), bm.getPixels(), bm.rowBytes()));
 
             const SkScalar x = kDrawSize * (i % kDrawsPerRow);
             const SkScalar y = kDrawSize * (i / kDrawsPerRow);

diff --git a/include/core/SkImageGenerator.h b/include/core/SkImageGenerator.h
index 8624359..846c380 100644 (file)
--- a/include/core/SkImageGenerator.h
+++ b/include/core/SkImageGenerator.h
@@ -156,8 +156,6 @@ public:
                                                              SkImage::BitDepth,
                                                              sk_sp<SkColorSpace>);
 
-    bool tryGenerateBitmap(SkBitmap* bm, const SkImageInfo& info, SkBitmap::Allocator* allocator);
-
 protected:
     enum {
         kNeedNewImageUniqueID = 0

diff --git a/src/core/SkImageCacherator.cpp b/src/core/SkImageCacherator.cpp
index d2bb14f..a9db283 100644 (file)
--- a/src/core/SkImageCacherator.cpp
+++ b/src/core/SkImageCacherator.cpp
@@ -89,6 +89,12 @@ SkImageCacherator::Validator::Validator(sk_sp<SharedGenerator> gen, const SkIRec
     fInfo   = info.makeWH(subset->width(), subset->height());
     fOrigin = SkIPoint::Make(subset->x(), subset->y());
 
+    // colortables are poorly to not-at-all supported in our resourcecache, so we
+    // bully them into N32 (the generator will perform the up-sample)
+    if (fInfo.colorType() == kIndex_8_SkColorType) {
+        fInfo = fInfo.makeColorType(kN32_SkColorType);
+    }
+
     // If the encoded data is in a strange color space (it's not an XYZ matrix space), we won't be
     // able to preserve the gamut of the encoded data when we decode it. Instead, we'll have to
     // decode to a known color space (linear sRGB is a good choice). But we need to adjust the
@@ -119,6 +125,7 @@ SkImageCacherator::SkImageCacherator(Validator* validator)
         fUniqueIDs[i] = kNeedNewImageUniqueID;
     }
     SkASSERT(fSharedGenerator);
+    SkASSERT(fInfo.colorType() != kIndex_8_SkColorType);
 }
 
 SkImageCacherator::~SkImageCacherator() {}
@@ -135,6 +142,31 @@ static bool check_output_bitmap(const SkBitmap& bitmap, uint32_t expectedID) {
     return true;
 }
 
+static bool reset_and_return_false(SkBitmap* bitmap) {
+    bitmap->reset();
+    return false;
+}
+
+static bool try_generate_bitmap(SkImageGenerator* gen, SkBitmap* bitmap, const SkImageInfo& info,
+                                SkBitmap::Allocator* allocator) {
+    SkASSERT(info.colorType() != kIndex_8_SkColorType);
+    if (0 == info.getSafeSize(info.minRowBytes())) {
+        return false;
+    }
+    if (!bitmap->setInfo(info)) {
+        return reset_and_return_false(bitmap);
+    }
+    if (!bitmap->tryAllocPixels(allocator, nullptr)) {
+        return reset_and_return_false(bitmap);
+    }
+    SkASSERT(bitmap->getPixels());  // we're already locked
+
+    if (!gen->getPixels(bitmap->info(), bitmap->getPixels(), bitmap->rowBytes())) {
+        return reset_and_return_false(bitmap);
+    }
+    return true;
+}
+
 // Note, this returns a new, mutable, bitmap, with a new genID.
 // If you want the immutable bitmap with the same ID as our cacherator, call tryLockAsBitmap()
 //
@@ -146,18 +178,18 @@ bool SkImageCacherator::generateBitmap(SkBitmap* bitmap, const SkImageInfo& deco
     if (decodeInfo.dimensions() == genInfo.dimensions()) {
         SkASSERT(fOrigin.x() == 0 && fOrigin.y() == 0);
         // fast-case, no copy needed
-        return generator->tryGenerateBitmap(bitmap, decodeInfo, allocator);
+        return try_generate_bitmap(generator, bitmap, decodeInfo, allocator);
     } else {
         // need to handle subsetting, so we first generate the full size version, and then
         // "read" from it to get our subset. See https://bug.skia.org/4213
 
         SkBitmap full;
-        if (!generator->tryGenerateBitmap(&full,
-                                          decodeInfo.makeWH(genInfo.width(), genInfo.height()),
-                                          allocator)) {
+        if (!try_generate_bitmap(generator, &full,
+                                 decodeInfo.makeWH(genInfo.width(), genInfo.height()), allocator)) {
             return false;
         }
-        if (!bitmap->tryAllocPixels(decodeInfo, sk_ref_sp(full.getColorTable()))) {
+        SkASSERT(decodeInfo.colorType() != kIndex_8_SkColorType);
+        if (!bitmap->tryAllocPixels(decodeInfo)) {
             return false;
         }
         return full.readPixels(bitmap->info(), bitmap->getPixels(), bitmap->rowBytes(),

diff --git a/src/core/SkImageGenerator.cpp b/src/core/SkImageGenerator.cpp
index 2f209ca..f47bb1d 100644 (file)
--- a/src/core/SkImageGenerator.cpp
+++ b/src/core/SkImageGenerator.cpp
@@ -120,76 +120,6 @@ bool SkImageGenerator::onGetPixels(const SkImageInfo& info, void* dst, size_t rb
 #include "SkBitmap.h"
 #include "SkColorTable.h"
 
-static bool reset_and_return_false(SkBitmap* bitmap) {
-    bitmap->reset();
-    return false;
-}
-
-bool SkImageGenerator::tryGenerateBitmap(SkBitmap* bitmap, const SkImageInfo& info,
-                                         SkBitmap::Allocator* allocator) {
-    if (0 == info.getSafeSize(info.minRowBytes())) {
-        return false;
-    }
-    if (!bitmap->setInfo(info)) {
-        return reset_and_return_false(bitmap);
-    }
-
-    SkPMColor ctStorage[256];
-    memset(ctStorage, 0xFF, sizeof(ctStorage)); // init with opaque-white for the moment
-    sk_sp<SkColorTable> ctable(new SkColorTable(ctStorage, 256));
-    if (!bitmap->tryAllocPixels(allocator, ctable.get())) {
-        // SkResourceCache's custom allcator can'thandle ctables, so it may fail on
-        // kIndex_8_SkColorTable.
-        // https://bug.skia.org/4355
-#if 1
-        // ignore the allocator, and see if we can succeed without it
-        if (!bitmap->tryAllocPixels(nullptr, ctable.get())) {
-            return reset_and_return_false(bitmap);
-        }
-#else
-        // this is the up-scale technique, not fully debugged, but we keep it here at the moment
-        // to remind ourselves that this might be better than ignoring the allocator.
-
-        info = SkImageInfo::MakeN32(info.width(), info.height(), info.alphaType());
-        if (!bitmap->setInfo(info)) {
-            return reset_and_return_false(bitmap);
-        }
-        // we pass nullptr for the ctable arg, since we are now explicitly N32
-        if (!bitmap->tryAllocPixels(allocator, nullptr)) {
-            return reset_and_return_false(bitmap);
-        }
-#endif
-    }
-
-    bitmap->lockPixels();
-    if (!bitmap->getPixels()) {
-        return reset_and_return_false(bitmap);
-    }
-
-    int ctCount = 0;
-    if (!this->getPixels(bitmap->info(), bitmap->getPixels(), bitmap->rowBytes(),
-                         ctStorage, &ctCount)) {
-        return reset_and_return_false(bitmap);
-    }
-
-    if (ctCount > 0) {
-        SkASSERT(kIndex_8_SkColorType == bitmap->colorType());
-        // we and bitmap should be owners
-        SkASSERT(!ctable->unique());
-
-        // Now we need to overwrite the ctable we built earlier, with the correct colors.
-        // This does mean that we may have made the table too big, but that cannot be avoided
-        // until we can change SkImageGenerator's API to return us the ctable *before* we have to
-        // allocate space for all the pixels.
-        ctable->dangerous_overwriteColors(ctStorage, ctCount);
-    } else {
-        SkASSERT(kIndex_8_SkColorType != bitmap->colorType());
-        // we should be the only owner
-        SkASSERT(ctable->unique());
-    }
-    return true;
-}
-
 #include "SkGraphics.h"
 
 static SkGraphics::ImageGeneratorFromEncodedDataFactory gFactory;