Revert "Revert "simplify api to bitmapcache""

Remove (for now) overly agressive assert

This reverts commit dd1b4e94c4.

Bug: skia:
Change-Id: Ibdcf0b2e92cf108ae710cd180d4fa359f81176d1
Reviewed-on: https://skia-review.googlesource.com/10129
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Reed <reed@google.com>
This commit is contained in:
Mike Reed 2017-03-25 09:51:00 -04:00 committed by Skia Commit-Bot
parent 1b52df3392
commit 5fa3d6d4b1
8 changed files with 101 additions and 92 deletions

View File

@ -50,23 +50,45 @@ static SkIRect get_bounds_from_bitmap(const SkBitmap& bm) {
* return that subset (see get_bounds_from_bitmap).
*/
static SkIRect get_bounds_from_image(const SkImage* image) {
SkASSERT(image->width() > 0 && image->height() > 0);
return SkIRect::MakeWH(image->width(), image->height());
}
SkBitmapCacheDesc SkBitmapCacheDesc::Make(const SkBitmap& bm, int width, int height) {
return { bm.getGenerationID(), width, height, get_bounds_from_bitmap(bm) };
SkBitmapCacheDesc SkBitmapCacheDesc::Make(uint32_t imageID, int origWidth, int origHeight) {
SkASSERT(imageID);
SkASSERT(origWidth > 0 && origHeight > 0);
return { imageID, 0, 0, {0, 0, origWidth, origHeight} };
}
SkBitmapCacheDesc SkBitmapCacheDesc::Make(const SkBitmap& bm, int scaledWidth, int scaledHeight) {
SkASSERT(bm.width() > 0 && bm.height() > 0);
SkASSERT(scaledWidth > 0 && scaledHeight > 0);
SkASSERT(scaledWidth != bm.width() || scaledHeight != bm.height());
return { bm.getGenerationID(), scaledWidth, scaledHeight, get_bounds_from_bitmap(bm) };
}
SkBitmapCacheDesc SkBitmapCacheDesc::Make(const SkBitmap& bm) {
return Make(bm, bm.width(), bm.height());
SkASSERT(bm.width() > 0 && bm.height() > 0);
SkASSERT(bm.pixelRefOrigin() == SkIPoint::Make(0, 0));
return { bm.getGenerationID(), 0, 0, get_bounds_from_bitmap(bm) };
}
SkBitmapCacheDesc SkBitmapCacheDesc::Make(const SkImage* image, int width, int height) {
return { image->uniqueID(), width, height, get_bounds_from_image(image) };
SkBitmapCacheDesc SkBitmapCacheDesc::Make(const SkImage* image, int scaledWidth, int scaledHeight) {
SkASSERT(image->width() > 0 && image->height() > 0);
SkASSERT(scaledWidth > 0 && scaledHeight > 0);
// If the dimensions are the same, should we set them to 0,0?
//SkASSERT(scaledWidth != image->width() || scaledHeight != image->height());
return { image->uniqueID(), scaledWidth, scaledHeight, get_bounds_from_image(image) };
}
SkBitmapCacheDesc SkBitmapCacheDesc::Make(const SkImage* image) {
return Make(image, image->width(), image->height());
SkASSERT(image->width() > 0 && image->height() > 0);
return { image->uniqueID(), 0, 0, get_bounds_from_image(image) };
}
namespace {
@ -74,36 +96,21 @@ static unsigned gBitmapKeyNamespaceLabel;
struct BitmapKey : public SkResourceCache::Key {
public:
BitmapKey(uint32_t genID, int width, int height, const SkIRect& bounds)
: fDesc({ genID, width, height, bounds })
{
this->init(&gBitmapKeyNamespaceLabel, SkMakeResourceCacheSharedIDForBitmap(fDesc.fImageID),
sizeof(fDesc));
}
BitmapKey(const SkBitmapCacheDesc& desc) : fDesc(desc) {
this->init(&gBitmapKeyNamespaceLabel, SkMakeResourceCacheSharedIDForBitmap(fDesc.fImageID),
sizeof(fDesc));
}
void dump() const {
SkDebugf("-- add [%d %d] %d [%d %d %d %d]\n", fDesc.fWidth, fDesc.fHeight, fDesc.fImageID,
fDesc.fBounds.x(), fDesc.fBounds.y(), fDesc.fBounds.width(), fDesc.fBounds.height());
SkDebugf("-- add [%d %d] %d [%d %d %d %d]\n",
fDesc.fScaledWidth, fDesc.fScaledHeight, fDesc.fImageID,
fDesc.fSubset.x(), fDesc.fSubset.y(), fDesc.fSubset.width(), fDesc.fSubset.height());
}
const SkBitmapCacheDesc fDesc;
};
struct BitmapRec : public SkResourceCache::Rec {
BitmapRec(uint32_t genID, int width, int height, const SkIRect& bounds, const SkBitmap& result)
: fKey(genID, width, height, bounds)
, fBitmap(result)
{
#ifdef TRACE_NEW_BITMAP_CACHE_RECS
fKey.dump();
#endif
}
BitmapRec(const SkBitmapCacheDesc& desc, const SkBitmap& result)
: fKey(desc)
, fBitmap(result)
@ -139,41 +146,21 @@ private:
#define CHECK_LOCAL(localCache, localName, globalName, ...) \
((localCache) ? localCache->localName(__VA_ARGS__) : SkResourceCache::globalName(__VA_ARGS__))
bool SkBitmapCache::FindWH(const SkBitmapCacheDesc& desc, SkBitmap* result,
SkResourceCache* localCache) {
if (0 == desc.fWidth || 0 == desc.fHeight) {
// degenerate
return false;
}
bool SkBitmapCache::Find(const SkBitmapCacheDesc& desc, SkBitmap* result,
SkResourceCache* localCache) {
desc.validate();
return CHECK_LOCAL(localCache, find, Find, BitmapKey(desc), BitmapRec::Finder, result);
}
bool SkBitmapCache::AddWH(const SkBitmapCacheDesc& desc, const SkBitmap& result,
SkResourceCache* localCache) {
if (0 == desc.fWidth || 0 == desc.fHeight) {
// degenerate, and the key we use for mipmaps
return false;
}
bool SkBitmapCache::Add(const SkBitmapCacheDesc& desc, const SkBitmap& result,
SkResourceCache* localCache) {
desc.validate();
SkASSERT(result.isImmutable());
BitmapRec* rec = new BitmapRec(desc, result);
CHECK_LOCAL(localCache, add, Add, rec);
return true;
}
bool SkBitmapCache::Find(uint32_t genID, SkBitmap* result, SkResourceCache* localCache) {
BitmapKey key(genID, SK_Scalar1, SK_Scalar1, SkIRect::MakeEmpty());
return CHECK_LOCAL(localCache, find, Find, key, BitmapRec::Finder, result);
}
void SkBitmapCache::Add(uint32_t genID, const SkBitmap& result, SkResourceCache* localCache) {
SkASSERT(result.isImmutable());
BitmapRec* rec = new BitmapRec(genID, 1, 1, SkIRect::MakeEmpty(), result);
CHECK_LOCAL(localCache, add, Add, rec);
}
//////////////////////////////////////////////////////////////////////////////////////////
//////////////////////////////////////////////////////////////////////////////////////////
@ -182,21 +169,26 @@ static unsigned gMipMapKeyNamespaceLabel;
struct MipMapKey : public SkResourceCache::Key {
public:
MipMapKey(uint32_t genID, SkDestinationSurfaceColorMode colorMode, const SkIRect& bounds)
: fGenID(genID), fColorMode(static_cast<uint32_t>(colorMode)), fBounds(bounds)
MipMapKey(uint32_t imageID, const SkIRect& subset, SkDestinationSurfaceColorMode colorMode)
: fImageID(imageID)
, fColorMode(static_cast<uint32_t>(colorMode))
, fSubset(subset)
{
this->init(&gMipMapKeyNamespaceLabel, SkMakeResourceCacheSharedIDForBitmap(genID),
sizeof(fGenID) + sizeof(fColorMode) + sizeof(fBounds));
SkASSERT(fImageID);
SkASSERT(!subset.isEmpty());
this->init(&gMipMapKeyNamespaceLabel, SkMakeResourceCacheSharedIDForBitmap(fImageID),
sizeof(fImageID) + sizeof(fColorMode) + sizeof(fSubset));
}
uint32_t fGenID;
uint32_t fImageID;
uint32_t fColorMode;
SkIRect fBounds;
SkIRect fSubset;
};
struct MipMapRec : public SkResourceCache::Rec {
MipMapRec(const SkBitmap& src, SkDestinationSurfaceColorMode colorMode, const SkMipMap* result)
: fKey(src.getGenerationID(), colorMode, get_bounds_from_bitmap(src))
MipMapRec(uint32_t imageID, const SkIRect& subset, SkDestinationSurfaceColorMode colorMode,
const SkMipMap* result)
: fKey(imageID, subset, colorMode)
, fMipMap(result)
{
fMipMap->attachToCacheAndRef();
@ -236,8 +228,9 @@ private:
const SkMipMap* SkMipMapCache::FindAndRef(const SkBitmapCacheDesc& desc,
SkDestinationSurfaceColorMode colorMode,
SkResourceCache* localCache) {
// Note: we ignore width/height from desc, just need id and bounds
MipMapKey key(desc.fImageID, colorMode, desc.fBounds);
SkASSERT(desc.fScaledWidth == 0);
SkASSERT(desc.fScaledHeight == 0);
MipMapKey key(desc.fImageID, desc.fSubset, colorMode);
const SkMipMap* result;
if (!CHECK_LOCAL(localCache, find, Find, key, MipMapRec::Finder, &result)) {
@ -256,7 +249,8 @@ const SkMipMap* SkMipMapCache::AddAndRef(const SkBitmap& src,
SkResourceCache* localCache) {
SkMipMap* mipmap = SkMipMap::Build(src, colorMode, get_fact(localCache));
if (mipmap) {
MipMapRec* rec = new MipMapRec(src, colorMode, mipmap);
MipMapRec* rec = new MipMapRec(src.getGenerationID(), get_bounds_from_bitmap(src),
colorMode, mipmap);
CHECK_LOCAL(localCache, add, Add, rec);
src.pixelRef()->notifyAddedToCache();
}

View File

@ -19,15 +19,27 @@ uint64_t SkMakeResourceCacheSharedIDForBitmap(uint32_t bitmapGenID);
void SkNotifyBitmapGenIDIsStale(uint32_t bitmapGenID);
struct SkBitmapCacheDesc {
uint32_t fImageID;
int32_t fWidth;
int32_t fHeight;
SkIRect fBounds;
uint32_t fImageID; // != 0
int32_t fScaledWidth; // 0 for unscaled
int32_t fScaledHeight; // 0 for unscaled
SkIRect fSubset; // always set to a valid rect (entire or subset)
static SkBitmapCacheDesc Make(const SkBitmap&, int width, int height);
void validate() const {
SkASSERT(fImageID);
if (fScaledWidth || fScaledHeight) {
SkASSERT(fScaledWidth && fScaledHeight);
}
SkASSERT(fSubset.fLeft >= 0 && fSubset.fTop >= 0);
SkASSERT(fSubset.width() > 0 && fSubset.height() > 0);
}
static SkBitmapCacheDesc Make(const SkBitmap&, int scaledWidth, int scaledHeight);
static SkBitmapCacheDesc Make(const SkBitmap&);
static SkBitmapCacheDesc Make(const SkImage*, int width, int height);
static SkBitmapCacheDesc Make(const SkImage*, int scaledWidth, int scaledHeight);
static SkBitmapCacheDesc Make(const SkImage*);
// Use with care -- width/height must match the original bitmap/image
static SkBitmapCacheDesc Make(uint32_t genID, int origWidth, int origHeight);
};
class SkBitmapCache {
@ -42,22 +54,19 @@ public:
* Search based on the desc. If found, returns true and
* result will be set to the matching bitmap with its pixels already locked.
*/
static bool FindWH(const SkBitmapCacheDesc&, SkBitmap* result,
SkResourceCache* localCache = nullptr);
static bool Find(const SkBitmapCacheDesc&, SkBitmap* result,
SkResourceCache* localCache = nullptr);
/*
* result must be marked isImmutable()
*/
static bool AddWH(const SkBitmapCacheDesc&, const SkBitmap& result,
SkResourceCache* localCache = nullptr);
static bool Find(uint32_t genID, SkBitmap* result, SkResourceCache* localCache = nullptr);
// todo: eliminate the need to specify ID, since it should == the bitmap's
static void Add(uint32_t genID, const SkBitmap&, SkResourceCache* localCache = nullptr);
static bool Add(const SkBitmapCacheDesc&, const SkBitmap& result,
SkResourceCache* localCache = nullptr);
};
class SkMipMapCache {
public:
// Note: the scaled width/height in desc must be 0, as any other value would not make sense.
static const SkMipMap* FindAndRef(const SkBitmapCacheDesc&, SkDestinationSurfaceColorMode,
SkResourceCache* localCache = nullptr);
static const SkMipMap* AddAndRef(const SkBitmap& src, SkDestinationSurfaceColorMode,

View File

@ -124,7 +124,7 @@ bool SkDefaultBitmapControllerState::processHQRequest(const SkBitmapProvider& pr
const int dstH = SkScalarRoundToScalar(provider.height() / invScaleY);
const SkBitmapCacheDesc desc = provider.makeCacheDesc(dstW, dstH);
if (!SkBitmapCache::FindWH(desc, &fResultBitmap)) {
if (!SkBitmapCache::Find(desc, &fResultBitmap)) {
SkBitmap orig;
if (!provider.asBitmap(&orig)) {
return false;
@ -141,7 +141,7 @@ bool SkDefaultBitmapControllerState::processHQRequest(const SkBitmapProvider& pr
SkASSERT(fResultBitmap.getPixels());
fResultBitmap.setImmutable();
if (!provider.isVolatile()) {
if (SkBitmapCache::AddWH(desc, fResultBitmap)) {
if (SkBitmapCache::Add(desc, fResultBitmap)) {
provider.notifyAddedToCache();
}
}

View File

@ -180,7 +180,8 @@ bool SkImageCacherator::directGeneratePixels(const SkImageInfo& info, void* pixe
bool SkImageCacherator::lockAsBitmapOnlyIfAlreadyCached(SkBitmap* bitmap, CachedFormat format) {
return kNeedNewImageUniqueID != fUniqueIDs[format] &&
SkBitmapCache::Find(fUniqueIDs[format], bitmap) &&
SkBitmapCache::Find(SkBitmapCacheDesc::Make(fUniqueIDs[format],
fInfo.width(), fInfo.height()), bitmap) &&
check_output_bitmap(*bitmap, fUniqueIDs[format]);
}
@ -199,7 +200,8 @@ bool SkImageCacherator::tryLockAsBitmap(SkBitmap* bitmap, const SkImage* client,
}
bitmap->pixelRef()->setImmutableWithID(fUniqueIDs[format]);
if (SkImage::kAllow_CachingHint == chint) {
SkBitmapCache::Add(fUniqueIDs[format], *bitmap);
SkBitmapCache::Add(SkBitmapCacheDesc::Make(fUniqueIDs[format],
fInfo.width(), fInfo.height()), *bitmap);
if (client) {
as_IB(client)->notifyAddedToCache();
}
@ -259,7 +261,8 @@ bool SkImageCacherator::lockAsBitmap(GrContext* context, SkBitmap* bitmap, const
bitmap->pixelRef()->setImmutableWithID(fUniqueIDs[format]);
if (SkImage::kAllow_CachingHint == chint) {
SkBitmapCache::Add(fUniqueIDs[format], *bitmap);
SkBitmapCache::Add(SkBitmapCacheDesc::Make(fUniqueIDs[format],
fInfo.width(), fInfo.height()), *bitmap);
if (client) {
as_IB(client)->notifyAddedToCache();
}

View File

@ -401,7 +401,8 @@ public:
}
bool onGetROPixels(SkBitmap* dst) const override {
if (SkBitmapCache::Find(this->uniqueID(), dst)) {
const auto desc = SkBitmapCacheDesc::Make(this->uniqueID(), this->width(), this->height());
if (SkBitmapCache::Find(desc, dst)) {
SkASSERT(dst->getGenerationID() == this->uniqueID());
SkASSERT(dst->isImmutable());
SkASSERT(dst->getPixels());
@ -427,7 +428,7 @@ public:
}
dst->pixelRef()->setImmutableWithID(this->uniqueID());
SkBitmapCache::Add(this->uniqueID(), *dst);
SkBitmapCache::Add(desc, *dst);
fAddedRasterVersionToCache.store(true);
return true;
}

View File

@ -63,7 +63,8 @@ SkImageInfo SkImage_Gpu::onImageInfo() const {
}
bool SkImage_Gpu::getROPixels(SkBitmap* dst, SkColorSpace* dstColorSpace, CachingHint chint) const {
if (SkBitmapCache::Find(this->uniqueID(), dst)) {
const auto desc = SkBitmapCacheDesc::Make(this);
if (SkBitmapCache::Find(desc, dst)) {
SkASSERT(dst->getGenerationID() == this->uniqueID());
SkASSERT(dst->isImmutable());
SkASSERT(dst->getPixels());
@ -86,7 +87,7 @@ bool SkImage_Gpu::getROPixels(SkBitmap* dst, SkColorSpace* dstColorSpace, Cachin
dst->pixelRef()->setImmutableWithID(this->uniqueID());
if (kAllow_CachingHint == chint) {
SkBitmapCache::Add(this->uniqueID(), *dst);
SkBitmapCache::Add(desc, *dst);
fAddedRasterVersionToCache.store(true);
}
return true;

View File

@ -411,6 +411,7 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(c, reporter, ctxInfo) {
SkImageInfo info = SkImageInfo::MakeN32(20, 20, kOpaque_SkAlphaType);
sk_sp<SkImage> image(create_gpu_image(ctxInfo.grContext()));
const uint32_t uniqueID = image->uniqueID();
const auto desc = SkBitmapCacheDesc::Make(image.get());
auto surface(SkSurface::MakeRaster(info));
@ -418,13 +419,13 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(c, reporter, ctxInfo) {
{
SkBitmap cachedBitmap;
REPORTER_ASSERT(reporter, !SkBitmapCache::Find(uniqueID, &cachedBitmap));
REPORTER_ASSERT(reporter, !SkBitmapCache::Find(desc, &cachedBitmap));
}
surface->getCanvas()->drawImage(image, 0, 0);
{
SkBitmap cachedBitmap;
if (SkBitmapCache::Find(uniqueID, &cachedBitmap)) {
if (SkBitmapCache::Find(desc, &cachedBitmap)) {
REPORTER_ASSERT(reporter, cachedBitmap.getGenerationID() == uniqueID);
REPORTER_ASSERT(reporter, cachedBitmap.isImmutable());
REPORTER_ASSERT(reporter, cachedBitmap.getPixels());
@ -438,7 +439,7 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(c, reporter, ctxInfo) {
image.reset(nullptr);
{
SkBitmap cachedBitmap;
REPORTER_ASSERT(reporter, !SkBitmapCache::Find(uniqueID, &cachedBitmap));
REPORTER_ASSERT(reporter, !SkBitmapCache::Find(desc, &cachedBitmap));
}
}

View File

@ -89,8 +89,8 @@ static void test_mipmap_notify(skiatest::Reporter* reporter, SkResourceCache* ca
}
for (int i = 0; i < N; ++i) {
const SkMipMap* mipmap = SkMipMapCache::FindAndRef(SkBitmapCacheDesc::Make(src[i]),
colorMode, cache);
const auto desc = SkBitmapCacheDesc::Make(src[i]);
const SkMipMap* mipmap = SkMipMapCache::FindAndRef(desc, colorMode, cache);
if (cache) {
// if cache is null, we're working on the global cache, and other threads might purge
// it, making this check fragile.
@ -100,7 +100,7 @@ static void test_mipmap_notify(skiatest::Reporter* reporter, SkResourceCache* ca
src[i].reset(); // delete the underlying pixelref, which *should* remove us from the cache
mipmap = SkMipMapCache::FindAndRef(SkBitmapCacheDesc::Make(src[i]), colorMode, cache);
mipmap = SkMipMapCache::FindAndRef(desc, colorMode, cache);
REPORTER_ASSERT(reporter, !mipmap);
}
}
@ -156,14 +156,14 @@ static void test_discarded_image(skiatest::Reporter* reporter, const SkMatrix& t
canvas->concat(transform);
canvas->drawImage(image, 0, 0, &paint);
auto imageId = image->uniqueID();
const auto desc = SkBitmapCacheDesc::Make(image.get());
// delete the image
image.reset(nullptr);
// all resources should have been purged
SkBitmap result;
REPORTER_ASSERT(reporter, !SkBitmapCache::Find(imageId, &result));
REPORTER_ASSERT(reporter, !SkBitmapCache::Find(desc, &result));
}
}