Snap advance widths to integers

Fractional advance widths were causing subtle problems with text
positioning when the same text was drawn with different spans in the
hwui renderer. Quantizing the coordinates on layout (as opposed to
waiting until the renderer draws the glyphs) solves the problem.

This patch also fixes a discrepancy between x position and advance
widths when letterspacing.

Bug: 17347779
Change-Id: Ia705944047408c2839d5ad078eefd6bbec446872

2 files changed, 24 insertions, 8 deletions

diff --git a/include/minikin/MinikinFont.h b/include/minikin/MinikinFont.h
index 9b25f92..3f07589 100644
--- a/include/minikin/MinikinFont.h
+++ b/include/minikin/MinikinFont.h
@@ -49,6 +49,12 @@ struct MinikinPaint {
     std::string fontFeatureSettings;
 };
 
+// Only a few flags affect layout, but those that do should have values
+// consistent with Android's paint flags.
+enum MinikinPaintFlags {
+    LinearTextFlag = 0x40,
+};
+
 struct MinikinRect {
     float mLeft, mTop, mRight, mBottom;
     bool isEmpty() const {
diff --git a/libs/minikin/Layout.cpp b/libs/minikin/Layout.cpp
index c3d4c13..fcae6cc 100644
--- a/libs/minikin/Layout.cpp
+++ b/libs/minikin/Layout.cpp
@@ -676,7 +676,14 @@ void Layout::doLayoutRun(const uint16_t* buf, size_t start, size_t count, size_t
     double size = ctx->paint.size;
     double scaleX = ctx->paint.scaleX;
     double letterSpace = ctx->paint.letterSpacing * size * scaleX;
-    double letterSpaceHalf = letterSpace * .5;
+    double letterSpaceHalfLeft;
+    if ((ctx->paint.paintFlags & LinearTextFlag) == 0) {
+        letterSpace = round(letterSpace);
+        letterSpaceHalfLeft = floor(letterSpace * 0.5);
+    } else {
+        letterSpaceHalfLeft = letterSpace * 0.5;
+    }
+    double letterSpaceHalfRight = letterSpace - letterSpaceHalfLeft;
 
     float x = mAdvance;
     float y = 0;
@@ -721,8 +728,8 @@ void Layout::doLayoutRun(const uint16_t* buf, size_t start, size_t count, size_t
             hb_glyph_position_t* positions = hb_buffer_get_glyph_positions(buffer, NULL);
             if (numGlyphs)
             {
-                mAdvances[info[0].cluster - start] += letterSpaceHalf;
-                x += letterSpaceHalf;
+                mAdvances[info[0].cluster - start] += letterSpaceHalfLeft;
+                x += letterSpaceHalfLeft;
             }
             for (unsigned int i = 0; i < numGlyphs; i++) {
     #ifdef VERBOSE
@@ -730,9 +737,9 @@ void Layout::doLayoutRun(const uint16_t* buf, size_t start, size_t count, size_t
                 ": " << HBFixedToFloat(positions[i].x_advance) << "; " << positions[i].x_offset << ", " << positions[i].y_offset << std::endl;
     #endif
                 if (i > 0 && info[i - 1].cluster != info[i].cluster) {
-                    mAdvances[info[i - 1].cluster - start] += letterSpaceHalf;
-                    mAdvances[info[i].cluster - start] += letterSpaceHalf;
-                    x += letterSpaceHalf;
+                    mAdvances[info[i - 1].cluster - start] += letterSpaceHalfRight;
+                    mAdvances[info[i].cluster - start] += letterSpaceHalfLeft;
+                    x += letterSpace;
                 }
 
                 hb_codepoint_t glyph_ix = info[i].codepoint;
@@ -742,6 +749,9 @@ void Layout::doLayoutRun(const uint16_t* buf, size_t start, size_t count, size_t
                 LayoutGlyph glyph = {font_ix, glyph_ix, x + xoff, y + yoff};
                 mGlyphs.push_back(glyph);
                 float xAdvance = HBFixedToFloat(positions[i].x_advance);
+                if ((ctx->paint.paintFlags & LinearTextFlag) == 0) {
+                    xAdvance = roundf(xAdvance);
+                }
                 MinikinRect glyphBounds;
                 ctx->paint.font->GetBounds(&glyphBounds, glyph_ix, ctx->paint);
                 glyphBounds.offset(x + xoff, y + yoff);
@@ -751,8 +761,8 @@ void Layout::doLayoutRun(const uint16_t* buf, size_t start, size_t count, size_t
             }
             if (numGlyphs)
             {
-                mAdvances[info[numGlyphs - 1].cluster - start] += letterSpaceHalf;
-                x += letterSpaceHalf;
+                mAdvances[info[numGlyphs - 1].cluster - start] += letterSpaceHalfRight;
+                x += letterSpaceHalfRight;
             }
         }
     }