Make GrTextBlob initial position const

* Convert from fInitialX, fInitialY to an SkPoint fInitialOrigin
* Start putting params in a consistent order
* inline and remove single use functions

Change-Id: I53eecf6098b475ef05f4f5c4133510ee1104073f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/257756
Auto-Submit: Herb Derby <herb@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>

src/core/SkGlyphRunPainter.cpp

@@ -425,7 +425,7 @@ void GrTextContext::drawGlyphRunList(
             // but we'd have to clear the subrun information
             textBlobCache->remove(cacheBlob.get());
             cacheBlob = textBlobCache->makeCachedBlob(
-                    glyphRunList, key, blurRec, listPaint, forceW, color, grStrikeCache);
+                    glyphRunList, grStrikeCache, key, blurRec, color, forceW);
             cacheBlob->generateFromGlyphRunList(
                     *context->priv().caps()->shaderCaps(), fOptions,
                     listPaint, viewMatrix, props,
@@ -435,7 +435,7 @@ void GrTextContext::drawGlyphRunList(
 
             if (CACHE_SANITY_CHECK) {
                 sk_sp<GrTextBlob> sanityBlob(textBlobCache->makeBlob(
-                        glyphRunList, forceW, color, grStrikeCache));
+                        glyphRunList, grStrikeCache, color, forceW));
                 sanityBlob->setupKey(key, blurRec, listPaint);
                 cacheBlob->generateFromGlyphRunList(
                         *context->priv().caps()->shaderCaps(), fOptions,
@@ -447,9 +447,9 @@ void GrTextContext::drawGlyphRunList(
     } else {
         if (canCache) {
             cacheBlob = textBlobCache->makeCachedBlob(
-                    glyphRunList, key, blurRec, listPaint, forceW, color, grStrikeCache);
+                    glyphRunList, grStrikeCache, key, blurRec, color, forceW);
         } else {
-            cacheBlob = textBlobCache->makeBlob(glyphRunList, forceW, color, grStrikeCache);
+            cacheBlob = textBlobCache->makeBlob(glyphRunList, grStrikeCache, color, forceW);
         }
         cacheBlob->generateFromGlyphRunList(
                 *context->priv().caps()->shaderCaps(), fOptions, listPaint,
@@ -468,10 +468,8 @@ void GrTextBlob::generateFromGlyphRunList(const GrShaderCaps& shaderCaps,
                                           const SkSurfaceProps& props,
                                           const SkGlyphRunList& glyphRunList,
                                           SkGlyphRunListPainter* glyphPainter) {
-    SkPoint origin = glyphRunList.origin();
     const SkPaint& runPaint = glyphRunList.paint();
-    this->initReusableBlob(SkPaintPriv::ComputeLuminanceColor(runPaint), viewMatrix,
+    this->initReusableBlob(SkPaintPriv::ComputeLuminanceColor(runPaint), viewMatrix);
-                           origin.x(), origin.y());
 
     glyphPainter->processGlyphRunList(glyphRunList,
                                       viewMatrix,
@@ -494,8 +492,8 @@ GrTextBlob::SubRun::SubRun(SubRunType type, GrTextBlob* textBlob, const SkStrike
         , fStrikeSpec{strikeSpec}
         , fStrike{grStrike}
         , fColor{textBlob->fColor}
-        , fX{textBlob->fInitialX}
+        , fX{textBlob->fInitialOrigin.x()}
-        , fY{textBlob->fInitialY}
+        , fY{textBlob->fInitialOrigin.y()}
         , fCurrentViewMatrix{textBlob->fInitialViewMatrix} {
     SkASSERT(type != kTransformedPath);
 }
@@ -704,7 +702,7 @@ std::unique_ptr<GrDrawOp> GrTextContext::createOp_TestingOnly(GrRecordingContext
     auto glyphRunList = builder.useGlyphRunList();
     sk_sp<GrTextBlob> blob;
     if (!glyphRunList.empty()) {
-        blob = direct->priv().getTextBlobCache()->makeBlob(glyphRunList, false, color, strikeCache);
+        blob = direct->priv().getTextBlobCache()->makeBlob(glyphRunList, strikeCache, color, false);
         blob->generateFromGlyphRunList(
                 *context->priv().caps()->shaderCaps(), textContext->fOptions,
                 skPaint, viewMatrix, surfaceProps,

src/gpu/text/GrTextBlob.cpp

@@ -24,9 +24,10 @@ template <size_t N> static size_t sk_align(size_t s) {
 }
 
 sk_sp<GrTextBlob> GrTextBlob::Make(int glyphCount,
-                                   bool forceWForDistanceFields,
+                                   GrStrikeCache* strikeCache,
+                                   SkPoint origin,
                                    GrColor color,
-                                   GrStrikeCache* strikeCache) {
+                                   bool forceWForDistanceFields) {
     // We allocate size for the GrTextBlob itself, plus size for the vertices array,
     // and size for the glyphIds array.
     size_t verticesCount = glyphCount * kVerticesPerGlyph * kMaxVASize;
@@ -43,7 +44,7 @@ sk_sp<GrTextBlob> GrTextBlob::Make(int glyphCount,
     }
 
     sk_sp<GrTextBlob> blob{new (allocation) GrTextBlob{
-        size, strikeCache, color, forceWForDistanceFields}};
+        size, strikeCache, origin, color, forceWForDistanceFields}};
 
     // setup offsets for vertices / glyphs
     blob->fVertices = SkTAddOffset<char>(blob.get(), vertex);
@@ -89,11 +90,8 @@ bool GrTextBlob::mustRegenerate(const SkPaint& paint, bool anyRunHasSubpixelPosi
     // Mixed blobs must be regenerated.  We could probably figure out a way to do integer scrolls
     // for mixed blobs if this becomes an issue.
     if (this->hasBitmap() && this->hasDistanceField()) {
-        // Identical viewmatrices and we can reuse in all cases
+        // Identical view matrices and we can reuse in all cases
-        if (fInitialViewMatrix.cheapEqualTo(viewMatrix) && x == fInitialX && y == fInitialY) {
+        return !(fInitialViewMatrix.cheapEqualTo(viewMatrix) && SkPoint{x, y} == fInitialOrigin);
-            return false;
-        }
-        return true;
     }
 
     if (this->hasBitmap()) {
@@ -114,12 +112,12 @@ bool GrTextBlob::mustRegenerate(const SkPaint& paint, bool anyRunHasSubpixelPosi
         // already generated vertex coordinates to move them to the correct position.
         // Figure out the translation in view space given a translation in source space.
         SkScalar transX = viewMatrix.getTranslateX() +
-                          viewMatrix.getScaleX() * (x - fInitialX) +
+                          viewMatrix.getScaleX() * (x - fInitialOrigin.x()) +
-                          viewMatrix.getSkewX() * (y - fInitialY) -
+                          viewMatrix.getSkewX() * (y - fInitialOrigin.y()) -
                           fInitialViewMatrix.getTranslateX();
         SkScalar transY = viewMatrix.getTranslateY() +
-                          viewMatrix.getSkewY() * (x - fInitialX) +
+                          viewMatrix.getSkewY() * (x - fInitialOrigin.x()) +
-                          viewMatrix.getScaleY() * (y - fInitialY) -
+                          viewMatrix.getScaleY() * (y - fInitialOrigin.y()) -
                           fInitialViewMatrix.getTranslateY();
         if (!SkScalarIsInt(transX) || !SkScalarIsInt(transY)) {
             return true;
@@ -212,7 +210,7 @@ void GrTextBlob::flush(GrTextTarget* target, const SkSurfaceProps& props,
                              || runPaint.getMaskFilter();
 
             // The origin for the blob may have changed, so figure out the delta.
-            SkVector originShift = SkPoint{x, y} - SkPoint{fInitialX, fInitialY};
+            SkVector originShift = SkPoint{x, y} - fInitialOrigin;
 
             for (const auto& pathGlyph : subRun.fPaths) {
                 SkMatrix ctm{viewMatrix};
@@ -354,13 +352,48 @@ void GrTextBlob::AssertEqual(const GrTextBlob& l, const GrTextBlob& r) {
     }
 }
 
-GrTextBlob::GrTextBlob(size_t size, GrStrikeCache* strikeCache, GrColor color,
+GrTextBlob::GrTextBlob(size_t size,
+                       GrStrikeCache* strikeCache,
+                       SkPoint origin,
+                       GrColor color,
                        bool forceWForDistanceFields)
         : fSize{size}
         , fStrikeCache{strikeCache}
+        , fInitialOrigin{origin}
         , fForceWForDistanceFields{forceWForDistanceFields}
         , fColor{color} { }
 
+void GrTextBlob::computeSubRunBounds(SkRect* outBounds, const GrTextBlob::SubRun& subRun,
+                                     const SkMatrix& viewMatrix, SkScalar x, SkScalar y,
+                                     bool needsGlyphTransform) {
+    // We don't yet position distance field text on the cpu, so we have to map the vertex bounds
+    // into device space.
+    // We handle vertex bounds differently for distance field text and bitmap text because
+    // the vertex bounds of bitmap text are in device space.  If we are flushing multiple runs
+    // from one blob then we are going to pay the price here of mapping the rect for each run.
+    *outBounds = subRun.vertexBounds();
+    if (needsGlyphTransform) {
+        // Distance field text is positioned with the (X,Y) as part of the glyph position,
+        // and currently the view matrix is applied on the GPU
+        outBounds->offset(SkPoint{x, y} - fInitialOrigin);
+        viewMatrix.mapRect(outBounds);
+    } else {
+        // Bitmap text is fully positioned on the CPU, and offset by an (X,Y) translate in
+        // device space.
+        SkMatrix boundsMatrix = fInitialViewMatrixInverse;
+
+        boundsMatrix.postTranslate(-fInitialOrigin.x(), -fInitialOrigin.y());
+
+        boundsMatrix.postTranslate(x, y);
+
+        boundsMatrix.postConcat(viewMatrix);
+        boundsMatrix.mapRect(outBounds);
+
+        // Due to floating point numerical inaccuracies, we have to round out here
+        outBounds->roundOut(outBounds);
+    }
+}
+
 void GrTextBlob::SubRun::computeTranslation(const SkMatrix& viewMatrix,
                                                 SkScalar x, SkScalar y, SkScalar* transX,
                                                 SkScalar* transY) {

src/gpu/text/GrTextBlob.h

@@ -68,11 +68,11 @@ public:
 
     // Make an empty GrTextBlob, with all the invariants set to make the right decisions when
     // adding SubRuns.
-    static sk_sp<GrTextBlob> Make(
+    static sk_sp<GrTextBlob> Make(int glyphCount,
-            int glyphCount,
+                                  GrStrikeCache* strikeCache,
-            bool forceWForDistanceFields,
+                                  SkPoint origin,
-            GrColor color,
+                                  GrColor color,
-            GrStrikeCache* strikeCache);
+                                  bool forceWForDistanceFields);
 
     struct Key {
         Key() {
@@ -163,34 +163,7 @@ public:
 
     void computeSubRunBounds(SkRect* outBounds, const SubRun& subRun,
                              const SkMatrix& viewMatrix, SkScalar x, SkScalar y,
-                             bool needsGlyphTransform) {
+                             bool needsGlyphTransform);
-        // We don't yet position distance field text on the cpu, so we have to map the vertex bounds
-        // into device space.
-        // We handle vertex bounds differently for distance field text and bitmap text because
-        // the vertex bounds of bitmap text are in device space.  If we are flushing multiple runs
-        // from one blob then we are going to pay the price here of mapping the rect for each run.
-        *outBounds = subRun.vertexBounds();
-        if (needsGlyphTransform) {
-            // Distance field text is positioned with the (X,Y) as part of the glyph position,
-            // and currently the view matrix is applied on the GPU
-            outBounds->offset(x - fInitialX, y - fInitialY);
-            viewMatrix.mapRect(outBounds);
-        } else {
-            // Bitmap text is fully positioned on the CPU, and offset by an (X,Y) translate in
-            // device space.
-            SkMatrix boundsMatrix = fInitialViewMatrixInverse;
-
-            boundsMatrix.postTranslate(-fInitialX, -fInitialY);
-
-            boundsMatrix.postTranslate(x, y);
-
-            boundsMatrix.postConcat(viewMatrix);
-            boundsMatrix.mapRect(outBounds);
-
-            // Due to floating point numerical inaccuracies, we have to round out here
-            outBounds->roundOut(outBounds);
-        }
-    }
 
     // position + local coord
     static const size_t kColorTextVASize = sizeof(SkPoint) + sizeof(SkIPoint16);
@@ -204,17 +177,19 @@ public:
 
     static void AssertEqual(const GrTextBlob&, const GrTextBlob&);
 
+    // This function will only be called when we are generating a blob from scratch.
     // The color here is the GrPaint color, and it is used to determine whether we
     // have to regenerate LCD text blobs.
-    // We use this color vs the SkPaint color because it has the colorfilter applied.
+    // We use this color vs the SkPaint color because it has the color filter applied. We record the
-    void initReusableBlob(SkColor luminanceColor, const SkMatrix& viewMatrix,
+    // initial view matrix and initial offsets(x,y), because we record vertex bounds relative to
-                          SkScalar x, SkScalar y) {
+    // these numbers.  When blobs are reused with new matrices, we need to return to source space so
+    // we can update the vertex bounds appropriately.
+    void initReusableBlob(SkColor luminanceColor, const SkMatrix& viewMatrix) {
         fLuminanceColor = luminanceColor;
-        this->setupViewMatrix(viewMatrix, x, y);
+        fInitialViewMatrix = viewMatrix;
-    }
+        if (!viewMatrix.invert(&fInitialViewMatrixInverse)) {
-
+            fInitialViewMatrixInverse = SkMatrix::I();
-    void initThrowawayBlob(const SkMatrix& viewMatrix, SkScalar x, SkScalar y) {
+        }
-        this->setupViewMatrix(viewMatrix, x, y);
     }
 
     const Key& key() const { return fKey; }
@@ -377,8 +352,11 @@ public:
                  SkScalar maxScale);
 
 private:
-    GrTextBlob(
+    GrTextBlob(size_t size,
-            size_t size, GrStrikeCache* strikeCache, GrColor color, bool forceWForDistanceFields);
+               GrStrikeCache* strikeCache,
+               SkPoint origin,
+               GrColor color,
+               bool forceWForDistanceFields);
 
     struct StrokeInfo {
         SkScalar fFrameWidth;
@@ -391,19 +369,6 @@ private:
         kHasBitmap_TextType = 0x2,
     };
 
-    // This function will only be called when we are generating a blob from scratch. We record the
-    // initial view matrix and initial offsets(x,y), because we record vertex bounds relative to
-    // these numbers.  When blobs are reused with new matrices, we need to return to model space so
-    // we can update the vertex bounds appropriately.
-    void setupViewMatrix(const SkMatrix& viewMatrix, SkScalar x, SkScalar y) {
-        fInitialViewMatrix = viewMatrix;
-        if (!viewMatrix.invert(&fInitialViewMatrixInverse)) {
-            fInitialViewMatrixInverse = SkMatrix::I();
-        }
-        fInitialX = x;
-        fInitialY = y;
-    }
-
     std::unique_ptr<GrAtlasTextOp> makeOp(
             SubRun& info, int glyphCount,
             const SkMatrix& viewMatrix, SkScalar x, SkScalar y, const SkIRect& clipRect,
@@ -433,6 +398,10 @@ private:
     // The GrRecordingContext also owns the GrTextBlob cache which owns this GrTextBlob.
     GrStrikeCache* const fStrikeCache;
 
+    // Initial position of this blob. Used for calculating position differences when reusing this
+    // blob.
+    const SkPoint fInitialOrigin;
+
     // From the distance field options to force distance fields to have a W coordinate.
     const bool fForceWForDistanceFields;
 
@@ -457,10 +426,9 @@ private:
     SkMatrix fInitialViewMatrix;
     SkMatrix fInitialViewMatrixInverse;
     SkColor fLuminanceColor;
-    SkScalar fInitialX;
-    SkScalar fInitialY;
 
-    // We can reuse distance field text, but only if the new viewmatrix would not result in
+
+    // We can reuse distance field text, but only if the new view matrix would not result in
     // a mip change.  Because there can be multiple runs in a blob, we track the overall
     // maximum minimum scale, and minimum maximum scale, we can support before we need to regen
     SkScalar fMaxMinScale{-SK_ScalarMax};

src/gpu/text/GrTextBlobCache.h

@@ -34,21 +34,22 @@ public:
     ~GrTextBlobCache();
 
     sk_sp<GrTextBlob> makeBlob(const SkGlyphRunList& glyphRunList,
-                               bool forceW,
+                               GrStrikeCache* strikeCache,
                                GrColor color,
-                               GrStrikeCache* strikeCache) {
+                               bool forceW) {
-        return GrTextBlob::Make(glyphRunList.totalGlyphCount(), forceW, color, strikeCache);
+        return GrTextBlob::Make(
+                glyphRunList.totalGlyphCount(), strikeCache, glyphRunList.origin(), color, forceW);
     }
 
     sk_sp<GrTextBlob> makeCachedBlob(const SkGlyphRunList& glyphRunList,
+                                     GrStrikeCache* strikeCache,
                                      const GrTextBlob::Key& key,
                                      const SkMaskFilterBase::BlurRec& blurRec,
-                                     const SkPaint& paint,
-                                     bool forceW,
                                      GrColor color,
-                                     GrStrikeCache* strikeCache) {
+                                     bool forceW) {
-        sk_sp<GrTextBlob> cacheBlob(makeBlob(glyphRunList, forceW, color, strikeCache));
+        sk_sp<GrTextBlob> cacheBlob(
-        cacheBlob->setupKey(key, blurRec, paint);
+                this->makeBlob(glyphRunList, strikeCache, color, forceW));
+        cacheBlob->setupKey(key, blurRec, glyphRunList.paint());
         this->add(cacheBlob);
         glyphRunList.temporaryShuntBlobNotifyAddedToCache(fUniqueID);
         return cacheBlob;