From 518a2923f11b819fa44ed5cff54155326959540f Mon Sep 17 00:00:00 2001 From: mtklein Date: Tue, 1 Sep 2015 12:32:24 -0700 Subject: [PATCH] Stop using SkScalerContext::getAdvance() in SkGlyphCache. We think it'll simplify things to just always get the full metrics. On most platforms, it's no different, and we think the platforms that do differ (FreeType) will be nearly just as cheap. Removing this distinction helps us make SkGlyphCaches concurrent by removing a state (we-have-only-advances) from its logical state machine. We see no significant changes running SKPs before and after this CL. That makes sense, of course, because the SKPs bake some of this into drawPosText. BUG=skia: Review URL: https://codereview.chromium.org/1321243004 --- src/core/SkGlyphCache.cpp | 37 +++++++++++++------------------------ src/core/SkGlyphCache.h | 15 ++++----------- 2 files changed, 17 insertions(+), 35 deletions(-) diff --git a/src/core/SkGlyphCache.cpp b/src/core/SkGlyphCache.cpp index f83aab5942..309707c66f 100644 --- a/src/core/SkGlyphCache.cpp +++ b/src/core/SkGlyphCache.cpp @@ -119,40 +119,40 @@ int SkGlyphCache::countCachedGlyphs() const { const SkGlyph& SkGlyphCache::getUnicharAdvance(SkUnichar charCode) { VALIDATE(); - return *this->lookupByChar(charCode, kJustAdvance_MetricsType); + return *this->lookupByChar(charCode); } const SkGlyph& SkGlyphCache::getGlyphIDAdvance(uint16_t glyphID) { VALIDATE(); PackedGlyphID packedGlyphID = SkGlyph::MakeID(glyphID); - return *this->lookupByPackedGlyphID(packedGlyphID, kJustAdvance_MetricsType); + return *this->lookupByPackedGlyphID(packedGlyphID); } /////////////////////////////////////////////////////////////////////////////// const SkGlyph& SkGlyphCache::getUnicharMetrics(SkUnichar charCode) { VALIDATE(); - return *this->lookupByChar(charCode, kFull_MetricsType); + return *this->lookupByChar(charCode); } const SkGlyph& SkGlyphCache::getUnicharMetrics(SkUnichar charCode, SkFixed x, SkFixed y) { VALIDATE(); - return *this->lookupByChar(charCode, kFull_MetricsType, x, y); + return *this->lookupByChar(charCode, x, y); } const SkGlyph& SkGlyphCache::getGlyphIDMetrics(uint16_t glyphID) { VALIDATE(); PackedGlyphID packedGlyphID = SkGlyph::MakeID(glyphID); - return *this->lookupByPackedGlyphID(packedGlyphID, kFull_MetricsType); + return *this->lookupByPackedGlyphID(packedGlyphID); } const SkGlyph& SkGlyphCache::getGlyphIDMetrics(uint16_t glyphID, SkFixed x, SkFixed y) { VALIDATE(); PackedGlyphID packedGlyphID = SkGlyph::MakeID(glyphID, x, y); - return *this->lookupByPackedGlyphID(packedGlyphID, kFull_MetricsType); + return *this->lookupByPackedGlyphID(packedGlyphID); } -SkGlyph* SkGlyphCache::lookupByChar(SkUnichar charCode, MetricsType type, SkFixed x, SkFixed y) { +SkGlyph* SkGlyphCache::lookupByChar(SkUnichar charCode, SkFixed x, SkFixed y) { PackedUnicharID id = SkGlyph::MakeID(charCode, x, y); CharGlyphRec* rec = this->getCharGlyphRec(id); if (rec->fPackedUnicharID != id) { @@ -161,26 +161,21 @@ SkGlyph* SkGlyphCache::lookupByChar(SkUnichar charCode, MetricsType type, SkFixe // this ID is based on the glyph index PackedGlyphID combinedID = SkGlyph::MakeID(fScalerContext->charToGlyphID(charCode), x, y); rec->fPackedGlyphID = combinedID; - return this->lookupByPackedGlyphID(combinedID, type); + return this->lookupByPackedGlyphID(combinedID); } else { - return this->lookupByPackedGlyphID(rec->fPackedGlyphID, type); + return this->lookupByPackedGlyphID(rec->fPackedGlyphID); } } -SkGlyph* SkGlyphCache::lookupByPackedGlyphID(PackedGlyphID packedGlyphID, MetricsType type) { +SkGlyph* SkGlyphCache::lookupByPackedGlyphID(PackedGlyphID packedGlyphID) { SkGlyph* glyph = fGlyphMap.find(packedGlyphID); - if (nullptr == glyph) { - glyph = this->allocateNewGlyph(packedGlyphID, type); - } else { - if (type == kFull_MetricsType && glyph->isJustAdvance()) { - fScalerContext->getMetrics(glyph); - } + glyph = this->allocateNewGlyph(packedGlyphID); } return glyph; } -SkGlyph* SkGlyphCache::allocateNewGlyph(PackedGlyphID packedGlyphID, MetricsType mtype) { +SkGlyph* SkGlyphCache::allocateNewGlyph(PackedGlyphID packedGlyphID) { fMemoryUsed += sizeof(SkGlyph); SkGlyph* glyphPtr; @@ -189,13 +184,7 @@ SkGlyph* SkGlyphCache::allocateNewGlyph(PackedGlyphID packedGlyphID, MetricsType glyph.initGlyphFromCombinedID(packedGlyphID); glyphPtr = fGlyphMap.set(glyph); } - - if (kJustAdvance_MetricsType == mtype) { - fScalerContext->getAdvance(glyphPtr); - } else { - SkASSERT(kFull_MetricsType == mtype); - fScalerContext->getMetrics(glyphPtr); - } + fScalerContext->getMetrics(glyphPtr); SkASSERT(glyphPtr->fID != SkGlyph::kImpossibleID); return glyphPtr; diff --git a/src/core/SkGlyphCache.h b/src/core/SkGlyphCache.h index 2f1f417250..4eb6b5bc60 100644 --- a/src/core/SkGlyphCache.h +++ b/src/core/SkGlyphCache.h @@ -181,11 +181,6 @@ public: private: friend class SkGlyphCache_Globals; - enum MetricsType { - kJustAdvance_MetricsType, - kFull_MetricsType - }; - enum { kHashBits = 8, kHashCount = 1 << kHashBits, @@ -213,15 +208,13 @@ private: // 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. - SkGlyph* lookupByPackedGlyphID(PackedGlyphID packedGlyphID, MetricsType type); + SkGlyph* lookupByPackedGlyphID(PackedGlyphID packedGlyphID); // Return a SkGlyph* associated with unicode id and position x and y. - SkGlyph* lookupByChar(SkUnichar id, MetricsType type, SkFixed x = 0, SkFixed y = 0); + SkGlyph* lookupByChar(SkUnichar id, SkFixed x = 0, SkFixed y = 0); - // Return a new SkGlyph for the glyph ID and subpixel position id. Limit the amount - // of work - // using type. - SkGlyph* allocateNewGlyph(PackedGlyphID packedGlyphID, MetricsType type); + // Return a new SkGlyph for the glyph ID and subpixel position id. + SkGlyph* allocateNewGlyph(PackedGlyphID packedGlyphID); static bool DetachProc(const SkGlyphCache*, void*) { return true; }