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
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
{"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 @@
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 @@
#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 @@
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 @@
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 @@
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 @@
}
}
+#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 @@
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],