From 4b3d3bebda80be611ea79ec63a92d632c0375f20 Mon Sep 17 00:00:00 2001 From: reed Date: Thu, 17 Sep 2015 13:35:19 -0700 Subject: [PATCH] use allocator (if present) when we allocate our cache bitmap Remove some bogus tests on the cache, as they are not thread-reliable. Running w/ discardable these are racy. BUG=532981 Review URL: https://codereview.chromium.org/1351453004 --- include/core/SkColorTable.h | 11 +++++ include/core/SkImageGenerator.h | 13 +++--- src/core/SkImageCacherator.cpp | 7 ++- src/core/SkImageGenerator.cpp | 82 +++++++++++++++++++++++---------- 4 files changed, 81 insertions(+), 32 deletions(-) diff --git a/include/core/SkColorTable.h b/include/core/SkColorTable.h index cb613befb1..ccea7ed550 100644 --- a/include/core/SkColorTable.h +++ b/include/core/SkColorTable.h @@ -67,6 +67,17 @@ private: void init(const SkPMColor* colors, int count); + friend class SkImageGenerator; + // Only call if no other thread or cache has seen this table. + void dangerous_overwriteColors(const SkPMColor newColors[], int count) { + if (count < 0 || count > fCount) { + sk_throw(); + } + // assumes that f16BitCache nas NOT been initialized yet, so we don't try to update it + memcpy(fColors, newColors, count * sizeof(SkPMColor)); + fCount = count; // update fCount, in case count is smaller + } + typedef SkRefCnt INHERITED; }; diff --git a/include/core/SkImageGenerator.h b/include/core/SkImageGenerator.h index 5b96671c98..fdafa0b87b 100644 --- a/include/core/SkImageGenerator.h +++ b/include/core/SkImageGenerator.h @@ -8,6 +8,7 @@ #ifndef SkImageGenerator_DEFINED #define SkImageGenerator_DEFINED +#include "SkBitmap.h" #include "SkColor.h" #include "SkImageInfo.h" @@ -166,18 +167,18 @@ public: const SkPaint*); bool tryGenerateBitmap(SkBitmap* bm) { - return this->tryGenerateBitmap(bm, nullptr); + return this->tryGenerateBitmap(bm, nullptr, nullptr); } - bool tryGenerateBitmap(SkBitmap* bm, const SkImageInfo& info) { - return this->tryGenerateBitmap(bm, &info); + bool tryGenerateBitmap(SkBitmap* bm, const SkImageInfo& info, SkBitmap::Allocator* allocator) { + return this->tryGenerateBitmap(bm, &info, allocator); } void generateBitmap(SkBitmap* bm) { - if (!this->tryGenerateBitmap(bm, nullptr)) { + if (!this->tryGenerateBitmap(bm, nullptr, nullptr)) { sk_throw(); } } void generateBitmap(SkBitmap* bm, const SkImageInfo& info) { - if (!this->tryGenerateBitmap(bm, &info)) { + if (!this->tryGenerateBitmap(bm, &info, nullptr)) { sk_throw(); } } @@ -197,7 +198,7 @@ protected: return nullptr; } - bool tryGenerateBitmap(SkBitmap* bm, const SkImageInfo* optionalInfo); + bool tryGenerateBitmap(SkBitmap* bm, const SkImageInfo* optionalInfo, SkBitmap::Allocator*); private: const SkImageInfo fInfo; diff --git a/src/core/SkImageCacherator.cpp b/src/core/SkImageCacherator.cpp index f5ec5e3477..91e342a79b 100644 --- a/src/core/SkImageCacherator.cpp +++ b/src/core/SkImageCacherator.cpp @@ -11,6 +11,7 @@ #include "SkMallocPixelRef.h" #include "SkNextID.h" #include "SkPixelRef.h" +#include "SkResourceCache.h" #if SK_SUPPORT_GPU #include "GrContext.h" @@ -82,18 +83,20 @@ static bool check_output_bitmap(const SkBitmap& bitmap, uint32_t expectedID) { // If you want the immutable bitmap with the same ID as our cacherator, call tryLockAsBitmap() // bool SkImageCacherator::generateBitmap(SkBitmap* bitmap) { + SkBitmap::Allocator* allocator = SkResourceCache::GetAllocator(); + ScopedGenerator generator(this); const SkImageInfo& genInfo = generator->getInfo(); if (fInfo.dimensions() == genInfo.dimensions()) { SkASSERT(fOrigin.x() == 0 && fOrigin.y() == 0); // fast-case, no copy needed - return generator->tryGenerateBitmap(bitmap, fInfo); + return generator->tryGenerateBitmap(bitmap, fInfo, allocator); } else { // need to handle subsetting, so we first generate the full size version, and then // "read" from it to get our subset. See skbug.com/4213 SkBitmap full; - if (!generator->tryGenerateBitmap(&full, genInfo)) { + if (!generator->tryGenerateBitmap(&full, genInfo, allocator)) { return false; } if (!bitmap->tryAllocPixels(fInfo, nullptr, full.getColorTable())) { diff --git a/src/core/SkImageGenerator.cpp b/src/core/SkImageGenerator.cpp index 6fe9f84a08..187b46047a 100644 --- a/src/core/SkImageGenerator.cpp +++ b/src/core/SkImageGenerator.cpp @@ -128,41 +128,75 @@ bool SkImageGenerator::onGetPixels(const SkImageInfo& info, void* dst, size_t rb #include "SkBitmap.h" #include "SkColorTable.h" -static void release_malloc_proc(void* pixels, void* ctx) { - sk_free(pixels); +static bool reset_and_return_false(SkBitmap* bitmap) { + bitmap->reset(); + return false; } -bool SkImageGenerator::tryGenerateBitmap(SkBitmap* bitmap, const SkImageInfo* infoPtr) { - const SkImageInfo info = infoPtr ? *infoPtr : this->getInfo(); - const size_t rowBytes = info.minRowBytes(); - const size_t pixelSize = info.getSafeSize(rowBytes); - if (0 == pixelSize) { +bool SkImageGenerator::tryGenerateBitmap(SkBitmap* bitmap, const SkImageInfo* infoPtr, + SkBitmap::Allocator* allocator) { + SkImageInfo info = infoPtr ? *infoPtr : this->getInfo(); + if (0 == info.getSafeSize(info.minRowBytes())) { return false; } + if (!bitmap->setInfo(info)) { + return reset_and_return_false(bitmap); + } - SkAutoFree pixelStorage(sk_malloc_flags(pixelSize, 0)); - void* pixels = pixelStorage.get(); - if (!pixels) { - return false; - } - SkPMColor ctStorage[256]; + memset(ctStorage, 0xFF, sizeof(ctStorage)); // init with opaque-white for the moment + SkAutoTUnref ctable(new SkColorTable(ctStorage, 256)); + if (!bitmap->tryAllocPixels(allocator, ctable)) { + // SkResourceCache's custom allcator can'thandle ctables, so it may fail on + // kIndex_8_SkColorTable. + // skbug.com/4355 +#if 1 + // ignroe the allocator, and see if we can succeed without it + if (!bitmap->tryAllocPixels(nullptr, ctable)) { + 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(info, pixels, rowBytes, ctStorage, &ctCount)) { - return false; + if (!this->getPixels(bitmap->info(), bitmap->getPixels(), bitmap->rowBytes(), + ctStorage, &ctCount)) { + return reset_and_return_false(bitmap); } - - SkAutoTUnref ctable; + if (ctCount > 0) { - SkASSERT(kIndex_8_SkColorType == info.colorType()); - ctable.reset(new SkColorTable(ctStorage, ctCount)); + 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 != info.colorType()); + SkASSERT(kIndex_8_SkColorType != bitmap->colorType()); + // we should be the only owner + SkASSERT(ctable->unique()); } - - return bitmap->installPixels(info, pixelStorage.detach(), rowBytes, ctable, - release_malloc_proc, nullptr); + return true; } #include "SkGraphics.h"