Remove directGeneratePixels, and do some follow-up refactoring
This caused visually different results depending on what you passed for
cacheHint *and* whether or not the image had already been cached. That
nondeterminism is not worth the slight performance boost.
With that path gone, things are much simpler, and getROPixels and
lockAsBitmap can be folded together.
Bug: skia:
Change-Id: I9535764a56cef57feb241fd8c86c6c96ef89c142
Reviewed-on: https://skia-review.googlesource.com/c/164040
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
diff --git a/src/image/SkImage_Lazy.cpp b/src/image/SkImage_Lazy.cpp
index 6ac7e7d..fa88db1 100644
--- a/src/image/SkImage_Lazy.cpp
+++ b/src/image/SkImage_Lazy.cpp
@@ -135,36 +135,6 @@
//////////////////////////////////////////////////////////////////////////////////////////////////
-static bool check_output_bitmap(const SkBitmap& bitmap, const SkImageInfo& info) {
- SkASSERT(bitmap.isImmutable());
- SkASSERT(bitmap.getPixels());
- SkASSERT(bitmap.colorType() == info.colorType());
- SkASSERT(SkColorSpace::Equals(bitmap.colorSpace(), info.colorSpace()));
- return true;
-}
-
-bool SkImage_Lazy::directGeneratePixels(const SkImageInfo& info, void* pixels, size_t rb,
- int srcX, int srcY) const {
- ScopedGenerator generator(fSharedGenerator);
- const SkImageInfo& genInfo = generator->getInfo();
- // Currently generators do not natively handle subsets, so check that first.
- if (srcX || srcY || genInfo.width() != info.width() || genInfo.height() != info.height()) {
- return false;
- }
-
- return generator->getPixels(info, pixels, rb);
-}
-
-//////////////////////////////////////////////////////////////////////////////////////////////////
-
-bool SkImage_Lazy::lockAsBitmapOnlyIfAlreadyCached(SkBitmap* bitmap,
- const SkImageInfo& dstInfo) const {
- auto desc = SkBitmapCacheDesc::Make(fUniqueID, dstInfo.colorType(), dstInfo.colorSpace(),
- SkIRect::MakeSize(fInfo.dimensions()));
- return SkBitmapCache::Find(desc, bitmap) &&
- check_output_bitmap(*bitmap, dstInfo);
-}
-
static bool generate_pixels(SkImageGenerator* gen, const SkPixmap& pmap, int originX, int originY) {
const int genW = gen->getInfo().width();
const int genH = gen->getInfo().height();
@@ -201,45 +171,39 @@
return true;
}
-bool SkImage_Lazy::lockAsBitmap(SkBitmap* bitmap, SkImage::CachingHint chint,
- const SkImageInfo& info) const {
- if (this->lockAsBitmapOnlyIfAlreadyCached(bitmap, info)) {
+bool SkImage_Lazy::getROPixels(SkBitmap* bitmap, SkImage::CachingHint chint) const {
+ auto check_output_bitmap = [bitmap]() {
+ SkASSERT(bitmap->isImmutable());
+ SkASSERT(bitmap->getPixels());
+ (void)bitmap;
+ };
+
+ auto desc = SkBitmapCacheDesc::Make(this);
+ if (SkBitmapCache::Find(desc, bitmap)) {
+ check_output_bitmap();
return true;
}
- SkBitmap tmpBitmap;
- SkBitmapCache::RecPtr cacheRec;
- SkPixmap pmap;
if (SkImage::kAllow_CachingHint == chint) {
- auto desc = SkBitmapCacheDesc::Make(fUniqueID, info.colorType(), info.colorSpace(),
- SkIRect::MakeSize(info.dimensions()));
- cacheRec = SkBitmapCache::Alloc(desc, info, &pmap);
- if (!cacheRec) {
+ SkPixmap pmap;
+ SkBitmapCache::RecPtr cacheRec = SkBitmapCache::Alloc(desc, fInfo, &pmap);
+ if (!cacheRec ||
+ !generate_pixels(ScopedGenerator(fSharedGenerator), pmap,
+ fOrigin.x(), fOrigin.y())) {
return false;
}
- } else {
- if (!tmpBitmap.tryAllocPixels(info)) {
- return false;
- }
- if (!tmpBitmap.peekPixels(&pmap)) {
- return false;
- }
- }
-
- ScopedGenerator generator(fSharedGenerator);
- if (!generate_pixels(generator, pmap, fOrigin.x(), fOrigin.y())) {
- return false;
- }
-
- if (cacheRec) {
SkBitmapCache::Add(std::move(cacheRec), bitmap);
this->notifyAddedToRasterCache();
} else {
- *bitmap = tmpBitmap;
+ if (!bitmap->tryAllocPixels(fInfo) ||
+ !generate_pixels(ScopedGenerator(fSharedGenerator), bitmap->pixmap(),
+ fOrigin.x(), fOrigin.y())) {
+ return false;
+ }
bitmap->setImmutable();
}
- check_output_bitmap(*bitmap, info);
+ check_output_bitmap();
return true;
}
@@ -248,22 +212,6 @@
bool SkImage_Lazy::onReadPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t dstRB,
int srcX, int srcY, CachingHint chint) const {
SkBitmap bm;
-#ifdef SK_USE_LEGACY_LAZY_IMAGE_DECODE
- if (kDisallow_CachingHint == chint) {
- if (this->lockAsBitmapOnlyIfAlreadyCached(&bm, dstInfo)) {
- return bm.readPixels(dstInfo, dstPixels, dstRB, srcX, srcY);
- } else {
- // Try passing the caller's buffer directly down to the generator. If this fails we
- // may still succeed in the general case, as the generator may prefer some other
- // config, which we could then convert via SkBitmap::readPixels.
- if (this->directGeneratePixels(dstInfo, dstPixels, dstRB, srcX, srcY)) {
- return true;
- }
- // else fall through
- }
- }
-#endif
-
if (this->getROPixels(&bm, chint)) {
return bm.readPixels(dstInfo, dstPixels, dstRB, srcX, srcY);
}
@@ -275,10 +223,6 @@
return generator->refEncodedData();
}
-bool SkImage_Lazy::getROPixels(SkBitmap* bitmap, CachingHint chint) const {
- return this->lockAsBitmap(bitmap, chint, fInfo);
-}
-
bool SkImage_Lazy::onIsValid(GrContext* context) const {
ScopedGenerator generator(fSharedGenerator);
return generator->isValid(context);
@@ -498,7 +442,7 @@
// 4. Ask the generator to return RGB(A) data, which the GPU can convert
SkBitmap bitmap;
- if (!proxy && this->lockAsBitmap(&bitmap, chint, fInfo)) {
+ if (!proxy && this->getROPixels(&bitmap, chint)) {
if (willBeMipped) {
proxy = proxyProvider->createMipMapProxyFromBitmap(bitmap);
}