font remoting: Fix errors with sub-pixel positioning mismatch.

1) Use correct offset from SkTextBlobRunIterator. Only
kHorizontal_Positioning needs to use the y offset specified on the run.

2) Use the same mapping function from SkFindAndPlaceGlyph for mapping
the glyph position from transform for the run. This avoids minor
rounding differences between analysis and draw.

R=bungeman@google.com,herb@google.com

Bug:829622
Change-Id: Iaef74a8d01f0850ce042149458b2ea795c2a2b61
Reviewed-on: https://skia-review.googlesource.com/131169
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Khusal Sagar <khushalsagar@chromium.org>

2 files changed, 36 insertions, 59 deletions

diff --git a/src/core/SkFindAndPlaceGlyph.h b/src/core/SkFindAndPlaceGlyph.h
index 69debe4d59..6b157358e8 100644
--- a/src/core/SkFindAndPlaceGlyph.h
+++ b/src/core/SkFindAndPlaceGlyph.h
@@ -85,6 +85,33 @@ public:
         return {0.0f, 0.0f};
     }
 
+    // MapperInterface given a point map it through the matrix. There are several shortcut
+    // variants.
+    // * TranslationMapper - assumes a translation only matrix.
+    // * XScaleMapper - assumes an X scaling and a translation.
+    // * GeneralMapper - Does all other matricies.
+    class MapperInterface {
+    public:
+        virtual ~MapperInterface() {}
+
+        virtual SkPoint map(SkPoint position) const = 0;
+    };
+
+    static MapperInterface* CreateMapper(const SkMatrix& matrix, const SkPoint& offset,
+                                         int scalarsPerPosition, SkArenaAlloc* arena) {
+        auto mtype = matrix.getType();
+        if (mtype & (SkMatrix::kAffine_Mask | SkMatrix::kPerspective_Mask) ||
+            scalarsPerPosition == 2) {
+            return arena->make<GeneralMapper>(matrix, offset);
+        }
+
+        if (mtype & SkMatrix::kScale_Mask) {
+            return arena->make<XScaleMapper>(matrix, offset);
+        }
+
+        return arena->make<TranslationMapper>(matrix, offset);
+    }
+
 private:
     // GlyphFinderInterface is the polymorphic base for classes that parse a stream of chars into
     // the right UniChar (or GlyphID) and lookup up the glyph on the cache. The concrete
@@ -230,18 +257,6 @@ private:
         const SkScalar* fPositions;
     };
 
