From f2114d5646194100242206b267ddd6e7194b7da9 Mon Sep 17 00:00:00 2001 From: Raph Levien Date: Sun, 1 Jun 2014 15:54:47 -0700 Subject: [PATCH] Better Minikin integration This patch improves the Minikin integration in a number of ways, including: Software rendering does text decorations and handles alignment correctly. This fixes bug 15139067 "Centered text isn't working". Paint implements getTextPath. This fixes bug 15143354 "Text rendering in Maps Navigation wrong typeface?" Also a bit of refactoring, since there was duplicated code for iterating font runs that's now a static method in MinikinUtils. Change-Id: I4cfdb2c0559982376348325a757d95235fab1768 --- core/jni/android/graphics/Canvas.cpp | 61 ++++++++++------ core/jni/android/graphics/MinikinUtils.cpp | 17 ++++- core/jni/android/graphics/MinikinUtils.h | 21 ++++++ core/jni/android/graphics/Paint.cpp | 73 +++++++++++++++++-- core/jni/android/graphics/TextLayoutCache.cpp | 6 ++ core/jni/android_view_GLES20Canvas.cpp | 61 +++++++++------- graphics/java/android/graphics/Paint.java | 12 +-- 7 files changed, 186 insertions(+), 65 deletions(-) diff --git a/core/jni/android/graphics/Canvas.cpp b/core/jni/android/graphics/Canvas.cpp index f7acbd70561d1..ec935cc215ad3 100644 --- a/core/jni/android/graphics/Canvas.cpp +++ b/core/jni/android/graphics/Canvas.cpp @@ -821,32 +821,45 @@ public: } #ifdef USE_MINIKIN - static void drawGlyphsToSkia(SkCanvas* canvas, SkPaint* paint, Layout* layout, float x, float y) { - size_t nGlyphs = layout->nGlyphs(); - uint16_t *glyphs = new uint16_t[nGlyphs]; - SkPoint *pos = new SkPoint[nGlyphs]; - SkTypeface *lastFace = NULL; - SkTypeface *skFace = NULL; - size_t start = 0; + class DrawTextFunctor { + public: + DrawTextFunctor(const Layout& layout, SkCanvas* canvas, jfloat x, jfloat y, SkPaint* paint, + uint16_t* glyphs, SkPoint* pos) + : layout(layout), canvas(canvas), x(x), y(y), paint(paint), glyphs(glyphs), + pos(pos) { } - paint->setTextEncoding(SkPaint::kGlyphID_TextEncoding); - for (size_t i = 0; i < nGlyphs; i++) { - MinikinFontSkia *mfs = static_cast(layout->getFont(i)); - skFace = mfs->GetSkTypeface(); - glyphs[i] = layout->getGlyphId(i); - pos[i].fX = x + layout->getX(i); - pos[i].fY = y + layout->getY(i); - if (i > 0 && skFace != lastFace) { - paint->setTypeface(lastFace); - canvas->drawPosText(glyphs + start, (i - start) << 1, pos + start, *paint); - start = i; + void operator()(SkTypeface* t, size_t start, size_t end) { + for (size_t i = start; i < end; i++) { + glyphs[i] = layout.getGlyphId(i); + pos[i].fX = x + layout.getX(i); + pos[i].fY = y + layout.getY(i); } - lastFace = skFace; - } - if (skFace != NULL) { - paint->setTypeface(skFace); - canvas->drawPosText(glyphs + start, (nGlyphs - start) << 1, pos + start, *paint); + paint->setTypeface(t); + canvas->drawPosText(glyphs + start, (end - start) << 1, pos + start, *paint); } + private: + const Layout& layout; + SkCanvas* canvas; + jfloat x; + jfloat y; + SkPaint* paint; + uint16_t* glyphs; + SkPoint* pos; + }; + + static void drawGlyphsToSkia(SkCanvas* canvas, SkPaint* paint, const Layout& layout, float x, float y) { + size_t nGlyphs = layout.nGlyphs(); + uint16_t* glyphs = new uint16_t[nGlyphs]; + SkPoint* pos = new SkPoint[nGlyphs]; + + x += MinikinUtils::xOffsetForTextAlign(paint, layout); + SkPaint::Align align = paint->getTextAlign(); + paint->setTextAlign(SkPaint::kLeft_Align); + paint->setTextEncoding(SkPaint::kGlyphID_TextEncoding); + DrawTextFunctor f(layout, canvas, x, y, paint, glyphs, pos); + MinikinUtils::forFontRun(layout, f); + doDrawTextDecorations(canvas, x, y, layout.getAdvance(), paint); + paint->setTextAlign(align); delete[] glyphs; delete[] pos; } @@ -868,7 +881,7 @@ public: Layout layout; MinikinUtils::SetLayoutProperties(&layout, paint, flags, typeface); layout.doLayout(textArray + start, count); - drawGlyphsToSkia(canvas, paint, &layout, x, y); + drawGlyphsToSkia(canvas, paint, layout, x, y); #else sp value = TextLayoutEngine::getInstance().getValue(paint, textArray, start, count, contextCount, flags); diff --git a/core/jni/android/graphics/MinikinUtils.cpp b/core/jni/android/graphics/MinikinUtils.cpp index 79381ad9d3a63..597160b22767b 100644 --- a/core/jni/android/graphics/MinikinUtils.cpp +++ b/core/jni/android/graphics/MinikinUtils.cpp @@ -17,6 +17,7 @@ #include "SkPaint.h" #include "minikin/Layout.h" #include "TypefaceImpl.h" +#include "MinikinSkia.h" #include "MinikinUtils.h" @@ -42,4 +43,18 @@ void MinikinUtils::SetLayoutProperties(Layout* layout, SkPaint* paint, int flags layout->setProperties(css); } -} \ No newline at end of file +float MinikinUtils::xOffsetForTextAlign(SkPaint* paint, const Layout& layout) { + switch (paint->getTextAlign()) { + case SkPaint::kCenter_Align: + return layout.getAdvance() * -0.5f; + break; + case SkPaint::kRight_Align: + return -layout.getAdvance(); + break; + default: + break; + } + return 0; +} + +} diff --git a/core/jni/android/graphics/MinikinUtils.h b/core/jni/android/graphics/MinikinUtils.h index 42f5e2f19dc4f..f85074cd572c3 100644 --- a/core/jni/android/graphics/MinikinUtils.h +++ b/core/jni/android/graphics/MinikinUtils.h @@ -30,6 +30,27 @@ class MinikinUtils { public: static void SetLayoutProperties(Layout* layout, SkPaint* paint, int flags, TypefaceImpl* face); + static float xOffsetForTextAlign(SkPaint* paint, const Layout& layout); + + // f is a functor of type void f(SkTypeface *, size_t start, size_t end); + template + static void forFontRun(const Layout& layout, F& f) { + SkTypeface* lastFace = NULL; + size_t start = 0; + size_t nGlyphs = layout.nGlyphs(); + for (size_t i = 0; i < nGlyphs; i++) { + MinikinFontSkia* mfs = static_cast(layout.getFont(i)); + SkTypeface* skFace = mfs->GetSkTypeface(); + if (i > 0 && skFace != lastFace) { + f(lastFace, start, i); + start = i; + } + lastFace = skFace; + } + if (nGlyphs > start) { + f(lastFace, start, nGlyphs); + } + } }; } // namespace android diff --git a/core/jni/android/graphics/Paint.cpp b/core/jni/android/graphics/Paint.cpp index 8418162b65c6c..4642cfd0b4148 100644 --- a/core/jni/android/graphics/Paint.cpp +++ b/core/jni/android/graphics/Paint.cpp @@ -822,26 +822,83 @@ public: return result; } - static void getTextPath(JNIEnv* env, SkPaint* paint, const jchar* text, jint count, - jint bidiFlags, jfloat x, jfloat y, SkPath *path) { +#ifdef USE_MINIKIN + class GetTextFunctor { + public: + GetTextFunctor(const Layout& layout, SkPath* path, jfloat x, jfloat y, SkPaint* paint, + uint16_t* glyphs, SkPoint* pos) + : layout(layout), path(path), x(x), y(y), paint(paint), glyphs(glyphs), pos(pos) { + } + + void operator()(SkTypeface* t, size_t start, size_t end) { + for (size_t i = start; i < end; i++) { + glyphs[i] = layout.getGlyphId(i); + pos[i].fX = x + layout.getX(i); + pos[i].fY = y + layout.getY(i); + } + paint->setTypeface(t); + if (start == 0) { + paint->getPosTextPath(glyphs + start, (end - start) << 1, pos + start, path); + } else { + paint->getPosTextPath(glyphs + start, (end - start) << 1, pos + start, &tmpPath); + path->addPath(tmpPath); + } + } + private: + const Layout& layout; + SkPath* path; + jfloat x; + jfloat y; + SkPaint* paint; + uint16_t* glyphs; + SkPoint* pos; + SkPath tmpPath; + }; +#endif + + static void getTextPath(JNIEnv* env, SkPaint* paint, TypefaceImpl* typeface, const jchar* text, + jint count, jint bidiFlags, jfloat x, jfloat y, SkPath* path) { +#ifdef USE_MINIKIN + Layout layout; + MinikinUtils::SetLayoutProperties(&layout, paint, bidiFlags, typeface); + layout.doLayout(text, count); + size_t nGlyphs = layout.nGlyphs(); + uint16_t* glyphs = new uint16_t[nGlyphs]; + SkPoint* pos = new SkPoint[nGlyphs]; + + x += MinikinUtils::xOffsetForTextAlign(paint, layout); + SkPaint::Align align = paint->getTextAlign(); + paint->setTextAlign(SkPaint::kLeft_Align); + paint->setTextEncoding(SkPaint::kGlyphID_TextEncoding); + GetTextFunctor f(layout, path, x, y, paint, glyphs, pos); + MinikinUtils::forFontRun(layout, f); + paint->setTextAlign(align); + delete[] glyphs; + delete[] pos; +#else TextLayout::getTextPath(paint, text, count, bidiFlags, x, y, path); +#endif } - static void getTextPath___C(JNIEnv* env, jobject clazz, jlong paintHandle, jint bidiFlags, + static void getTextPath___C(JNIEnv* env, jobject clazz, jlong paintHandle, + jlong typefaceHandle, jint bidiFlags, jcharArray text, jint index, jint count, jfloat x, jfloat y, jlong pathHandle) { SkPaint* paint = reinterpret_cast(paintHandle); + TypefaceImpl* typeface = reinterpret_cast(typefaceHandle); SkPath* path = reinterpret_cast(pathHandle); const jchar* textArray = env->GetCharArrayElements(text, NULL); - getTextPath(env, paint, textArray + index, count, bidiFlags, x, y, path); + getTextPath(env, paint, typeface, textArray + index, count, bidiFlags, x, y, path); env->ReleaseCharArrayElements(text, const_cast(textArray), JNI_ABORT); } - static void getTextPath__String(JNIEnv* env, jobject clazz, jlong paintHandle, jint bidiFlags, + static void getTextPath__String(JNIEnv* env, jobject clazz, jlong paintHandle, + jlong typefaceHandle, jint bidiFlags, jstring text, jint start, jint end, jfloat x, jfloat y, jlong pathHandle) { SkPaint* paint = reinterpret_cast(paintHandle); + TypefaceImpl* typeface = reinterpret_cast(typefaceHandle); SkPath* path = reinterpret_cast(pathHandle); const jchar* textArray = env->GetStringChars(text, NULL); - getTextPath(env, paint, textArray + start, end - start, bidiFlags, x, y, path); + getTextPath(env, paint, typeface, textArray + start, end - start, bidiFlags, x, y, path); env->ReleaseStringChars(text, textArray); } @@ -1035,8 +1092,8 @@ static JNINativeMethod methods[] = { {"native_getTextRunCursor", "(J[CIIIII)I", (void*) SkPaintGlue::getTextRunCursor___C}, {"native_getTextRunCursor", "(JLjava/lang/String;IIIII)I", (void*) SkPaintGlue::getTextRunCursor__String}, - {"native_getTextPath","(JI[CIIFFJ)V", (void*) SkPaintGlue::getTextPath___C}, - {"native_getTextPath","(JILjava/lang/String;IIFFJ)V", (void*) SkPaintGlue::getTextPath__String}, + {"native_getTextPath","(JJI[CIIFFJ)V", (void*) SkPaintGlue::getTextPath___C}, + {"native_getTextPath","(JJILjava/lang/String;IIFFJ)V", (void*) SkPaintGlue::getTextPath__String}, {"nativeGetStringBounds", "(JLjava/lang/String;IIILandroid/graphics/Rect;)V", (void*) SkPaintGlue::getStringBounds }, {"nativeGetCharArrayBounds", "(J[CIIILandroid/graphics/Rect;)V", diff --git a/core/jni/android/graphics/TextLayoutCache.cpp b/core/jni/android/graphics/TextLayoutCache.cpp index 9279758213ea7..495d08a0f159b 100644 --- a/core/jni/android/graphics/TextLayoutCache.cpp +++ b/core/jni/android/graphics/TextLayoutCache.cpp @@ -17,6 +17,7 @@ #define LOG_TAG "TextLayoutCache" #include +#include #include "TextLayoutCache.h" #include "TextLayout.h" @@ -89,6 +90,11 @@ void TextLayoutCache::purgeCaches() { sp TextLayoutCache::getValue(const SkPaint* paint, const jchar* text, jint start, jint count, jint contextCount, jint dirFlags) { AutoMutex _l(mLock); +#ifdef USE_MINIKIN + // We want to get rid of all legacy calls in the Minikin case, so log + ALOGW("TextLayoutCache being invoked!"); + CallStack _cs(LOG_TAG); +#endif nsecs_t startTime = 0; if (mDebugEnabled) { startTime = systemTime(SYSTEM_TIME_MONOTONIC); diff --git a/core/jni/android_view_GLES20Canvas.cpp b/core/jni/android_view_GLES20Canvas.cpp index c5dd06fec81a4..820da17d88f99 100644 --- a/core/jni/android_view_GLES20Canvas.cpp +++ b/core/jni/android_view_GLES20Canvas.cpp @@ -632,6 +632,7 @@ static void android_view_GLES20Canvas_resetPaintFilter(JNIEnv* env, jobject claz // Text // ---------------------------------------------------------------------------- +// TODO: this is moving to MinikinUtils, remove with USE_MINIKIN ifdef static float xOffsetForTextAlign(SkPaint* paint, float totalAdvance) { switch (paint->getTextAlign()) { case SkPaint::kCenter_Align: @@ -647,43 +648,51 @@ static float xOffsetForTextAlign(SkPaint* paint, float totalAdvance) { } #ifdef USE_MINIKIN + +class RenderTextFunctor { +public: + RenderTextFunctor(const Layout& layout, OpenGLRenderer* renderer, jfloat x, jfloat y, + SkPaint* paint, uint16_t* glyphs, float* pos, float totalAdvance, + uirenderer::Rect& bounds) + : layout(layout), renderer(renderer), x(x), y(y), paint(paint), glyphs(glyphs), + pos(pos), totalAdvance(totalAdvance), bounds(bounds) { } + void operator()(SkTypeface* t, size_t start, size_t end) { + for (size_t i = start; i < end; i++) { + glyphs[i] = layout.getGlyphId(i); + pos[2 * i] = layout.getX(i); + pos[2 * i + 1] = layout.getY(i); + } + paint->setTypeface(t); + size_t glyphsCount = end - start; + int bytesCount = glyphsCount * sizeof(jchar); + renderer->drawText((const char*) (glyphs + start), bytesCount, glyphsCount, + x, y, pos + 2 * start, paint, totalAdvance, bounds); + } +private: + const Layout& layout; + OpenGLRenderer* renderer; + jfloat x; + jfloat y; + SkPaint* paint; + uint16_t* glyphs; + float* pos; + float totalAdvance; + uirenderer::Rect& bounds; +}; + static void renderTextLayout(OpenGLRenderer* renderer, Layout* layout, jfloat x, jfloat y, SkPaint* paint) { size_t nGlyphs = layout->nGlyphs(); float* pos = new float[nGlyphs * 2]; uint16_t* glyphs = new uint16_t[nGlyphs]; - SkTypeface* lastFace = 0; - SkTypeface* skFace = 0; - size_t start = 0; MinikinRect b; layout->getBounds(&b); android::uirenderer::Rect bounds(b.mLeft, b.mTop, b.mRight, b.mBottom); bounds.translate(x, y); float totalAdvance = layout->getAdvance(); - for (size_t i = 0; i < nGlyphs; i++) { - MinikinFontSkia* mfs = static_cast(layout->getFont(i)); - skFace = mfs->GetSkTypeface(); - glyphs[i] = layout->getGlyphId(i); - pos[2 * i] = layout->getX(i); - pos[2 * i + 1] = layout->getY(i); - if (i > 0 && skFace != lastFace) { - paint->setTypeface(lastFace); - size_t glyphsCount = i - start; - int bytesCount = glyphsCount * sizeof(jchar); - renderer->drawText((const char*) (glyphs + start), bytesCount, glyphsCount, - x, y, pos + 2 * start, paint, totalAdvance, bounds); - start = i; - } - lastFace = skFace; - } - if (skFace != NULL) { - paint->setTypeface(skFace); - size_t glyphsCount = nGlyphs - start; - int bytesCount = glyphsCount * sizeof(jchar); - renderer->drawText((const char*) (glyphs + start), bytesCount, glyphsCount, - x, y, pos + 2 * start, paint, totalAdvance, bounds); - } + RenderTextFunctor f(*layout, renderer, x, y, paint, glyphs, pos, totalAdvance, bounds); + MinikinUtils::forFontRun(*layout, f); delete[] glyphs; delete[] pos; } diff --git a/graphics/java/android/graphics/Paint.java b/graphics/java/android/graphics/Paint.java index 92cfd6b6b87e3..f97add88458ce 100644 --- a/graphics/java/android/graphics/Paint.java +++ b/graphics/java/android/graphics/Paint.java @@ -2131,7 +2131,7 @@ public class Paint { if ((index | count) < 0 || index + count > text.length) { throw new ArrayIndexOutOfBoundsException(); } - native_getTextPath(mNativePaint, mBidiFlags, text, index, count, x, y, + native_getTextPath(mNativePaint, mNativeTypeface, mBidiFlags, text, index, count, x, y, path.ni()); } @@ -2153,7 +2153,7 @@ public class Paint { if ((start | end | (end - start) | (text.length() - end)) < 0) { throw new IndexOutOfBoundsException(); } - native_getTextPath(mNativePaint, mBidiFlags, text, start, end, x, y, + native_getTextPath(mNativePaint, mNativeTypeface, mBidiFlags, text, start, end, x, y, path.ni()); } @@ -2261,10 +2261,10 @@ public class Paint { private native int native_getTextRunCursor(long native_object, String text, int contextStart, int contextEnd, int flags, int offset, int cursorOpt); - private static native void native_getTextPath(long native_object, int bidiFlags, - char[] text, int index, int count, float x, float y, long path); - private static native void native_getTextPath(long native_object, int bidiFlags, - String text, int start, int end, float x, float y, long path); + private static native void native_getTextPath(long native_object, long native_typeface, + int bidiFlags, char[] text, int index, int count, float x, float y, long path); + private static native void native_getTextPath(long native_object, long native_typeface, + int bidiFlags, String text, int start, int end, float x, float y, long path); private static native void nativeGetStringBounds(long nativePaint, String text, int start, int end, int bidiFlags, Rect bounds); private static native void nativeGetCharArrayBounds(long nativePaint,