diff --git a/src/core/SkBitmapCache.cpp b/src/core/SkBitmapCache.cpp index 4a028d457c..1ac8f36b4d 100644 --- a/src/core/SkBitmapCache.cpp +++ b/src/core/SkBitmapCache.cpp @@ -51,29 +51,22 @@ static SkIRect get_bounds_from_image(const SkImage* image) { return SkIRect::MakeWH(image->width(), image->height()); } -static uint64_t get_hash_from_color_space(SkColorSpace* colorSpace) { - return colorSpace ? colorSpace->hash() : 0; -} - -SkBitmapCacheDesc SkBitmapCacheDesc::Make(uint32_t imageID, SkColorType colorType, - SkColorSpace* colorSpace, const SkIRect& subset) { +SkBitmapCacheDesc SkBitmapCacheDesc::Make(uint32_t imageID, const SkIRect& subset) { SkASSERT(imageID); SkASSERT(subset.width() > 0 && subset.height() > 0); - return { imageID, colorType, get_hash_from_color_space(colorSpace), subset }; + return { imageID, subset }; } SkBitmapCacheDesc SkBitmapCacheDesc::Make(const SkBitmap& bm) { SkASSERT(bm.width() > 0 && bm.height() > 0); - return { bm.getGenerationID(), bm.colorType(), get_hash_from_color_space(bm.colorSpace()), - get_bounds_from_bitmap(bm) }; + return { bm.getGenerationID(), get_bounds_from_bitmap(bm) }; } SkBitmapCacheDesc SkBitmapCacheDesc::Make(const SkImage* image) { SkASSERT(image->width() > 0 && image->height() > 0); - return { image->uniqueID(), image->colorType(), get_hash_from_color_space(image->colorSpace()), - get_bounds_from_image(image) }; + return { image->uniqueID(), get_bounds_from_image(image) }; } namespace { @@ -117,9 +110,8 @@ public: { SkASSERT(!(fDM && fMalloc)); // can't have both - // We need an ID to return with the bitmap/pixelref. We can't necessarily use the key/desc - // ID - lazy images cache the same ID with multiple keys (in different color types). - fPrUniqueID = SkNextID::ImageID(); + // We need an ID to return with the bitmap/pixelref - return the same ID as the key/desc + fPrUniqueID = desc.fImageID; REC_TRACE(" Rec(%d): [%d %d] %d\n", sk_atomic_inc(&gRecCounter), fInfo.width(), fInfo.height(), fPrUniqueID); } @@ -258,8 +250,6 @@ SkBitmapCache::RecPtr SkBitmapCache::Alloc(const SkBitmapCacheDesc& desc, const // Ensure that the info matches the subset (i.e. the subset is the entire image) SkASSERT(info.width() == desc.fSubset.width()); SkASSERT(info.height() == desc.fSubset.height()); - SkASSERT(info.colorType() == desc.fColorType); - SkASSERT(get_hash_from_color_space(info.colorSpace()) == desc.fColorSpaceHash); const size_t rb = info.minRowBytes(); size_t size = info.computeByteSize(rb); diff --git a/src/core/SkBitmapCache.h b/src/core/SkBitmapCache.h index f7e97e32c2..3909f73182 100644 --- a/src/core/SkBitmapCache.h +++ b/src/core/SkBitmapCache.h @@ -20,21 +20,17 @@ void SkNotifyBitmapGenIDIsStale(uint32_t bitmapGenID); struct SkBitmapCacheDesc { uint32_t fImageID; // != 0 - SkColorType fColorType; - uint64_t fColorSpaceHash; SkIRect fSubset; // always set to a valid rect (entire or subset) void validate() const { SkASSERT(fImageID); SkASSERT(fSubset.fLeft >= 0 && fSubset.fTop >= 0); SkASSERT(fSubset.width() > 0 && fSubset.height() > 0); - SkASSERT(kUnknown_SkColorType != fColorType); } static SkBitmapCacheDesc Make(const SkBitmap&); static SkBitmapCacheDesc Make(const SkImage*); - static SkBitmapCacheDesc Make(uint32_t genID, SkColorType, SkColorSpace*, - const SkIRect& subset); + static SkBitmapCacheDesc Make(uint32_t genID, const SkIRect& subset); }; class SkBitmapCache { diff --git a/src/core/SkSpecialImage.cpp b/src/core/SkSpecialImage.cpp index ddff0756c9..f89abd5af0 100644 --- a/src/core/SkSpecialImage.cpp +++ b/src/core/SkSpecialImage.cpp @@ -396,8 +396,7 @@ public: } bool onGetROPixels(SkBitmap* dst) const override { - const auto desc = SkBitmapCacheDesc::Make(this->uniqueID(), kN32_SkColorType, - fColorSpace.get(), this->subset()); + const auto desc = SkBitmapCacheDesc::Make(this->uniqueID(), this->subset()); if (SkBitmapCache::Find(desc, dst)) { SkASSERT(dst->getGenerationID() == this->uniqueID()); SkASSERT(dst->isImmutable()); diff --git a/src/image/SkImage_GpuBase.cpp b/src/image/SkImage_GpuBase.cpp index a718fdb3d6..b5105abf09 100644 --- a/src/image/SkImage_GpuBase.cpp +++ b/src/image/SkImage_GpuBase.cpp @@ -61,6 +61,7 @@ bool SkImage_GpuBase::getROPixels(SkBitmap* dst, SkColorSpace*, CachingHint chin // rolled the dstColorSpace into the key). const auto desc = SkBitmapCacheDesc::Make(this); if (SkBitmapCache::Find(desc, dst)) { + SkASSERT(dst->getGenerationID() == this->uniqueID()); SkASSERT(dst->isImmutable()); SkASSERT(dst->getPixels()); return true; diff --git a/src/image/SkImage_Lazy.cpp b/src/image/SkImage_Lazy.cpp index 7071b4b6f6..2350cb8ea6 100644 --- a/src/image/SkImage_Lazy.cpp +++ b/src/image/SkImage_Lazy.cpp @@ -136,11 +136,10 @@ SkImage_Lazy::~SkImage_Lazy() { ////////////////////////////////////////////////////////////////////////////////////////////////// -static bool check_output_bitmap(const SkBitmap& bitmap, const SkImageInfo& info) { +static bool check_output_bitmap(const SkBitmap& bitmap, uint32_t expectedID) { + SkASSERT(bitmap.getGenerationID() == expectedID); SkASSERT(bitmap.isImmutable()); SkASSERT(bitmap.getPixels()); - SkASSERT(bitmap.colorType() == info.colorType()); - SkASSERT(SkColorSpace::Equals(bitmap.colorSpace(), info.colorSpace())); return true; } @@ -158,12 +157,10 @@ bool SkImage_Lazy::directGeneratePixels(const SkImageInfo& info, void* pixels, s ////////////////////////////////////////////////////////////////////////////////////////////////// -bool SkImage_Lazy::lockAsBitmapOnlyIfAlreadyCached(SkBitmap* bitmap, - const SkImageInfo& dstInfo) const { - auto desc = SkBitmapCacheDesc::Make(fUniqueID, dstInfo.colorType(), dstInfo.colorSpace(), - SkIRect::MakeSize(fInfo.dimensions())); +bool SkImage_Lazy::lockAsBitmapOnlyIfAlreadyCached(SkBitmap* bitmap) const { + auto desc = SkBitmapCacheDesc::Make(fUniqueID, SkIRect::MakeSize(fInfo.dimensions())); return SkBitmapCache::Find(desc, bitmap) && - check_output_bitmap(*bitmap, dstInfo); + check_output_bitmap(*bitmap, fUniqueID); } static bool generate_pixels(SkImageGenerator* gen, const SkPixmap& pmap, int originX, int originY) { @@ -204,7 +201,8 @@ static bool generate_pixels(SkImageGenerator* gen, const SkPixmap& pmap, int ori bool SkImage_Lazy::lockAsBitmap(SkBitmap* bitmap, SkImage::CachingHint chint, const SkImageInfo& info) const { - if (this->lockAsBitmapOnlyIfAlreadyCached(bitmap, info)) { + // TODO: Verify dstColorSpace here + if (this->lockAsBitmapOnlyIfAlreadyCached(bitmap)) { return true; } @@ -212,8 +210,7 @@ bool SkImage_Lazy::lockAsBitmap(SkBitmap* bitmap, SkImage::CachingHint chint, SkBitmapCache::RecPtr cacheRec; SkPixmap pmap; if (SkImage::kAllow_CachingHint == chint) { - auto desc = SkBitmapCacheDesc::Make(fUniqueID, info.colorType(), info.colorSpace(), - SkIRect::MakeSize(info.dimensions())); + auto desc = SkBitmapCacheDesc::Make(fUniqueID, SkIRect::MakeSize(info.dimensions())); cacheRec = SkBitmapCache::Alloc(desc, info, &pmap); if (!cacheRec) { return false; @@ -234,13 +231,16 @@ bool SkImage_Lazy::lockAsBitmap(SkBitmap* bitmap, SkImage::CachingHint chint, if (cacheRec) { SkBitmapCache::Add(std::move(cacheRec), bitmap); + SkASSERT(bitmap->getPixels()); // we're locked + SkASSERT(bitmap->isImmutable()); + SkASSERT(bitmap->getGenerationID() == fUniqueID); this->notifyAddedToRasterCache(); } else { *bitmap = tmpBitmap; - bitmap->setImmutable(); + bitmap->pixelRef()->setImmutableWithID(fUniqueID); } - check_output_bitmap(*bitmap, info); + check_output_bitmap(*bitmap, fUniqueID); return true; } @@ -251,7 +251,7 @@ bool SkImage_Lazy::onReadPixels(const SkImageInfo& dstInfo, void* dstPixels, siz SkColorSpace* dstColorSpace = dstInfo.colorSpace(); SkBitmap bm; if (kDisallow_CachingHint == chint) { - if (this->lockAsBitmapOnlyIfAlreadyCached(&bm, dstInfo)) { + if (this->lockAsBitmapOnlyIfAlreadyCached(&bm)) { return bm.readPixels(dstInfo, dstPixels, dstRB, srcX, srcY); } else { // Try passing the caller's buffer directly down to the generator. If this fails we diff --git a/src/image/SkImage_Lazy.h b/src/image/SkImage_Lazy.h index a2b434a2b1..3b8cfc5465 100644 --- a/src/image/SkImage_Lazy.h +++ b/src/image/SkImage_Lazy.h @@ -59,8 +59,8 @@ public: bool onIsValid(GrContext*) const override; - // Only return true if the generate has already been cached, in a format that matches the info. - bool lockAsBitmapOnlyIfAlreadyCached(SkBitmap*, const SkImageInfo&) const; + // Only return true if the generate has already been cached. + bool lockAsBitmapOnlyIfAlreadyCached(SkBitmap*) const; // Call the underlying generator directly bool directGeneratePixels(const SkImageInfo& dstInfo, void* dstPixels, size_t dstRB, int srcX, int srcY) const; diff --git a/tests/ImageTest.cpp b/tests/ImageTest.cpp index 7942169c86..445b5498e5 100644 --- a/tests/ImageTest.cpp +++ b/tests/ImageTest.cpp @@ -337,6 +337,7 @@ DEF_TEST(image_newfrombitmap, reporter) { DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SkImage_Gpu2Cpu, reporter, ctxInfo) { SkImageInfo info = SkImageInfo::MakeN32(20, 20, kOpaque_SkAlphaType); sk_sp image(create_gpu_image(ctxInfo.grContext())); + const uint32_t uniqueID = image->uniqueID(); const auto desc = SkBitmapCacheDesc::Make(image.get()); auto surface(SkSurface::MakeRaster(info)); @@ -352,6 +353,7 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SkImage_Gpu2Cpu, reporter, ctxInfo) { { SkBitmap cachedBitmap; if (SkBitmapCache::Find(desc, &cachedBitmap)) { + REPORTER_ASSERT(reporter, cachedBitmap.getGenerationID() == uniqueID); REPORTER_ASSERT(reporter, cachedBitmap.isImmutable()); REPORTER_ASSERT(reporter, cachedBitmap.getPixels()); } else {