-    // MapperInterface given a point map it through the matrix. There are several shortcut
-    // variants.
-    // * TranslationMapper - assumes a translation only matrix.
-    // * XScaleMapper - assumes an X scaling and a translation.
-    // * GeneralMapper - Does all other matricies.
-    class MapperInterface {
-    public:
-        virtual ~MapperInterface() { }
-
-        virtual SkPoint map(SkPoint position) const = 0;
-    };
-
     class TranslationMapper final : public MapperInterface {
     public:
         TranslationMapper(const SkMatrix& matrix, const SkPoint origin)
@@ -457,16 +472,7 @@ inline void SkFindAndPlaceGlyph::ProcessPosText(
         positionReader = arena.make<HorizontalPositions>(pos);
     }
 
-    MapperInterface* mapper = nullptr;
-    if (mtype & (SkMatrix::kAffine_Mask | SkMatrix::kPerspective_Mask)
-        || scalarsPerPosition == 2) {
-        mapper = arena.make<GeneralMapper>(matrix, offset);
-    } else if (mtype & SkMatrix::kScale_Mask) {
-        mapper = arena.make<XScaleMapper>(matrix, offset);
-    } else {
-        mapper = arena.make<TranslationMapper>(matrix, offset);
-    }
-
+    MapperInterface* mapper = CreateMapper(matrix, offset, scalarsPerPosition, &arena);
     GlyphFindAndPlaceInterface<ProcessOneGlyph>* findAndPosition = nullptr;
     if (cache->isSubpixel()) {
         findAndPosition = getSubpixel<ProcessOneGlyph>(&arena, axisAlignment, glyphFinder);
diff --git a/src/core/SkRemoteGlyphCache.cpp b/src/core/SkRemoteGlyphCache.cpp
index 17edc83064..1d52f46dbd 100644
--- a/src/core/SkRemoteGlyphCache.cpp
+++ b/src/core/SkRemoteGlyphCache.cpp
@@ -276,8 +276,6 @@ void SkTextBlobCacheDiffCanvas::processGlyphRun(
 
     SkMatrix runMatrix{fDeviceMatrix};
     runMatrix.preConcat(this->getTotalMatrix());
-    runMatrix.preTranslate(position.x(), position.y());
-    runMatrix.preTranslate(it.offset().x(), it.offset().y());
 
 #if SK_SUPPORT_GPU
     GrTextContext::Options options;
@@ -305,55 +303,28 @@ void SkTextBlobCacheDiffCanvas::processGlyphRun(
 
     using PosFn = SkPoint(*)(int index, const SkScalar* pos);
     PosFn posFn;
+    SkSTArenaAlloc<120> arena;
+    SkFindAndPlaceGlyph::MapperInterface* mapper = nullptr;
     switch (it.positioning()) {
         case SkTextBlob::kHorizontal_Positioning:
             posFn = [](int index, const SkScalar* pos) {
                 return SkPoint{pos[index], 0};
             };
-
+            mapper = SkFindAndPlaceGlyph::CreateMapper(
+                    runMatrix, SkPoint::Make(position.x(), position.y() + it.offset().y()), 1,
+                    &arena);
             break;
-
         case SkTextBlob::kFull_Positioning:
             posFn = [](int index, const SkScalar* pos) {
                 return SkPoint{pos[2 * index], pos[2 * index + 1]};
             };
+            mapper = SkFindAndPlaceGlyph::CreateMapper(runMatrix, position, 2, &arena);
             break;
-
         default:
             posFn = nullptr;
             SK_ABORT("unhandled positioning mode");
     }
 
-    using MapFn = SkPoint(*)(const SkMatrix& m, SkPoint pt);
-    MapFn mapFn;
-    switch ((int)runMatrix.getType()) {
-        case SkMatrix::kIdentity_Mask:
-        case SkMatrix::kTranslate_Mask:
-            mapFn = [](const SkMatrix& m, SkPoint pt) {
-                pt.offset(m.getTranslateX(), m.getTranslateY());
-                return pt;
-            };
-            break;
-        case SkMatrix::kScale_Mask:
-        case SkMatrix::kScale_Mask | SkMatrix::kTranslate_Mask:
-            mapFn = [](const SkMatrix& m, SkPoint pt) {
-                return SkPoint{pt.x() * m.getScaleX() + m.getTranslateX(),
-                               pt.y() * m.getScaleY() + m.getTranslateY()};
-            };
-            break;
-        case SkMatrix::kAffine_Mask | SkMatrix::kScale_Mask:
-        case SkMatrix::kAffine_Mask | SkMatrix::kScale_Mask | SkMatrix::kTranslate_Mask:
-            mapFn = [](const SkMatrix& m, SkPoint pt) {
-                return SkPoint{
-                        pt.x() * m.getScaleX() + pt.y() * m.getSkewX() + m.getTranslateX(),
-                        pt.x() * m.getSkewY() + pt.y() * m.getScaleY() + m.getTranslateY()};
-            };
-            break;
-        default:
-            mapFn = nullptr;
-            SK_ABORT("Bad matrix.");
-    }
-
     SkScalerContextRec deviceSpecificRec;
     SkScalerContextEffects effects;
     auto* glyphCacheState =
@@ -371,8 +342,8 @@ void SkTextBlobCacheDiffCanvas::processGlyphRun(
     const uint16_t* glyphs = it.glyphs();
     for (uint32_t index = 0; index < it.glyphCount(); index++) {
         SkIPoint subPixelPos{0, 0};
-        if (runPaint.isAntiAlias() && isSubpixel) {
-            SkPoint glyphPos = mapFn(runMatrix, posFn(index, pos));
+        if (isSubpixel) {
+            SkPoint glyphPos = mapper->map(posFn(index, pos));
             subPixelPos = SkFindAndPlaceGlyph::SubpixelAlignment(axisAlignment, glyphPos);
         }