Store pointers to GrGlyph directly in BitmapTextBlob

src/gpu/GrAtlasTextContext.cpp

Index: src/gpu/GrAtlasTextContext.cpp
diff --git a/src/gpu/GrAtlasTextContext.cpp b/src/gpu/GrAtlasTextContext.cpp
index 5fbb6a998360205a3de7e5666f4c39ec7682240d..8591ec6e4f177b1f0b71ed7e881d2e501c639f91 100644
--- a/src/gpu/GrAtlasTextContext.cpp
+++ b/src/gpu/GrAtlasTextContext.cpp
@@ -1165,8 +1165,10 @@ void GrAtlasTextContext::bmpAppendGlyph(BitmapTextBlob* blob, int runIndex,
                                         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);
@@ -1199,8 +1201,6 @@ void GrAtlasTextContext::bmpAppendGlyph(BitmapTextBlob* blob, int runIndex,
         return;
     }
 
-    Run& run = blob->fRuns[runIndex];
-
     GrMaskFormat format = glyph->fMaskFormat;
 
     PerSubRunInfo* subRun = &run.fSubRunInfo.back();
@@ -1219,7 +1219,7 @@ void GrAtlasTextContext::bmpAppendGlyph(BitmapTextBlob* blob, int runIndex,
     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,
@@ -1228,8 +1228,10 @@ bool GrAtlasTextContext::dfAppendGlyph(BitmapTextBlob* blob, int runIndex,
                                        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);
@@ -1276,8 +1278,6 @@ bool GrAtlasTextContext::dfAppendGlyph(BitmapTextBlob* blob, int runIndex,
         return true;
     }
 
-    Run& run = blob->fRuns[runIndex];
-
     PerSubRunInfo* subRun = &run.fSubRunInfo.back();
     SkASSERT(glyph->fMaskFormat == kA8_GrMaskFormat);
     subRun->fMaskFormat = kA8_GrMaskFormat;
@@ -1286,7 +1286,7 @@ bool GrAtlasTextContext::dfAppendGlyph(BitmapTextBlob* blob, int runIndex,
 
     bool useColorVerts = !subRun->fUseLCDText;
     this->appendGlyphCommon(blob, &run, subRun, glyphRect, color, vertexStride, useColorVerts,
-                            packed);
+                            glyph);
     return true;
 }
 
@@ -1309,8 +1309,8 @@ inline void GrAtlasTextContext::appendGlyphCommon(BitmapTextBlob* blob, Run* run
                                                   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;
 
@@ -1509,7 +1509,6 @@ public:
         const SkDescriptor* desc = NULL;
         SkGlyphCache* cache = NULL;
         GrFontScaler* scaler = NULL;
-        GrBatchTextStrike* strike = NULL;
         SkTypeface* typeface = NULL;
 
         int instancesToFlush = 0;
@@ -1541,6 +1540,17 @@ public:
             if (regenerateTextureCoords || regenerateColors || regeneratePositions) {
                 // first regenerate texture coordinates / colors if need be
                 bool brokenRun = false;
+

[bsalomon, 2015/04/21 12:32:17]

Probably should have a parallel (but shorter) comment in the cache header where
it returns the grbatchtextstrike. Something like, "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 the cache since the strike was received."

+                // 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();
 
@@ -1557,16 +1567,30 @@ public:
                         desc = newDesc;
                         cache = SkGlyphCache::DetachCache(run.fTypeface, desc);
                         scaler = GrTextContext::GetGrFontScaler(cache);
-                        strike = fFontCache->getStrike(scaler);
+                        strike = run.fStrike;
                         typeface = run.fTypeface;
                     }
+
+                    if (run.fStrike->isAbandoned()) {
+                        regenerateGlyphs = true;
+                        strike = fFontCache->getStrike(scaler);
+                    } else {
+                        strike = run.fStrike;
+                    }
                 }
-                for (int glyphIdx = 0; glyphIdx < glyphCount; glyphIdx++) {
-                    GrGlyph::PackedID glyphID = blob->fGlyphIDs[glyphIdx + info.fGlyphStartIndex];
 
+                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) &&
@@ -1577,7 +1601,8 @@ public:
                             instancesToFlush = 0;
                             brokenRun = glyphIdx > 0;
 
-                            SkDEBUGCODE(bool success =) strike->addGlyphToAtlas(batchTarget, glyph,
+                            SkDEBUGCODE(bool success =) strike->addGlyphToAtlas(batchTarget,
+                                                                                glyph,
                                                                                 scaler);
                             SkASSERT(success);
                         }
@@ -1615,6 +1640,9 @@ public:
                 // 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);
                 }