Revert "Don't send strike with no pending glyphs"
This reverts commit 9e081d164c816bdf6df9c1f11b6928dab08c753c.
Reason for revert: Spike in cache misses: chromium:996636
BUG=chromium:996636
Original change's description:
> Don't send strike with no pending glyphs
>
> This changes tracks the strikes to send as a set of pointers
> to SkGlyphCacheState instead of as descriptors. This removes a descriptor
> lookup allowing rapid calculation of the actual number of strikes to send.
> Knowing the number of strikes with changes allows us to remove sending a boolean
> if strike has no pending glyphs.
>
> Change-Id: Id173336e238daebc19564248a818eb9dbdbe6db6
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/235827
> Reviewed-by: Mike Klein <mtklein@google.com>
> Reviewed-by: Khushal Sagar <khushalsagar@chromium.org>
> Commit-Queue: Herb Derby <herb@google.com>
TBR=mtklein@google.com,herb@google.com,khushalsagar@chromium.org,khushalsagar@google.com
# Not skipping CQ checks because original CL landed > 1 day ago.
Change-Id: I0e0c8877be8d08be4b62a029005889068b6e4cc3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/236351
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: Herb Derby <herb@google.com>
diff --git a/src/core/SkRemoteGlyphCache.cpp b/src/core/SkRemoteGlyphCache.cpp
index bf3fbd4..2e973d0 100644
--- a/src/core/SkRemoteGlyphCache.cpp
+++ b/src/core/SkRemoteGlyphCache.cpp
@@ -77,7 +77,7 @@
class Serializer {
public:
- explicit Serializer(std::vector<uint8_t>* buffer) : fBuffer{buffer} { }
+ Serializer(std::vector<uint8_t>* buffer) : fBuffer{buffer} { }
template <typename T, typename... Args>
T* emplace(Args&&... args) {
@@ -229,16 +229,14 @@
void onAboutToExitScope() override {}
+private:
bool hasPendingGlyphs() const {
return !fPendingGlyphImages.empty() || !fPendingGlyphPaths.empty();
}
-
- void resetScalerContext();
-
-private:
void writeGlyphPath(const SkPackedGlyphID& glyphID, Serializer* serializer) const;
void ensureScalerContext();
+ void resetScalerContext();
// The set of glyphs cached on the remote client.
SkTHashSet<SkPackedGlyphID> fCachedGlyphImages;
@@ -360,6 +358,9 @@
};
// -- SkTextBlobCacheDiffCanvas -------------------------------------------------------------------
+// DEPRECATED
+// TODO(herb): remove uses in Chrome
+
SkTextBlobCacheDiffCanvas::SkTextBlobCacheDiffCanvas(int width, int height,
const SkSurfaceProps& props,
SkStrikeServer* strikeServer,
@@ -427,34 +428,22 @@
}
void SkStrikeServer::writeStrikeData(std::vector<uint8_t>* memory) {
- size_t countStrikesToSend = 0;
- for (SkGlyphCacheState* strike : fStrikesToSend) {
- if (strike->hasPendingGlyphs()) {
- // Only send strikes with pending glyphs.
- countStrikesToSend++;
- } else {
- strike->resetScalerContext();
- }
- }
-
- if (fTypefacesToSend.empty() && countStrikesToSend == 0) {
+ if (fLockedDescs.empty() && fTypefacesToSend.empty()) {
return;
}
Serializer serializer(memory);
serializer.emplace<uint64_t>(fTypefacesToSend.size());
- for (const WireTypeface& tf : fTypefacesToSend) {
- serializer.write<WireTypeface>(tf);
- }
+ for (const auto& tf : fTypefacesToSend) serializer.write<WireTypeface>(tf);
fTypefacesToSend.clear();
- serializer.emplace<uint64_t>(countStrikesToSend);
- for (SkGlyphCacheState* strike : fStrikesToSend) {
- if (strike->hasPendingGlyphs()) {
- strike->writePendingGlyphs(&serializer);
- }
+ serializer.emplace<uint64_t>(fLockedDescs.size());
+ for (const auto* desc : fLockedDescs) {
+ auto it = fRemoteGlyphStateMap.find(desc);
+ SkASSERT(it != fRemoteGlyphStateMap.end());
+ it->second->writePendingGlyphs(&serializer);
}
- fStrikesToSend.clear();
+ fLockedDescs.clear();
}
SkStrikeServer::SkGlyphCacheState* SkStrikeServer::getOrCreateCache(
@@ -510,26 +499,29 @@
)
);
+ // Already locked.
+ if (fLockedDescs.find(&desc) != fLockedDescs.end()) {
+ auto it = fRemoteGlyphStateMap.find(&desc);
+ SkASSERT(it != fRemoteGlyphStateMap.end());
+ SkGlyphCacheState* cache = it->second.get();
+ cache->setTypefaceAndEffects(&typeface, effects);
+ return cache;
+ }
+
// Try to lock.
auto it = fRemoteGlyphStateMap.find(&desc);
if (it != fRemoteGlyphStateMap.end()) {
SkGlyphCacheState* cache = it->second.get();
- cache->setTypefaceAndEffects(&typeface, effects);
- bool inserted;
- std::tie(std::ignore, inserted) = fStrikesToSend.insert(cache);
- if (!inserted) {
- // Already existed in fStrikesToSend.
+ bool locked = fDiscardableHandleManager->lockHandle(it->second->discardableHandleId());
+ if (locked) {
+ fLockedDescs.insert(it->first);
+ cache->setTypefaceAndEffects(&typeface, effects);
return cache;
- } else {
- bool locked = fDiscardableHandleManager->lockHandle(cache->discardableHandleId());
- if (locked) {
- return cache;
- }
- // If the lock failed, the entry was deleted on the client. Remove our
- // tracking.
- fStrikesToSend.erase(cache);
- fRemoteGlyphStateMap.erase(it);
}
+
+ // If the lock failed, the entry was deleted on the client. Remove our
+ // tracking.
+ fRemoteGlyphStateMap.erase(it);
}
const SkFontID typefaceId = typeface.uniqueID();
@@ -548,7 +540,7 @@
auto* cacheStatePtr = cacheState.get();
- fStrikesToSend.insert(cacheStatePtr);
+ fLockedDescs.insert(&cacheStatePtr->getDescriptor());
fRemoteGlyphStateMap[&cacheStatePtr->getDescriptor()] = std::move(cacheState);
checkForDeletedEntries();
@@ -571,6 +563,13 @@
}
void SkStrikeServer::SkGlyphCacheState::writePendingGlyphs(Serializer* serializer) {
+ // TODO(khushalsagar): Write a strike only if it has any pending glyphs.
+ serializer->emplace<bool>(this->hasPendingGlyphs());
+ if (!this->hasPendingGlyphs()) {
+ this->resetScalerContext();
+ return;
+ }
+
// Write the desc.
serializer->emplace<StrikeSpec>(fContext->getTypeface()->uniqueID(), fDiscardableHandleId);
serializer->writeDescriptor(*fDescriptor.getDesc());
@@ -782,6 +781,11 @@
if (!deserializer.read<uint64_t>(&strikeCount)) READ_FAILURE
for (size_t i = 0; i < strikeCount; ++i) {
+ bool has_glyphs = false;
+ if (!deserializer.read<bool>(&has_glyphs)) READ_FAILURE
+
+ if (!has_glyphs) continue;
+
StrikeSpec spec;
if (!deserializer.read<StrikeSpec>(&spec)) READ_FAILURE
diff --git a/src/core/SkRemoteGlyphCache.h b/src/core/SkRemoteGlyphCache.h
index 385255c..2f1bac9 100644
--- a/src/core/SkRemoteGlyphCache.h
+++ b/src/core/SkRemoteGlyphCache.h
@@ -47,6 +47,9 @@
using SkDescriptorMap = std::unordered_map<const SkDescriptor*, T, SkDescriptorMapOperators,
SkDescriptorMapOperators>;
+using SkDescriptorSet =
+ std::unordered_set<const SkDescriptor*, SkDescriptorMapOperators, SkDescriptorMapOperators>;
+
// A SkTextBlobCacheDiffCanvas is used to populate the SkStrikeServer with ops
// which will be serialized and rendered using the SkStrikeClient.
class SkTextBlobCacheDiffCanvas : public SkNoDrawCanvas {
@@ -152,7 +155,7 @@
SkTHashMap<SkFontID, sk_sp<SkData>> fSerializedTypefaces;
// State cached until the next serialization.
- std::unordered_set<SkGlyphCacheState*> fStrikesToSend;
+ SkDescriptorSet fLockedDescs;
std::vector<WireTypeface> fTypefacesToSend;
};
