Attempt to improve lifetime management of SkGlyphCache in Ganesh atlas text code.
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1983353003
Review-Url: https://codereview.chromium.org/1983353003
diff --git a/src/core/SkDescriptor.h b/src/core/SkDescriptor.h
index 7360219..71f7133 100644
--- a/src/core/SkDescriptor.h
+++ b/src/core/SkDescriptor.h
@@ -102,6 +102,7 @@
} while (aa < stop);
return true;
}
+ bool operator!=(const SkDescriptor& other) const { return !(*this == other); }
uint32_t getChecksum() const { return fChecksum; }
diff --git a/src/core/SkGlyphCache.h b/src/core/SkGlyphCache.h
index fd3ef66..2a96370 100644
--- a/src/core/SkGlyphCache.h
+++ b/src/core/SkGlyphCache.h
@@ -277,7 +277,7 @@
public:
/** deprecated: use get() */
SkGlyphCache* getCache() const { return this->get(); }
-
+ SkAutoGlyphCache() = default;
SkAutoGlyphCache(SkGlyphCache* cache) : INHERITED(cache) {}
SkAutoGlyphCache(SkTypeface* typeface, const SkScalerContextEffects& effects,
const SkDescriptor* desc)
diff --git a/src/gpu/batches/GrAtlasTextBatch.cpp b/src/gpu/batches/GrAtlasTextBatch.cpp
index 2da4938..5a41457 100644
--- a/src/gpu/batches/GrAtlasTextBatch.cpp
+++ b/src/gpu/batches/GrAtlasTextBatch.cpp
@@ -132,23 +132,17 @@
unsigned char* currVertex = reinterpret_cast<unsigned char*>(vertices);
- // We cache some values to avoid going to the glyphcache for the same fontScaler twice
- // in a row
- const SkDescriptor* desc = nullptr;
- SkGlyphCache* cache = nullptr;
- SkTypeface* typeface = nullptr;
-
GrBlobRegenHelper helper(this, target, &flushInfo);
-
+ SkAutoGlyphCache glyphCache;
for (int i = 0; i < fGeoCount; i++) {
const Geometry& args = fGeoData[i];
Blob* blob = args.fBlob;
size_t byteCount;
void* blobVertices;
int subRunGlyphCount;
- blob->regenInBatch(target, fFontCache, &helper, args.fRun, args.fSubRun, &cache,
- &typeface, &desc, vertexStride, args.fViewMatrix, args.fX,
- args.fY, args.fColor, &blobVertices, &byteCount, &subRunGlyphCount);
+ blob->regenInBatch(target, fFontCache, &helper, args.fRun, args.fSubRun, &glyphCache,
+ vertexStride, args.fViewMatrix, args.fX, args.fY, args.fColor,
+ &blobVertices, &byteCount, &subRunGlyphCount);
// now copy all vertices
memcpy(currVertex, blobVertices, byteCount);
@@ -172,10 +166,6 @@
currVertex += byteCount;
}
- // Make sure to attach the last cache if applicable
- if (cache) {
- SkGlyphCache::AttachCache(cache);
- }
this->flush(target, &flushInfo);
}
diff --git a/src/gpu/text/GrAtlasTextBlob.h b/src/gpu/text/GrAtlasTextBlob.h
index 788eaba..d9083b2 100644
--- a/src/gpu/text/GrAtlasTextBlob.h
+++ b/src/gpu/text/GrAtlasTextBlob.h
@@ -256,12 +256,15 @@
this->setupViewMatrix(viewMatrix, x, y);
}
+ /**
+ * Consecutive calls to regenInBatch often use the same SkGlyphCache. If the same instance of
+ * SkAutoGlyphCache is passed to multiple calls of regenInBatch then it can save the cost of
+ * multiple detach/attach operations of SkGlyphCache.
+ */
void regenInBatch(GrDrawBatch::Target* target, GrBatchFontCache* fontCache,
- GrBlobRegenHelper *helper, int run, int subRun, SkGlyphCache** cache,
- SkTypeface** typeface, const SkDescriptor** desc, size_t vertexStride,
- const SkMatrix& viewMatrix, SkScalar x, SkScalar y,
- GrColor color,
- void** vertices, size_t* byteCount, int* glyphCount);
+ GrBlobRegenHelper *helper, int run, int subRun, SkAutoGlyphCache*,
+ size_t vertexStride, const SkMatrix& viewMatrix, SkScalar x, SkScalar y,
+ GrColor color, void** vertices, size_t* byteCount, int* glyphCount);
const Key& key() const { return fKey; }
@@ -489,9 +492,9 @@
void regenInBatch(GrDrawBatch::Target* target,
GrBatchFontCache* fontCache,
GrBlobRegenHelper* helper,
- Run* run, Run::SubRunInfo* info, SkGlyphCache** cache,
- SkTypeface** typeface, const SkDescriptor** desc,
- int glyphCount, size_t vertexStride,
+ Run* run, Run::SubRunInfo* info,
+ SkAutoGlyphCache*, int glyphCount,
+ size_t vertexStride,
GrColor color, SkScalar transX,
SkScalar transY) const;
diff --git a/src/gpu/text/GrAtlasTextBlob_regenInBatch.cpp b/src/gpu/text/GrAtlasTextBlob_regenInBatch.cpp
index e5a8956..28d3219 100644
--- a/src/gpu/text/GrAtlasTextBlob_regenInBatch.cpp
+++ b/src/gpu/text/GrAtlasTextBlob_regenInBatch.cpp
@@ -142,35 +142,27 @@
GrBatchFontCache* fontCache,
GrBlobRegenHelper *helper,
Run* run,
- Run::SubRunInfo* info, SkGlyphCache** cache,
- SkTypeface** typeface,
- const SkDescriptor** desc,
+ Run::SubRunInfo* info,
+ SkAutoGlyphCache* lazyCache,
int glyphCount, size_t vertexStride,
GrColor color, SkScalar transX,
SkScalar transY) const {
+ SkASSERT(lazyCache);
static_assert(!regenGlyphs || regenTexCoords, "must regenTexCoords along regenGlyphs");
GrBatchTextStrike* strike = nullptr;
if (regenTexCoords) {
info->resetBulkUseToken();
- // We can reuse if we have a valid strike and our descriptors / typeface are the
- // same. The override descriptor is only for the non distance field text within
- // a run
- const SkDescriptor* newDesc = (run->fOverrideDescriptor && !info->drawAsDistanceFields()) ?
- run->fOverrideDescriptor->getDesc() :
- run->fDescriptor.getDesc();
- if (!*cache || !SkTypeface::Equal(*typeface, run->fTypeface) ||
- !(**desc == *newDesc)) {
- if (*cache) {
- SkGlyphCache::AttachCache(*cache);
- }
- *desc = newDesc;
- *cache = SkGlyphCache::DetachCache(run->fTypeface, run->fEffects, *desc);
- *typeface = run->fTypeface;
+ const SkDescriptor* desc = (run->fOverrideDescriptor && !info->drawAsDistanceFields())
+ ? run->fOverrideDescriptor->getDesc()
+ : run->fDescriptor.getDesc();
+
+ if (!*lazyCache || (*lazyCache)->getDescriptor() != *desc) {
+ lazyCache->reset(SkGlyphCache::DetachCache(run->fTypeface, run->fEffects, desc));
}
if (regenGlyphs) {
- strike = fontCache->getStrike(*cache);
+ strike = fontCache->getStrike(lazyCache->get());
} else {
strike = info->strike();
}
@@ -187,20 +179,20 @@
// Get the id from the old glyph, and use the new strike to lookup
// the glyph.
GrGlyph::PackedID id = fGlyphs[glyphOffset]->fPackedID;
- fGlyphs[glyphOffset] = strike->getGlyph(id, info->maskFormat(), *cache);
+ fGlyphs[glyphOffset] = strike->getGlyph(id, info->maskFormat(), lazyCache->get());
SkASSERT(id == fGlyphs[glyphOffset]->fPackedID);
}
glyph = fGlyphs[glyphOffset];
SkASSERT(glyph && glyph->fMaskFormat == info->maskFormat());
if (!fontCache->hasGlyph(glyph) &&
- !strike->addGlyphToAtlas(target, glyph, *cache, info->maskFormat())) {
+ !strike->addGlyphToAtlas(target, glyph, lazyCache->get(), info->maskFormat())) {
helper->flush();
brokenRun = glyphIdx > 0;
SkDEBUGCODE(bool success =) strike->addGlyphToAtlas(target,
glyph,
- *cache,
+ lazyCache->get(),
info->maskFormat());
SkASSERT(success);
}
@@ -238,7 +230,7 @@
kRegenGlyph = 0x8 | kRegenTex, // we have to regenerate the texture coords when we regen glyphs
// combinations
- kRegenPosCol = kRegenPos | kRegenCol,
+ kRegenPosCol = kRegenPos | kRegenCol,
kRegenPosTex = kRegenPos | kRegenTex,
kRegenPosTexGlyph = kRegenPos | kRegenGlyph,
kRegenPosColTex = kRegenPos | kRegenCol | kRegenTex,
@@ -247,14 +239,13 @@
kRegenColTexGlyph = kRegenCol | kRegenGlyph,
};
-#define REGEN_ARGS target, fontCache, helper, &run, &info, cache, typeface, desc, \
+#define REGEN_ARGS target, fontCache, helper, &run, &info, lazyCache, \
*glyphCount, vertexStride, color, transX, transY
void GrAtlasTextBlob::regenInBatch(GrDrawBatch::Target* target,
GrBatchFontCache* fontCache,
GrBlobRegenHelper *helper,
- int runIndex, int subRunIndex, SkGlyphCache** cache,
- SkTypeface** typeface, const SkDescriptor** desc,
+ int runIndex, int subRunIndex, SkAutoGlyphCache* lazyCache,
size_t vertexStride, const SkMatrix& viewMatrix,
SkScalar x, SkScalar y, GrColor color,
void** vertices, size_t* byteCount, int* glyphCount) {