Reland "store GlyphIDs in the PathOpSubmitter"

This is a reland of 24d732f4ca

I removed glyphIDs and Paths sharing memory. They are just in
separate arrays. Having them share causes a race in threaded
environments like DDLs. Now everything is const. This allows
passing just the SkDescriptor instead of the sk_sp<SkStrike>.

Original change's description:
> store GlyphIDs in the PathOpSubmitter
>
> In order to support serializing, have the PathOpSubmitter store
> SkPathIDs until the paths are needed for drawing, and then lookup
> the paths and store them in the memory that the IDs occupied.
> This is a one time operation. Wire the strike down from the
> SkGlyphRunPainter down to the PathOpSubmitter.
>
> Change-Id: Id9382fefc252eebafeb9792f380d24ec0939316d
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/512937
> Reviewed-by: Ben Wagner <bungeman@google.com>
> Commit-Queue: Herb Derby <herb@google.com>

Change-Id: I99ecfc1cb052b0d85d11403b5ef24d738f0a1d2d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/512942
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Herb Derby <herb@google.com>

src/core/SkGlyphRunPainter.cpp

@@ -420,7 +420,10 @@ void SkGlyphRunListPainter::processGlyphRun(SkGlyphRunPainterInterface* process,
             if (process && !fAccepted.empty()) {
                 // processSourcePaths must be called even if there are no glyphs to make sure
                 // runs are set correctly.
-                process->processSourcePaths(fAccepted.accepted(), runFont, strikeToSourceScale);
+                process->processSourcePaths(fAccepted.accepted(),
+                                            runFont,
+                                            strikeSpec.descriptor(),
+                                            strikeToSourceScale);
             }
         }
     }

src/core/SkGlyphRunPainter.h

