From 6740536e3927d25bf5c2567e5f6e8c175973cbb7 Mon Sep 17 00:00:00 2001 From: Raph Levien Date: Wed, 3 Sep 2014 10:37:05 -0700 Subject: 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 --- include/minikin/MinikinFont.h | 6 ++++++ libs/minikin/Layout.cpp | 26 ++++++++++++++++++-------- 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; } } } -- cgit v1.2.3