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 <herb@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>

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;

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"
 

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 <cctype>
 
 SkGlyphCache::SkGlyphCache(
-        const SkDescriptor& desc,
-        std::unique_ptr<SkScalerContext> scaler,
-        const SkPaint::FontMetrics& fontMetrics)
-    : Node{desc}
+    const SkDescriptor& desc,
+    std::unique_ptr<SkScalerContext> 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
 
 

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 <memory>
 
-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<SkScalerContext> 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<SkScalerContext> 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<SkScalerContext> 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

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"
 

src/core/SkRemoteGlyphCache.cpp

@@ -13,6 +13,7 @@
 
 #include "SkDevice.h"
 #include "SkFindAndPlaceGlyph.h"
+#include "SkStrikeCache.h"
 #include "SkTypeface_remote.h"
 
 template <typename T>

src/core/SkStrikeCache.cpp

@@ -28,17 +28,69 @@ static SkStrikeCache& get_globals() {
     return *globals;
 }
 
+struct SkStrikeCache::Node {
+    Node(const SkDescriptor& desc,
+         std::unique_ptr<SkScalerContext> 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<void(const SkGlyphCache&)> 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);
-}
+}
+
+#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

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<void, SkGlyphCache, SkStrikeCache::AttachCache>>;
+    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<void(const SkGlyphCache&)> 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};

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:

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"

src/pdf/SkPDFFont.cpp

@@ -6,6 +6,7 @@
  */
 
 #include "SkData.h"
+#include "SkGlyphCache.h"
 #include "SkMakeUnique.h"
 #include "SkPDFCanon.h"
 #include "SkPDFConvertType1FontStream.h"

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"

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"