Skip to content

Commit

Permalink
Revert "Only send the RemoteStrikes that have pending glyphs."
Browse files Browse the repository at this point in the history
This reverts commit 3783375.

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 <herb@google.com>
> Reviewed-by: Khushal Sagar <khushalsagar@chromium.org>

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 <khushalsagar@chromium.org>
Commit-Queue: Khushal Sagar <khushalsagar@chromium.org>
  • Loading branch information
khushalsagar authored and Skia Commit-Bot committed Sep 3, 2019
1 parent 8203a18 commit 94c6647
Showing 1 changed file with 12 additions and 26 deletions.
38 changes: 12 additions & 26 deletions src/core/SkRemoteGlyphCache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<SkPackedGlyphID> fCachedGlyphImages;
Expand Down Expand Up @@ -427,18 +425,7 @@ sk_sp<SkData> SkStrikeServer::serializeTypeface(SkTypeface* tf) {
}

void SkStrikeServer::writeStrikeData(std::vector<uint8_t>* 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;
}

Expand All @@ -449,25 +436,19 @@ void SkStrikeServer::writeStrikeData(std::vector<uint8_t>* memory) {
}
fTypefacesToSend.clear();

serializer.emplace<uint64_t>(SkTo<uint64_t>(strikesToSend));
serializer.emplace<uint64_t>(SkTo<uint64_t>(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);
}

#else
[&serializer](RemoteStrike* strike) {
if (strike->hasPendingGlyphs()) {
strike->writePendingGlyphs(&serializer);
strike->resetScalerContext();
}
strike->writePendingGlyphs(&serializer);
}
#endif
);
Expand Down Expand Up @@ -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<bool>(this->hasPendingGlyphs());
SkASSERT(this->hasPendingGlyphs());
if (!this->hasPendingGlyphs()) {
this->resetScalerContext();
return;
}

// Write the desc.
serializer->emplace<StrikeSpec>(fContext->getTypeface()->uniqueID(), fDiscardableHandleId);
Expand Down Expand Up @@ -633,6 +618,7 @@ void SkStrikeServer::RemoteStrike::writePendingGlyphs(Serializer* serializer) {
writeGlyphPath(glyphID, serializer);
}
fPendingGlyphPaths.clear();
this->resetScalerContext();
}

void SkStrikeServer::RemoteStrike::ensureScalerContext() {
Expand Down

0 comments on commit 94c6647

Please sign in to comment.