Store pointers to GrGlyph directly in BitmapTextBlob. This patch improves performance by avoiding hashmap lookups under normal use
BUG=skia:
Review URL: https://codereview.chromium.org/1087203004
diff --git a/src/gpu/GrAtlasTextContext.cpp b/src/gpu/GrAtlasTextContext.cpp
index 7b877a0..b5e8693 100644
--- a/src/gpu/GrAtlasTextContext.cpp
+++ b/src/gpu/GrAtlasTextContext.cpp
@@ -1164,8 +1164,10 @@
GrGlyph::PackedID packed,
int vx, int vy, GrColor color, GrFontScaler* scaler,
const SkIRect& clipRect) {
+ Run& run = blob->fRuns[runIndex];
if (!fCurrStrike) {
fCurrStrike = fContext->getBatchFontCache()->getStrike(scaler);
+ run.fStrike.reset(SkRef(fCurrStrike));
}
GrGlyph* glyph = fCurrStrike->getGlyph(packed, scaler);
@@ -1198,8 +1200,6 @@
return;
}
- Run& run = blob->fRuns[runIndex];
-
GrMaskFormat format = glyph->fMaskFormat;
PerSubRunInfo* subRun = &run.fSubRunInfo.back();
@@ -1218,7 +1218,7 @@
r.fBottom = r.fTop + SkIntToScalar(height);
subRun->fMaskFormat = format;
this->appendGlyphCommon(blob, &run, subRun, r, color, vertexStride, kA8_GrMaskFormat == format,
- packed);
+ glyph);
}
bool GrAtlasTextContext::dfAppendGlyph(BitmapTextBlob* blob, int runIndex,
@@ -1227,8 +1227,10 @@
GrFontScaler* scaler,
const SkIRect& clipRect,
SkScalar textRatio, const SkMatrix& viewMatrix) {
+ Run& run = blob->fRuns[runIndex];
if (!fCurrStrike) {
fCurrStrike = fContext->getBatchFontCache()->getStrike(scaler);
+ run.fStrike.reset(SkRef(fCurrStrike));
}
GrGlyph* glyph = fCurrStrike->getGlyph(packed, scaler);
@@ -1275,8 +1277,6 @@
return true;
}
- Run& run = blob->fRuns[runIndex];
-
PerSubRunInfo* subRun = &run.fSubRunInfo.back();
SkASSERT(glyph->fMaskFormat == kA8_GrMaskFormat);
subRun->fMaskFormat = kA8_GrMaskFormat;
@@ -1285,7 +1285,7 @@
bool useColorVerts = !subRun->fUseLCDText;
this->appendGlyphCommon(blob, &run, subRun, glyphRect, color, vertexStride, useColorVerts,
- packed);
+ glyph);
return true;
}
@@ -1308,8 +1308,8 @@
Run::SubRunInfo* subRun,
const SkRect& positions, GrColor color,
size_t vertexStride, bool useVertexColor,
- GrGlyph::PackedID packed) {
- blob->fGlyphIDs[subRun->fGlyphEndIndex] = packed;
+ GrGlyph* glyph) {
+ blob->fGlyphs[subRun->fGlyphEndIndex] = glyph;
run->fVertexBounds.joinNonEmptyArg(positions);
run->fColor = color;
@@ -1508,7 +1508,6 @@
const SkDescriptor* desc = NULL;
SkGlyphCache* cache = NULL;
GrFontScaler* scaler = NULL;
- GrBatchTextStrike* strike = NULL;
SkTypeface* typeface = NULL;
int instancesToFlush = 0;
@@ -1540,6 +1539,17 @@
if (regenerateTextureCoords || regenerateColors || regeneratePositions) {
// first regenerate texture coordinates / colors if need be
bool brokenRun = false;
+
+ // Because the GrBatchFontCache may evict the strike a blob depends on using for
+ // generating its texture coords, we have to track whether or not the strike has
+ // been abandoned. If it hasn't been abandoned, then we can use the GrGlyph*s as is
+ // otherwise we have to get the new strike, and use that to get the correct glyphs.
+ // Because we do not have the packed ids, and thus can't look up our glyphs in the
+ // new strike, we instead keep our ref to the old strike and use the packed ids from
+ // it. These ids will still be valid as long as we hold the ref. When we are done
+ // updating our cache of the GrGlyph*s, we drop our ref on the old strike
+ bool regenerateGlyphs = false;
+ GrBatchTextStrike* strike = NULL;
if (regenerateTextureCoords) {
info.fBulkUseToken.reset();
@@ -1556,16 +1566,30 @@
desc = newDesc;
cache = SkGlyphCache::DetachCache(run.fTypeface, desc);
scaler = GrTextContext::GetGrFontScaler(cache);
- strike = fFontCache->getStrike(scaler);
+ strike = run.fStrike;
typeface = run.fTypeface;
}
- }
- for (int glyphIdx = 0; glyphIdx < glyphCount; glyphIdx++) {
- GrGlyph::PackedID glyphID = blob->fGlyphIDs[glyphIdx + info.fGlyphStartIndex];
+ if (run.fStrike->isAbandoned()) {
+ regenerateGlyphs = true;
+ strike = fFontCache->getStrike(scaler);
+ } else {
+ strike = run.fStrike;
+ }
+ }
+
+ for (int glyphIdx = 0; glyphIdx < glyphCount; glyphIdx++) {
if (regenerateTextureCoords) {
- // Upload the glyph only if needed
- GrGlyph* glyph = strike->getGlyph(glyphID, scaler);
+ size_t glyphOffset = glyphIdx + info.fGlyphStartIndex;
+ GrGlyph* glyph;
+ if (regenerateGlyphs) {
+ // Get the id from the old glyph, and use the new strike to lookup
+ // the glyph.
+ glyph = blob->fGlyphs[glyphOffset];
+ blob->fGlyphs[glyphOffset] = strike->getGlyph(glyph->fPackedID,
+ scaler);
+ }
+ glyph = blob->fGlyphs[glyphOffset];
SkASSERT(glyph);
if (!fFontCache->hasGlyph(glyph) &&
@@ -1576,7 +1600,8 @@
instancesToFlush = 0;
brokenRun = glyphIdx > 0;
- SkDEBUGCODE(bool success =) strike->addGlyphToAtlas(batchTarget, glyph,
+ SkDEBUGCODE(bool success =) strike->addGlyphToAtlas(batchTarget,
+ glyph,
scaler);
SkASSERT(success);
}
@@ -1614,6 +1639,9 @@
// We my have changed the color so update it here
run.fColor = args.fColor;
if (regenerateTextureCoords) {
+ if (regenerateGlyphs) {
+ run.fStrike.reset(SkRef(strike));
+ }
info.fAtlasGeneration = brokenRun ? GrBatchAtlas::kInvalidAtlasGeneration :
fFontCache->atlasGeneration(fMaskFormat);
}
diff --git a/src/gpu/GrAtlasTextContext.h b/src/gpu/GrAtlasTextContext.h
index d5fa0d7..4c833b2 100644
--- a/src/gpu/GrAtlasTextContext.h
+++ b/src/gpu/GrAtlasTextContext.h
@@ -11,6 +11,7 @@
#include "GrTextContext.h"
#include "GrBatchAtlas.h"
+#include "GrBatchFontCache.h"
#include "GrGeometryProcessor.h"
#include "SkDescriptor.h"
#include "GrMemoryPool.h"
@@ -18,7 +19,6 @@
#include "SkTextBlob.h"
#include "SkTInternalLList.h"
-class GrBatchTextStrike;
class GrPipelineBuilder;
class GrTextBlobCache;
@@ -175,6 +175,7 @@
int fSubRunCount;
int fSubRunAllocation;
};
+ SkAutoTUnref<GrBatchTextStrike> fStrike;
SkAutoTUnref<SkTypeface> fTypeface;
SkRect fVertexBounds;
SubRunInfoArray fSubRunInfo;
@@ -222,7 +223,7 @@
// all glyph / vertex offsets are into these pools.
unsigned char* fVertices;
- GrGlyph::PackedID* fGlyphIDs;
+ GrGlyph** fGlyphs;
Run* fRuns;
GrMemoryPool* fPool;
SkMaskFilter::BlurRec fBlurRec;
@@ -287,7 +288,7 @@
inline void appendGlyphCommon(BitmapTextBlob*, Run*, Run::SubRunInfo*,
const SkRect& positions, GrColor color,
size_t vertexStride, bool useVertexColor,
- GrGlyph::PackedID);
+ GrGlyph*);
inline void flushRunAsPaths(const SkTextBlob::RunIterator&, const SkPaint&, SkDrawFilter*,
const SkMatrix& viewMatrix, const SkIRect& clipBounds, SkScalar x,
diff --git a/src/gpu/GrBatchFontCache.cpp b/src/gpu/GrBatchFontCache.cpp
index 653649a..7e44fa5 100644
--- a/src/gpu/GrBatchFontCache.cpp
+++ b/src/gpu/GrBatchFontCache.cpp
@@ -74,7 +74,7 @@
GrBatchFontCache::~GrBatchFontCache() {
SkTDynamicHash<GrBatchTextStrike, GrFontDescKey>::Iter iter(&fCache);
while (!iter.done()) {
- SkDELETE(&(*iter));
+ (*iter).unref();
++iter;
}
for (int i = 0; i < kMaskFormatCount; ++i) {
@@ -85,7 +85,7 @@
void GrBatchFontCache::freeAll() {
SkTDynamicHash<GrBatchTextStrike, GrFontDescKey>::Iter iter(&fCache);
while (!iter.done()) {
- SkDELETE(&(*iter));
+ (*iter).unref();
++iter;
}
fCache.rewind();
@@ -118,7 +118,8 @@
// triggered the eviction
if (strike != fontCache->fPreserveStrike && 0 == strike->fAtlasedGlyphs) {
fontCache->fCache.remove(*(strike->fFontScalerKey));
- SkDELETE(strike);
+ strike->fIsAbandoned = true;
+ strike->unref();
}
}
}
@@ -155,7 +156,8 @@
GrBatchTextStrike::GrBatchTextStrike(GrBatchFontCache* cache, const GrFontDescKey* key)
: fFontScalerKey(SkRef(key))
, fPool(9/*start allocations at 512 bytes*/)
- , fAtlasedGlyphs(0) {
+ , fAtlasedGlyphs(0)
+ , fIsAbandoned(false) {
fBatchFontCache = cache; // no need to ref, it won't go away before we do
}
diff --git a/src/gpu/GrBatchFontCache.h b/src/gpu/GrBatchFontCache.h
index 6aa14e6..f60e2c8 100644
--- a/src/gpu/GrBatchFontCache.h
+++ b/src/gpu/GrBatchFontCache.h
@@ -24,7 +24,7 @@
* is abstracted by GrGlyph, and indexed by a PackedID and GrFontScaler. The GrFontScaler is what
* actually creates the mask.
*/
-class GrBatchTextStrike {
+class GrBatchTextStrike : public SkNVRefCnt<GrBatchTextStrike> {
public:
GrBatchTextStrike(GrBatchFontCache*, const GrFontDescKey* fontScalerKey);
~GrBatchTextStrike();
@@ -52,6 +52,9 @@
// remove any references to this plot
void removeID(GrBatchAtlas::AtlasID);
+ // If a TextStrike is abandoned by the cache, then the caller must get a new strike
+ bool isAbandoned() const { return fIsAbandoned; }
+
static const GrFontDescKey& GetKey(const GrBatchTextStrike& ts) {
return *(ts.fFontScalerKey);
}
@@ -66,6 +69,7 @@
GrBatchFontCache* fBatchFontCache;
int fAtlasedGlyphs;
+ bool fIsAbandoned;
GrGlyph* generateGlyph(GrGlyph::PackedID packed, GrFontScaler* scaler);
@@ -84,7 +88,10 @@
public:
GrBatchFontCache(GrContext*);
~GrBatchFontCache();
-
+ // The user of the cache may hold a long-lived ref to the returned strike. However, actions by
+ // another client of the cache may cause the strike to be purged while it is still reffed.
+ // Therefore, the caller must check GrBatchTextStrike::isAbandoned() if there are other
+ // interactions with the cache since the strike was received.
inline GrBatchTextStrike* getStrike(GrFontScaler* scaler) {
GrBatchTextStrike* strike = fCache.find(*(scaler->getKey()));
if (NULL == strike) {
diff --git a/src/gpu/GrTextBlobCache.cpp b/src/gpu/GrTextBlobCache.cpp
index d32e24b..d2153bd 100644
--- a/src/gpu/GrTextBlobCache.cpp
+++ b/src/gpu/GrTextBlobCache.cpp
@@ -20,16 +20,15 @@
size_t verticesCount = glyphCount * kVerticesPerGlyph * maxVASize;
size_t size = sizeof(BitmapTextBlob) +
verticesCount +
- glyphCount * sizeof(GrGlyph::PackedID) +
+ glyphCount * sizeof(GrGlyph**) +
sizeof(BitmapTextBlob::Run) * runCount;
BitmapTextBlob* cacheBlob = SkNEW_PLACEMENT(fPool.allocate(size), BitmapTextBlob);
// setup offsets for vertices / glyphs
cacheBlob->fVertices = sizeof(BitmapTextBlob) + reinterpret_cast<unsigned char*>(cacheBlob);
- cacheBlob->fGlyphIDs =
- reinterpret_cast<GrGlyph::PackedID*>(cacheBlob->fVertices + verticesCount);
- cacheBlob->fRuns = reinterpret_cast<BitmapTextBlob::Run*>(cacheBlob->fGlyphIDs + glyphCount);
+ cacheBlob->fGlyphs = reinterpret_cast<GrGlyph**>(cacheBlob->fVertices + verticesCount);
+ cacheBlob->fRuns = reinterpret_cast<BitmapTextBlob::Run*>(cacheBlob->fGlyphs + glyphCount);
// Initialize runs
for (int i = 0; i < runCount; i++) {