@@ -144,6 +144,7 @@ public:
 
     virtual void processSourcePaths(const SkZip<SkGlyphVariant, SkPoint>& accepted,
                                     const SkFont& runFont,
+                                    const SkDescriptor& descriptor,
                                     SkScalar strikeToSourceScale) = 0;
 
     virtual void processSourceDrawables(const SkZip<SkGlyphVariant, SkPoint>& accepted,

src/gpu/text/GrTextBlob.cpp

@@ -406,18 +406,20 @@ std::tuple<bool, SkVector> can_use_direct(
 // -- PathOpSubmitter ------------------------------------------------------------------------------
 // Shared code for submitting GPU ops for drawing glyphs as paths.
 class PathOpSubmitter {
-    struct PathAndPosition;
+    union Variant;
+
 public:
     PathOpSubmitter(bool isAntiAliased,
                     SkScalar strikeToSourceScale,
-                    SkSpan<PathAndPosition> paths,
+                    SkSpan<SkPoint> positions,
-                    std::unique_ptr<PathAndPosition[], GrSubRunAllocator::ArrayDestroyer> pathData);
+                    SkSpan<SkGlyphID> glyphIDs,
-
+                    std::unique_ptr<SkPath[], GrSubRunAllocator::ArrayDestroyer> paths,
-    PathOpSubmitter(PathOpSubmitter&& that);
+                    const SkDescriptor& descriptor);
 
     static PathOpSubmitter Make(const SkZip<SkGlyphVariant, SkPoint>& accepted,
                                 bool isAntiAliased,
                                 SkScalar strikeToSourceScale,
+                                const SkDescriptor& descriptor,
                                 GrSubRunAllocator* alloc);
 
     void submitOps(SkCanvas*,
@@ -428,50 +430,52 @@ public:
                    skgpu::v1::SurfaceDrawContext* sdc) const;
 
 private:
-    struct PathAndPosition {
-        SkPath fPath;
-        SkPoint fPosition;
-        // Support for serialization.
-        SkPackedGlyphID fPackedID;
-    };
     const bool fIsAntiAliased;
     const SkScalar fStrikeToSourceScale;
-    const SkSpan<const PathAndPosition> fPaths;
+    const SkSpan<const SkPoint> fPositions;
-    std::unique_ptr<PathAndPosition[], GrSubRunAllocator::ArrayDestroyer> fPathData;
+    const SkSpan<const SkGlyphID> fGlyphIDs;
+    std::unique_ptr<SkPath[], GrSubRunAllocator::ArrayDestroyer> fPaths;
+    const SkAutoDescriptor fDescriptor;
 };
 
 PathOpSubmitter::PathOpSubmitter(
-        bool isAntiAliased,
+    bool isAntiAliased,
-        SkScalar strikeToSourceScale,
+    SkScalar strikeToSourceScale,
-        SkSpan<PathAndPosition> paths,
+    SkSpan<SkPoint> positions,
-        std::unique_ptr<PathAndPosition[], GrSubRunAllocator::ArrayDestroyer> pathData)
+    SkSpan<SkGlyphID> glyphIDs,
-            : fIsAntiAliased{isAntiAliased}
+    std::unique_ptr<SkPath[], GrSubRunAllocator::ArrayDestroyer> paths,
-            , fStrikeToSourceScale{strikeToSourceScale}
+    const SkDescriptor& descriptor)
-            , fPaths{paths}
+        : fIsAntiAliased{isAntiAliased}
-            , fPathData{std::move(pathData)} {
+        , fStrikeToSourceScale{strikeToSourceScale}
-    SkASSERT(!fPaths.empty());
+        , fPositions{positions}
+        , fGlyphIDs{glyphIDs}
+        , fPaths{std::move(paths)}
+        , fDescriptor {descriptor} {
+    SkASSERT(!fPositions.empty());
 }
 
-PathOpSubmitter::PathOpSubmitter(PathOpSubmitter&& that)
-    : fIsAntiAliased{that.fIsAntiAliased}
-    , fStrikeToSourceScale{that.fStrikeToSourceScale}
-    , fPaths{that.fPaths}
-    , fPathData{std::move(that.fPathData)} {}
-
 PathOpSubmitter PathOpSubmitter::Make(const SkZip<SkGlyphVariant, SkPoint>& accepted,
                                       bool isAntiAliased,
                                       SkScalar strikeToSourceScale,
+                                      const SkDescriptor& descriptor,
                                       GrSubRunAllocator* alloc) {
-    auto pathData = alloc->makeUniqueArray<PathAndPosition>(
+    int glyphCount = SkCount(accepted);
-            accepted.size(),
+    SkPoint* positions = alloc->makePODArray<SkPoint>(glyphCount);
-            [&](int i){
+    SkGlyphID* glyphIDs = alloc->makePODArray<SkGlyphID>(glyphCount);
-                auto [variant, pos] = accepted[i];
+    auto paths = alloc->makeUniqueArray<SkPath>(glyphCount);
-                const SkGlyph& glyph = *variant.glyph();
-                return PathAndPosition{*glyph.path(), pos, glyph.getPackedID()};
-            });
-    SkSpan<PathAndPosition> paths{pathData.get(), accepted.size()};
 
-    return PathOpSubmitter{isAntiAliased, strikeToSourceScale, paths, std::move(pathData)};
+    for (auto [i, variant, pos] : SkMakeEnumerate(accepted)) {
+        positions[i] = pos;
+        glyphIDs[i] = variant.glyph()->getGlyphID();
+        paths[i] = *variant.glyph()->path();
+    }
+
+    return PathOpSubmitter{isAntiAliased,
+                           strikeToSourceScale,
+                           SkMakeSpan(positions, glyphCount),
+                           SkMakeSpan(glyphIDs, glyphCount),
+                           std::move(paths),
+                           descriptor};
 }
 
 void PathOpSubmitter::submitOps(SkCanvas* canvas,
@@ -496,9 +500,7 @@ void PathOpSubmitter::submitOps(SkCanvas* canvas,
     SkMatrix strikeToSource = SkMatrix::Scale(fStrikeToSourceScale, fStrikeToSourceScale);
     strikeToSource.postTranslate(drawOrigin.x(), drawOrigin.y());
     if (!needsExactCTM) {
-        for (const auto& pathPos : fPaths) {
+        for (auto [path, pos] : SkMakeZip(fPaths.get(), fPositions)) {
-            const SkPath& path = pathPos.fPath;
-            const SkPoint pos = pathPos.fPosition;
             // Transform the glyph to source space.
             SkMatrix pathMatrix = strikeToSource;
             pathMatrix.postTranslate(pos.x(), pos.y());
@@ -510,9 +512,7 @@ void PathOpSubmitter::submitOps(SkCanvas* canvas,
     } else {
         // Transform the path to device because the deviceMatrix must be unchanged to
         // draw effect, filter or shader paths.
-        for (const auto& pathPos : fPaths) {
+        for (auto [path, pos] : SkMakeZip(fPaths.get(), fPositions)) {
-            const SkPath& path = pathPos.fPath;
-            const SkPoint pos = pathPos.fPosition;
             // Transform the glyph to source space.
             SkMatrix pathMatrix = strikeToSource;
             pathMatrix.postTranslate(pos.x(), pos.y());
@@ -533,9 +533,11 @@ public:
     static GrSubRunOwner Make(const SkZip<SkGlyphVariant, SkPoint>& accepted,
                               bool isAntiAliased,
                               SkScalar strikeToSourceScale,
+                              const SkDescriptor& descriptor,
                               GrSubRunAllocator* alloc) {
         return alloc->makeUnique<PathSubRun>(
-                PathOpSubmitter::Make(accepted, isAntiAliased, strikeToSourceScale, alloc));
+                PathOpSubmitter::Make(
+                        accepted, isAntiAliased, strikeToSourceScale, descriptor, alloc));
     }
 
     void draw(SkCanvas* canvas,
@@ -2026,9 +2028,10 @@ void GrTextBlob::processDeviceMasks(
 
 void GrTextBlob::processSourcePaths(const SkZip<SkGlyphVariant, SkPoint>& accepted,
                                     const SkFont& runFont,
+                                    const SkDescriptor& descriptor,
                                     SkScalar strikeToSourceScale) {
     fSubRunList.append(PathSubRun::Make(
-            accepted, has_some_antialiasing(runFont), strikeToSourceScale, &fAlloc));
+        accepted, has_some_antialiasing(runFont), strikeToSourceScale, descriptor, &fAlloc));
 }
 
 void GrTextBlob::processSourceDrawables(const SkZip<SkGlyphVariant, SkPoint>& accepted,
@@ -2733,11 +2736,13 @@ void GrSubRunNoCachePainter::processSourceMasks(const SkZip<SkGlyphVariant, SkPo
 
 void GrSubRunNoCachePainter::processSourcePaths(const SkZip<SkGlyphVariant, SkPoint>& accepted,
                                                 const SkFont& runFont,
+                                                const SkDescriptor& descriptor,
                                                 SkScalar strikeToSourceScale) {
     PathOpSubmitter pathDrawing =
             PathOpSubmitter::Make(accepted,
                                   has_some_antialiasing(runFont),
                                   strikeToSourceScale,
+                                  descriptor,
                                   fAlloc);
 
     pathDrawing.submitOps(fCanvas, fClip, fViewMatrix, fGlyphRunList.origin(), fPaint, fSDC);
@@ -2815,7 +2820,7 @@ public:
             SkScalar strikeToSourceScale) override;
     void processSourcePaths(
             const SkZip<SkGlyphVariant, SkPoint>& accepted, const SkFont& runFont,
-            SkScalar strikeToSourceScale) override;
+            const SkDescriptor& descriptor, SkScalar strikeToSourceScale) override;
     void processSourceDrawables(
             const SkZip<SkGlyphVariant, SkPoint>& drawables, const SkFont& runFont,
             SkScalar strikeToSourceScale) override;
@@ -3408,9 +3413,13 @@ sk_sp<Slug> Slug::Make(const SkMatrixProvider& viewMatrix,
 void Slug::processSourcePaths(const SkZip<SkGlyphVariant,
                               SkPoint>& accepted,
                               const SkFont& runFont,
+                              const SkDescriptor& descriptor,
                               SkScalar strikeToSourceScale) {
-    fSubRuns.append(PathSubRun::Make(
+    fSubRuns.append(PathSubRun::Make(accepted,
-            accepted, has_some_antialiasing(runFont), strikeToSourceScale, &fAlloc));
+                                     has_some_antialiasing(runFont),
+                                     strikeToSourceScale,
+                                     descriptor,
+                                     &fAlloc));
 }
 
 void Slug::processSourceDrawables(const SkZip<SkGlyphVariant, SkPoint>& accepted,

src/gpu/text/GrTextBlob.h

@@ -23,6 +23,7 @@
 #include "src/core/SkTInternalLList.h"
 #include "src/core/SkTLazy.h"
 #include "src/gpu/GrColor.h"
+#include "src/gpu/GrResourceProvider.h"
 #include "src/gpu/GrSubRunAllocator.h"
 #include "src/gpu/ops/GrOp.h"
 
@@ -262,6 +263,7 @@ private:
                             sk_sp<SkStrike>&& strike) override;
     void processSourcePaths(const SkZip<SkGlyphVariant, SkPoint>& accepted,
                             const SkFont& runFont,
+                            const SkDescriptor& descriptor,
                             SkScalar strikeToSourceScale) override;
     void processSourceDrawables(const SkZip<SkGlyphVariant, SkPoint>& accepted,
                                 const SkFont& runFont,
@@ -315,6 +317,7 @@ public:
                             SkScalar strikeToSourceScale) override;
     void processSourcePaths(const SkZip<SkGlyphVariant, SkPoint>& accepted,
                             const SkFont& runFont,
+                            const SkDescriptor& descriptor,
                             SkScalar strikeToSourceScale) override;
     void processSourceDrawables(const SkZip<SkGlyphVariant, SkPoint>& accepted,
                                 const SkFont& runFont,