From 8823c85f0a98988a21bddedf1e1c3fb725258ba1 Mon Sep 17 00:00:00 2001 From: Roozbeh Pournader Date: Thu, 9 Jun 2016 18:36:47 -0700 Subject: [PATCH] Clean up TextUtils.doesNotNeedBidi() One of the signatures of the method that used CharSequence was unused. The other's implementation was too crude, resulting in too many false negatives. The new code now shares logic with BoringLayout in order to detect potential bidi-affecting characters. The logic is intentionally rough in order to be efficient: we still have false negatives, but not as many as before. Bug: 29254696 Change-Id: I106133369e93e49d7b2fe82ffc433e4556740ee8 --- core/java/android/text/BoringLayout.java | 12 +-------- core/java/android/text/TextUtils.java | 32 ++++++++++++++++-------- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/core/java/android/text/BoringLayout.java b/core/java/android/text/BoringLayout.java index 683a2c9ec70c2..eddfe033713e9 100644 --- a/core/java/android/text/BoringLayout.java +++ b/core/java/android/text/BoringLayout.java @@ -247,17 +247,7 @@ public class BoringLayout extends Layout implements TextUtils.EllipsizeCallback 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 - ) { + if (c == '\n' || c == '\t' || TextUtils.couldAffectRtl(c)) { return true; } } diff --git a/core/java/android/text/TextUtils.java b/core/java/android/text/TextUtils.java index b8fd2ffc8e5eb..d7e786a6f2c5e 100644 --- a/core/java/android/text/TextUtils.java +++ b/core/java/android/text/TextUtils.java @@ -1303,22 +1303,32 @@ public class TextUtils { return width; } - private static final char FIRST_RIGHT_TO_LEFT = '\u0590'; - + // Returns true if the character's presence could affect RTL layout. + // + // In order to be fast, the code is intentionally rough and quite conservative in its + // considering inclusion of any non-BMP or surrogate characters or anything in the bidi + // blocks or any bidi formatting characters with a potential to affect RTL layout. /* package */ - static boolean doesNotNeedBidi(CharSequence s, int start, int end) { - for (int i = start; i < end; i++) { - if (s.charAt(i) >= FIRST_RIGHT_TO_LEFT) { - return false; - } - } - return true; + static boolean couldAffectRtl(char c) { + return (0x0590 <= c && c <= 0x08FF) || // RTL scripts + c == 0x200E || // Bidi format character + c == 0x200F || // Bidi format character + (0x202A <= c && c <= 0x202E) || // Bidi format characters + (0x2066 <= c && c <= 0x2069) || // Bidi format characters + (0xD800 <= c && c <= 0xDFFF) || // Surrogate pairs + (0xFB1D <= c && c <= 0xFDFF) || // Hebrew and Arabic presentation forms + (0xFE70 <= c && c <= 0xFEFE); // Arabic presentation forms } + // Returns true if there is no character present that may potentially affect RTL layout. + // Since this calls couldAffectRtl() above, it's also quite conservative, in the way that + // it may return 'false' (needs bidi) although careful consideration may tell us it should + // return 'true' (does not need bidi). /* package */ static boolean doesNotNeedBidi(char[] text, int start, int len) { - for (int i = start, e = i + len; i < e; i++) { - if (text[i] >= FIRST_RIGHT_TO_LEFT) { + final int end = start + len; + for (int i = start; i < end; i++) { + if (couldAffectRtl(text[i])) { return false; } }