From dce19a76e9ea90a42829cc7d13e23b3507e83bfe Mon Sep 17 00:00:00 2001 From: Herb Derby Date: Wed, 18 Apr 2018 16:02:17 -0400 Subject: Untangle strike cache and glyph cache The strike cache and the glpyh cache have been friends for a long time. Untangle this twisted relationship. BUG=skia:7515 Change-Id: Ie77393f6923e9886ec90ff7a60a1200e78319937 Reviewed-on: https://skia-review.googlesource.com/122084 Commit-Queue: Herb Derby Reviewed-by: Ben Wagner --- bench/PathTextBench.cpp | 1 + samplecode/SamplePathText.cpp | 1 + src/core/SkGlyphCache.cpp | 30 +++----- src/core/SkGlyphCache.h | 26 +++---- src/core/SkOverdrawCanvas.cpp | 1 + src/core/SkRemoteGlyphCache.cpp | 1 + src/core/SkStrikeCache.cpp | 167 ++++++++++++++++++++++++++++------------ src/core/SkStrikeCache.h | 52 +++++++------ src/core/SkTextToPathIter.h | 3 +- src/gpu/ops/GrAtlasTextOp.cpp | 2 +- src/pdf/SkPDFFont.cpp | 1 + src/pdf/SkPDFFont.h | 2 +- src/xps/SkXPSDevice.cpp | 1 + 13 files changed, 180 insertions(+), 108 deletions(-) diff --git a/bench/PathTextBench.cpp b/bench/PathTextBench.cpp index 38cc9e7732..b1c9e01a80 100644 --- a/bench/PathTextBench.cpp +++ b/bench/PathTextBench.cpp @@ -11,6 +11,7 @@ #include "SkPaint.h" #include "SkPath.h" #include "SkRandom.h" +#include "SkStrikeCache.h" #include "sk_tool_utils.h" static constexpr int kScreenWidth = 1500; diff --git a/samplecode/SamplePathText.cpp b/samplecode/SamplePathText.cpp index 3dd3287a5f..fd3c799b2d 100644 --- a/samplecode/SamplePathText.cpp +++ b/samplecode/SamplePathText.cpp @@ -12,6 +12,7 @@ #include "SkPaint.h" #include "SkPath.h" #include "SkRandom.h" +#include "SkStrikeCache.h" #include "SkTaskGroup.h" #include "sk_tool_utils.h" diff --git a/src/core/SkGlyphCache.cpp b/src/core/SkGlyphCache.cpp index 3475b6037d..6930b37e6a 100644 --- a/src/core/SkGlyphCache.cpp +++ b/src/core/SkGlyphCache.cpp @@ -6,6 +6,7 @@ */ #include "SkGlyphCache.h" + #include "SkGraphics.h" #include "SkMutex.h" #include "SkOnce.h" @@ -15,10 +16,10 @@ #include SkGlyphCache::SkGlyphCache( - const SkDescriptor& desc, - std::unique_ptr scaler, - const SkPaint::FontMetrics& fontMetrics) - : Node{desc} + const SkDescriptor& desc, + std::unique_ptr scaler, + const SkPaint::FontMetrics& fontMetrics) + : fDesc{desc} , fScalerContext{std::move(scaler)} , fFontMetrics(fontMetrics) { @@ -34,6 +35,10 @@ SkGlyphCache::~SkGlyphCache() { }); } +const SkDescriptor& SkGlyphCache::getDescriptor() const { + return *fDesc.getDesc(); +} + SkGlyphCache::CharGlyphRec* SkGlyphCache::getCharGlyphRec(SkPackedUnicharID packedUnicharID) { if (!fPackedUnicharIDToPackedGlyphID) { fPackedUnicharIDToPackedGlyphID.reset(new CharGlyphRec[kHashCount]); @@ -401,23 +406,6 @@ void SkGlyphCache::validate() const { #endif } -void SkStrikeCache::validate() const { - size_t computedBytes = 0; - int computedCount = 0; - - const SkGlyphCache* head = fHead; - while (head != nullptr) { - computedBytes += head->fMemoryUsed; - computedCount += 1; - head = head->fNext; - } - - SkASSERTF(fCacheCount == computedCount, "fCacheCount: %d, computedCount: %d", fCacheCount, - computedCount); - SkASSERTF(fTotalMemoryUsed == computedBytes, "fTotalMemoryUsed: %d, computedBytes: %d", - fTotalMemoryUsed, computedBytes); -} - #endif diff --git a/src/core/SkGlyphCache.h b/src/core/SkGlyphCache.h index 8d5098a496..30928da744 100644 --- a/src/core/SkGlyphCache.h +++ b/src/core/SkGlyphCache.h @@ -10,15 +10,12 @@ #include "SkArenaAlloc.h" #include "SkDescriptor.h" #include "SkGlyph.h" -#include "SkStrikeCache.h" #include "SkPaint.h" #include "SkTHash.h" #include "SkScalerContext.h" #include "SkTemplates.h" #include -class SkTraceMemoryDump; - /** \class SkGlyphCache This class represents a strike: a specific combination of typeface, size, matrix, etc., and @@ -32,8 +29,15 @@ class SkTraceMemoryDump; The Find*Exclusive() method returns SkExclusiveStrikePtr, which releases exclusive ownership when they go out of scope. */ -class SkGlyphCache : private SkStrikeCache::Node { +class SkGlyphCache { public: + SkGlyphCache(const SkDescriptor& desc, + std::unique_ptr scaler, + const SkPaint::FontMetrics&); + ~SkGlyphCache(); + + const SkDescriptor& getDescriptor() const; + /** Return true if glyph is cached. */ bool isGlyphCached(SkGlyphID glyphID, SkFixed x, SkFixed y) const; @@ -106,8 +110,6 @@ public: return fFontMetrics; } - using Node::getDescriptor; - SkMask::Format getMaskFormat() const { return fScalerContext->getMaskFormat(); } @@ -149,8 +151,6 @@ public: }; private: - friend class SkStrikeCache; - enum MetricsType { kNothing_MetricsType, kJustAdvance_MetricsType, @@ -168,10 +168,6 @@ private: SkPackedGlyphID fPackedGlyphID; }; - SkGlyphCache(const SkDescriptor& desc, std::unique_ptr scaler, - const SkPaint::FontMetrics&); - ~SkGlyphCache(); - // Return the SkGlyph* associated with MakeID. The id parameter is the // combined glyph/x/y id generated by MakeID. If it is just a glyph id // then x and y are assumed to be zero. @@ -201,6 +197,7 @@ private: static const SkGlyph::Intercept* MatchBounds(const SkGlyph* glyph, const SkScalar bounds[2]); + const SkAutoDescriptor fDesc; const std::unique_ptr fScalerContext; SkPaint::FontMetrics fFontMetrics; @@ -220,7 +217,9 @@ private: size_t fMemoryUsed; }; +// This will be deleted soon, and keeps android building. #ifdef SK_BUILD_FOR_ANDROID_FRAMEWORK +#include "SkStrikeCache.h" class SkAutoGlyphCacheNoGamma : public SkExclusiveStrikePtr { public: SkAutoGlyphCacheNoGamma(const SkPaint& paint, @@ -228,9 +227,8 @@ public: const SkMatrix* matrix) : SkExclusiveStrikePtr(SkStrikeCache::FindOrCreateStrikeExclusive( paint, surfaceProps, SkScalerContextFlags::kNone, matrix)) {} - }; #define SkAutoGlyphCacheNoGamma(...) SK_REQUIRE_LOCAL_VAR(SkAutoGlyphCacheNoGamma) #endif -#endif +#endif // SkGlyphCache_DEFINED diff --git a/src/core/SkOverdrawCanvas.cpp b/src/core/SkOverdrawCanvas.cpp index 118b9431e3..050ddf76fb 100644 --- a/src/core/SkOverdrawCanvas.cpp +++ b/src/core/SkOverdrawCanvas.cpp @@ -16,6 +16,7 @@ #include "SkPath.h" #include "SkRRect.h" #include "SkRSXform.h" +#include "SkStrikeCache.h" #include "SkTextBlob.h" #include "SkTextBlobRunIterator.h" diff --git a/src/core/SkRemoteGlyphCache.cpp b/src/core/SkRemoteGlyphCache.cpp index d73622327f..ed484dd6fd 100644 --- a/src/core/SkRemoteGlyphCache.cpp +++ b/src/core/SkRemoteGlyphCache.cpp @@ -13,6 +13,7 @@ #include "SkDevice.h" #include "SkFindAndPlaceGlyph.h" +#include "SkStrikeCache.h" #include "SkTypeface_remote.h" template diff --git a/src/core/SkStrikeCache.cpp b/src/core/SkStrikeCache.cpp index f5ae5bce35..0c6cb83447 100644 --- a/src/core/SkStrikeCache.cpp +++ b/src/core/SkStrikeCache.cpp @@ -28,17 +28,69 @@ static SkStrikeCache& get_globals() { return *globals; } +struct SkStrikeCache::Node { + Node(const SkDescriptor& desc, + std::unique_ptr scaler, + const SkPaint::FontMetrics& metrics) + : fCache{desc, std::move(scaler), metrics} {} + Node* fNext{nullptr}; + Node* fPrev{nullptr}; + SkGlyphCache fCache; +}; + +SkStrikeCache::ExclusiveStrikePtr::ExclusiveStrikePtr(SkStrikeCache::Node* node) : fNode{node} {} +SkStrikeCache::ExclusiveStrikePtr::ExclusiveStrikePtr() : fNode{nullptr} {} +SkStrikeCache::ExclusiveStrikePtr::ExclusiveStrikePtr(ExclusiveStrikePtr&& o) + : fNode{o.fNode} { + o.fNode = nullptr; +} + +SkStrikeCache::ExclusiveStrikePtr& +SkStrikeCache::ExclusiveStrikePtr::operator = (ExclusiveStrikePtr&& o) { + Attach(fNode); + fNode = o.fNode; + o.fNode = nullptr; + return *this; +} + +SkStrikeCache::ExclusiveStrikePtr::~ExclusiveStrikePtr() { + SkStrikeCache::Attach(fNode); +} +SkGlyphCache* SkStrikeCache::ExclusiveStrikePtr::get() const { + return &fNode->fCache; +} +SkGlyphCache* SkStrikeCache::ExclusiveStrikePtr::operator -> () const { + return this->get(); +} +SkGlyphCache& SkStrikeCache::ExclusiveStrikePtr::operator * () const { + return *this->get(); +} +SkStrikeCache::ExclusiveStrikePtr::operator bool () const { + return fNode != nullptr; +} + +bool operator == (const SkStrikeCache::ExclusiveStrikePtr& lhs, + const SkStrikeCache::ExclusiveStrikePtr& rhs) { + return lhs.fNode == rhs.fNode; +} +bool operator == (const SkStrikeCache::ExclusiveStrikePtr& lhs, decltype(nullptr)) { + return lhs.fNode == nullptr; +} +bool operator == (decltype(nullptr), const SkStrikeCache::ExclusiveStrikePtr& rhs) { + return nullptr == rhs.fNode; +} + SkStrikeCache::~SkStrikeCache() { - SkGlyphCache* cache = fHead; - while (cache) { - SkGlyphCache* next = cache->fNext; - delete cache; - cache = next; + Node* node = fHead; + while (node) { + Node* next = node->fNext; + delete node; + node = next; } } -void SkStrikeCache::AttachCache(SkGlyphCache* cache) { - get_globals().attachCache(cache); +void SkStrikeCache::Attach(Node* node) { + get_globals().attachNode(node); } SkExclusiveStrikePtr SkStrikeCache::FindStrikeExclusive(const SkDescriptor& desc) { @@ -131,27 +183,27 @@ void SkStrikeCache::DumpMemoryStatistics(SkTraceMemoryDump* dump) { } -void SkStrikeCache::attachCache(SkGlyphCache *cache) { - if (cache == nullptr) { +void SkStrikeCache::attachNode(Node* node) { + if (node == nullptr) { return; } SkAutoExclusive ac(fLock); this->validate(); - cache->validate(); + node->fCache.validate(); - this->internalAttachCacheToHead(cache); + this->internalAttachToHead(node); this->internalPurge(); } SkExclusiveStrikePtr SkStrikeCache::findStrikeExclusive(const SkDescriptor& desc) { - SkGlyphCache* cache; - SkAutoExclusive ac(fLock); + Node* node; + SkAutoExclusive ac(fLock); - for (cache = internalGetHead(); cache != nullptr; cache = cache->fNext) { - if (cache->getDescriptor() == desc) { - this->internalDetachCache(cache); - return SkExclusiveStrikePtr(cache); + for (node = internalGetHead(); node != nullptr; node = node->fNext) { + if (node->fCache.getDescriptor() == desc) { + this->internalDetachCache(node); + return SkExclusiveStrikePtr(node); } } @@ -232,19 +284,19 @@ void SkStrikeCache::forEachStrike(std::function visit this->validate(); - for (SkGlyphCache* cache = this->internalGetHead(); cache != nullptr; cache = cache->fNext) { - visitor(*cache); + for (Node* node = this->internalGetHead(); node != nullptr; node = node->fNext) { + visitor(node->fCache); } } -SkGlyphCache* SkStrikeCache::internalGetTail() const { - SkGlyphCache* cache = fHead; - if (cache) { - while (cache->fNext) { - cache = cache->fNext; +SkStrikeCache::Node* SkStrikeCache::internalGetTail() const { + Node* node = fHead; + if (node) { + while (node->fNext) { + node = node->fNext; } } - return cache; + return node; } size_t SkStrikeCache::internalPurge(size_t minBytesNeeded) { @@ -277,16 +329,16 @@ size_t SkStrikeCache::internalPurge(size_t minBytesNeeded) { // we start at the tail and proceed backwards, as the linklist is in LRU // order, with unimportant entries at the tail. - SkGlyphCache* cache = this->internalGetTail(); - while (cache != nullptr && + Node* node = this->internalGetTail(); + while (node != nullptr && (bytesFreed < bytesNeeded || countFreed < countNeeded)) { - SkGlyphCache* prev = cache->fPrev; - bytesFreed += cache->fMemoryUsed; + Node* prev = node->fPrev; + bytesFreed += node->fCache.getMemoryUsed(); countFreed += 1; - this->internalDetachCache(cache); - delete cache; - cache = prev; + this->internalDetachCache(node); + delete node; + node = prev; } this->validate(); @@ -301,32 +353,32 @@ size_t SkStrikeCache::internalPurge(size_t minBytesNeeded) { return bytesFreed; } -void SkStrikeCache::internalAttachCacheToHead(SkGlyphCache* cache) { - SkASSERT(nullptr == cache->fPrev && nullptr == cache->fNext); +void SkStrikeCache::internalAttachToHead(Node* node) { + SkASSERT(nullptr == node->fPrev && nullptr == node->fNext); if (fHead) { - fHead->fPrev = cache; - cache->fNext = fHead; + fHead->fPrev = node; + node->fNext = fHead; } - fHead = cache; + fHead = node; fCacheCount += 1; - fTotalMemoryUsed += cache->fMemoryUsed; + fTotalMemoryUsed += node->fCache.getMemoryUsed(); } -void SkStrikeCache::internalDetachCache(SkGlyphCache* cache) { +void SkStrikeCache::internalDetachCache(Node* node) { SkASSERT(fCacheCount > 0); fCacheCount -= 1; - fTotalMemoryUsed -= cache->fMemoryUsed; + fTotalMemoryUsed -= node->fCache.getMemoryUsed(); - if (cache->fPrev) { - cache->fPrev->fNext = cache->fNext; + if (node->fPrev) { + node->fPrev->fNext = node->fNext; } else { - fHead = cache->fNext; + fHead = node->fNext; } - if (cache->fNext) { - cache->fNext->fPrev = cache->fPrev; + if (node->fNext) { + node->fNext->fPrev = node->fPrev; } - cache->fPrev = cache->fNext = nullptr; + node->fPrev = node->fNext = nullptr; } size_t SkGraphics::GetFontCacheLimit() { @@ -378,7 +430,7 @@ SkExclusiveStrikePtr SkStrikeCache::CreateStrikeExclusive( scaler->getFontMetrics(&fontMetrics); } - return SkExclusiveStrikePtr(new SkGlyphCache(desc, move(scaler), fontMetrics)); + return SkExclusiveStrikePtr(new Node(desc, move(scaler), fontMetrics)); } SkExclusiveStrikePtr SkStrikeCache::FindOrCreateStrikeExclusive( @@ -412,4 +464,23 @@ SkExclusiveStrikePtr SkStrikeCache::FindOrCreateStrikeExclusive( SkExclusiveStrikePtr SkStrikeCache::FindOrCreateStrikeExclusive(const SkPaint& paint) { return FindOrCreateStrikeExclusive( paint, nullptr, kFakeGammaAndBoostContrast, nullptr); -} \ No newline at end of file +} + +#ifdef SK_DEBUG +void SkStrikeCache::validate() const { + size_t computedBytes = 0; + int computedCount = 0; + + const Node* node = fHead; + while (node != nullptr) { + computedBytes += node->fCache.getMemoryUsed(); + computedCount += 1; + node = node->fNext; + } + + SkASSERTF(fCacheCount == computedCount, "fCacheCount: %d, computedCount: %d", fCacheCount, + computedCount); + SkASSERTF(fTotalMemoryUsed == computedBytes, "fTotalMemoryUsed: %d, computedBytes: %d", + fTotalMemoryUsed, computedBytes); +} +#endif \ No newline at end of file diff --git a/src/core/SkStrikeCache.h b/src/core/SkStrikeCache.h index a72e71f232..a98b508c15 100644 --- a/src/core/SkStrikeCache.h +++ b/src/core/SkStrikeCache.h @@ -11,7 +11,6 @@ #include "SkDescriptor.h" #include "SkSpinlock.h" #include "SkTemplates.h" -#include "SkArenaAlloc.h" class SkGlyphCache; class SkTraceMemoryDump; @@ -30,19 +29,35 @@ class SkTraceMemoryDump; /////////////////////////////////////////////////////////////////////////////// -class SkGlyphCache; - class SkStrikeCache { + struct Node; + public: SkStrikeCache() = default; ~SkStrikeCache(); - static void AttachCache(SkGlyphCache* cache); - using ExclusiveStrikePtr = - std::unique_ptr< - SkGlyphCache, - SkFunctionWrapper>; + class ExclusiveStrikePtr { + public: + explicit ExclusiveStrikePtr(Node*); + ExclusiveStrikePtr(); + ExclusiveStrikePtr(const ExclusiveStrikePtr&) = delete; + ExclusiveStrikePtr& operator = (const ExclusiveStrikePtr&) = delete; + ExclusiveStrikePtr(ExclusiveStrikePtr&&); + ExclusiveStrikePtr& operator = (ExclusiveStrikePtr&&); + ~ExclusiveStrikePtr(); + + SkGlyphCache* get() const; + SkGlyphCache* operator -> () const; + SkGlyphCache& operator * () const; + explicit operator bool () const; + friend bool operator == (const ExclusiveStrikePtr&, const ExclusiveStrikePtr&); + friend bool operator == (const ExclusiveStrikePtr&, decltype(nullptr)); + friend bool operator == (decltype(nullptr), const ExclusiveStrikePtr&); + + private: + Node* fNode; + }; static ExclusiveStrikePtr FindStrikeExclusive(const SkDescriptor&); @@ -76,7 +91,7 @@ public: static void DumpMemoryStatistics(SkTraceMemoryDump* dump); // call when a glyphcache is available for caching (i.e. not in use) - void attachCache(SkGlyphCache *cache); + void attachNode(Node* node); ExclusiveStrikePtr findStrikeExclusive(const SkDescriptor&); void purgeAll(); // does not change budget @@ -99,20 +114,13 @@ public: #endif private: - friend class SkGlyphCache; - struct Node { - Node(const SkDescriptor& desc) : fDesc{desc} {} - const SkDescriptor& getDescriptor() const {return *fDesc.getDesc(); } - SkGlyphCache* fNext{nullptr}; - SkGlyphCache* fPrev{nullptr}; - SkAutoDescriptor fDesc; - }; + static void Attach(Node* node); // The following methods can only be called when mutex is already held. - SkGlyphCache* internalGetHead() const { return fHead; } - SkGlyphCache* internalGetTail() const; - void internalDetachCache(SkGlyphCache*); - void internalAttachCacheToHead(SkGlyphCache*); + Node* internalGetHead() const { return fHead; } + Node* internalGetTail() const; + void internalDetachCache(Node*); + void internalAttachToHead(Node*); // Checkout budgets, modulated by the specified min-bytes-needed-to-purge, // and attempt to purge caches to match. @@ -122,7 +130,7 @@ private: void forEachStrike(std::function visitor) const; mutable SkSpinlock fLock; - SkGlyphCache* fHead{nullptr}; + Node* fHead{nullptr}; size_t fTotalMemoryUsed{0}; size_t fCacheSizeLimit{SK_DEFAULT_FONT_CACHE_LIMIT}; int32_t fCacheCountLimit{SK_DEFAULT_FONT_CACHE_COUNT_LIMIT}; diff --git a/src/core/SkTextToPathIter.h b/src/core/SkTextToPathIter.h index 219833468c..49564d96af 100644 --- a/src/core/SkTextToPathIter.h +++ b/src/core/SkTextToPathIter.h @@ -9,8 +9,9 @@ #define SkTextToPathIter_DEFINED #include "SkAutoKern.h" -#include "SkGlyphCache.h" #include "SkPaint.h" +#include "SkStrikeCache.h" + class SkTextBaseIter { protected: diff --git a/src/gpu/ops/GrAtlasTextOp.cpp b/src/gpu/ops/GrAtlasTextOp.cpp index 6a534235d8..c4a2f153cb 100644 --- a/src/gpu/ops/GrAtlasTextOp.cpp +++ b/src/gpu/ops/GrAtlasTextOp.cpp @@ -10,10 +10,10 @@ #include "GrContext.h" #include "GrOpFlushState.h" #include "GrResourceProvider.h" -#include "SkGlyphCache.h" #include "SkMathPriv.h" #include "SkMatrixPriv.h" #include "SkPoint3.h" +#include "SkStrikeCache.h" #include "effects/GrBitmapTextGeoProc.h" #include "effects/GrDistanceFieldGeoProc.h" #include "text/GrAtlasManager.h" diff --git a/src/pdf/SkPDFFont.cpp b/src/pdf/SkPDFFont.cpp index 11c4b5828b..8c35b53d7c 100644 --- a/src/pdf/SkPDFFont.cpp +++ b/src/pdf/SkPDFFont.cpp @@ -6,6 +6,7 @@ */ #include "SkData.h" +#include "SkGlyphCache.h" #include "SkMakeUnique.h" #include "SkPDFCanon.h" #include "SkPDFConvertType1FontStream.h" diff --git a/src/pdf/SkPDFFont.h b/src/pdf/SkPDFFont.h index 44cebf9fef..746f4fab25 100644 --- a/src/pdf/SkPDFFont.h +++ b/src/pdf/SkPDFFont.h @@ -11,7 +11,7 @@ #include "SkAdvancedTypefaceMetrics.h" #include "SkBitSet.h" -#include "SkGlyphCache.h" +#include "SkStrikeCache.h" #include "SkPDFTypes.h" #include "SkTDArray.h" #include "SkTypeface.h" diff --git a/src/xps/SkXPSDevice.cpp b/src/xps/SkXPSDevice.cpp index 856cafe158..215951622c 100644 --- a/src/xps/SkXPSDevice.cpp +++ b/src/xps/SkXPSDevice.cpp @@ -44,6 +44,7 @@ #include "SkShader.h" #include "SkSize.h" #include "SkStream.h" +#include "SkStrikeCache.h" #include "SkTDArray.h" #include "SkTLazy.h" #include "SkTScopedComPtr.h" -- cgit v1.2.3