combine direct position calculation

Make the glyph buffers use the same Direct position calculations
for Bitmap and GPU processing. This will create fewer changes when
switching to SkGlyphDigest based API.

Bug: skia:13192

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

src/core/BUILD.bazel

@@ -2225,6 +2225,7 @@ generated_cc_atom(
         ":SkGlyph_hdr",
         ":SkScalerContext_hdr",
         "//include/core:SkDrawable_hdr",
+        "//include/core:SkScalar_hdr",
         "//src/pathops:SkPathOpsCubic_hdr",
         "//src/pathops:SkPathOpsQuad_hdr",
     ],

src/core/SkGlyph.cpp

@@ -8,6 +8,7 @@
 #include "src/core/SkGlyph.h"
 
 #include "include/core/SkDrawable.h"
+#include "include/core/SkScalar.h"
 #include "src/core/SkArenaAlloc.h"
 #include "src/core/SkScalerContext.h"
 #include "src/pathops/SkPathOpsCubic.h"
@@ -29,6 +30,7 @@ SkMask SkGlyph::mask() const {
 }
 
 SkMask SkGlyph::mask(SkPoint position) const {
+    SkASSERT(SkScalarIsInt(position.x()) && SkScalarIsInt(position.y()));
     SkMask answer = this->mask();
     answer.fBounds.offset(SkScalarFloorToInt(position.x()), SkScalarFloorToInt(position.y()));
     return answer;

src/core/SkGlyphBuffer.cpp

@@ -40,49 +40,21 @@ void SkDrawableGlyphBuffer::startSource(const SkZip<const SkGlyphID, const SkPoi
     SkDEBUGCODE(fPhase = kInput);
 }
 
-void SkDrawableGlyphBuffer::startBitmapDevice(
+void SkDrawableGlyphBuffer::startDevicePositioning(
         const SkZip<const SkGlyphID, const SkPoint>& source,
-        SkPoint origin, const SkMatrix& viewMatrix,
+        const SkMatrix& positionMatrix,
-        const SkGlyphPositionRoundingSpec& roundingSpec) {
-    fInputSize = source.size();
-    fAcceptedSize = 0;
-
-    // Map the positions including subpixel position.
-    auto positions = source.get<1>();
-    SkMatrix matrix = viewMatrix;
-    matrix.preTranslate(origin.x(), origin.y());
-    SkPoint halfSampleFreq = roundingSpec.halfAxisSampleFreq;
-    matrix.postTranslate(halfSampleFreq.x(), halfSampleFreq.y());
-    matrix.mapPoints(fPositions, positions.data(), positions.size());
-
-    // Mask for controlling axis alignment.
-    SkIPoint mask = roundingSpec.ignorePositionFieldMask;
-
-    // Convert glyph ids and positions to packed glyph ids.
-    SkZip<const SkGlyphID, const SkPoint> withMappedPos =
-            SkMakeZip(source.get<0>(), fPositions.get());
-    SkGlyphVariant* packedIDCursor = fMultiBuffer.get();
-    for (auto [glyphID, pos] : withMappedPos) {
-        *packedIDCursor++ = SkPackedGlyphID{glyphID, pos, mask};
-    }
-    SkDEBUGCODE(fPhase = kInput);
-}
-
-void SkDrawableGlyphBuffer::startGPUDevice(
-        const SkZip<const SkGlyphID, const SkPoint>& source,
-        const SkMatrix& drawMatrix,
         const SkGlyphPositionRoundingSpec& roundingSpec) {
     fInputSize = source.size();
     fAcceptedSize = 0;
 
     // Build up the mapping from source space to device space. Add the rounding constant
-    // halfSampleFreq so we just need to floor to get the device result.
+    // halfSampleFreq, so we just need to floor to get the device result.
-    SkMatrix device = drawMatrix;
+    SkMatrix positionMatrixWithRounding = positionMatrix;
     SkPoint halfSampleFreq = roundingSpec.halfAxisSampleFreq;
-    device.postTranslate(halfSampleFreq.x(), halfSampleFreq.y());
+    positionMatrixWithRounding.postTranslate(halfSampleFreq.x(), halfSampleFreq.y());
 
     auto positions = source.get<1>();
-    device.mapPoints(fPositions, positions.data(), positions.size());
+    positionMatrixWithRounding.mapPoints(fPositions, positions.data(), positions.size());
 
     auto floor = [](SkPoint pt) -> SkPoint {
         return {SkScalarFloorToScalar(pt.x()), SkScalarFloorToScalar(pt.y())};

src/core/SkGlyphBuffer.h

@@ -145,15 +145,9 @@ public:
     // during drawing.
     void startSource(const SkZip<const SkGlyphID, const SkPoint>& source);
 
-    // Load the buffer with SkPackedGlyphIDs and positions using the device transform.
+    // Load the buffer with SkPackedGlyphIDs, calculating positions, so they can be constant.
-    void startBitmapDevice(
-            const SkZip<const SkGlyphID, const SkPoint>& source,
-            SkPoint origin, const SkMatrix& viewMatrix,
-            const SkGlyphPositionRoundingSpec& roundingSpec);
-
-    // Load the buffer with SkPackedGlyphIDs, calculating positions so they can be constant.
     //
-    // The positions are calculated integer positions in devices space, and the mapping of the
+    // The positions are calculated integer positions in devices space, and the mapping of
     // the source origin through the initial matrix is returned. It is given that these positions
     // are only reused when the blob is translated by an integral amount. Thus, the shifted
     // positions are given by the following equation where (ix, iy) is the integer positions of
@@ -165,9 +159,11 @@ public:
     //
     // In theory, newMappedOrigin - initialMappedOrigin should be integer, but the vagaries of
     // floating point don't guarantee that, so force it to integer.
-    void startGPUDevice(
+    //
+    // N.B. The positionMatrix is already translated by the origin of the glyph run list.
+    void startDevicePositioning(
             const SkZip<const SkGlyphID, const SkPoint>& source,
-            const SkMatrix& drawMatrix,
+            const SkMatrix& positionMatrix,
             const SkGlyphPositionRoundingSpec& roundingSpec);
 
     SkString dumpInput() const;

src/core/SkGlyphRunPainter.cpp

@@ -82,7 +82,7 @@ SkGlyphRunListPainter::SkGlyphRunListPainter(const skgpu::v1::SurfaceDrawContext
 
 void SkGlyphRunListPainter::drawForBitmapDevice(
         SkCanvas* canvas, const BitmapDevicePainter* bitmapDevice,
-        const SkGlyphRunList& glyphRunList, const SkPaint& paint, const SkMatrix& deviceMatrix) {
+        const SkGlyphRunList& glyphRunList, const SkPaint& paint, const SkMatrix& drawMatrix) {
     ScopedBuffers _ = this->ensureBuffers(glyphRunList);
 
     // TODO: fStrikeCache is only used for GPU, and some compilers complain about it during the no
@@ -96,12 +96,14 @@ void SkGlyphRunListPainter::drawForBitmapDevice(
                   : fBitmapFallbackProps;
 
     SkPoint drawOrigin = glyphRunList.origin();
+    SkMatrix positionMatrix{drawMatrix};
+    positionMatrix.preTranslate(drawOrigin.x(), drawOrigin.y());
     for (auto& glyphRun : glyphRunList) {
         const SkFont& runFont = glyphRun.font();
 
         fRejected.setSource(glyphRun.source());
 
-        if (SkStrikeSpec::ShouldDrawAsPath(paint, runFont, deviceMatrix)) {
+        if (SkStrikeSpec::ShouldDrawAsPath(paint, runFont, positionMatrix)) {
 
             auto [strikeSpec, strikeToSourceScale] =
                     SkStrikeSpec::MakePath(runFont, paint, props, fScalerContextFlags);
@@ -168,21 +170,20 @@ void SkGlyphRunListPainter::drawForBitmapDevice(
                 }
             }
         }
-        if (!fRejected.source().empty() && !deviceMatrix.hasPerspective()) {
+        if (!fRejected.source().empty() && !positionMatrix.hasPerspective()) {
             SkStrikeSpec strikeSpec = SkStrikeSpec::MakeMask(
-                    runFont, paint, props, fScalerContextFlags, deviceMatrix);
+                    runFont, paint, props, fScalerContextFlags, positionMatrix);
 
             auto strike = strikeSpec.findOrCreateStrike();
 
-            fAccepted.startBitmapDevice(
+            fAccepted.startDevicePositioning(
-                    fRejected.source(), drawOrigin, deviceMatrix, strike->roundingSpec());
+                    fRejected.source(), positionMatrix, strike->roundingSpec());
+
             strike->prepareForDrawingMasksCPU(&fAccepted);
             fRejected.flipRejectsToSource();
             bitmapDevice->paintMasks(&fAccepted, paint);
         }
         if (!fRejected.source().empty()) {
-            SkMatrix runMatrix = deviceMatrix;
-            runMatrix.preTranslate(drawOrigin.x(), drawOrigin.y());
             std::vector<SkPoint> sourcePositions;
 
             // Create a strike is source space to calculate scale information.
@@ -203,7 +204,7 @@ void SkGlyphRunListPainter::drawForBitmapDevice(
                 sourcePositions.push_back(srcPos);
                 SkRect rect = glyph->rect();
                 rect.makeOffset(srcPos);
-                runMatrix.mapRectToQuad(corners, rect);
+                positionMatrix.mapRectToQuad(corners, rect);
                 // left top -> right top
                 SkScalar scale = (corners[1] - corners[0]).length() / rect.width();
                 maxScale = std::max(maxScale, scale);
@@ -229,8 +230,8 @@ void SkGlyphRunListPainter::drawForBitmapDevice(
             auto strike = strikeSpec.findOrCreateStrike();
 
             // Figure out all the positions and packed glyphIDs based on the device matrix.
-            fAccepted.startBitmapDevice(
+            fAccepted.startDevicePositioning(
-                    fRejected.source(), drawOrigin, deviceMatrix, strike->roundingSpec());
+                    fRejected.source(), positionMatrix, strike->roundingSpec());
 
             strike->prepareForDrawingMasksCPU(&fAccepted);
             auto variants = fAccepted.accepted().get<0>();
@@ -277,7 +278,7 @@ void SkGlyphRunListPainter::drawForBitmapDevice(
 #if SK_SUPPORT_GPU
 void SkGlyphRunListPainter::processGlyphRun(SkGlyphRunPainterInterface* process,
                                             const SkGlyphRun& glyphRun,
-                                            const SkMatrix& drawMatrix,
+                                            const SkMatrix& positionMatrix,
                                             const SkPaint& runPaint,
                                             const GrSDFTControl& control,
                                             const char* tag,
@@ -293,8 +294,8 @@ void SkGlyphRunListPainter::processGlyphRun(SkGlyphRunPainterInterface* process,
         }
         msg.appendf("\n   matrix\n");
         msg.appendf("   %7.3g %7.3g %7.3g\n   %7.3g %7.3g %7.3g\n",
-                    drawMatrix[0], drawMatrix[1], drawMatrix[2],
+                    positionMatrix[0], positionMatrix[1], positionMatrix[2],
-                    drawMatrix[3], drawMatrix[4], drawMatrix[5]);
+                    positionMatrix[3], positionMatrix[4], positionMatrix[5]);
     #endif
     ScopedBuffers _ = this->ensureBuffers(glyphRun);
     fRejected.setSource(glyphRun.source());
@@ -302,14 +303,14 @@ void SkGlyphRunListPainter::processGlyphRun(SkGlyphRunPainterInterface* process,
 
     // Only consider using direct or SDFT drawing if not drawing hairlines and not perspective.
     if ((runPaint.getStyle() != SkPaint::kStroke_Style || runPaint.getStrokeWidth() != 0)
-             && !drawMatrix.hasPerspective()) {
+             && !positionMatrix.hasPerspective()) {
         SkScalar approximateDeviceTextSize =
-                SkFontPriv::ApproximateTransformedTextSize(runFont, drawMatrix);
+                SkFontPriv::ApproximateTransformedTextSize(runFont, positionMatrix);
 
         if (control.isSDFT(approximateDeviceTextSize, runPaint)) {
             // Process SDFT - This should be the .009% case.
             const auto& [strikeSpec, strikeToSourceScale, matrixRange] =
-                    SkStrikeSpec::MakeSDFT(runFont, runPaint, fDeviceProps, drawMatrix, control);
+                SkStrikeSpec::MakeSDFT(runFont, runPaint, fDeviceProps, positionMatrix, control);
 
             #if defined(SK_TRACE_GLYPH_RUN_PROCESS)
                 msg.appendf("  SDFT case:\n%s", strikeSpec.dump().c_str());
@@ -342,7 +343,7 @@ void SkGlyphRunListPainter::processGlyphRun(SkGlyphRunPainterInterface* process,
             // This will handle medium size emoji that are sharing the run with SDFT drawn text.
             // If things are too big they will be passed along to the drawing of last resort below.
             SkStrikeSpec strikeSpec = SkStrikeSpec::MakeMask(
-                    runFont, runPaint, fDeviceProps, fScalerContextFlags, drawMatrix);
+                    runFont, runPaint, fDeviceProps, fScalerContextFlags, positionMatrix);
 
             #if defined(SK_TRACE_GLYPH_RUN_PROCESS)
                 msg.appendf("  Mask case:\n%s", strikeSpec.dump().c_str());
@@ -350,7 +351,8 @@ void SkGlyphRunListPainter::processGlyphRun(SkGlyphRunPainterInterface* process,
 
             SkScopedStrikeForGPU strike = strikeSpec.findOrCreateScopedStrike(fStrikeCache);
 
-            fAccepted.startGPUDevice(fRejected.source(), drawMatrix, strike->roundingSpec());
+            fAccepted.startDevicePositioning(
+                    fRejected.source(), positionMatrix, strike->roundingSpec());
             #if defined(SK_TRACE_GLYPH_RUN_PROCESS)
                 msg.appendf("    glyphs:(x,y):\n      %s\n", fAccepted.dumpInput().c_str());
             #endif

src/core/SkGlyphRunPainter.h

@@ -79,14 +79,15 @@ public:
 
     void drawForBitmapDevice(
             SkCanvas* canvas, const BitmapDevicePainter* bitmapDevice,
-            const SkGlyphRunList& glyphRunList, const SkPaint& paint, const SkMatrix& deviceMatrix);
+            const SkGlyphRunList& glyphRunList, const SkPaint& paint, const SkMatrix& drawMatrix);
 
 #if SK_SUPPORT_GPU
     // A nullptr for process means that the calls to the cache will be performed, but none of the
     // callbacks will be called.
+    // N.B. The positionMatrix has already been translated to the glyph run list origin.
     void processGlyphRun(SkGlyphRunPainterInterface* process,
                          const SkGlyphRun& glyphRun,
-                         const SkMatrix& drawMatrix,
+                         const SkMatrix& positionMatrix,
                          const SkPaint& drawPaint,
                          const GrSDFTControl& control,
                          const char* tag = nullptr,

tests/SkGlyphBufferTest.cpp

@@ -172,12 +172,16 @@ DEF_TEST(SkDrawableGlyphBufferBasic, reporter) {
         accepted.ensureSize(100);
         SkMatrix matrix = SkMatrix::Scale(0.5, 0.5);
         SkGlyphPositionRoundingSpec rounding{true, SkAxisAlignment::kX};
-        accepted.startBitmapDevice(source, {100, 100}, matrix, rounding);
+        SkMatrix positionMatrix{matrix};
+        positionMatrix.preTranslate(100, 100);
+        accepted.startDevicePositioning(source, positionMatrix, rounding);
         for (auto [i, packedID, pos] : SkMakeEnumerate(accepted.input())) {
             REPORTER_ASSERT(reporter, glyphIDs[i] == packedID.packedID().glyphID());
             REPORTER_ASSERT(reporter,
-                    pos.x() == positions[i].x() * 0.5 + 50 + SkPackedGlyphID::kSubpixelRound);
+                pos.x() == SkScalarFloorToInt(positions[i].x() * 0.5 + 50 +
-            REPORTER_ASSERT(reporter, pos.y() == positions[i].y() * 0.5 + 50 + 0.5);
+                                              SkPackedGlyphID::kSubpixelRound));
+            REPORTER_ASSERT(reporter,
+                            pos.y() == SkScalarFloorToInt(positions[i].y() * 0.5 + 50 + 0.5));
         }
     }
 

tests/SkScalerCacheTest.cpp

@@ -69,8 +69,8 @@ DEF_TEST(SkScalerCacheMultiThread, Reporter) {
                 accepted.ensureSize(glyphCount);
                 rejected.setSource(local);
 
-                accepted.startBitmapDevice(rejected.source(), {0, 0}, SkMatrix::I(),
+                accepted.startDevicePositioning(
-                                           scalerCache.roundingSpec());
+                        rejected.source(), SkMatrix::I(), scalerCache.roundingSpec());
                 scalerCache.prepareForMaskDrawing(&accepted, &rejected);
                 rejected.flipRejectsToSource();
                 accepted.reset();