Remove search of desperation
Change-Id: Id862057ad5b853e979a9e93fc43a559360b400d9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/268842
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Reviewed-by: Khushal Sagar <khushalsagar@chromium.org>
diff --git a/src/core/SkRemoteGlyphCache.h b/src/core/SkRemoteGlyphCache.h
index d78b431..4a3bd36 100644
--- a/src/core/SkRemoteGlyphCache.h
+++ b/src/core/SkRemoteGlyphCache.h
@@ -176,11 +176,11 @@
kGlyphImage = 2,
kGlyphPath = 3,
- // The original glyph could not be found and a fallback was used.
+ // (DEPRECATED) The original glyph could not be found and a fallback was used.
kGlyphMetricsFallback = 4,
kGlyphPathFallback = 5,
- kLast = kGlyphPathFallback
+ kLast = kGlyphPath
};
// An interface to delete handles that may be pinned by the remote server.
diff --git a/src/core/SkStrike.cpp b/src/core/SkStrike.cpp
index cf49926..7f828ef 100644
--- a/src/core/SkStrike.cpp
+++ b/src/core/SkStrike.cpp
@@ -124,20 +124,6 @@
return glyph;
}
-const SkGlyph* SkStrike::getCachedGlyphAnySubPix(SkGlyphID glyphID, SkPackedGlyphID vetoID) const {
- for (SkFixed subY = 0; subY < SK_Fixed1; subY += SK_FixedQuarter) {
- for (SkFixed subX = 0; subX < SK_Fixed1; subX += SK_FixedQuarter) {
- SkPackedGlyphID packedGlyphID{glyphID, subX, subY};
- if (packedGlyphID == vetoID) continue;
- if (SkGlyph* glyphPtr = fGlyphMap.findOrNull(packedGlyphID)) {
- return glyphPtr;
- }
- }
- }
-
- return nullptr;
-}
-
SkSpan<const SkGlyph*> SkStrike::metrics(SkSpan<const SkGlyphID> glyphIDs,
const SkGlyph* results[]) {
return this->internalPrepare(glyphIDs, kMetricsOnly, results);
diff --git a/src/core/SkStrike.h b/src/core/SkStrike.h
index 0ae57ac..20d0d3f 100644
--- a/src/core/SkStrike.h
+++ b/src/core/SkStrike.h
@@ -63,11 +63,6 @@
void findIntercepts(const SkScalar bounds[2], SkScalar scale, SkScalar xPos,
SkGlyph* , SkScalar* array, int* count);
- /** Find any glyph in this cache with the given ID, regardless of subpixel positioning.
- * If set and present, skip over the glyph with vetoID.
- */
- const SkGlyph* getCachedGlyphAnySubPix(SkGlyphID,
- SkPackedGlyphID vetoID = SkPackedGlyphID()) const;
/** Return the vertical metrics for this strike.
*/
diff --git a/src/core/SkStrikeCache.cpp b/src/core/SkStrikeCache.cpp
index 482776d..205ae28 100644
--- a/src/core/SkStrikeCache.cpp
+++ b/src/core/SkStrikeCache.cpp
@@ -279,84 +279,6 @@
return nullptr;
}
-
-static bool loose_compare(const SkDescriptor& lhs, const SkDescriptor& rhs) {
- uint32_t size;
- auto ptr = lhs.findEntry(kRec_SkDescriptorTag, &size);
- SkScalerContextRec lhsRec;
- std::memcpy((void*)&lhsRec, ptr, size);
-
- ptr = rhs.findEntry(kRec_SkDescriptorTag, &size);
- SkScalerContextRec rhsRec;
- std::memcpy((void*)&rhsRec, ptr, size);
-
- // If these don't match, there's no way we can use these strikes interchangeably.
- // Note that a typeface from each renderer maps to a unique proxy typeface on the GPU,
- // keyed in the glyph cache using fontID in the SkDescriptor. By limiting this search
- // to descriptors with the same fontID, we ensure that a renderer never uses glyphs
- // generated by a different renderer.
- return
- lhsRec.fFontID == rhsRec.fFontID &&
- lhsRec.fTextSize == rhsRec.fTextSize &&
- lhsRec.fPreScaleX == rhsRec.fPreScaleX &&
- lhsRec.fPreSkewX == rhsRec.fPreSkewX &&
- lhsRec.fPost2x2[0][0] == rhsRec.fPost2x2[0][0] &&
- lhsRec.fPost2x2[0][1] == rhsRec.fPost2x2[0][1] &&
- lhsRec.fPost2x2[1][0] == rhsRec.fPost2x2[1][0] &&
- lhsRec.fPost2x2[1][1] == rhsRec.fPost2x2[1][1];
-}
-
-bool SkStrikeCache::desperationSearchForImage(const SkDescriptor& desc, SkGlyph* glyph,
- SkStrike* targetCache) {
- SkAutoSpinlock ac(fLock);
-
- SkGlyphID glyphID = glyph->getGlyphID();
- for (Node* node = fHead; node != nullptr; node = node->fNext) {
- if (loose_compare(node->fStrike.getDescriptor(), desc)) {
- 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.
- targetCache->mergeGlyphAndImage(glyph->getPackedID(), *fallback);
- return true;
- }
-
- // Look for any sub-pixel pos for this glyph, in case there is a pos mismatch.
- if (const auto* fallback = node->fStrike.getCachedGlyphAnySubPix(glyphID)) {
- targetCache->mergeGlyphAndImage(glyph->getPackedID(), *fallback);
- return true;
- }
- }
- }
-
- return false;
-}
-
-bool SkStrikeCache::desperationSearchForPath(
- const SkDescriptor& desc, SkGlyphID glyphID, SkPath* path) {
- SkAutoSpinlock ac(fLock);
-
- // The following is wrong there is subpixel positioning with paths...
- // Paths are only ever at sub-pixel position (0,0), so we can just try that directly rather
- // than try our packed position first then search all others on failure like for masks.
- //
- // This will have to search the sub-pixel positions too.
- // There is also a problem with accounting for cache size with shared path data.
- for (Node* node = fHead; node != nullptr; node = node->fNext) {
- if (loose_compare(node->fStrike.getDescriptor(), desc)) {
- if (SkGlyph *from = node->fStrike.glyphOrNull(SkPackedGlyphID{glyphID})) {
- if (from->setPathHasBeenCalled() && from->path() != nullptr) {
- // We can just copy the path out by value here, so no need to worry
- // about the lifetime of this desperate-match node.
- *path = *from->path();
- return true;
- }
- }
- }
- }
- return false;
-}
-
SkExclusiveStrikePtr SkStrikeCache::createStrikeExclusive(
const SkDescriptor& desc,
std::unique_ptr<SkScalerContext> scaler,
@@ -588,5 +510,3 @@
}
}
#endif
-
-////////////////////////////////////////////////////////////////////////////////////////////////////
diff --git a/src/core/SkStrikeCache.h b/src/core/SkStrikeCache.h
index 47b5f26..db5ee6b 100644
--- a/src/core/SkStrikeCache.h
+++ b/src/core/SkStrikeCache.h
@@ -82,13 +82,6 @@
const SkScalerContextEffects& effects,
const SkTypeface& typeface);
- // Routines to find suitable data when working in a remote cache situation. These are
- // suitable as substitutes for similar calls in SkScalerContext.
- bool desperationSearchForImage(const SkDescriptor& desc,
- SkGlyph* glyph,
- SkStrike* targetCache);
- bool desperationSearchForPath(const SkDescriptor& desc, SkGlyphID glyphID, SkPath* path);
-
SkScopedStrikeForGPU findOrCreateScopedStrike(const SkDescriptor& desc,
const SkScalerContextEffects& effects,
const SkTypeface& typeface) override;
diff --git a/src/core/SkTypeface_remote.cpp b/src/core/SkTypeface_remote.cpp
index dd6c2e2..c08c2e9 100644
--- a/src/core/SkTypeface_remote.cpp
+++ b/src/core/SkTypeface_remote.cpp
@@ -42,27 +42,6 @@
}
glyph->fMaskFormat = fRec.fMaskFormat;
-
- // Since the scaler context is being called, we don't have the needed data. Try to find a
- // fallback before failing.
- if (fCache && fCache->glyphOrNull(glyph->getPackedID()) != nullptr) {
- // First check the original cache, in case there is a sub-pixel pos mismatch.
- if (const SkGlyph* from =
- fCache->getCachedGlyphAnySubPix(glyph->getGlyphID(), glyph->getPackedID())) {
- fCache->mergeGlyphAndImage(glyph->getPackedID(), *from);
- fDiscardableManager->notifyCacheMiss(
- SkStrikeClient::CacheMissType::kGlyphMetricsFallback);
- return;
- }
-
- // Now check other caches for a desc mismatch.
- if (fStrikeCache->desperationSearchForImage(fCache->getDescriptor(), glyph, fCache)) {
- fDiscardableManager->notifyCacheMiss(
- SkStrikeClient::CacheMissType::kGlyphMetricsFallback);
- return;
- }
- }
-
glyph->zeroMetrics();
fDiscardableManager->notifyCacheMiss(SkStrikeClient::CacheMissType::kGlyphMetrics);
}
@@ -84,14 +63,8 @@
SkDebugf("GlyphCacheMiss generatePath: %s\n", this->getRec().dump().c_str());
}
- // Since the scaler context is being called, we don't have the needed data. Try to find a
- // fallback before failing.
- auto desc = SkScalerContext::DescriptorGivenRecAndEffects(this->getRec(), this->getEffects());
- bool foundPath = fStrikeCache && fStrikeCache->desperationSearchForPath(*desc, glyphID, path);
- fDiscardableManager->notifyCacheMiss(foundPath
- ? SkStrikeClient::CacheMissType::kGlyphPathFallback
- : SkStrikeClient::CacheMissType::kGlyphPath);
- return foundPath;
+ fDiscardableManager->notifyCacheMiss(SkStrikeClient::CacheMissType::kGlyphPath);
+ return false;
}
void SkScalerContextProxy::generateFontMetrics(SkFontMetrics* metrics) {
diff --git a/tests/SkRemoteGlyphCacheTest.cpp b/tests/SkRemoteGlyphCacheTest.cpp
index fd327e8..c3b21c2 100644
--- a/tests/SkRemoteGlyphCacheTest.cpp
+++ b/tests/SkRemoteGlyphCacheTest.cpp
@@ -821,122 +821,6 @@
discardableManager->unlockAndDeleteAll();
}
-DEF_TEST(SkRemoteGlyphCache_SearchOfDesperation, reporter) {
- // Build proxy typeface on the client for initializing the cache.
- sk_sp<DiscardableManager> discardableManager = sk_make_sp<DiscardableManager>();
- SkStrikeServer server(discardableManager.get());
- SkStrikeClient client(discardableManager, false);
-
- auto serverTf = SkTypeface::MakeFromName("monospace", SkFontStyle());
- auto tfData = server.serializeTypeface(serverTf.get());
- auto clientTf = client.deserializeTypeface(tfData->data(), tfData->size());
- REPORTER_ASSERT(reporter, clientTf);
-
- SkFont font;
- font.setTypeface(clientTf);
- font.setSubpixel(true);
- SkPaint paint;
- paint.setAntiAlias(true);
- paint.setColor(SK_ColorRED);
-
- auto lostGlyphID = SkPackedGlyphID(1, SK_FixedHalf, SK_FixedHalf);
- const uint8_t glyphImage[] = {0xFF, 0xFF};
-
- SkStrikeCache strikeCache;
-
- // Build a fallback cache.
- {
- SkAutoDescriptor ad;
- SkScalerContextRec rec;
- SkScalerContextEffects effects;
- SkScalerContextFlags flags = SkScalerContextFlags::kFakeGammaAndBoostContrast;
- SkScalerContext::MakeRecAndEffects(
- font, paint, SkSurfacePropsCopyOrDefault(nullptr), flags,
- SkMatrix::I(), &rec, &effects);
- auto desc = SkScalerContext::AutoDescriptorGivenRecAndEffects(rec, effects, &ad);
-
- auto fallbackCache = strikeCache.findOrCreateStrikeExclusive(*desc, effects, *clientTf);
- SkGlyphPrototype proto = {lostGlyphID, 0.f, 0.f, 2, 1, 0, 0, SkMask::kA8_Format, false};
- fallbackCache->glyphFromPrototype(proto, (void*)glyphImage);
- }
-
- // Make sure we can find the fall back cache.
- {
- SkAutoDescriptor ad;
- SkScalerContextRec rec;
- SkScalerContextEffects effects;
- SkScalerContextFlags flags = SkScalerContextFlags::kFakeGammaAndBoostContrast;
- SkScalerContext::MakeRecAndEffects(
- font, paint, SkSurfacePropsCopyOrDefault(nullptr), flags,
- SkMatrix::I(), &rec, &effects);
- auto desc = SkScalerContext::AutoDescriptorGivenRecAndEffects(rec, effects, &ad);
- auto testCache = strikeCache.findStrikeExclusive(*desc);
- REPORTER_ASSERT(reporter, !(testCache == nullptr));
- }
-
- // Create the target cache.
- SkExclusiveStrikePtr testCache;
- SkAutoDescriptor ad;
- SkScalerContextRec rec;
- SkScalerContextEffects effects;
- SkScalerContextFlags flags = SkScalerContextFlags::kNone;
- SkScalerContext::MakeRecAndEffects(
- font, paint, SkSurfacePropsCopyOrDefault(nullptr), flags,
- SkMatrix::I(), &rec, &effects);
- auto desc = SkScalerContext::AutoDescriptorGivenRecAndEffects(rec, effects, &ad);
- testCache = strikeCache.findStrikeExclusive(*desc);
- REPORTER_ASSERT(reporter, testCache == nullptr);
- testCache = strikeCache.createStrikeExclusive(*desc,
- clientTf->createScalerContext(effects, desc));
- auto scalerProxy = static_cast<SkScalerContextProxy*>(testCache->getScalerContext());
- scalerProxy->initCache(testCache.get(), &strikeCache);
-
- auto rounding = testCache->roundingSpec();
- SkBulkGlyphMetricsAndImages metricsAndImages{std::move(testCache)};
-
- // Look for the lost glyph.
- {
- SkPoint pt{SkFixedToScalar(lostGlyphID.getSubXFixed()),
- SkFixedToScalar(lostGlyphID.getSubYFixed())};
- SkPackedGlyphID packedID{
- lostGlyphID.glyphID(), pt, rounding.ignorePositionFieldMask};
-
- const SkGlyph* lostGlyph = metricsAndImages.glyph(packedID);
-
- 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.
- {
- SkPoint pt{SkFixedToScalar(SK_FixedQuarter),
- SkFixedToScalar(SK_FixedQuarter)};
- SkPackedGlyphID packedID{
- lostGlyphID.glyphID(), pt, rounding.ignorePositionFieldMask};
- const SkGlyph* lostGlyph = metricsAndImages.glyph(packedID);
-
- 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) {
- if (i == SkStrikeClient::CacheMissType::kGlyphMetricsFallback ||
- i == SkStrikeClient::CacheMissType::kFontMetrics) {
- REPORTER_ASSERT(reporter, discardableManager->cacheMissCount(i) == 2);
- } else {
- REPORTER_ASSERT(reporter, discardableManager->cacheMissCount(i) == 0);
- }
- }
- strikeCache.validateGlyphCacheDataSize();
-
- // Must unlock everything on termination, otherwise valgrind complains about memory leaks.
- discardableManager->unlockAndDeleteAll();
-}
-
DEF_TEST(SkRemoteGlyphCache_ReWriteGlyph, reporter) {
// Build proxy typeface on the client for initializing the cache.
sk_sp<DiscardableManager> discardableManager = sk_make_sp<DiscardableManager>();
