Attempt to improve lifetime management of SkGlyphCache in Ganesh atlas text code.

src/gpu/text/GrAtlasTextBlob_regenInBatch.cpp

 /*
  * Copyright 2016 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "GrAtlasTextBlob.h"
 
 #include "GrBatchFlushState.h"
@@ skipping 119 matching lines @@
         *vcolor = color;
     }
 
     if (regenTexCoords) {
         uint16_t* textureCoords = reinterpret_cast<uint16_t*>(vertex + texCoordOffset);
         textureCoords[0] = (uint16_t)u1;
         textureCoords[1] = (uint16_t)v0;
     }
 }
 
+void GrAtlasTextBlob::LazyGlyphCache::initIfNecessary(Run* run, Run::SubRunInfo* info) {
+    // 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 a run
+    const SkDescriptor* newDesc = (run->fOverrideDescriptor && !info->drawAsDistanceFields())
+                                  ? run->fOverrideDescriptor->getDesc()
+                                  : run->fDescriptor.getDesc();
+    if (!fCache || !SkTypeface::Equal(fTypeface, run->fTypeface) ||

[bungeman-skia, 2016/05/18 15:33:07]

if instead of using fTypeface here, we just used
fCache->getScalerContext()->getTypeface() would that be enough? The one issue I
can see is that fCache->getScalerContext()->getTypeface() is always resolved (if
you pass in nullptr this will be the default font). If run->fTypeface is ever
nullptr then this may not be equivalent, I'm not sure if that's allowed or not.
If it does work it means fTypeface isn't needed.

+        fCache->getDescriptor() != *newDesc) {

[bungeman-skia, 2016/05/18 15:33:07]

nit: I know it isn't a documented style, but it would be nice if this were more
like

if (!fCache ||
    !SkTypeface::Equal(fTypeface, run->fTypeface) ||
    fCache->getDescriptor() != *newDesc)
{

so that the logic is easier to follow, and so the '{' breaks up the equal
indentation of the conditional and the body. As things are, it's difficult to
visually distinguish between the conditional logic and the body.

+        fCache.reset(SkGlyphCache::DetachCache(run->fTypeface, run->fEffects, newDesc));
+        SkASSERT(*newDesc == fCache->getDescriptor());
+        fTypeface = run->fTypeface;
+    }
+}
+
 template <bool regenPos, bool regenCol, bool regenTexCoords, bool regenGlyphs>
 void GrAtlasTextBlob::regenInBatch(GrDrawBatch::Target* target,
                                    GrBatchFontCache* fontCache,
                                    GrBlobRegenHelper *helper,
                                    Run* run,
-                                   Run::SubRunInfo* info, SkGlyphCache** cache,
-                                   SkTypeface** typeface,
-                                   const SkDescriptor** desc,
+                                   Run::SubRunInfo* info,
+                                   LazyGlyphCache* lazyCache,
                                    int glyphCount, size_t vertexStride,
                                    GrColor color, SkScalar transX,
                                    SkScalar transY) const {
     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;
-        }
+        lazyCache->initIfNecessary(run, info);
 
         if (regenGlyphs) {
-            strike = fontCache->getStrike(*cache);
+            strike = fontCache->getStrike(lazyCache->cache());
         } else {
             strike = info->strike();
         }
     }
 
     bool brokenRun = false;
     for (int glyphIdx = 0; glyphIdx < glyphCount; glyphIdx++) {
         GrGlyph* glyph = nullptr;
         int log2Width = 0, log2Height = 0;
         if (regenTexCoords) {
             size_t glyphOffset = glyphIdx + info->glyphStartIndex();
 
             if (regenGlyphs) {
                 // 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->cache());
                 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->cache(), info->maskFormat())) {
                 helper->flush();
                 brokenRun = glyphIdx > 0;
 
                 SkDEBUGCODE(bool success =) strike->addGlyphToAtlas(target,
                                                                     glyph,
-                                                                    *cache,
+                                                                    lazyCache->cache(),
                                                                     info->maskFormat());
                 SkASSERT(success);
             }
             fontCache->addGlyphToBulkAndSetUseToken(info->bulkUseToken(), glyph,
                                                     target->nextDrawToken());
             log2Width = fontCache->log2Width(info->maskFormat());
             log2Height = fontCache->log2Height(info->maskFormat());
         }
 
         intptr_t vertex = reinterpret_cast<intptr_t>(fVertices);
@@ skipping 17 matching lines @@
 }
 
 enum RegenMask {
     kNoRegen    = 0x0,
     kRegenPos   = 0x1,
     kRegenCol   = 0x2,
     kRegenTex   = 0x4,
     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,
     kRegenPosColTexGlyph = kRegenPos | kRegenCol | kRegenGlyph,
     kRegenColTex = kRegenCol | kRegenTex,
     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, LazyGlyphCache* lazyCache,
                                    size_t vertexStride, const SkMatrix& viewMatrix,
                                    SkScalar x, SkScalar y, GrColor color,
                                    void** vertices, size_t* byteCount, int* glyphCount) {
     Run& run = fRuns[runIndex];
     Run::SubRunInfo& info = run.fSubRunInfo[subRunIndex];
 
     uint64_t currentAtlasGen = fontCache->atlasGeneration(info.maskFormat());
 
     // Compute translation if any
     SkScalar transX, transY;
@@ skipping 42 matching lines @@
             // set use tokens for all of the glyphs in our subrun.  This is only valid if we
             // have a valid atlas generation
             fontCache->setUseTokenBulk(*info.bulkUseToken(), target->nextDrawToken(),
                                         info.maskFormat());
             break;
     }
 
     *byteCount = info.byteCount();
     *vertices = fVertices + info.vertexStartIndex();
 }