Fix text redering issue where the text was sometimes truncated

- mostly was visible in Settings apps / Wi-Fi networks summary info for each network
- correctly setup the local SkPaint for advances computation
- improve test app for adding live resizing

Change-Id: Ia031fe1b115b521ba55c7e68f2a26300f02e48ca

7 files changed, 157 insertions, 61 deletions

diff --git a/core/jni/android/graphics/HarfbuzzSkia.cpp b/core/jni/android/graphics/HarfbuzzSkia.cpp
index 58fb32b..92c743f 100644
--- a/core/jni/android/graphics/HarfbuzzSkia.cpp
+++ b/core/jni/android/graphics/HarfbuzzSkia.cpp
@@ -34,6 +34,8 @@
 #include "SkRect.h"
 #include "SkTypeface.h"
 
+#include <utils/Log.h>
+
 extern "C" {
 #include "harfbuzz-shaper.h"
 }
@@ -43,20 +45,13 @@ extern "C" {
 
 namespace android {
 
-static HB_Fixed SkiaScalarToHarfbuzzFixed(SkScalar value)
-{
-    // HB_Fixed is a 26.6 fixed point format.
-    return value * 64;
-}
-
 static void setupPaintWithFontData(SkPaint* paint, FontData* data) {
-    paint->setAntiAlias(true);
-    paint->setSubpixelText(true);
-    paint->setHinting(SkPaint::kSlight_Hinting);
-    paint->setTextSize(SkFloatToScalar(data->textSize));
     paint->setTypeface(data->typeFace);
-    paint->setFakeBoldText(data->fakeBold);
-    paint->setTextSkewX(data->fakeItalic ? -SK_Scalar1/4 : 0);
+    paint->setTextSize(data->textSize);
+    paint->setTextSkewX(data->textSkewX);
+    paint->setTextScaleX(data->textScaleX);
+    paint->setFlags(data->flags);
+    paint->setHinting(data->hinting);
 }
 
 static HB_Bool stringToGlyphs(HB_Font hbFont, const HB_UChar16* characters, hb_uint32 length,
@@ -67,16 +62,13 @@ static HB_Bool stringToGlyphs(HB_Font hbFont, const HB_UChar16* characters, hb_u
     setupPaintWithFontData(&paint, data);
 
     paint.setTextEncoding(SkPaint::kUTF16_TextEncoding);
-    int numGlyphs = paint.textToGlyphs(characters, length * sizeof(uint16_t),
-            reinterpret_cast<uint16_t*>(glyphs));
+    uint16_t* skiaGlyphs = reinterpret_cast<uint16_t*>(glyphs);
+    int numGlyphs = paint.textToGlyphs(characters, length * sizeof(uint16_t), skiaGlyphs);
 
     // HB_Glyph is 32-bit, but Skia outputs only 16-bit numbers. So our
     // |glyphs| array needs to be converted.
     for (int i = numGlyphs - 1; i >= 0; --i) {
-        uint16_t value;
-        // We use a memcpy to avoid breaking strict aliasing rules.
-        memcpy(&value, reinterpret_cast<char*>(glyphs) + sizeof(uint16_t) * i, sizeof(value));
-        glyphs[i] = value;
+        glyphs[i] = skiaGlyphs[i];
     }
 
     *glyphsSize = numGlyphs;
@@ -97,16 +89,17 @@ static void glyphsToAdvances(HB_Font hbFont, const HB_Glyph* glyphs, hb_uint32 n
         return;
     for (unsigned i = 0; i < numGlyphs; ++i)
         glyphs16[i] = glyphs[i];
-    paint.getTextWidths(glyphs16, numGlyphs * sizeof(uint16_t), reinterpret_cast<SkScalar*>(advances));
+    SkScalar* scalarAdvances = reinterpret_cast<SkScalar*>(advances);
+    paint.getTextWidths(glyphs16, numGlyphs * sizeof(uint16_t), scalarAdvances);
 
     // The |advances| values which Skia outputs are SkScalars, which are floats
     // in Chromium. However, Harfbuzz wants them in 26.6 fixed point format.
     // These two formats are both 32-bits long.
     for (unsigned i = 0; i < numGlyphs; ++i) {
-        float value;
-        // We use a memcpy to avoid breaking strict aliasing rules.
-        memcpy(&value, reinterpret_cast<char*>(advances) + sizeof(float) * i, sizeof(value));
-        advances[i] = SkiaScalarToHarfbuzzFixed(value);
+        advances[i] = SkScalarToHBFixed(scalarAdvances[i]);
+#if DEBUG_ADVANCES
+        LOGD("glyphsToAdvances -- advances[%d]=%d", i, advances[i]);
+#endif
     }
     delete glyphs16;
 }
@@ -156,8 +149,8 @@ static HB_Error getOutlinePoint(HB_Font hbFont, HB_Glyph glyph, int flags, hb_ui
         return HB_Err_Invalid_SubTable;
     // Skia does let us get a single point from the path.
     path.getPoints(points, point + 1);
-    *xPos = SkiaScalarToHarfbuzzFixed(points[point].fX);
-    *yPos = SkiaScalarToHarfbuzzFixed(points[point].fY);
+    *xPos = SkScalarToHBFixed(points[point].fX);
+    *yPos = SkScalarToHBFixed(points[point].fY);
     *resultingNumPoints = numPoints;
     delete points;
 
@@ -176,12 +169,12 @@ static void getGlyphMetrics(HB_Font hbFont, HB_Glyph glyph, HB_GlyphMetrics* met
     SkRect bounds;
     paint.getTextWidths(&glyph16, sizeof(glyph16), &width, &bounds);
 
-    metrics->x = SkiaScalarToHarfbuzzFixed(bounds.fLeft);
-    metrics->y = SkiaScalarToHarfbuzzFixed(bounds.fTop);
-    metrics->width = SkiaScalarToHarfbuzzFixed(bounds.width());
-    metrics->height = SkiaScalarToHarfbuzzFixed(bounds.height());
+    metrics->x = SkScalarToHBFixed(bounds.fLeft);
+    metrics->y = SkScalarToHBFixed(bounds.fTop);
+    metrics->width = SkScalarToHBFixed(bounds.width());
+    metrics->height = SkScalarToHBFixed(bounds.height());
 
-    metrics->xOffset = SkiaScalarToHarfbuzzFixed(width);
+    metrics->xOffset = SkScalarToHBFixed(width);
     // We can't actually get the |y| correct because Skia doesn't export
     // the vertical advance. However, nor we do ever render vertical text at
     // the moment so it's unimportant.
@@ -199,7 +192,7 @@ static HB_Fixed getFontMetric(HB_Font hbFont, HB_FontMetric metric)
 
     switch (metric) {
     case HB_FontAscent:
-        return SkiaScalarToHarfbuzzFixed(-skiaMetrics.fAscent);
+        return SkScalarToHBFixed(-skiaMetrics.fAscent);
     // We don't support getting the rest of the metrics and Harfbuzz doesn't seem to need them.
     default:
         return 0;
diff --git a/core/jni/android/graphics/HarfbuzzSkia.h b/core/jni/android/graphics/HarfbuzzSkia.h
index d057d76..99b389a 100644
--- a/core/jni/android/graphics/HarfbuzzSkia.h
+++ b/core/jni/android/graphics/HarfbuzzSkia.h
@@ -27,22 +27,38 @@
 #ifndef HarfbuzzSkia_h
 #define HarfbuzzSkia_h
 
+#include "SkScalar.h"
 #include "SkTypeface.h"
+#include "SkPaint.h"
 
 extern "C" {
 #include "harfbuzz-shaper.h"
 }
 
 namespace android {
-    typedef struct {
-        SkTypeface* typeFace;
-        float textSize;
-        bool fakeBold;
-        bool fakeItalic;
-    } FontData;
-
-    HB_Error harfbuzzSkiaGetTable(void* voidface, const HB_Tag, HB_Byte* buffer, HB_UInt* len);
-    extern const HB_FontClass harfbuzzSkiaClass;
+
+static inline float HBFixedToFloat(HB_Fixed v) {
+    // Harfbuzz uses 26.6 fixed point values for pixel offsets
+    return v * (1.0f / 64);
+}
+
+static inline HB_Fixed SkScalarToHBFixed(SkScalar value) {
+    // HB_Fixed is a 26.6 fixed point format.
+    return SkScalarToFloat(value) * 64.0f;
+}
+
+typedef struct {
+    SkTypeface* typeFace;
+    SkScalar textSize;
+    SkScalar textSkewX;
+    SkScalar textScaleX;
+    uint32_t flags;
+    SkPaint::Hinting hinting;
+} FontData;
+
+HB_Error harfbuzzSkiaGetTable(void* voidface, const HB_Tag, HB_Byte* buffer, HB_UInt* len);
+extern const HB_FontClass harfbuzzSkiaClass;
+
 }  // namespace android
 
 #endif
diff --git a/core/jni/android/graphics/Paint.cpp b/core/jni/android/graphics/Paint.cpp
index 5c3497f..27be871 100644
--- a/core/jni/android/graphics/Paint.cpp
+++ b/core/jni/android/graphics/Paint.cpp
@@ -414,6 +414,7 @@ public:
         for (int i = 0; i < glyphCount; i++) {
             glyphsArray[i] = (jchar) shaperItem.glyphs[i];
         }
+        env->ReleaseCharArrayElements(glyphs, glyphsArray, JNI_ABORT);
         return glyphCount;
     }
 
@@ -442,6 +443,21 @@ public:
         return totalAdvance;
     }
 
+    static jfloat doTextRunAdvancesICU(JNIEnv *env, SkPaint *paint, const jchar *text,
+                                    jint start, jint count, jint contextCount, jint flags,
+                                    jfloatArray advances, jint advancesIndex) {
+        jfloat advancesArray[count];
+        jfloat totalAdvance;
+
+        TextLayout::getTextRunAdvancesICU(paint, text, start, count, contextCount, flags,
+                                       advancesArray, totalAdvance);
+
+        if (advances != NULL) {
+            env->SetFloatArrayRegion(advances, advancesIndex, count, advancesArray);
+        }
+        return totalAdvance;
+    }
+
     static float getTextRunAdvances___CIIIII_FI(JNIEnv* env, jobject clazz, SkPaint* paint,
             jcharArray text, jint index, jint count, jint contextIndex, jint contextCount,
             jint flags, jfloatArray advances, jint advancesIndex) {
@@ -463,6 +479,27 @@ public:
         return result;
     }
 
+    static float getTextRunAdvancesICU___CIIIII_FI(JNIEnv* env, jobject clazz, SkPaint* paint,
+            jcharArray text, jint index, jint count, jint contextIndex, jint contextCount,
+            jint flags, jfloatArray advances, jint advancesIndex) {
+        jchar* textArray = env->GetCharArrayElements(text, NULL);
+        jfloat result = doTextRunAdvancesICU(env, paint, textArray + contextIndex,
+            index - contextIndex, count, contextCount, flags, advances, advancesIndex);
+        env->ReleaseCharArrayElements(text, textArray, JNI_ABORT);
+        return result;
+    }
+
+    static float getTextRunAdvancesICU__StringIIIII_FI(JNIEnv* env, jobject clazz, SkPaint* paint,
+            jstring text, jint start, jint end, jint contextStart, jint contextEnd, jint flags,
+            jfloatArray advances, jint advancesIndex) {
+        const jchar* textArray = env->GetStringChars(text, NULL);
+        jfloat result = doTextRunAdvancesICU(env, paint, textArray + contextStart,
+            start - contextStart, end - start, contextEnd - contextStart, flags, advances,
+            advancesIndex);
+        env->ReleaseStringChars(text, textArray);
+        return result;
+    }
+
     static jint doTextRunCursor(JNIEnv *env, SkPaint* paint, const jchar *text, jint start,
             jint count, jint flags, jint offset, jint opt) {
         SkScalar scalarArray[count];
@@ -748,10 +785,14 @@ static JNINativeMethod methods[] = {
     {"native_breakText","(Ljava/lang/String;ZF[F)I", (void*) SkPaintGlue::breakTextS},
     {"native_getTextWidths","(I[CII[F)I", (void*) SkPaintGlue::getTextWidths___CII_F},
     {"native_getTextWidths","(ILjava/lang/String;II[F)I", (void*) SkPaintGlue::getTextWidths__StringII_F},
-    {"native_getTextRunAdvances","(I[CIIIII[FI)F", (void*)
-        SkPaintGlue::getTextRunAdvances___CIIIII_FI},
+    {"native_getTextRunAdvances","(I[CIIIII[FI)F",
+        (void*) SkPaintGlue::getTextRunAdvances___CIIIII_FI},
     {"native_getTextRunAdvances","(ILjava/lang/String;IIIII[FI)F",
         (void*) SkPaintGlue::getTextRunAdvances__StringIIIII_FI},
+    {"native_getTextRunAdvancesICU","(I[CIIIII[FI)F",
+        (void*) SkPaintGlue::getTextRunAdvancesICU___CIIIII_FI},
+    {"native_getTextRunAdvancesICU","(ILjava/lang/String;IIIII[FI)F",
+        (void*) SkPaintGlue::getTextRunAdvancesICU__StringIIIII_FI},
     {"native_getTextGlyphs","(ILjava/lang/String;IIIII[C)I",
         (void*) SkPaintGlue::getTextGlyphs__StringIIIII_C},
     {"native_getTextRunCursor", "(I[CIIIII)I", (void*) SkPaintGlue::getTextRunCursor___C},
diff --git a/core/jni/android/graphics/RtlProperties.h b/core/jni/android/graphics/RtlProperties.h
index 2c68fa3..f41f4a1 100644
--- a/core/jni/android/graphics/RtlProperties.h
+++ b/core/jni/android/graphics/RtlProperties.h
@@ -45,7 +45,12 @@ static RtlDebugLevel readRtlDebugLevel() {
     return kRtlDebugDisabled;
 }
 
+// Define if we want to use Harfbuzz (1) or not (0)
 #define RTL_USE_HARFBUZZ 1
 
+// Define if we want (1) to have Advances debug values or not (0)
+#define DEBUG_ADVANCES 0
+
+
 } // namespace android
 #endif // ANDROID_RTL_PROPERTIES_H
diff --git a/core/jni/android/graphics/TextLayout.cpp b/core/jni/android/graphics/TextLayout.cpp
index f1bb696..434f63b 100644
--- a/core/jni/android/graphics/TextLayout.cpp
+++ b/core/jni/android/graphics/TextLayout.cpp
@@ -269,6 +269,21 @@ void TextLayout::getTextRunAdvances(SkPaint* paint, const jchar* chars, jint sta
 #endif
 }
 
+void TextLayout::getTextRunAdvancesHB(SkPaint* paint, const jchar* chars, jint start,
+                                    jint count, jint contextCount, jint dirFlags,
+                                    jfloat* resultAdvances, jfloat& resultTotalAdvance) {
+    // Compute advances and return them
+    RunAdvanceDescription::computeAdvancesWithHarfbuzz(paint, chars, start, count, contextCount, dirFlags,
+            resultAdvances, &resultTotalAdvance);
+}
+
+void TextLayout::getTextRunAdvancesICU(SkPaint* paint, const jchar* chars, jint start,
+                                    jint count, jint contextCount, jint dirFlags,
+                                    jfloat* resultAdvances, jfloat& resultTotalAdvance) {
+    // Compute advances and return them
+    RunAdvanceDescription::computeAdvancesWithICU(paint, chars, start, count, contextCount, dirFlags,
+            resultAdvances, &resultTotalAdvance);
+}
 
 // Draws a paragraph of text on a single line, running bidi and shaping
 void TextLayout::drawText(SkPaint* paint, const jchar* text, jsize len,
diff --git a/core/jni/android/graphics/TextLayout.h b/core/jni/android/graphics/TextLayout.h
index a950d13..138983c 100644
--- a/core/jni/android/graphics/TextLayout.h
+++ b/core/jni/android/graphics/TextLayout.h
@@ -73,6 +73,14 @@ public:
                                    jint count, jint contextCount, jint dirFlags,
                                    jfloat* resultAdvances, jfloat& resultTotalAdvance);
 
+    static void getTextRunAdvancesICU(SkPaint* paint, const jchar* chars, jint start,
+                                   jint count, jint contextCount, jint dirFlags,
+                                   jfloat* resultAdvances, jfloat& resultTotalAdvance);
+
+    static void getTextRunAdvancesHB(SkPaint* paint, const jchar* chars, jint start,
+                                   jint count, jint contextCount, jint dirFlags,
+                                   jfloat* resultAdvances, jfloat& resultTotalAdvance);
+
     static void drawText(SkPaint* paint, const jchar* text, jsize len,
                          jint bidiFlags, jfloat x, jfloat y, SkCanvas* canvas);
 
diff --git a/core/jni/android/graphics/TextLayoutCache.h b/core/jni/android/graphics/TextLayoutCache.h
index 925bb7c..31d802c 100644
--- a/core/jni/android/graphics/TextLayoutCache.h
+++ b/core/jni/android/graphics/TextLayoutCache.h
@@ -19,17 +19,20 @@
 
 #include "RtlProperties.h"
 
-#include "stddef.h"
+#include <stddef.h>
 #include <utils/threads.h>
 #include <utils/String16.h>
-#include "utils/GenerationCache.h"
-#include "utils/Compare.h"
+#include <utils/GenerationCache.h>
+#include <utils/Compare.h>
 
-#include "SkPaint.h"
-#include "SkTemplates.h"
+#include <SkPaint.h>
+#include <SkTemplates.h>
+#include <SkUtils.h>
+#include <SkScalerContext.h>
+#include <SkAutoKern.h>
 
-#include "unicode/ubidi.h"
-#include "unicode/ushape.h"
+#include <unicode/ubidi.h>
+#include <unicode/ushape.h>
 #include "HarfbuzzSkia.h"
 #include "harfbuzz-shaper.h"
 
@@ -54,14 +57,8 @@
 // Define the interval in number of cache hits between two statistics dump
 #define DEFAULT_DUMP_STATS_CACHE_HIT_INTERVAL 100
 
-// Define if we want to have Advances debug values
-#define DEBUG_ADVANCES 0
-
 namespace android {
 
-// Harfbuzz uses 26.6 fixed point values for pixel offsets
-#define HB_FIXED_TO_FLOAT(v) (((float) v) * (1.0 / 64))
-
 /**
  * TextLayoutCacheKey is the Cache key
  */
@@ -202,6 +199,8 @@ public:
         shaperItem->font = font;
         shaperItem->face = HB_NewFace(shaperItem->font, harfbuzzSkiaGetTable);
 
+        shaperItem->kerning_applied = false;
+
         // We cannot know, ahead of time, how many glyphs a given script run
         // will produce. We take a guess that script runs will not produce more
         // than twice as many glyphs as there are code points plus a bit of
@@ -222,10 +221,12 @@ public:
         shaperItem->string = chars;
         shaperItem->stringLength = contextCount;
 
-        fontData->textSize = paint->getTextSize();
-        fontData->fakeBold = paint->isFakeBoldText();
-        fontData->fakeItalic = (paint->getTextSkewX() > 0);
         fontData->typeFace = paint->getTypeface();
+        fontData->textSize = paint->getTextSize();
+        fontData->textSkewX = paint->getTextSkewX();
+        fontData->textScaleX = paint->getTextScaleX();
+        fontData->flags = paint->getFlags();
+        fontData->hinting = paint->getHinting();
 
         shaperItem->font->userData = fontData;
     }
@@ -248,6 +249,21 @@ public:
         }
     }
 
+#define SkAutoKern_AdjustF(prev, next) (((next) - (prev) + 32) >> 6 << 16)
+
+    static int adjust(int prev, int next) {
+        int delta = next - prev;
+        if (delta >= 32) {
+            return -1;
+        }
+        else if (delta < -32) {
+            return +1;
+        }
+        else {
+            return 0;
+        }
+    }
+
     static void computeAdvancesWithHarfbuzz(SkPaint* paint, const UChar* chars, size_t start,
             size_t count, size_t contextCount, int dirFlags,
             jfloat* outAdvances, jfloat* outTotalAdvance) {
@@ -261,13 +277,15 @@ public:
                 contextCount, dirFlags);
 
 #if DEBUG_ADVANCES
-        LOGD("HARFBUZZ -- num_glypth=%d", shaperItem.num_glyphs);
+        LOGD("HARFBUZZ -- num_glypth=%d - kerning_applied=%d", shaperItem.num_glyphs, shaperItem.kerning_applied);
+        LOGD("         -- string= '%s'", String8(chars, contextCount).string());
+        LOGD("         -- isDevKernText=%d", paint->isDevKernText());
 #endif
 
         jfloat totalAdvance = 0;
+
         for (size_t i = 0; i < count; i++) {
-            // Be careful: we need to use ceilf() for doing the same way as what Skia is doing
-            totalAdvance += outAdvances[i] = ceilf(HB_FIXED_TO_FLOAT(shaperItem.advances[i]));
+            totalAdvance += outAdvances[i] = HBFixedToFloat(shaperItem.advances[i]);
 
 #if DEBUG_ADVANCES
             LOGD("hb-adv = %d - rebased = %f - total = %f", shaperItem.advances[i], outAdvances[i],