From 5536c4fdee8684ebbb2d17af72ce94757a5e1a80 Mon Sep 17 00:00:00 2001 From: Roozbeh Pournader Date: Wed, 1 Jun 2016 16:49:27 -0700 Subject: [PATCH] Use bidi heuristics correctly in BoringLayout#isBoring() in order to determine if the text is RTL, the previous code ran chunks of the text (in code units) through the TextDirectionHeuristic instead of the whole text, which could result in misdetection as RTL in various cases, or missing some cases due to RTL characters getting split across chunks. Now we look at the whole text instead. This also refactors the code to make it clearer to understand and removes an unused signature for isBoring. Bug: 27702584 Change-Id: I8d98614a0af28c0d4e61af5ab4a27a8a3ab8c9dc --- core/java/android/text/BoringLayout.java | 125 +++++++++++------------ 1 file changed, 58 insertions(+), 67 deletions(-) diff --git a/core/java/android/text/BoringLayout.java b/core/java/android/text/BoringLayout.java index 7f0021d729d05..683a2c9ec70c2 100644 --- a/core/java/android/text/BoringLayout.java +++ b/core/java/android/text/BoringLayout.java @@ -220,16 +220,6 @@ public class BoringLayout extends Layout implements TextUtils.EllipsizeCallback return isBoring(text, paint, TextDirectionHeuristics.FIRSTSTRONG_LTR, null); } - /** - * Returns null if not boring; the width, ascent, and descent if boring. - * @hide - */ - public static Metrics isBoring(CharSequence text, - TextPaint paint, - TextDirectionHeuristic textDir) { - return isBoring(text, paint, textDir, null); - } - /** * Returns null if not boring; the width, ascent, and descent in the * provided Metrics object (or a new one if the provided one was null) @@ -239,6 +229,45 @@ public class BoringLayout extends Layout implements TextUtils.EllipsizeCallback return isBoring(text, paint, TextDirectionHeuristics.FIRSTSTRONG_LTR, metrics); } + /** + * Returns true if the text contains any RTL characters, bidi format characters, or surrogate + * code units. + */ + private static boolean hasAnyInterestingChars(CharSequence text, int textLength) { + final int MAX_BUF_LEN = 500; + final char[] buffer = TextUtils.obtain(MAX_BUF_LEN); + try { + for (int start = 0; start < textLength; start += MAX_BUF_LEN) { + final int end = Math.min(start + MAX_BUF_LEN, textLength); + + // No need to worry about getting half codepoints, since we consider surrogate code + // units "interesting" as soon we see one. + TextUtils.getChars(text, start, end, buffer, 0); + + final int len = end - start; + for (int i = 0; i < len; i++) { + final char c = buffer[i]; + + if (c == '\n' || c == '\t' || + (c >= 0x0590 && c <= 0x08FF) || // RTL scripts + c == 0x200E || // Bidi format character + c == 0x200F || // Bidi format character + (c >= 0x202A && c <= 0x202E) || // Bidi format characters + (c >= 0x2066 && c <= 0x2069) || // Bidi format characters + (c >= 0xD800 && c <= 0xDFFF) || // surrogate pairs + (c >= 0xFB1D && c <= 0xFDFF) || // Hebrew and Arabic presentation forms + (c >= 0xFE70 && c <= 0xFEFE) // Arabic presentation forms + ) { + return true; + } + } + } + return false; + } finally { + TextUtils.recycle(buffer); + } + } + /** * Returns null if not boring; the width, ascent, and descent in the * provided Metrics object (or a new one if the provided one was null) @@ -247,73 +276,35 @@ public class BoringLayout extends Layout implements TextUtils.EllipsizeCallback */ public static Metrics isBoring(CharSequence text, TextPaint paint, TextDirectionHeuristic textDir, Metrics metrics) { - final int MAX_BUF_LEN = 500; - final char[] buffer = TextUtils.obtain(MAX_BUF_LEN); final int textLength = text.length(); - boolean boring = true; - - outer: - for (int start = 0; start < textLength; start += MAX_BUF_LEN) { - final int end = Math.min(start + MAX_BUF_LEN, textLength); - - // No need to worry about getting half codepoints, since we reject surrogate code units - // as non-boring as soon we see one. - TextUtils.getChars(text, start, end, buffer, 0); - - final int len = end - start; - for (int i = 0; i < len; i++) { - final char c = buffer[i]; - - if (c == '\n' || c == '\t' || - (c >= 0x0590 && c <= 0x08FF) || // RTL scripts - c == 0x200F || // Bidi format character - (c >= 0x202A && c <= 0x202E) || // Bidi format characters - (c >= 0x2066 && c <= 0x2069) || // Bidi format characters - (c >= 0xD800 && c <= 0xDFFF) || // surrogate pairs - (c >= 0xFB1D && c <= 0xFDFF) || // Hebrew and Arabic presentation forms - (c >= 0xFE70 && c <= 0xFEFE) // Arabic presentation forms - ) { - boring = false; - break outer; - } - } - - // TODO: This looks a little suspicious, and in some cases can result in O(n^2) - // run time. Consider moving outside the loop. - if (textDir != null && textDir.isRtl(buffer, 0, len)) { - boring = false; - break outer; - } + if (hasAnyInterestingChars(text, textLength)) { + return null; // There are some interesting characters. Not boring. } - - TextUtils.recycle(buffer); - - if (boring && text instanceof Spanned) { + if (textDir != null && textDir.isRtl(text, 0, textLength)) { + return null; // The heuristic considers the whole text RTL. Not boring. + } + if (text instanceof Spanned) { Spanned sp = (Spanned) text; Object[] styles = sp.getSpans(0, textLength, ParagraphStyle.class); if (styles.length > 0) { - boring = false; + return null; // There are some PargraphStyle spans. Not boring. } } - if (boring) { - Metrics fm = metrics; - if (fm == null) { - fm = new Metrics(); - } else { - fm.reset(); - } - - TextLine line = TextLine.obtain(); - line.set(paint, text, 0, textLength, Layout.DIR_LEFT_TO_RIGHT, - Layout.DIRS_ALL_LEFT_TO_RIGHT, false, null); - fm.width = (int) Math.ceil(line.metrics(fm)); - TextLine.recycle(line); - - return fm; + Metrics fm = metrics; + if (fm == null) { + fm = new Metrics(); } else { - return null; + fm.reset(); } + + TextLine line = TextLine.obtain(); + line.set(paint, text, 0, textLength, Layout.DIR_LEFT_TO_RIGHT, + Layout.DIRS_ALL_LEFT_TO_RIGHT, false, null); + fm.width = (int) Math.ceil(line.metrics(fm)); + TextLine.recycle(line); + + return fm; } @Override