From 94c66475560de551a499334818df85966db7681c Mon Sep 17 00:00:00 2001 From: Khushal Sagar Date: Tue, 3 Sep 2019 00:30:28 +0000 Subject: [PATCH] Revert "Only send the RemoteStrikes that have pending glyphs." This reverts commit 3783375c4d418e8fc4ccdc7d1b185f8407fc7824. Reason for revert: Glyph cache miss issues are back with this change (landed in 78.0.3898.0) : https://crash.corp.google.com/browse?q=product_name%3D%22Chrome%22+AND+expanded_custom_data.ChromeCrashProto.channel+IN+%28%22canary%22%29+AND+expanded_custom_data.ChromeCrashProto.magic_signature_1.name%3D%22%5BDump+without+crash%5D+SkScalerContextProxy%3A%3AgenerateMetrics%22 Original change's description: > Only send the RemoteStrikes that have pending glyphs. > > Count the number of strikes in fRemoteStrikesToSend that have > glyphs to send. The strike may not have glyphs to send, because they > were sent previously. Then, send the strikes that have glyphs to send. > Keep the bool in the serialization which indicates that strike > has glyphs and should always be true now. The bool will be removed in the next CL. > > Change-Id: I8b75d1bda574fd71adfd21cb20ff912340fc2e33 > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/237815 > Commit-Queue: Herb Derby > Reviewed-by: Khushal Sagar TBR=herb@google.com,khushalsagar@chromium.org,khushalsagar@google.com # Not skipping CQ checks because original CL landed > 1 day ago. Change-Id: I910734d9db570784b73e108d7b5abedd2efc5ad7 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/238778 Reviewed-by: Khushal Sagar Commit-Queue: Khushal Sagar --- src/core/SkRemoteGlyphCache.cpp | 38 +++++++++++---------------------- 1 file changed, 12 insertions(+), 26 deletions(-) diff --git a/src/core/SkRemoteGlyphCache.cpp b/src/core/SkRemoteGlyphCache.cpp index c7b331f5844be..b3af9b7c1740e 100644 --- a/src/core/SkRemoteGlyphCache.cpp +++ b/src/core/SkRemoteGlyphCache.cpp @@ -221,16 +221,14 @@ class SkStrikeServer::RemoteStrike : public SkStrikeInterface { 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 fCachedGlyphImages; @@ -427,18 +425,7 @@ sk_sp SkStrikeServer::serializeTypeface(SkTypeface* tf) { } void SkStrikeServer::writeStrikeData(std::vector* memory) { - size_t strikesToSend = 0; - fRemoteStrikesToSend.foreach( - [&strikesToSend](RemoteStrike* strike) { - if (strike->hasPendingGlyphs()) { - strikesToSend++; - } else { - strike->resetScalerContext(); - } - } - ); - - if (strikesToSend == 0 && fTypefacesToSend.empty()) { + if (fRemoteStrikesToSend.empty() && fTypefacesToSend.empty()) { return; } @@ -449,14 +436,11 @@ void SkStrikeServer::writeStrikeData(std::vector* memory) { } fTypefacesToSend.clear(); - serializer.emplace(SkTo(strikesToSend)); + serializer.emplace(SkTo(fRemoteStrikesToSend.count())); fRemoteStrikesToSend.foreach( #ifdef SK_DEBUG [&serializer, this](RemoteStrike* strike) { - if (strike->hasPendingGlyphs()) { - strike->writePendingGlyphs(&serializer); - strike->resetScalerContext(); - } + strike->writePendingGlyphs(&serializer); auto it = fDescToRemoteStrike.find(&strike->getDescriptor()); SkASSERT(it != fDescToRemoteStrike.end()); SkASSERT(it->second.get() == strike); @@ -464,10 +448,7 @@ void SkStrikeServer::writeStrikeData(std::vector* memory) { #else [&serializer](RemoteStrike* strike) { - if (strike->hasPendingGlyphs()) { - strike->writePendingGlyphs(&serializer); - strike->resetScalerContext(); - } + strike->writePendingGlyphs(&serializer); } #endif ); @@ -588,8 +569,12 @@ static void writeGlyph(SkGlyph* glyph, Serializer* serializer) { } void SkStrikeServer::RemoteStrike::writePendingGlyphs(Serializer* serializer) { + // TODO(khushalsagar): Write a strike only if it has any pending glyphs. serializer->emplace(this->hasPendingGlyphs()); - SkASSERT(this->hasPendingGlyphs()); + if (!this->hasPendingGlyphs()) { + this->resetScalerContext(); + return; + } // Write the desc. serializer->emplace(fContext->getTypeface()->uniqueID(), fDiscardableHandleId); @@ -633,6 +618,7 @@ void SkStrikeServer::RemoteStrike::writePendingGlyphs(Serializer* serializer) { writeGlyphPath(glyphID, serializer); } fPendingGlyphPaths.clear(); + this->resetScalerContext(); } void SkStrikeServer::RemoteStrike::ensureScalerContext() {