From a553477ddf55d170a66410ed325ae5e5d3005965 Mon Sep 17 00:00:00 2001 From: Seigo Nonaka Date: Thu, 15 Mar 2018 00:22:20 -0700 Subject: Make PrecomputedText Spannable for supporting selection This is 2nd attempt of I072dfd70b9a687d9c47e310d8cdb34f988fbb32e The root cause of crashing is unexpected copying of NoCopySpan by SpannableString constructor. To prevent crashing, stop copying NoCopySpan by passing ignoreNoCopySpan=true to SpannableString copy constructor. The original commit message is following: To support selectable TextView, make PrecomputedText spannable. By this change, TextView start using DynamicLayout instead of StaticLayout. DynamicLayout requires boundary rectangle of the text, so this CL also adds getBounds method to PrecomputedText which retrieves measured boundary box from native. By this change, the selectable TextView performance for the precomputed text 10x faster. On the other hand, the performacne for the non-selectable text gets 2.5x slower. However, we concluded that we accept this performance regression since it still 10 times faster than non precomputed text. Here is a precomputed text performance result of TextView. android.widget.TextViewPrecomputedTextPerfTest: newLayout_PrecomputedText : 736,130 -> 1,648,694: (+124.0%) newLayout_PrecomputedText_Selectable: 17,379,765 -> 1,700,146: (-90.2%) onDraw_PrecomputedText : 1,274,921 -> 1,848,076: (+45.0%) onDraw_PrecomputedText_Selectable : 17,367,238 -> 1,399,169: (-91.9%) onMeasure_PrecomputedText : 752,875 -> 1,766,606: (+134.6%) onMeasure_PrecomputedText_Selectable: 17,647,842 -> 1,810,704: (-89.7%) setText_PrecomputedText : 92,894 -> 135,471: (+45.8%) setText_PrecomputedText_Selectable : 145,134 -> 215,757: (+48.7%) Bug: 72998298 Test: atest CtsWidgetTestCases:EditTextTest CtsWidgetTestCases:TextViewFadingEdgeTest FrameworksCoreTests:TextViewFallbackLineSpacingTest FrameworksCoreTests:TextViewTest FrameworksCoreTests:TypefaceTest CtsGraphicsTestCases:TypefaceTest CtsWidgetTestCases:TextViewTest CtsTextTestCases FrameworksCoreTests:android.text CtsWidgetTestCases:TextViewPrecomputedTextTest Change-Id: Ie98c75d8b4ba962eaf0a544357b2ff1ade891118 --- api/current.txt | 4 +- core/java/android/text/DynamicLayout.java | 7 ++- core/java/android/text/MeasuredParagraph.java | 16 +++++++ core/java/android/text/PrecomputedText.java | 61 ++++++++++++++++++++++++--- core/java/android/widget/TextView.java | 9 ++-- core/jni/android_text_MeasuredParagraph.cpp | 29 +++++++++++++ 6 files changed, 115 insertions(+), 11 deletions(-) diff --git a/api/current.txt b/api/current.txt index 332e2a494920..d2c63b72f6f7 100644 --- a/api/current.txt +++ b/api/current.txt @@ -44063,7 +44063,7 @@ package android.text { method public abstract int getSpanTypeId(); } - public class PrecomputedText implements android.text.Spanned { + public class PrecomputedText implements android.text.Spannable { method public char charAt(int); method public static android.text.PrecomputedText create(java.lang.CharSequence, android.text.PrecomputedText.Params); method public int getParagraphCount(); @@ -44077,6 +44077,8 @@ package android.text { method public java.lang.CharSequence getText(); method public int length(); method public int nextSpanTransition(int, int, java.lang.Class); + method public void removeSpan(java.lang.Object); + method public void setSpan(java.lang.Object, int, int, int); method public java.lang.CharSequence subSequence(int, int); } diff --git a/core/java/android/text/DynamicLayout.java b/core/java/android/text/DynamicLayout.java index 18431cacbfaf..febca7ec9de1 100644 --- a/core/java/android/text/DynamicLayout.java +++ b/core/java/android/text/DynamicLayout.java @@ -704,7 +704,12 @@ public class DynamicLayout extends Layout { // Spans other than ReplacementSpan can be ignored because line top and bottom are // disjunction of all tops and bottoms, although it's not optimal. final Paint paint = getPaint(); - paint.getTextBounds(text, start, end, mTempRect); + if (text instanceof PrecomputedText) { + PrecomputedText precomputed = (PrecomputedText) text; + precomputed.getBounds(start, end, mTempRect); + } else { + paint.getTextBounds(text, start, end, mTempRect); + } final Paint.FontMetricsInt fm = paint.getFontMetricsInt(); return mTempRect.top < fm.top || mTempRect.bottom > fm.bottom; } diff --git a/core/java/android/text/MeasuredParagraph.java b/core/java/android/text/MeasuredParagraph.java index 980f47043269..a107cab58966 100644 --- a/core/java/android/text/MeasuredParagraph.java +++ b/core/java/android/text/MeasuredParagraph.java @@ -21,6 +21,7 @@ import android.annotation.IntRange; import android.annotation.NonNull; import android.annotation.Nullable; import android.graphics.Paint; +import android.graphics.Rect; import android.text.AutoGrowArray.ByteArray; import android.text.AutoGrowArray.FloatArray; import android.text.AutoGrowArray.IntArray; @@ -296,6 +297,18 @@ public class MeasuredParagraph { } } + /** + * Retrieves the bounding rectangle that encloses all of the characters, with an implied origin + * at (0, 0). + * + * This is available only if the MeasuredParagraph is computed with buildForStaticLayout. + */ + public void getBounds(@NonNull Paint paint, @IntRange(from = 0) int start, + @IntRange(from = 0) int end, @NonNull Rect bounds) { + nGetBounds(mNativePtr, mCopiedBuffer, paint.getNativeInstance(), start, end, + paint.getBidiFlags(), bounds); + } + /** * Generates new MeasuredParagraph for Bidi computation. * @@ -728,4 +741,7 @@ public class MeasuredParagraph { @CriticalNative private static native int nGetMemoryUsage(/* Non Zero */ long nativePtr); + + private static native void nGetBounds(long nativePtr, char[] buf, long paintPtr, int start, + int end, int bidiFlag, Rect rect); } diff --git a/core/java/android/text/PrecomputedText.java b/core/java/android/text/PrecomputedText.java index 9458184ee888..413df05d86ee 100644 --- a/core/java/android/text/PrecomputedText.java +++ b/core/java/android/text/PrecomputedText.java @@ -19,6 +19,8 @@ package android.text; import android.annotation.IntRange; import android.annotation.NonNull; import android.annotation.Nullable; +import android.graphics.Rect; +import android.text.style.MetricAffectingSpan; import com.android.internal.util.Preconditions; @@ -57,10 +59,12 @@ import java.util.Objects; * * * Note that the {@link PrecomputedText} created from different parameters of the target - * {@link android.widget.TextView} will be rejected internally and compute the text layout again - * with the current {@link android.widget.TextView} parameters. + * {@link android.widget.TextView} will be rejected. + * + * Note that any {@link android.text.NoCopySpan} attached to the original text won't be passed to + * PrecomputedText. */ -public class PrecomputedText implements Spanned { +public class PrecomputedText implements Spannable { private static final char LINE_FEED = '\n'; /** @@ -283,7 +287,7 @@ public class PrecomputedText implements Spanned { // The original text. - private final @NonNull SpannedString mText; + private final @NonNull SpannableString mText; // The inclusive start offset of the measuring target. private final @IntRange(from = 0) int mStart; @@ -304,6 +308,9 @@ public class PrecomputedText implements Spanned { * presented can save work on the UI thread. *

* + * Note that any {@link android.text.NoCopySpan} attached to the text won't be passed to the + * created PrecomputedText. + * * @param text the text to be measured * @param params parameters that define how text will be precomputed * @return A {@link PrecomputedText} @@ -347,7 +354,7 @@ public class PrecomputedText implements Spanned { private PrecomputedText(@NonNull CharSequence text, @IntRange(from = 0) int start, @IntRange(from = 0) int end, @NonNull Params params, @NonNull ParagraphInfo[] paraInfo) { - mText = new SpannedString(text); + mText = new SpannableString(text, true /* ignoreNoCopySpan */); mStart = start; mEnd = end; mParams = params; @@ -457,6 +464,21 @@ public class PrecomputedText implements Spanned { return getMeasuredParagraph(paraIndex).getWidth(start - paraStart, end - paraStart); } + /** @hide */ + public void getBounds(@IntRange(from = 0) int start, @IntRange(from = 0) int end, + @NonNull Rect bounds) { + final int paraIndex = findParaIndex(start); + final int paraStart = getParagraphStart(paraIndex); + final int paraEnd = getParagraphEnd(paraIndex); + if (start < paraStart || paraEnd < end) { + throw new RuntimeException("Cannot measured across the paragraph:" + + "para: (" + paraStart + ", " + paraEnd + "), " + + "request: (" + start + ", " + end + ")"); + } + getMeasuredParagraph(paraIndex).getBounds(mParams.mPaint, + start - paraStart, end - paraStart, bounds); + } + /** * Returns the size of native PrecomputedText memory usage. * @@ -471,6 +493,35 @@ public class PrecomputedText implements Spanned { return r; } + /////////////////////////////////////////////////////////////////////////////////////////////// + // Spannable overrides + // + // Do not allow to modify MetricAffectingSpan + + /** + * @throws IllegalArgumentException if {@link MetricAffectingSpan} is specified. + */ + @Override + public void setSpan(Object what, int start, int end, int flags) { + if (what instanceof MetricAffectingSpan) { + throw new IllegalArgumentException( + "MetricAffectingSpan can not be set to PrecomputedText."); + } + mText.setSpan(what, start, end, flags); + } + + /** + * @throws IllegalArgumentException if {@link MetricAffectingSpan} is specified. + */ + @Override + public void removeSpan(Object what) { + if (what instanceof MetricAffectingSpan) { + throw new IllegalArgumentException( + "MetricAffectingSpan can not be removed from PrecomputedText."); + } + mText.removeSpan(what); + } + /////////////////////////////////////////////////////////////////////////////////////////////// // Spanned overrides // diff --git a/core/java/android/widget/TextView.java b/core/java/android/widget/TextView.java index 72c8426482aa..54ca171fbb56 100644 --- a/core/java/android/widget/TextView.java +++ b/core/java/android/widget/TextView.java @@ -5635,6 +5635,8 @@ public class TextView extends View implements ViewTreeObserver.OnPreDrawListener needEditableForNotification = true; } + PrecomputedText precomputed = + (text instanceof PrecomputedText) ? (PrecomputedText) text : null; if (type == BufferType.EDITABLE || getKeyListener() != null || needEditableForNotification) { createEditorIfNeeded(); @@ -5644,10 +5646,7 @@ public class TextView extends View implements ViewTreeObserver.OnPreDrawListener setFilters(t, mFilters); InputMethodManager imm = InputMethodManager.peekInstance(); if (imm != null) imm.restartInput(this); - } else if (type == BufferType.SPANNABLE || mMovement != null) { - text = mSpannableFactory.newSpannable(text); - } else if (text instanceof PrecomputedText) { - PrecomputedText precomputed = (PrecomputedText) text; + } else if (precomputed != null) { if (mTextDir == null) { mTextDir = getTextDirectionHeuristic(); } @@ -5660,6 +5659,8 @@ public class TextView extends View implements ViewTreeObserver.OnPreDrawListener + "PrecomputedText: " + precomputed.getParams() + "TextView: " + getTextMetricsParams()); } + } else if (type == BufferType.SPANNABLE || mMovement != null) { + text = mSpannableFactory.newSpannable(text); } else if (!(text instanceof CharWrapper)) { text = TextUtils.stringOrSpannedString(text); } diff --git a/core/jni/android_text_MeasuredParagraph.cpp b/core/jni/android_text_MeasuredParagraph.cpp index d33337dabae0..9d794175de18 100644 --- a/core/jni/android_text_MeasuredParagraph.cpp +++ b/core/jni/android_text_MeasuredParagraph.cpp @@ -16,6 +16,7 @@ #define LOG_TAG "MeasuredParagraph" +#include "GraphicsJNI.h" #include "ScopedIcuLocale.h" #include "unicode/locid.h" #include "unicode/brkiter.h" @@ -109,6 +110,33 @@ static jfloat nGetWidth(jlong ptr, jint start, jint end) { return r; } +// Regular JNI +static void nGetBounds(JNIEnv* env, jobject, jlong ptr, jcharArray javaText, jlong paintPtr, + jint start, jint end, jint bidiFlags, jobject bounds) { + ScopedCharArrayRO text(env, javaText); + const minikin::U16StringPiece textBuffer(text.get(), text.size()); + + minikin::MeasuredText* mt = toMeasuredParagraph(ptr); + Paint* paint = toPaint(paintPtr); + const Typeface* typeface = Typeface::resolveDefault(paint->getAndroidTypeface()); + minikin::Layout layout = MinikinUtils::doLayout(paint, + static_cast(bidiFlags), typeface, textBuffer.data(), start, end - start, + textBuffer.size(), mt); + + minikin::MinikinRect rect; + layout.getBounds(&rect); + + SkRect r; + r.fLeft = rect.mLeft; + r.fTop = rect.mTop; + r.fRight = rect.mRight; + r.fBottom = rect.mBottom; + + SkIRect ir; + r.roundOut(&ir); + GraphicsJNI::irect_to_jrect(ir, env, bounds); +} + // CriticalNative static jlong nGetReleaseFunc() { return toJLong(&releaseMeasuredParagraph); @@ -128,6 +156,7 @@ static const JNINativeMethod gMethods[] = { // MeasuredParagraph native functions. {"nGetWidth", "(JII)F", (void*) nGetWidth}, // Critical Natives + {"nGetBounds", "(J[CJIIILandroid/graphics/Rect;)V", (void*) nGetBounds}, // Regular JNI {"nGetReleaseFunc", "()J", (void*) nGetReleaseFunc}, // Critical Natives {"nGetMemoryUsage", "(J)I", (void*) nGetMemoryUsage}, // Critical Native }; -- cgit v1.2.3-59-g8ed1b