Use a monotonic counter for atlas and plot generations

Currently when the GPU resources are freed, the generation counters
are reset back to 1. This could allow stale data to be retained in the
SubRun texture coordinates. In addition, it confuses managing the
GrStrikes.

Use monotonic counters so that no number is ever repeated. This allows
for a simple check of equality without any additional checks or constrinats.

Bug: chromium:1045016

Change-Id: Ib58abf9a99107a37927fa73aef88a95900f70a5f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/266618
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>

src/gpu/GrDrawOpAtlas.cpp

@@ -39,14 +39,16 @@ std::unique_ptr<GrDrawOpAtlas> GrDrawOpAtlas::Make(GrProxyProvider* proxyProvide
                                                    const GrBackendFormat& format,
                                                    GrColorType colorType, int width,
                                                    int height, int plotWidth, int plotHeight,
+                                                   GenerationCounter* generationCounter,
                                                    AllowMultitexturing allowMultitexturing,
                                                    EvictionCallback* evictor) {
     if (!format.isValid()) {
         return nullptr;
     }
 
-    std::unique_ptr<GrDrawOpAtlas> atlas(new GrDrawOpAtlas(proxyProvider, format, colorType, width,
+    std::unique_ptr<GrDrawOpAtlas> atlas(new GrDrawOpAtlas(proxyProvider, format, colorType,
-                                                           height, plotWidth, plotHeight,
+                                                           width, height, plotWidth, plotHeight,
+                                                           generationCounter,
                                                            allowMultitexturing));
     if (!atlas->getViews()[0].proxy()) {
         return nullptr;
@@ -61,14 +63,15 @@ static bool gDumpAtlasData = false;
 #endif
 
 ////////////////////////////////////////////////////////////////////////////////
-GrDrawOpAtlas::Plot::Plot(int pageIndex, int plotIndex, uint64_t genID, int offX, int offY,
+GrDrawOpAtlas::Plot::Plot(int pageIndex, int plotIndex, GenerationCounter* generationCounter,
-                          int width, int height, GrColorType colorType)
+        int offX, int offY, int width, int height, GrColorType colorType)
         : fLastUpload(GrDeferredUploadToken::AlreadyFlushedToken())
         , fLastUse(GrDeferredUploadToken::AlreadyFlushedToken())
         , fFlushesSinceLastUse(0)
         , fPageIndex(pageIndex)
         , fPlotIndex(plotIndex)
-        , fGenID(genID)
+        , fGenerationCounter(generationCounter)
+        , fGenID(fGenerationCounter->next())
         , fPlotLocator(CreatePlotLocator(fPageIndex, fPlotIndex, fGenID))
         , fData(nullptr)
         , fWidth(width)
@@ -161,7 +164,7 @@ void GrDrawOpAtlas::Plot::uploadToTexture(GrDeferredTextureUploadWritePixelsFn&
 void GrDrawOpAtlas::Plot::resetRects() {
     fRectanizer.reset();
 
-    fGenID++;
+    fGenID = fGenerationCounter->next();
     fPlotLocator = CreatePlotLocator(fPageIndex, fPlotIndex, fGenID);
     fLastUpload = GrDeferredUploadToken::AlreadyFlushedToken();
     fLastUse = GrDeferredUploadToken::AlreadyFlushedToken();
@@ -177,16 +180,18 @@ void GrDrawOpAtlas::Plot::resetRects() {
 
 ///////////////////////////////////////////////////////////////////////////////
 
-GrDrawOpAtlas::GrDrawOpAtlas(GrProxyProvider* proxyProvider, const GrBackendFormat& format,
+GrDrawOpAtlas::GrDrawOpAtlas(
-                             GrColorType colorType, int width, int height,
+        GrProxyProvider* proxyProvider, const GrBackendFormat& format,
-                             int plotWidth, int plotHeight, AllowMultitexturing allowMultitexturing)
+        GrColorType colorType, int width, int height, int plotWidth, int plotHeight,
+        GenerationCounter* generationCounter, AllowMultitexturing allowMultitexturing)
         : fFormat(format)
         , fColorType(colorType)
         , fTextureWidth(width)
         , fTextureHeight(height)
         , fPlotWidth(plotWidth)
         , fPlotHeight(plotHeight)
-        , fAtlasGeneration(kInvalidAtlasGeneration + 1)
+        , fGenerationCounter(generationCounter)
+        , fAtlasGeneration(fGenerationCounter->next())
         , fPrevFlushToken(GrDeferredUploadToken::AlreadyFlushedToken())
         , fMaxPages(AllowMultitexturing::kYes == allowMultitexturing ? kMaxMultitexturePages : 1)
         , fNumActivePages(0) {
@@ -198,7 +203,7 @@ GrDrawOpAtlas::GrDrawOpAtlas(GrProxyProvider* proxyProvider, const GrBackendForm
 
     fNumPlots = numPlotsX * numPlotsY;
 
-    this->createPages(proxyProvider);
+    this->createPages(proxyProvider, generationCounter);
 }
 
 inline void GrDrawOpAtlas::processEviction(PlotLocator plotLocator) {
@@ -206,7 +211,7 @@ inline void GrDrawOpAtlas::processEviction(PlotLocator plotLocator) {
         evictor->evict(plotLocator);
     }
 
-    ++fAtlasGeneration;
+    fAtlasGeneration = fGenerationCounter->next();
 }
 
 inline bool GrDrawOpAtlas::updatePlot(GrDeferredUploadTarget* target,
@@ -517,7 +522,8 @@ void GrDrawOpAtlas::compact(GrDeferredUploadToken startTokenForNextFlush) {
     fPrevFlushToken = startTokenForNextFlush;
 }
 
-bool GrDrawOpAtlas::createPages(GrProxyProvider* proxyProvider) {
+bool GrDrawOpAtlas::createPages(
+        GrProxyProvider* proxyProvider, GenerationCounter* generationCounter) {
     SkASSERT(SkIsPow2(fTextureWidth) && SkIsPow2(fTextureHeight));
 
     GrSurfaceDesc desc;
@@ -545,8 +551,8 @@ bool GrDrawOpAtlas::createPages(GrProxyProvider* proxyProvider) {
         for (int y = numPlotsY - 1, r = 0; y >= 0; --y, ++r) {
             for (int x = numPlotsX - 1, c = 0; x >= 0; --x, ++c) {
                 uint32_t plotIndex = r * numPlotsX + c;
-                currPlot->reset(new Plot(i, plotIndex, 1, x, y, fPlotWidth, fPlotHeight,
+                currPlot->reset(new Plot(
-                                         fColorType));
+                    i, plotIndex, generationCounter, x, y, fPlotWidth, fPlotHeight, fColorType));
 
                 // build LRU list
                 fPages[i].fPlotList.addToHead(currPlot->get());

src/gpu/GrDrawOpAtlas.h

@@ -81,6 +81,20 @@ public:
         virtual void evict(PlotLocator plotLocator) = 0;
     };
 
+    /**
+     * Keep track of generation number for Atlases and Plots.
+     */
+    class GenerationCounter {
+    public:
+        static constexpr uint64_t kInvalidGeneration = 0;
+        uint64_t next() {
+            return fGeneration++;
+        }
+
+    private:
+        uint64_t fGeneration{1};
+    };
+
     /**
      * Returns a GrDrawOpAtlas. This function can be called anywhere, but the returned atlas
      * should only be used inside of GrMeshDrawOp::onPrepareDraws.
@@ -91,6 +105,7 @@ public:
      *                          direction
      *  @param numPlotsY        The number of plots the atlas should be broken up into in the Y
      *                          direction
+     *  @param atlasGeneration  a pointer to the context's generation counter.
      *  @param allowMultitexturing Can the atlas use more than one texture.
      *  @param evictor          A pointer to an eviction callback class.
      *
@@ -101,6 +116,7 @@ public:
                                                GrColorType,
                                                int width, int height,
                                                int plotWidth, int plotHeight,
+                                               GenerationCounter* generationCounter,
                                                AllowMultitexturing allowMultitexturing,
                                                EvictionCallback* evictor);
 
@@ -140,10 +156,10 @@ public:
         }
 
         uint32_t plot = GetPlotIndexFromID(plotLocator);
-        SkASSERT(plot < fNumPlots);
         uint32_t page = GetPageIndexFromID(plotLocator);
-        SkASSERT(page < fNumActivePages);
+        uint64_t plotGeneration = fPages[page].fPlotArray[plot]->genID();
-        return fPages[page].fPlotArray[plot]->genID() == GetGenerationFromID(plotLocator);
+        uint64_t locatorGeneration = GetGenerationFromID(plotLocator);
+        return plot < fNumPlots && page < fNumActivePages && plotGeneration == locatorGeneration;
     }
 
     /** To ensure the atlas does not evict a given entry, the client must set the last use token. */
@@ -247,7 +263,7 @@ public:
 
 private:
     GrDrawOpAtlas(GrProxyProvider*, const GrBackendFormat& format, GrColorType, int width,
-                  int height, int plotWidth, int plotHeight,
+                  int height, int plotWidth, int plotHeight, GenerationCounter* generationCounter,
                   AllowMultitexturing allowMultitexturing);
 
     /**
@@ -296,8 +312,8 @@ private:
         void incFlushesSinceLastUsed() { fFlushesSinceLastUse++; }
 
     private:
-        Plot(int pageIndex, int plotIndex, uint64_t genID, int offX, int offY,
+        Plot(int pageIndex, int plotIndex, GenerationCounter* generationCounter,
-             int width, int height, GrColorType colorType);
+             int offX, int offY, int width, int height, GrColorType colorType);
 
         ~Plot() override;
 
@@ -306,8 +322,8 @@ private:
          * the atlas
          */
         Plot* clone() const {
-            return new Plot(fPageIndex, fPlotIndex, fGenID + 1, fX, fY, fWidth, fHeight,
+            return new Plot(
-                            fColorType);
+                fPageIndex, fPlotIndex, fGenerationCounter, fX, fY, fWidth, fHeight, fColorType);
         }
 
         static GrDrawOpAtlas::PlotLocator CreatePlotLocator(
@@ -328,6 +344,7 @@ private:
             const uint32_t fPageIndex : 16;
             const uint32_t fPlotIndex : 16;
         };
+        GenerationCounter* const fGenerationCounter;
         uint64_t fGenID;
         GrDrawOpAtlas::PlotLocator fPlotLocator;
         unsigned char* fData;
@@ -376,7 +393,7 @@ private:
                       GrDeferredUploadTarget* target, int width, int height, const void* image,
                       SkIPoint16* loc);
 
-    bool createPages(GrProxyProvider*);
+    bool createPages(GrProxyProvider*, GenerationCounter*);
     bool activateNewPage(GrResourceProvider*);
     void deactivateLastPage();
 
@@ -394,7 +411,9 @@ private:
     int                   fPlotHeight;
     unsigned int          fNumPlots;
 
-    uint64_t              fAtlasGeneration;
+    GenerationCounter* const fGenerationCounter;
+    uint64_t                 fAtlasGeneration;
+
     // nextTokenToFlush() value at the end of the previous flush
     GrDeferredUploadToken fPrevFlushToken;
 

src/gpu/ops/GrSmallPathRenderer.cpp

@@ -890,8 +890,9 @@ bool GrSmallPathRenderer::onDrawPath(const DrawPathArgs& args) {
                                         kMaxAtlasTextureBytes);
         SkISize size = atlasConfig.atlasDimensions(kA8_GrMaskFormat);
         fAtlas = GrDrawOpAtlas::Make(args.fContext->priv().proxyProvider(), format,
-                                     GrColorType::kAlpha_8, size.width(), size.height(), kPlotWidth,
+                                     GrColorType::kAlpha_8, size.width(), size.height(),
-                                     kPlotHeight, GrDrawOpAtlas::AllowMultitexturing::kYes, this);
+                                     kPlotWidth, kPlotHeight, this,
+                                     GrDrawOpAtlas::AllowMultitexturing::kYes, this);
         if (!fAtlas) {
             return false;
         }
@@ -909,7 +910,8 @@ bool GrSmallPathRenderer::onDrawPath(const DrawPathArgs& args) {
 
 #if GR_TEST_UTILS
 
-struct GrSmallPathRenderer::PathTestStruct : public GrDrawOpAtlas::EvictionCallback {
+struct GrSmallPathRenderer::PathTestStruct : public GrDrawOpAtlas::EvictionCallback,
+                                             public GrDrawOpAtlas::GenerationCounter {
     PathTestStruct() : fContextID(SK_InvalidGenID), fAtlas(nullptr) {}
     ~PathTestStruct() override { this->reset(); }
 
@@ -979,6 +981,7 @@ GR_DRAW_OP_TEST_DEFINE(SmallPathOp) {
         gTestStruct.fAtlas =
                 GrDrawOpAtlas::Make(context->priv().proxyProvider(), format, GrColorType::kAlpha_8,
                                     size.width(), size.height(), kPlotWidth, kPlotHeight,
+                                    &gTestStruct,
                                     GrDrawOpAtlas::AllowMultitexturing::kYes, &gTestStruct);
     }
 

src/gpu/ops/GrSmallPathRenderer.h

@@ -24,7 +24,8 @@ class ShapeDataKey;
 
 class GrSmallPathRenderer : public GrPathRenderer,
                             public GrOnFlushCallbackObject,
-                            public GrDrawOpAtlas::EvictionCallback {
+                            public GrDrawOpAtlas::EvictionCallback,
+                            public GrDrawOpAtlas::GenerationCounter {
 public:
     GrSmallPathRenderer();
     ~GrSmallPathRenderer() override;

src/gpu/text/GrAtlasManager.cpp

@@ -161,7 +161,7 @@ bool GrAtlasManager::initAtlas(GrMaskFormat format) {
                 fProxyProvider, format, grColorType,
                 atlasDimensions.width(), atlasDimensions.height(),
                 plotDimensions.width(), plotDimensions.height(),
-                fAllowMultitexturing, fGlyphCache);
+                this, fAllowMultitexturing, fGlyphCache);
         if (!fAtlases[index]) {
             return false;
         }

src/gpu/text/GrAtlasManager.h

@@ -23,7 +23,7 @@ class GrTextStrike;
  *  This implies that all of the advanced atlasManager functionality (i.e.,
  *  adding glyphs to the atlas) are only available at flush time.
  */
-class GrAtlasManager : public GrOnFlushCallbackObject {
+class GrAtlasManager : public GrOnFlushCallbackObject, public GrDrawOpAtlas::GenerationCounter {
 public:
     GrAtlasManager(GrProxyProvider*, GrStrikeCache*,
                    size_t maxTextureBytes, GrDrawOpAtlas::AllowMultitexturing);
@@ -134,6 +134,7 @@ private:
 
     GrDrawOpAtlas::AllowMultitexturing fAllowMultitexturing;
     std::unique_ptr<GrDrawOpAtlas> fAtlases[kMaskFormatCount];
+    static_assert(kMaskFormatCount == 3);
     GrProxyProvider* fProxyProvider;
     sk_sp<const GrCaps> fCaps;
     GrStrikeCache* fGlyphCache;

src/gpu/text/GrStrikeCache.cpp

@@ -27,26 +27,10 @@ GrStrikeCache::~GrStrikeCache() {
 }
 
 void GrStrikeCache::freeAll() {
-    fCache.foreach([](sk_sp<GrTextStrike>* strike){
-        (*strike)->fIsAbandoned = true;
-    });
     fCache.reset();
 }
 
-void GrStrikeCache::evict(GrDrawOpAtlas::PlotLocator plotLocator) {
+void GrStrikeCache::evict(GrDrawOpAtlas::PlotLocator) { }
-    fCache.mutate([this, plotLocator](sk_sp<GrTextStrike>* cacheSlot){
-        GrTextStrike* strike = cacheSlot->get();
-        strike->removeID(plotLocator);
-
-        // clear out any empty strikes.  We will preserve the strike whose call to addToAtlas
-        // triggered the eviction
-        if (strike != fPreserveStrike && 0 == strike->fAtlasedGlyphs) {
-            strike->fIsAbandoned = true;
-            return false;  // Remove this entry from the cache.
-        }
-        return true;  // Keep this entry in the cache.
-    });
-}
 
 // expands each bit in a bitmask to 0 or ~0 of type INT_TYPE. Used to expand a BW glyph mask to
 // A8, RGB565, or RGBA8888.

src/gpu/text/GrStrikeCache.h

@@ -56,9 +56,6 @@ public:
     // remove any references to this plot
     void removeID(GrDrawOpAtlas::PlotLocator);
 
-    // If a TextStrike is abandoned by the cache, then the caller must get a new strike
-    bool isAbandoned() const { return fIsAbandoned; }
-
 private:
     struct HashTraits {
         // GetKey and Hash for the the hash table.
@@ -75,7 +72,6 @@ private:
     SkArenaAlloc fAlloc{512};
 
     int fAtlasedGlyphs{0};
-    bool fIsAbandoned{false};
 
     friend class GrStrikeCache;
 };

src/gpu/text/GrTextBlob.cpp

@@ -280,22 +280,6 @@ void GrTextBlob::SubRun::updateTexCoords(int begin, int end) {
     }
 }
 
-// GrStrikeCache may evict the strike a blob needs to generate texture coordinates.
-// If our strike has been abandoned in this way, we'll translate all our old glyphs
-// over to a new strike and carry on with that.
-void GrTextBlob::SubRun::updateStrikeIfNeeded(SkBulkGlyphMetricsAndImages* metricsAndImages,
-                                              GrStrikeCache* cache) {
-    if (this->strike()->isAbandoned()) {
-        sk_sp<GrTextStrike> newStrike = this->strikeSpec().findOrCreateGrStrike(cache);
-
-        // Take the glyphs from the old strike, and translate them a new strike.
-        for (size_t i = 0; i < fGlyphs.size(); i++) {
-            fGlyphs[i] = newStrike->getGlyph(fGlyphs[i]->fPackedID, metricsAndImages);
-        }
-
-        this->setStrike(newStrike);
-    }
-}
 
 void GrTextBlob::SubRun::setUseLCDText(bool useLCDText) { fFlags.useLCDText = useLCDText; }
 bool GrTextBlob::SubRun::hasUseLCDText() const { return fFlags.useLCDText; }
@@ -812,8 +796,6 @@ std::tuple<bool, int> GrTextBlob::VertexRegenerator::updateTextureCoordinatesMay
         fMetricsAndImages.init(strikeSpec);
     }
 
-    fSubRun->updateStrikeIfNeeded(fMetricsAndImages.get(), fGrStrikeCache);
-
     // Update the atlas information in the GrStrike.
     auto code = GrDrawOpAtlas::ErrorCode::kSucceeded;
     GrTextStrike* grStrike = fSubRun->strike();
@@ -860,10 +842,7 @@ std::tuple<bool, int> GrTextBlob::VertexRegenerator::regenerate(int begin, int e
     //  the atlas generation.
     fRegenerateTextureCoordinates =
             fRegenerateTextureCoordinates || fSubRun->fAtlasGeneration != currentAtlasGen;
-
+    if (fRegenerateTextureCoordinates) {
-    // The true || ... is to fix chrome bug 1045016. This is a temporary fix.
-    // TODO: figure out why the atlas number is getting off track, and restore the check.
-    if (true || fSubRun->strike()->isAbandoned() || fRegenerateTextureCoordinates) {
         return this->updateTextureCoordinatesMaybeStrike(begin, end);
     } else {
         // All glyphs are inserted into the atlas if fCurrGlyph is at the end of fGlyphs.

src/gpu/text/GrTextBlob.h

@@ -357,8 +357,6 @@ public:
     void translateVerticesIfNeeded(const SkMatrix& drawMatrix, SkPoint drawOrigin);
     void updateVerticesColorIfNeeded(GrColor newColor);
     void updateTexCoords(int begin, int end);
-    void updateStrikeIfNeeded(
-            SkBulkGlyphMetricsAndImages* metricsAndImages, GrStrikeCache* cache);
 
     // df properties
     void setUseLCDText(bool useLCDText);

tests/DrawOpAtlasTest.cpp

@@ -146,6 +146,7 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(BasicDrawOpAtlas, reporter, ctxInfo) {
                                                            GrRenderable::kNo);
 
     DummyEvict evictor;
+    GrDrawOpAtlas::GenerationCounter counter;
 
     std::unique_ptr<GrDrawOpAtlas> atlas = GrDrawOpAtlas::Make(
                                                 proxyProvider,
@@ -153,6 +154,7 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(BasicDrawOpAtlas, reporter, ctxInfo) {
                                                 GrColorType::kAlpha_8,
                                                 kAtlasSize, kAtlasSize,
                                                 kAtlasSize/kNumPlots, kAtlasSize/kNumPlots,
+                                                &counter,
                                                 GrDrawOpAtlas::AllowMultitexturing::kYes,
                                                 &evictor);
     check(reporter, atlas.get(), 0, 4, 0);