From 511dcfc33994902920eae6e95e97fa9f10a4d9bf Mon Sep 17 00:00:00 2001 From: Herb Derby Date: Mon, 24 Jun 2019 12:58:41 -0400 Subject: [PATCH] Remove getGlyphIDMatrics type calls. Converting to glyph() style calls that return SkGlyph*. This is mainly preparation for removing converting findImage(const SkGlyph&) to prepareImage(SkGlyph*). + Misc cleanups mainly fWidth -> width() type things. Change-Id: Id5c9b0ba5856b3ea54353ece4d05fa495cc5a640 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/223187 Commit-Queue: Herb Derby Reviewed-by: Mike Klein --- bench/SkGlyphCacheBench.cpp | 5 ++-- src/core/SkRemoteGlyphCache.cpp | 17 ++++++------ src/core/SkStrike.cpp | 40 +++++++++------------------ src/core/SkStrike.h | 22 ++------------- src/core/SkStrikeCache.cpp | 7 +---- src/pdf/SkPDFFont.cpp | 4 +-- tests/SkRemoteGlyphCacheTest.cpp | 47 +++++++++++++++++--------------- 7 files changed, 53 insertions(+), 89 deletions(-) diff --git a/bench/SkGlyphCacheBench.cpp b/bench/SkGlyphCacheBench.cpp index 66b3a668a4..d26e9461a9 100644 --- a/bench/SkGlyphCacheBench.cpp +++ b/bench/SkGlyphCacheBench.cpp @@ -31,11 +31,10 @@ static void do_font_stuff(SkFont* font) { } for (int lookups = 0; lookups < 10; lookups++) { for (int c = ' '; c < 'z'; c++) { - const SkGlyph& g = cache->getGlyphIDMetrics(glyphs[c]); - cache->findImage(g); + SkGlyph* g = cache->glyph(glyphs[c]); + cache->findImage(*g); } } - } } diff --git a/src/core/SkRemoteGlyphCache.cpp b/src/core/SkRemoteGlyphCache.cpp index f8414626ca..6ecd37c071 100644 --- a/src/core/SkRemoteGlyphCache.cpp +++ b/src/core/SkRemoteGlyphCache.cpp @@ -445,10 +445,10 @@ static void writeGlyph(SkGlyph* glyph, Serializer* serializer) { serializer->write(glyph->getPackedID()); serializer->write(glyph->advanceX()); serializer->write(glyph->advanceY()); - serializer->write(glyph->fWidth); - serializer->write(glyph->fHeight); - serializer->write(glyph->fTop); - serializer->write(glyph->fLeft); + serializer->write(glyph->width()); + serializer->write(glyph->height()); + serializer->write(glyph->top()); + serializer->write(glyph->left()); serializer->write(glyph->maskFormat()); } @@ -616,7 +616,7 @@ SkSpan SkStrikeServer::SkGlyphCacheState::prepareForDrawing( if (glyphPtr->maxDimension() <= maxDimension) { // do nothing - } else if (glyphPtr->fMaskFormat != SkMask::kARGB32_Format) { + } else if (!glyphPtr->isColor()) { // The glyph is too big for the atlas, but it is not color, so it is handled with a // path. @@ -628,8 +628,7 @@ SkSpan SkStrikeServer::SkGlyphCacheState::prepareForDrawing( } } else { // This will be handled by the fallback strike. - SkASSERT(glyphPtr->maxDimension() > maxDimension - && glyphPtr->fMaskFormat == SkMask::kARGB32_Format); + SkASSERT(glyphPtr->maxDimension() > maxDimension && glyphPtr->isColor()); } // Make sure to send the glyph to the GPU because we always send the image for a glyph. @@ -761,7 +760,7 @@ bool SkStrikeClient::readStrikeData(const volatile void* memory, size_t memorySi SkTLazy glyph; if (!SkStrikeClient::ReadGlyph(glyph, &deserializer)) READ_FAILURE - SkGlyph* allocatedGlyph = strike->getRawGlyphByID(glyph->getPackedID()); + SkGlyph* allocatedGlyph = strike->uninitializedGlyph(glyph->getPackedID()); // Update the glyph unless it's already got an image (from fallback), // preserving any path that might be present. @@ -790,7 +789,7 @@ bool SkStrikeClient::readStrikeData(const volatile void* memory, size_t memorySi SkTLazy glyph; if (!SkStrikeClient::ReadGlyph(glyph, &deserializer)) READ_FAILURE - SkGlyph* allocatedGlyph = strike->getRawGlyphByID(glyph->getPackedID()); + SkGlyph* allocatedGlyph = strike->uninitializedGlyph(glyph->getPackedID()); SkPath* pathPtr = nullptr; SkPath path; diff --git a/src/core/SkStrike.cpp b/src/core/SkStrike.cpp index cd9c346f8a..738c24e84b 100644 --- a/src/core/SkStrike.cpp +++ b/src/core/SkStrike.cpp @@ -65,13 +65,24 @@ SkGlyph* SkStrike::glyph(SkGlyphID glyphID) { return this->glyph(SkPackedGlyphID{glyphID}); } -SkGlyph* SkStrike::glyphFromPrototype(const SkGlyphPrototype& p) { +SkGlyph* SkStrike::glyph(SkGlyphID glyphID, SkPoint position) { + const SkFixed maskX = (!fIsSubpixel || fAxisAlignment == kY_SkAxisAlignment) ? 0 : ~0; + const SkFixed maskY = (!fIsSubpixel || fAxisAlignment == kX_SkAxisAlignment) ? 0 : ~0; + SkFixed subX = SkScalarToFixed(position.x()) & maskX, + subY = SkScalarToFixed(position.y()) & maskY; + return this->glyph(SkPackedGlyphID{glyphID, subX, subY}); +} + +SkGlyph* SkStrike::glyphFromPrototype(const SkGlyphPrototype& p, void* image) { SkGlyph* glyph = fGlyphMap.findOrNull(p.id); if (glyph == nullptr) { fMemoryUsed += sizeof(SkGlyph); glyph = fAlloc.make(p); fGlyphMap.set(glyph); } + if (glyph->setImage(&fAlloc, image)) { + fMemoryUsed += glyph->imageSize(); + } return glyph; } @@ -79,25 +90,6 @@ SkGlyph* SkStrike::glyphOrNull(SkPackedGlyphID id) const { return fGlyphMap.findOrNull(id); } -SkGlyph* SkStrike::getRawGlyphByID(SkPackedGlyphID id) { - return this->uninitializedGlyph(id); -} - -const SkGlyph& SkStrike::getGlyphIDMetrics(SkGlyphID glyphID) { - VALIDATE(); - return *this->glyph(glyphID); -} - -const SkGlyph& SkStrike::getGlyphIDMetrics(SkPackedGlyphID id) { - VALIDATE(); - return *this->glyph(id); -} - -const SkGlyph& SkStrike::getGlyphIDMetrics(SkGlyphID glyphID, SkFixed x, SkFixed y) { - SkPackedGlyphID packedGlyphID(glyphID, x, y); - return *this->glyph(packedGlyphID); -} - const SkPath* SkStrike::preparePath(SkGlyph* glyph) { if (glyph->setPath(&fAlloc, fScalerContext.get())) { fMemoryUsed += glyph->path()->approximateBytesUsed(); @@ -195,13 +187,7 @@ SkVector SkStrike::rounding() const { } const SkGlyph& SkStrike::getGlyphMetrics(SkGlyphID glyphID, SkPoint position) { - if (!fIsSubpixel) { - return this->getGlyphIDMetrics(glyphID); - } else { - SkIPoint lookupPosition = SkStrikeCommon::SubpixelLookup(fAxisAlignment, position); - - return this->getGlyphIDMetrics(glyphID, lookupPosition.x(), lookupPosition.y()); - } + return *this->glyph(glyphID, position); } // N.B. This glyphMetrics call culls all the glyphs which will not display based on a non-finite diff --git a/src/core/SkStrike.h b/src/core/SkStrike.h index 01b2faaa6b..0eeb0cf277 100644 --- a/src/core/SkStrike.h +++ b/src/core/SkStrike.h @@ -42,30 +42,12 @@ public: /** Return true if glyph is cached. */ bool isGlyphCached(SkGlyphID glyphID, SkFixed x, SkFixed y) const; - /** Return a glyph that has no information if it is not already filled out. */ - SkGlyph* getRawGlyphByID(SkPackedGlyphID); - - /** Returns a glyph with all fields valid except fImage and fPath, which may be null. If they - are null, call findImage or findPath for those. If they are not null, then they are valid. - - This call is potentially slower than the matching ...Advance call. If you only need the - fAdvance/fDevKern fields, call those instead. - */ - const SkGlyph& getGlyphIDMetrics(SkGlyphID); - - /** These are variants that take the device position of the glyph. Call these only if you are - drawing in subpixel mode. Passing 0, 0 is effectively the same as calling the variants - w/o the extra params, though a tiny bit slower. - */ - const SkGlyph& getGlyphIDMetrics(SkGlyphID, SkFixed x, SkFixed y); - - const SkGlyph& getGlyphIDMetrics(SkPackedGlyphID id); - // Return a glyph. Create it if it doesn't exist, and initialize the glyph with metrics and // advances. SkGlyph* glyph(SkPackedGlyphID packedID); SkGlyph* glyph(SkGlyphID glyphID); - SkGlyph* glyphFromPrototype(const SkGlyphPrototype& p); + SkGlyph* glyphFromPrototype(const SkGlyphPrototype& p, void* image = nullptr); + SkGlyph* glyph(SkGlyphID, SkPoint); // Return a glyph or nullptr if it does not exits in the strike. SkGlyph* glyphOrNull(SkPackedGlyphID id) const; diff --git a/src/core/SkStrikeCache.cpp b/src/core/SkStrikeCache.cpp index 4509d4e0c7..cc32bc02a2 100644 --- a/src/core/SkStrikeCache.cpp +++ b/src/core/SkStrikeCache.cpp @@ -306,14 +306,9 @@ bool SkStrikeCache::desperationSearchForImage(const SkDescriptor& desc, SkGlyph* SkAutoSpinlock ac(fLock); SkGlyphID glyphID = glyph->getGlyphID(); - SkFixed targetSubX = glyph->getSubXFixed(), - targetSubY = glyph->getSubYFixed(); - for (Node* node = internalGetHead(); node != nullptr; node = node->fNext) { if (loose_compare(node->fStrike.getDescriptor(), desc)) { - auto targetGlyphID = SkPackedGlyphID(glyphID, targetSubX, targetSubY); - if (node->fStrike.isGlyphCached(glyphID, targetSubX, targetSubY)) { - SkGlyph* fallback = node->fStrike.getRawGlyphByID(targetGlyphID); + if (SkGlyph *fallback = node->fStrike.glyphOrNull(glyph->getPackedID())) { // This desperate-match node may disappear as soon as we drop fLock, so we // need to copy the glyph from node into this strike, including a // deep copy of the mask. diff --git a/src/pdf/SkPDFFont.cpp b/src/pdf/SkPDFFont.cpp index 5bc45f29cf..716e64a47f 100644 --- a/src/pdf/SkPDFFont.cpp +++ b/src/pdf/SkPDFFont.cpp @@ -448,8 +448,8 @@ struct ImageAndOffset { SkIPoint fOffset; }; static ImageAndOffset to_image(SkGlyphID gid, SkStrike* cache) { - (void)cache->findImage(cache->getGlyphIDMetrics(gid)); - SkMask mask = cache->getGlyphIDMetrics(gid).mask(); + (void)cache->findImage(*cache->glyph(gid)); + SkMask mask = cache->glyph(gid)->mask(); if (!mask.fImage) { return {nullptr, {0, 0}}; } diff --git a/tests/SkRemoteGlyphCacheTest.cpp b/tests/SkRemoteGlyphCacheTest.cpp index 795f5fe978..af4983460c 100644 --- a/tests/SkRemoteGlyphCacheTest.cpp +++ b/tests/SkRemoteGlyphCacheTest.cpp @@ -824,6 +824,7 @@ DEF_TEST(SkRemoteGlyphCache_SearchOfDesperation, reporter) { SkFont font; font.setTypeface(clientTf); + font.setSubpixel(true); SkPaint paint; paint.setAntiAlias(true); paint.setColor(SK_ColorRED); @@ -846,8 +847,7 @@ DEF_TEST(SkRemoteGlyphCache_SearchOfDesperation, reporter) { auto fallbackCache = strikeCache.findOrCreateStrikeExclusive(*desc, effects, *clientTf); SkGlyphPrototype proto = {lostGlyphID, 0.f, 0.f, 2, 1, 0, 0, SkMask::kA8_Format, false}; - SkGlyph* glyph = fallbackCache->glyphFromPrototype(proto); - fallbackCache->initializeImage(glyphImage, glyph->imageSize(), glyph); + fallbackCache->glyphFromPrototype(proto, (void*)glyphImage); } // Make sure we can find the fall back cache. @@ -883,26 +883,28 @@ DEF_TEST(SkRemoteGlyphCache_SearchOfDesperation, reporter) { // Look for the lost glyph. { - const auto& lostGlyph = testCache->getGlyphIDMetrics( - lostGlyphID.code(), lostGlyphID.getSubXFixed(), lostGlyphID.getSubYFixed()); - testCache->findImage(lostGlyph); + SkPoint pt{SkFixedToScalar(lostGlyphID.getSubXFixed()), + SkFixedToScalar(lostGlyphID.getSubYFixed())}; + SkGlyph* lostGlyph = testCache->glyph(lostGlyphID.code(), pt); + testCache->findImage(*lostGlyph); - REPORTER_ASSERT(reporter, lostGlyph.fHeight == 1); - REPORTER_ASSERT(reporter, lostGlyph.fWidth == 2); - REPORTER_ASSERT(reporter, lostGlyph.maskFormat() == SkMask::kA8_Format); - REPORTER_ASSERT(reporter, memcmp(lostGlyph.fImage, glyphImage, sizeof(glyphImage)) == 0); + REPORTER_ASSERT(reporter, lostGlyph->height() == 1); + REPORTER_ASSERT(reporter, lostGlyph->width() == 2); + REPORTER_ASSERT(reporter, lostGlyph->maskFormat() == SkMask::kA8_Format); + REPORTER_ASSERT(reporter, memcmp(lostGlyph->image(), glyphImage, sizeof(glyphImage)) == 0); } // Look for the lost glyph with a different sub-pix position. { - const auto& lostGlyph = - testCache->getGlyphIDMetrics(lostGlyphID.code(), SK_FixedQuarter, SK_FixedQuarter); - testCache->findImage(lostGlyph); + SkPoint pt{SkFixedToScalar(SK_FixedQuarter), + SkFixedToScalar(SK_FixedQuarter)}; + SkGlyph* lostGlyph = testCache->glyph(lostGlyphID.code(), pt); + testCache->findImage(*lostGlyph); - REPORTER_ASSERT(reporter, lostGlyph.fHeight == 1); - REPORTER_ASSERT(reporter, lostGlyph.fWidth == 2); - REPORTER_ASSERT(reporter, lostGlyph.maskFormat() == SkMask::kA8_Format); - REPORTER_ASSERT(reporter, memcmp(lostGlyph.fImage, glyphImage, sizeof(glyphImage)) == 0); + REPORTER_ASSERT(reporter, lostGlyph->height() == 1); + REPORTER_ASSERT(reporter, lostGlyph->width() == 2); + REPORTER_ASSERT(reporter, lostGlyph->maskFormat() == SkMask::kA8_Format); + REPORTER_ASSERT(reporter, memcmp(lostGlyph->image(), glyphImage, sizeof(glyphImage)) == 0); } for (uint32_t i = 0; i <= SkStrikeClient::CacheMissType::kLast; ++i) { @@ -974,8 +976,7 @@ DEF_TEST(SkRemoteGlyphCache_ReWriteGlyph, reporter) { auto fallbackCache = strikeCache.findOrCreateStrikeExclusive(*desc, effects, *clientTf); fakeMask = (realMask == SkMask::kA8_Format) ? SkMask::kBW_Format : SkMask::kA8_Format; SkGlyphPrototype proto = {lostGlyphID, 0.f, 0.f, 2, 1, 0, 0, fakeMask, false}; - SkGlyph* glyph = fallbackCache->glyphFromPrototype(proto); - fallbackCache->initializeImage(glyphImage, glyph->imageSize(), glyph); + fallbackCache->glyphFromPrototype(proto, (void *)glyphImage); } // Send over the real glyph and make sure the client cache stays intact. @@ -1010,10 +1011,12 @@ DEF_TEST(SkRemoteGlyphCache_ReWriteGlyph, reporter) { auto fallbackCache = strikeCache.findStrikeExclusive(*desc); REPORTER_ASSERT(reporter, fallbackCache.get() != nullptr); - auto glyph = fallbackCache->getRawGlyphByID(lostGlyphID); - REPORTER_ASSERT(reporter, glyph->maskFormat() == fakeMask); - REPORTER_ASSERT(reporter, - memcmp(glyph->fImage, glyphImage, glyph->imageSize()) == 0); + auto glyph = fallbackCache->glyphOrNull(lostGlyphID); + REPORTER_ASSERT(reporter, glyph && glyph->maskFormat() == fakeMask); + if (glyph) { + REPORTER_ASSERT(reporter, + memcmp(glyph->image(), glyphImage, glyph->imageSize()) == 0); + } } strikeCache.validateGlyphCacheDataSize();