Make sure we mark Plots in use for all regenerate cases

Change-Id: I49b2cd7f634faa4a233038f3c1eaa507c630d95d Test: Added fontregen GM to verify fix. Bug: b/118850208 Merged-In: I986cd20ff387aa34e2750c10b53aa8a73ba85ed3
CustomROMs · Feb 7, 2019 · 5b3adf4 · 5b3adf4
1 parent 828b4e8
commit 5b3adf4
Show file tree

Hide file tree

Showing 6 changed files with 134 additions and 8 deletions.
diff --git a/gm/fontregen.cpp b/gm/fontregen.cpp
@@ -0,0 +1,118 @@
+/*
+ * Copyright 2018 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+// GM to stress TextBlob regeneration and the GPU font cache
+// It's not necessary to run this with CPU configs
+//
+// The point here is to draw a set of text that will fit in one Plot, and then some large
+// text. After a flush we draw the first set of text again with a slightly different color,
+// and then enough new large text to spill the entire atlas. What *should* happen is that
+// the Plot with the first set of text will not get overwritten by the new large text.
+
+#include "gm.h"
+
+#include "GrContext.h"
+#include "GrContextPriv.h"
+#include "GrContextOptions.h"
+#include "SkCanvas.h"
+#include "SkGraphics.h"
+#include "SkImage.h"
+#include "SkTypeface.h"
+#include "gm.h"
+
+#include "sk_tool_utils.h"
+
+static sk_sp<SkTextBlob> make_blob(const SkString& text, const SkFont& font) {
+    size_t len = text.size();
+    SkAutoTArray<SkScalar>  pos(len);
+    SkAutoTArray<SkGlyphID> glyphs(len);
+
+    font.textToGlyphs(text.c_str(), len, SkTextEncoding::kUTF8, glyphs.get(), len);
+    font.getXPos(glyphs.get(), len, pos.get());
+    return SkTextBlob::MakeFromPosTextH(text.c_str(), len, pos.get(), 0, font);
+}
+
+class FontRegenGM : public skiagm::GM {
+public:
+    FontRegenGM() {
+        this->setBGColor(SK_ColorLTGRAY);
+    }
+
+    void modifyGrContextOptions(GrContextOptions* options) override {
+        options->fGlyphCacheTextureMaximumBytes = 0;
+        options->fAllowMultipleGlyphCacheTextures = GrContextOptions::Enable::kNo;
+    }
+
+protected:
+    SkString onShortName() override {
+        SkString name("fontregen");
+        return name;
+    }
+
+    SkISize onISize() override { return SkISize::Make(kSize, kSize); }
+
+    void onOnceBeforeDraw() override {
+        auto tf = sk_tool_utils::create_portable_typeface("sans-serif", SkFontStyle::Normal());
+
+        static const SkString kTexts[] = {
+            SkString("abcdefghijklmnopqrstuvwxyz"),
+            SkString("ABCDEFGHI"),
+            SkString("NOPQRSTUV")
+        };
+
+        SkFont font;
+        font.setEdging(SkFont::Edging::kAntiAlias);
+        font.setSubpixel(false);
+        font.setSize(80);
+        font.setTypeface(tf);
+
+        fBlobs[0] = make_blob(kTexts[0], font);
+        font.setSize(162);
+        fBlobs[1] = make_blob(kTexts[1], font);
+        fBlobs[2] = make_blob(kTexts[2], font);
+    }
+
+    void onDraw(SkCanvas* canvas) override {
+        GrRenderTargetContext* renderTargetContext =
+            canvas->internal_private_accessTopLayerRenderTargetContext();
+        if (!renderTargetContext) {
+            skiagm::GM::DrawGpuOnlyMessage(canvas);
+            return;
+        }
+
+        SkPaint paint;
+        paint.setColor(SK_ColorBLACK);
+        canvas->drawTextBlob(fBlobs[0], 10, 80, paint);
+        canvas->drawTextBlob(fBlobs[1], 10, 225, paint);
+        canvas->flush();
+
+        paint.setColor(0xFF010101);
+        canvas->drawTextBlob(fBlobs[0], 10, 305, paint);
+        canvas->drawTextBlob(fBlobs[2], 10, 465, paint);
+
+        //  Debugging tool for GPU.
+        static const bool kShowAtlas = false;
+        if (kShowAtlas) {
+            if (auto ctx = canvas->getGrContext()) {
+                auto img = ctx->contextPriv().getFontAtlasImage_ForTesting(kA8_GrMaskFormat);
+                canvas->drawImage(img, 200, 0);
+            }
+        }
+    }
+
+private:
+    static constexpr SkScalar kSize = 512;
+
+    sk_sp<SkTextBlob> fBlobs[3];
+    typedef GM INHERITED;
+};
+
+constexpr SkScalar FontRegenGM::kSize;
+
+//////////////////////////////////////////////////////////////////////////////
+
+DEF_GM(return new FontRegenGM())
diff --git a/gn/gm.gni b/gn/gm.gni
@@ -138,6 +138,7 @@ gm_sources = [
   "$_gm/flippity.cpp",
   "$_gm/fontcache.cpp",
   "$_gm/fontmgr.cpp",
+  "$_gm/fontregen.cpp",
   "$_gm/fontscaler.cpp",
   "$_gm/fontscalerdistortable.cpp",
   "$_gm/gamma.cpp",

diff --git a/src/gpu/GrDrawOpAtlas.cpp b/src/gpu/GrDrawOpAtlas.cpp
@@ -271,9 +271,8 @@ bool GrDrawOpAtlas::addToAtlas(GrResourceProvider* resourceProvider,
     for (unsigned int pageIdx = 0; pageIdx < fNumActivePages; ++pageIdx) {
         Plot* plot = fPages[pageIdx].fPlotList.tail();
         SkASSERT(plot);
-        if ((fNumActivePages == this->maxPages() &&
-             plot->lastUseToken() < target->tokenTracker()->nextTokenToFlush()) ||
-            plot->flushesSinceLastUsed() >= kRecentlyUsedCount) {
+        if (fNumActivePages == this->maxPages() &&
+            plot->lastUseToken() < target->tokenTracker()->nextTokenToFlush()) {
             this->processEvictionAndResetRects(plot);
             SkASSERT(GrBytesPerPixel(fProxies[pageIdx]->config()) == plot->bpp());
             SkDEBUGCODE(bool verify = )plot->addSubImage(width, height, image, loc);

diff --git a/src/gpu/GrDrawOpAtlas.h b/src/gpu/GrDrawOpAtlas.h
@@ -162,12 +162,14 @@ class GrDrawOpAtlas {
             memcpy(fPlotAlreadyUpdated, that.fPlotAlreadyUpdated, sizeof(fPlotAlreadyUpdated));
         }
 
-        void add(AtlasID id) {
+        bool add(AtlasID id) {
             int index = GrDrawOpAtlas::GetPlotIndexFromID(id);
             int pageIdx = GrDrawOpAtlas::GetPageIndexFromID(id);
-            if (!this->find(pageIdx, index)) {
-                this->set(pageIdx, index);
+            if (this->find(pageIdx, index)) {
+                return false;
             }
+            this->set(pageIdx, index);
+            return true;
         }
 
         void reset() {

diff --git a/src/gpu/text/GrAtlasManager.cpp b/src/gpu/text/GrAtlasManager.cpp
@@ -108,8 +108,9 @@ void GrAtlasManager::addGlyphToBulkAndSetUseToken(GrDrawOpAtlas::BulkUseTokenUpd
                                                   GrGlyph* glyph,
                                                   GrDeferredUploadToken token) {
     SkASSERT(glyph);
-    updater->add(glyph->fID);
-    this->getAtlas(glyph->fMaskFormat)->setLastUseToken(glyph->fID, token);
+    if (updater->add(glyph->fID)) {
+        this->getAtlas(glyph->fMaskFormat)->setLastUseToken(glyph->fID, token);
+    }
 }
 
 #ifdef SK_DEBUG

diff --git a/src/gpu/text/GrAtlasTextBlobVertexRegenerator.cpp b/src/gpu/text/GrAtlasTextBlobVertexRegenerator.cpp
@@ -307,6 +307,11 @@ Regenerator::Result Regenerator::doRegen() {
         fSubRun->setAtlasGeneration(fBrokenRun
                                     ? GrDrawOpAtlas::kInvalidAtlasGeneration
                                     : fFullAtlasManager->atlasGeneration(fSubRun->maskFormat()));
+    } else {
+        // For the non-texCoords case we need to ensure that we update the associated use tokens
+        fFullAtlasManager->setUseTokenBulk(*fSubRun->bulkUseToken(),
+                                           fUploadTarget->tokenTracker()->nextDrawToken(),
+                                           fSubRun->maskFormat());
     }
     return result;
 }