From 7929e3ae76719f65111e4fe7a2f2444d53228c2b Mon Sep 17 00:00:00 2001 From: fmalita Date: Thu, 27 Oct 2016 08:15:44 -0700 Subject: Avoid separate allocation of SkImageCacherator Embed directly in SkImage_Generator, and add a helper to handle param validation. R=reed@google.com GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2453473004 Review-Url: https://codereview.chromium.org/2453473004 --- src/core/SkImageCacherator.cpp | 50 +++++++++++++++++++++++------------------ src/core/SkImageCacherator.h | 15 ++++++++++++- src/image/SkImage_Generator.cpp | 35 +++++++++++++---------------- 3 files changed, 57 insertions(+), 43 deletions(-) diff --git a/src/core/SkImageCacherator.cpp b/src/core/SkImageCacherator.cpp index 496ca74d9a..6858a4c09e 100644 --- a/src/core/SkImageCacherator.cpp +++ b/src/core/SkImageCacherator.cpp @@ -32,49 +32,55 @@ // see skbug.com/ 4971, 5128, ... //#define SK_SUPPORT_COMPRESSED_TEXTURES_IN_CACHERATOR -SkImageCacherator* SkImageCacherator::NewFromGenerator(SkImageGenerator* gen, - const SkIRect* subset) { +SkImageCacherator::Validator::Validator(SkImageGenerator* gen, const SkIRect* subset) + // We are required to take ownership of gen, regardless of whether we instantiate a cacherator + // or not. On instantiation, the client is responsible for transferring ownership. + : fGenerator(gen) { + if (!gen) { - return nullptr; + return; } - // We are required to take ownership of gen, regardless of if we return a cacherator or not - SkAutoTDelete genHolder(gen); - const SkImageInfo& info = gen->getInfo(); if (info.isEmpty()) { - return nullptr; + fGenerator.reset(); + return; } - uint32_t uniqueID = gen->uniqueID(); + fUniqueID = gen->uniqueID(); const SkIRect bounds = SkIRect::MakeWH(info.width(), info.height()); if (subset) { if (!bounds.contains(*subset)) { - return nullptr; + fGenerator.reset(); + return; } if (*subset != bounds) { // we need a different uniqueID since we really are a subset of the raw generator - uniqueID = SkNextID::ImageID(); + fUniqueID = SkNextID::ImageID(); } } else { subset = &bounds; } - // Now that we know we can hand-off the generator (to be owned by the cacherator) we can - // release our holder. (we DONT want to delete it here anymore) - genHolder.release(); + fInfo = info.makeWH(subset->width(), subset->height()); + fOrigin = SkIPoint::Make(subset->x(), subset->y()); +} - return new SkImageCacherator(gen, gen->getInfo().makeWH(subset->width(), subset->height()), - SkIPoint::Make(subset->x(), subset->y()), uniqueID); +SkImageCacherator* SkImageCacherator::NewFromGenerator(SkImageGenerator* gen, + const SkIRect* subset) { + Validator validator(gen, subset); + + return validator ? new SkImageCacherator(&validator) : nullptr; } -SkImageCacherator::SkImageCacherator(SkImageGenerator* gen, const SkImageInfo& info, - const SkIPoint& origin, uint32_t uniqueID) - : fNotThreadSafeGenerator(gen) - , fInfo(info) - , fOrigin(origin) - , fUniqueID(uniqueID) -{} +SkImageCacherator::SkImageCacherator(Validator* validator) + : fNotThreadSafeGenerator(validator->fGenerator.release()) // we take ownership + , fInfo(validator->fInfo) + , fOrigin(validator->fOrigin) + , fUniqueID(validator->fUniqueID) +{ + SkASSERT(fNotThreadSafeGenerator); +} SkData* SkImageCacherator::refEncoded(GrContext* ctx) { ScopedGenerator generator(this); diff --git a/src/core/SkImageCacherator.h b/src/core/SkImageCacherator.h index 3be69a5785..2d9e94cc4d 100644 --- a/src/core/SkImageCacherator.h +++ b/src/core/SkImageCacherator.h @@ -68,7 +68,18 @@ public: int srcX, int srcY); private: - SkImageCacherator(SkImageGenerator*, const SkImageInfo&, const SkIPoint&, uint32_t uniqueID); + struct Validator { + Validator(SkImageGenerator*, const SkIRect* subset); + + operator bool() const { return fGenerator.get(); } + + std::unique_ptr fGenerator; + SkImageInfo fInfo; + SkIPoint fOrigin; + uint32_t fUniqueID; + }; + + SkImageCacherator(Validator*); bool generateBitmap(SkBitmap*); bool tryLockAsBitmap(SkBitmap*, const SkImage*, SkImage::CachingHint); @@ -100,6 +111,8 @@ private: const uint32_t fUniqueID; friend class GrImageTextureMaker; + friend class SkImage; + friend class SkImage_Generator; }; #endif diff --git a/src/image/SkImage_Generator.cpp b/src/image/SkImage_Generator.cpp index 412f573ba6..8fa022de7b 100644 --- a/src/image/SkImage_Generator.cpp +++ b/src/image/SkImage_Generator.cpp @@ -16,20 +16,20 @@ class SkImage_Generator : public SkImage_Base { public: - SkImage_Generator(SkImageCacherator* cache) - : INHERITED(cache->info().width(), cache->info().height(), cache->uniqueID()) - , fCache(cache) // take ownership + SkImage_Generator(SkImageCacherator::Validator* validator) + : INHERITED(validator->fInfo.width(), validator->fInfo.height(), validator->fUniqueID) + , fCache(validator) {} virtual SkImageInfo onImageInfo() const override { - return fCache->info(); + return fCache.info(); } SkAlphaType onAlphaType() const override { - return fCache->info().alphaType(); + return fCache.info().alphaType(); } bool onReadPixels(const SkImageInfo&, void*, size_t, int srcX, int srcY, CachingHint) const override; - SkImageCacherator* peekCacherator() const override { return fCache; } + SkImageCacherator* peekCacherator() const override { return &fCache; } SkData* onRefEncoded(GrContext*) const override; sk_sp onMakeSubset(const SkIRect&) const override; bool getROPixels(SkBitmap*, CachingHint) const override; @@ -38,7 +38,7 @@ public: bool onIsLazyGenerated() const override { return true; } private: - SkAutoTDelete fCache; + mutable SkImageCacherator fCache; typedef SkImage_Base INHERITED; }; @@ -49,13 +49,13 @@ bool SkImage_Generator::onReadPixels(const SkImageInfo& dstInfo, void* dstPixels int srcX, int srcY, CachingHint chint) const { SkBitmap bm; if (kDisallow_CachingHint == chint) { - if (fCache->lockAsBitmapOnlyIfAlreadyCached(&bm)) { + if (fCache.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 // may still succeed in the general case, as the generator may prefer some other // config, which we could then convert via SkBitmap::readPixels. - if (fCache->directGeneratePixels(dstInfo, dstPixels, dstRB, srcX, srcY)) { + if (fCache.directGeneratePixels(dstInfo, dstPixels, dstRB, srcX, srcY)) { return true; } // else fall through @@ -69,16 +69,16 @@ bool SkImage_Generator::onReadPixels(const SkImageInfo& dstInfo, void* dstPixels } SkData* SkImage_Generator::onRefEncoded(GrContext* ctx) const { - return fCache->refEncoded(ctx); + return fCache.refEncoded(ctx); } bool SkImage_Generator::getROPixels(SkBitmap* bitmap, CachingHint chint) const { - return fCache->lockAsBitmap(bitmap, this, chint); + return fCache.lockAsBitmap(bitmap, this, chint); } GrTexture* SkImage_Generator::asTextureRef(GrContext* ctx, const GrTextureParams& params, SkSourceGammaTreatment gammaTreatment) const { - return fCache->lockAsTexture(ctx, params, gammaTreatment, this); + return fCache.lockAsTexture(ctx, params, gammaTreatment, this); } sk_sp SkImage_Generator::onMakeSubset(const SkIRect& subset) const { @@ -98,12 +98,7 @@ sk_sp SkImage_Generator::onMakeSubset(const SkIRect& subset) const { } sk_sp SkImage::MakeFromGenerator(SkImageGenerator* generator, const SkIRect* subset) { - if (!generator) { - return nullptr; - } - SkImageCacherator* cache = SkImageCacherator::NewFromGenerator(generator, subset); - if (!cache) { - return nullptr; - } - return sk_make_sp(cache); + SkImageCacherator::Validator validator(generator, subset); + + return validator ? sk_make_sp(&validator) : nullptr; } -- cgit v1.2.3