Redo glyph quad regeneration

Lift all the invariants for the loop into the onPrepare loops
from regenerate and updateTextureCoordinatesMaybeStrike.

* Simplify looping over geos and subRuns
* remove VertexRegenerator::Result
* remove fCurrGlyph

Change-Id: I75445c6d7113207a3b1544154b605af2c4cfcb31
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/263778
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>

src/atlastext/SkAtlasTextTarget.cpp

@@ -239,17 +239,16 @@ void GrAtlasTextOp::executeForTextTarget(SkAtlasTextTarget* target) {
                 resourceProvider, fGeoData[i].fSubRunPtr,
                 fGeoData[i].fDrawMatrix, fGeoData[i].fDrawOrigin,
                 fGeoData[i].fColor.toBytes_RGBA(), &context, glyphCache, atlasManager);
-        bool done = false;
+        auto subRun = fGeoData[i].fSubRunPtr;
-        while (!done) {
+        int subRunEnd = subRun->fGlyphs.size();
-            GrTextBlob::VertexRegenerator::Result result;
+        for (int subRunIndex = 0; subRunIndex < subRunEnd;) {
-            if (!regenerator.regenerate(&result)) {
+            auto [ok, glyphsRegenerated] = regenerator.regenerate(subRunIndex, subRunEnd);
-                break;
+            if (!ok) { break; }
-            }
-            done = result.fFinished;
 
-            context.recordDraw(result.fFirstVertex, result.fGlyphsRegenerated,
+            context.recordDraw(subRun->quadStart(subRunIndex), glyphsRegenerated,
                                fGeoData[i].fDrawMatrix, target->handle());
-            if (!result.fFinished) {
+            subRunIndex += glyphsRegenerated;
+            if (subRunIndex != subRunEnd) {
                 // Make space in the atlas so we can continue generating vertices.
                 context.flush();
             }

src/gpu/ops/GrAtlasTextOp.cpp

@@ -331,58 +331,75 @@ void GrAtlasTextOp::onPrepareDraws(Target* target) {
                 numActiveViews, filter, maskFormat, localMatrix, vmPerspective);
     }
 
-    size_t vertexStride = flushInfo.fGeometryProcessor->vertexStride();
+    int vertexStride = (int)flushInfo.fGeometryProcessor->vertexStride();
 
     // Ensure we don't request an insanely large contiguous vertex allocation.
-    static const size_t kMaxVertexBytes = GrBufferAllocPool::kDefaultBufferSize;
+    static const int kMaxVertexBytes = GrBufferAllocPool::kDefaultBufferSize;
-    int maxGlyphsInBuffer = kMaxVertexBytes / (vertexStride * kVerticesPerGlyph);
+    const int quadSize = vertexStride * kVerticesPerGlyph;
-    int totalGlyphCount = this->numGlyphs();
+    const int maxQuadsPerBuffer = kMaxVertexBytes / quadSize;
-    int bufferGlyphCount = std::min(totalGlyphCount, maxGlyphsInBuffer);
+
-    void* vertices = target->makeVertexSpace(vertexStride, bufferGlyphCount * kVerticesPerGlyph,
+    // Where the quad buffer begins and ends relative to totalGlyphsRegened.
-                                             &flushInfo.fVertexBuffer, &flushInfo.fVertexOffset);
+    int quadBufferBegin = 0;
+    int quadBufferEnd = std::min(this->numGlyphs(), maxQuadsPerBuffer);
+
     flushInfo.fIndexBuffer = resourceProvider->refNonAAQuadIndexBuffer();
+    void* vertices = target->makeVertexSpace(
+            vertexStride,
+            kVerticesPerGlyph * (quadBufferEnd - quadBufferBegin),
+            &flushInfo.fVertexBuffer,
+            &flushInfo.fVertexOffset);
     if (!vertices || !flushInfo.fVertexBuffer) {
         SkDebugf("Could not allocate vertices\n");
         return;
     }
 
-    char* currVertex = reinterpret_cast<char*>(vertices);
+    // totalGlyphsRegened is all the glyphs for the op [0, this->numGlyphs()). The subRun glyph and
-
+    // quad buffer indices are calculated from this.
-    // each of these is a SubRun
+    int totalGlyphsRegened = 0;
     for (int i = 0; i < fGeoCount; i++) {
         const Geometry& args = fGeoData[i];
+        auto subRun = args.fSubRunPtr;
+        SkASSERT((int)subRun->vertexStride() == vertexStride);
+
         // TODO4F: Preserve float colors
         GrTextBlob::VertexRegenerator regenerator(
                 resourceProvider, args.fSubRunPtr, args.fDrawMatrix, args.fDrawOrigin,
                 args.fColor.toBytes_RGBA(), target->deferredUploadTarget(), glyphCache,
                 atlasManager);
-        // This loop issues draws until regenerator says we're done with this geo. Regenerator
+
-        // breaks things up if inline uploads are necessary.
+        // Where the subRun begins and ends relative to totalGlyphsRegened.
-        GrTextBlob::VertexRegenerator::Result result;
+        int subRunBegin = totalGlyphsRegened;
-        while (!result.fFinished) {
+        int subRunEnd = subRunBegin + (int)subRun->fGlyphs.size();
-            // Copy regenerated vertices from the blob to our vertex buffer. If we overflow our
+
-            // vertex buffer we'll issue a draw and then get more vertex buffer space.
+        // Draw all the glyphs in the subRun.
-            do {
+        while (totalGlyphsRegened < subRunEnd) {
-                if (!bufferGlyphCount) {
+            // drawBegin and drawEnd are indices for the subRun on the
-                    this->flush(target, &flushInfo);
+            // interval [0, subRun->fGlyphs.size()).
-                    bufferGlyphCount = std::min(totalGlyphCount, maxGlyphsInBuffer);
+            int drawBegin = totalGlyphsRegened - subRunBegin;
-                    vertices = target->makeVertexSpace(
+            // drawEnd is either the end of the subRun or the end of the current quad buffer.
-                            vertexStride, bufferGlyphCount * kVerticesPerGlyph,
+            int drawEnd = std::min(subRunEnd, quadBufferEnd) - subRunBegin;
-                            &flushInfo.fVertexBuffer, &flushInfo.fVertexOffset);
+            auto[ok, glyphsRegenerated] = regenerator.regenerate(drawBegin, drawEnd);
-                    currVertex = reinterpret_cast<char*>(vertices);
+
-                }
+            // There was a problem allocating the glyph in the atlas. Bail.
-                if (!regenerator.regenerate(&result, bufferGlyphCount)) {
+            if(!ok) { return; }
-                    return;
+
-                }
+            // Update all the vertices for glyphsRegenerate glyphs.
-                int glyphCount = std::min(result.fGlyphsRegenerated, bufferGlyphCount);
+            if (glyphsRegenerated > 0) {
-                int vertexCount = glyphCount * kVerticesPerGlyph;
+                int quadBufferIndex = totalGlyphsRegened - quadBufferBegin;
-                size_t vertexBytes = vertexCount * vertexStride;
+                int subRunIndex = totalGlyphsRegened - subRunBegin;
+                auto regeneratedQuadBuffer =
+                        SkTAddOffset<char>(vertices, subRun->quadOffset(quadBufferIndex));
                 if (args.fClipRect.isEmpty()) {
-                    memcpy(currVertex, result.fFirstVertex, vertexBytes);
+                    memcpy(regeneratedQuadBuffer,
+                           subRun->quadStart(subRunIndex),
+                           glyphsRegenerated * quadSize);
                 } else {
                     SkASSERT(!vmPerspective);
-                    clip_quads(args.fClipRect, currVertex, result.fFirstVertex, vertexStride,
+                    clip_quads(args.fClipRect,
-                               glyphCount);
+                               regeneratedQuadBuffer,
+                               subRun->quadStart(subRunIndex),
+                               vertexStride,
+                               glyphsRegenerated);
                 }
                 if (fNeedsGlyphTransform && !args.fDrawMatrix.isIdentity()) {
                     // We always do the distance field view matrix transformation after copying
@@ -390,30 +407,54 @@ void GrAtlasTextOp::onPrepareDraws(Target* target) {
                     // successive arbitrary transformations would be complicated and accumulate
                     // error.
                     if (args.fDrawMatrix.hasPerspective()) {
-                        auto* pos = reinterpret_cast<SkPoint3*>(currVertex);
+                        auto* pos = reinterpret_cast<SkPoint3*>(regeneratedQuadBuffer);
-                        SkMatrixPriv::MapHomogeneousPointsWithStride(args.fDrawMatrix, pos,
+                        SkMatrixPriv::MapHomogeneousPointsWithStride(
-                                                                     vertexStride, pos,
+                                args.fDrawMatrix, pos,
-                                                                     vertexStride, vertexCount);
+                                vertexStride, pos,
+                                vertexStride,
+                                glyphsRegenerated * kVerticesPerGlyph);
                     } else {
-                        auto* pos = reinterpret_cast<SkPoint*>(currVertex);
+                        auto* pos = reinterpret_cast<SkPoint*>(regeneratedQuadBuffer);
                         SkMatrixPriv::MapPointsWithStride(args.fDrawMatrix, pos, vertexStride,
-                                                          vertexCount);
+                                                          glyphsRegenerated * kVerticesPerGlyph);
                     }
                 }
-                flushInfo.fGlyphsToFlush += glyphCount;
-                currVertex += vertexBytes;
-                result.fFirstVertex += vertexBytes;
-                result.fGlyphsRegenerated -= glyphCount;
-                bufferGlyphCount -= glyphCount;
-                totalGlyphCount -= glyphCount;
-            } while (result.fGlyphsRegenerated);
-            if (!result.fFinished) {
-                this->flush(target, &flushInfo);
             }
-        }  // for all vertices
+
+            totalGlyphsRegened += glyphsRegenerated;
+            flushInfo.fGlyphsToFlush += glyphsRegenerated;
+
+            // regenerate() has stopped part way through a SubRun. This means that either the atlas
+            // or the quad buffer is full or both. There is a case were the flow through
+            // the loop is strange. If we run out of quad buffer space at the same time the
+            // SubRun ends, then this is not triggered which is the right result for the last
+            // SubRun. But, if this is not the last SubRun, then advance to the next SubRun which
+            // will process no glyphs, and return to this point where the quad buffer will be
+            // expanded.
+            if (totalGlyphsRegened != subRunEnd) {
+                // Flush if not all glyphs drawn because either the quad buffer is full or the
+                // atlas is out of space.
+                this->flush(target, &flushInfo);
+                if (totalGlyphsRegened == quadBufferEnd) {
+                    // Quad buffer is full. Get more buffer.
+                    quadBufferBegin = totalGlyphsRegened;
+                    int quadBufferSize =
+                            std::min(maxQuadsPerBuffer, this->numGlyphs() - totalGlyphsRegened);
+                    quadBufferEnd = quadBufferBegin + quadBufferSize;
+
+                    vertices = target->makeVertexSpace(
+                            vertexStride,
+                            kVerticesPerGlyph * quadBufferSize,
+                            &flushInfo.fVertexBuffer,
+                            &flushInfo.fVertexOffset);
+                    if (!vertices || !flushInfo.fVertexBuffer) {
+                        SkDebugf("Could not allocate vertices\n");
+                        return;
+                    }
+                }
+            }
+        }
     }  // for all geometries
-    SkASSERT(!bufferGlyphCount);
-    SkASSERT(!totalGlyphCount);
     this->flush(target, &flushInfo);
 }
 

src/gpu/text/GrTextBlob.cpp

@@ -150,8 +150,11 @@ size_t GrTextBlob::SubRun::texCoordOffset() const {
 }
 
 char* GrTextBlob::SubRun::quadStart(size_t index) const {
-    return SkTAddOffset<char>(
+    return SkTAddOffset<char>(fVertexData.data(), this->quadOffset(index));
-            fVertexData.data(), index * kVerticesPerGlyph * this->vertexStride());
+}
+
+size_t GrTextBlob::SubRun::quadOffset(size_t index) const {
+    return index * kVerticesPerGlyph * this->vertexStride();
 }
 
 const SkRect& GrTextBlob::SubRun::vertexBounds() const { return fVertexBounds; }
@@ -843,10 +846,10 @@ std::tuple<bool, int> GrTextBlob::VertexRegenerator::updateTextureCoordinatesMay
         fFullAtlasManager->addGlyphToBulkAndSetUseToken(
                 fSubRun->bulkUseToken(), glyph, tokenTracker->nextDrawToken());
     }
-    int firstNotInAtlas = i;
+    int glyphsPlacedInAtlas = i - begin;
 
     // Update the quads with the new atlas coordinates.
-    fSubRun->updateTexCoords(begin, firstNotInAtlas);
+    fSubRun->updateTexCoords(begin, begin + glyphsPlacedInAtlas);
 
     if (code == GrDrawOpAtlas::ErrorCode::kSucceeded) {
         // If we reach here with begin > 0, some earlier call to regenerate() exhausted the atlas
@@ -857,39 +860,27 @@ std::tuple<bool, int> GrTextBlob::VertexRegenerator::updateTextureCoordinatesMay
                           : fFullAtlasManager->atlasGeneration(fSubRun->maskFormat());
     }
 
-    return {code != GrDrawOpAtlas::ErrorCode::kError, firstNotInAtlas};
+    return {code != GrDrawOpAtlas::ErrorCode::kError, glyphsPlacedInAtlas};
 }
 
-bool GrTextBlob::VertexRegenerator::regenerate(GrTextBlob::VertexRegenerator::Result* result,
+std::tuple<bool, int> GrTextBlob::VertexRegenerator::regenerate(int begin, int end) {
-                                               int maxGlyphs) {
     uint64_t currentAtlasGen = fFullAtlasManager->atlasGeneration(fSubRun->maskFormat());
     // If regenerate() is called multiple times then the atlas gen may have changed. So we check
     // this each time.
     fActions.regenTextureCoordinates |= fSubRun->fAtlasGeneration != currentAtlasGen;
     if (fActions.regenStrike) { SkASSERT(fActions.regenTextureCoordinates); }
 
-    bool ok = true;
-    const int begin = fCurrGlyph;
-    const int end = std::min((int)fSubRun->fGlyphs.size(), begin + maxGlyphs);
     if (fActions.regenStrike || fActions.regenTextureCoordinates) {
-        int firstGlyphNotInAtlas;
+        return this->updateTextureCoordinatesMaybeStrike(begin, end);
-        std::tie(ok, firstGlyphNotInAtlas) = this->updateTextureCoordinatesMaybeStrike(begin, end);
-        fCurrGlyph = firstGlyphNotInAtlas;
     } else {
-        fCurrGlyph = end;
         // All glyphs are inserted into the atlas if fCurrGlyph is at the end of fGlyphs.
-        if (fCurrGlyph == (int)fSubRun->fGlyphs.size()) {
+        if (end == (int)fSubRun->fGlyphs.size()) {
             // Set use tokens for all of the glyphs in our SubRun.  This is only valid if we
             // have a valid atlas generation
             fFullAtlasManager->setUseTokenBulk(*fSubRun->bulkUseToken(),
                                                fUploadTarget->tokenTracker()->nextDrawToken(),
                                                fSubRun->maskFormat());
         }
+        return {true, end - begin};
     }
-    if (ok) {
-        result->fFinished = fCurrGlyph == (int)fSubRun->fGlyphs.size();
-        result->fGlyphsRegenerated += fCurrGlyph - begin;
-        result->fFirstVertex = fSubRun->quadStart(begin);
-    }
-    return ok;
 }

src/gpu/text/GrTextBlob.h

@@ -300,29 +300,11 @@ public:
                       const SkMatrix& drawMatrix, SkPoint drawOrigin, GrColor color,
                       GrDeferredUploadTarget*, GrStrikeCache*, GrAtlasManager*);
 
-    struct Result {
+    // Return {success, number of glyphs regenerated}
-        /**
+    std::tuple<bool, int> regenerate(int begin, int end);
-         * Was regenerate() able to draw all the glyphs from the sub run? If not flush all glyph
-         * draws and call regenerate() again.
-         */
-        bool fFinished = false;
-
-        /**
-         * How many glyphs were regenerated. Will be equal to the sub run's glyph count if
-         * fType is kFinished.
-         */
-        int fGlyphsRegenerated = 0;
-
-        /**
-         * Pointer where the caller finds the first regenerated vertex.
-         */
-        const char* fFirstVertex = nullptr;
-    };
-
-    bool regenerate(Result*, int maxGlyphs = std::numeric_limits<int>::max());
 
 private:
-    // Return {success, index of first glyph not in atlas}
+    // Return {success, number of glyphs regenerated}
     std::tuple<bool, int> updateTextureCoordinatesMaybeStrike(int begin, int end);
 
     GrResourceProvider* fResourceProvider;
@@ -335,9 +317,6 @@ private:
         bool regenTextureCoordinates:1;
         bool regenStrike:1;
     } fActions = {false, false};
-
-    // fCurrGlyph indicates the next glyph to be placed in the atlas.
-    int fCurrGlyph = 0;
 };
 
 // -- GrTextBlob::SubRun ---------------------------------------------------------------------------
@@ -370,6 +349,7 @@ public:
     size_t colorOffset() const;
     size_t texCoordOffset() const;
     char* quadStart(size_t index) const;
+    size_t quadOffset(size_t index) const;
 
     const SkRect& vertexBounds() const;
     void joinGlyphBounds(const SkRect& glyphBounds);