From 0249b43f6ce59bfec104f0fe606d9059244f8797 Mon Sep 17 00:00:00 2001 From: Gilles Debunne Date: Mon, 9 Apr 2012 16:02:31 -0700 Subject: Faster and simpler replace in SSB, take two This is a new version of CL 179343 which had to be reverted. This problem of the previous CL is that the ComposingSpan that was part of the replacement text was correctly added during the replace but was immediately removed because it had a zero-length size. Swapping the add and remove blocks solves the problem. The new non-zero length enforcement also revealed a bug in the spell checker where we were creating useless range spans. Change-Id: I59cebd4708af3becc7ab625ae41bc36837f1a1cf --- core/java/android/text/SpannableStringBuilder.java | 147 +++++++++------------ core/java/android/widget/SpellChecker.java | 13 +- 2 files changed, 70 insertions(+), 90 deletions(-) (limited to 'core') diff --git a/core/java/android/text/SpannableStringBuilder.java b/core/java/android/text/SpannableStringBuilder.java index ae9042c..0f8efd1 100644 --- a/core/java/android/text/SpannableStringBuilder.java +++ b/core/java/android/text/SpannableStringBuilder.java @@ -50,6 +50,8 @@ public class SpannableStringBuilder implements CharSequence, GetChars, Spannable public SpannableStringBuilder(CharSequence text, int start, int end) { int srclen = end - start; + if (srclen < 0) throw new StringIndexOutOfBoundsException(); + int len = ArrayUtils.idealCharArraySize(srclen + 1); mText = new char[len]; mGapStart = srclen; @@ -87,7 +89,7 @@ public class SpannableStringBuilder implements CharSequence, GetChars, Spannable if (en > end - start) en = end - start; - setSpan(spans[i], st, en, fl); + setSpan(false, spans[i], st, en, fl); } } } @@ -149,7 +151,7 @@ public class SpannableStringBuilder implements CharSequence, GetChars, Spannable if (where == mGapStart) return; - boolean atend = (where == length()); + boolean atEnd = (where == length()); if (where < mGapStart) { int overlap = mGapStart - where; @@ -171,7 +173,7 @@ public class SpannableStringBuilder implements CharSequence, GetChars, Spannable else if (start == where) { int flag = (mSpanFlags[i] & START_MASK) >> START_SHIFT; - if (flag == POINT || (atend && flag == PARAGRAPH)) + if (flag == POINT || (atEnd && flag == PARAGRAPH)) start += mGapLength; } @@ -182,7 +184,7 @@ public class SpannableStringBuilder implements CharSequence, GetChars, Spannable else if (end == where) { int flag = (mSpanFlags[i] & END_MASK); - if (flag == POINT || (atend && flag == PARAGRAPH)) + if (flag == POINT || (atEnd && flag == PARAGRAPH)) end += mGapLength; } @@ -284,7 +286,7 @@ public class SpannableStringBuilder implements CharSequence, GetChars, Spannable } if (st != ost || en != oen) - setSpan(mSpans[i], st, en, mSpanFlags[i]); + setSpan(false, mSpans[i], st, en, mSpanFlags[i]); } } @@ -305,28 +307,6 @@ public class SpannableStringBuilder implements CharSequence, GetChars, Spannable TextUtils.getChars(tb, tbstart, tbend, mText, start); - if (tb instanceof Spanned) { - Spanned sp = (Spanned) tb; - Object[] spans = sp.getSpans(tbstart, tbend, Object.class); - - for (int i = 0; i < spans.length; i++) { - int st = sp.getSpanStart(spans[i]); - int en = sp.getSpanEnd(spans[i]); - - if (st < tbstart) - st = tbstart; - if (en > tbend) - en = tbend; - - if (getSpanStart(spans[i]) < 0) { - setSpan(false, spans[i], - st - tbstart + start, - en - tbstart + start, - sp.getSpanFlags(spans[i])); - } - } - } - if (end > start) { // no need for span fixup on pure insertion boolean atEnd = (mGapStart + mGapLength == mText.length); @@ -358,6 +338,25 @@ public class SpannableStringBuilder implements CharSequence, GetChars, Spannable } } } + + if (tb instanceof Spanned) { + Spanned sp = (Spanned) tb; + Object[] spans = sp.getSpans(tbstart, tbend, Object.class); + + for (int i = 0; i < spans.length; i++) { + int st = sp.getSpanStart(spans[i]); + int en = sp.getSpanEnd(spans[i]); + + if (st < tbstart) st = tbstart; + if (en > tbend) en = tbend; + + // Add span only if this object is not yet used as a span in this string + if (getSpanStart(spans[i]) < 0) { + setSpan(false, spans[i], st - tbstart + start, en - tbstart + start, + sp.getSpanFlags(spans[i])); + } + } + } } private void removeSpan(int i) { @@ -389,7 +388,7 @@ public class SpannableStringBuilder implements CharSequence, GetChars, Spannable // Documentation from interface public SpannableStringBuilder replace(final int start, final int end, - CharSequence tb, int tbstart, int tbend) { + CharSequence tb, int tbstart, int tbend) { int filtercount = mFilters.length; for (int i = 0; i < filtercount; i++) { CharSequence repl = mFilters[i].filter(tb, tbstart, tbend, this, start, end); @@ -411,53 +410,26 @@ public class SpannableStringBuilder implements CharSequence, GetChars, Spannable TextWatcher[] textWatchers = getSpans(start, start + origLen, TextWatcher.class); sendBeforeTextChanged(textWatchers, start, origLen, newLen); - if (origLen == 0 || newLen == 0) { - change(start, end, tb, tbstart, tbend); - } else { - int selstart = Selection.getSelectionStart(this); - int selend = Selection.getSelectionEnd(this); - - // XXX just make the span fixups in change() do the right thing - // instead of this madness! - - checkRange("replace", start, end); - moveGapTo(end); - - if (mGapLength < 2) - resizeFor(length() + 1); - - for (int i = mSpanCount - 1; i >= 0; i--) { - if (mSpanStarts[i] == mGapStart) - mSpanStarts[i]++; + // Try to keep the cursor / selection at the same relative position during + // a text replacement. If replaced or replacement text length is zero, this + // is already taken care of. + boolean adjustSelection = origLen != 0 && newLen != 0; + int selstart = 0; + int selend = 0; + if (adjustSelection) { + selstart = Selection.getSelectionStart(this); + selend = Selection.getSelectionEnd(this); + } - if (mSpanEnds[i] == mGapStart) - mSpanEnds[i]++; - } + checkRange("replace", start, end); - mText[mGapStart] = ' '; - mGapStart++; - mGapLength--; + change(start, end, tb, tbstart, tbend); - if (mGapLength < 1) { - new Exception("mGapLength < 1").printStackTrace(); - } - - change(start + 1, start + 1, tb, tbstart, tbend); - change(start, start + 1, "", 0, 0); - change(start + newLen, start + newLen + origLen, "", 0, 0); - - /* - * Special case to keep the cursor in the same position - * if it was somewhere in the middle of the replaced region. - * If it was at the start or the end or crossing the whole - * replacement, it should already be where it belongs. - * TODO: Is there some more general mechanism that could - * accomplish this? - */ + if (adjustSelection) { if (selstart > start && selstart < end) { long off = selstart - start; - off = off * newLen / (end - start); + off = off * newLen / origLen; selstart = (int) off + start; setSpan(false, Selection.SELECTION_START, selstart, selstart, @@ -466,7 +438,7 @@ public class SpannableStringBuilder implements CharSequence, GetChars, Spannable if (selend > start && selend < end) { long off = selend - start; - off = off * newLen / (end - start); + off = off * newLen / origLen; selend = (int) off + start; setSpan(false, Selection.SELECTION_END, selend, selend, Spanned.SPAN_POINT_POINT); @@ -489,12 +461,10 @@ public class SpannableStringBuilder implements CharSequence, GetChars, Spannable } private void setSpan(boolean send, Object what, int start, int end, int flags) { - int nstart = start; - int nend = end; - checkRange("setSpan", start, end); - if ((flags & START_MASK) == (PARAGRAPH << START_SHIFT)) { + int flagsStart = (flags & START_MASK) >> START_SHIFT; + if (flagsStart == PARAGRAPH) { if (start != 0 && start != length()) { char c = charAt(start - 1); @@ -503,7 +473,8 @@ public class SpannableStringBuilder implements CharSequence, GetChars, Spannable } } - if ((flags & END_MASK) == PARAGRAPH) { + int flagsEnd = flags & END_MASK; + if (flagsEnd == PARAGRAPH) { if (end != 0 && end != length()) { char c = charAt(end - 1); @@ -512,26 +483,33 @@ public class SpannableStringBuilder implements CharSequence, GetChars, Spannable } } - if (flags == Spanned.SPAN_EXCLUSIVE_EXCLUSIVE && start == end) { - throw new IllegalArgumentException( - "SPAN_EXCLUSIVE_EXCLUSIVE spans cannot have a zero length"); + // 0-length Spanned.SPAN_EXCLUSIVE_EXCLUSIVE + if (flagsStart == POINT && flagsEnd == MARK && start == end) { + if (send) { + throw new IllegalArgumentException( + "SPAN_EXCLUSIVE_EXCLUSIVE spans cannot have a zero length"); + } else { + // Silently ignore invalid spans when they are created from this class. + // This avoids the duplication of the above test code before all the + // calls to setSpan that are done in this class + return; + } } + int nstart = start; + int nend = end; + if (start > mGapStart) { start += mGapLength; } else if (start == mGapStart) { - int flag = (flags & START_MASK) >> START_SHIFT; - - if (flag == POINT || (flag == PARAGRAPH && start == length())) + if (flagsStart == POINT || (flagsStart == PARAGRAPH && start == length())) start += mGapLength; } if (end > mGapStart) { end += mGapLength; } else if (end == mGapStart) { - int flag = (flags & END_MASK); - - if (flag == POINT || (flag == PARAGRAPH && end == length())) + if (flagsEnd == POINT || (flagsEnd == PARAGRAPH && end == length())) end += mGapLength; } @@ -1231,6 +1209,7 @@ public class SpannableStringBuilder implements CharSequence, GetChars, Spannable private int mSpanCount; // TODO These value are tightly related to the public SPAN_MARK/POINT values in {@link Spanned} + private static final int MARK = 1; private static final int POINT = 2; private static final int PARAGRAPH = 3; diff --git a/core/java/android/widget/SpellChecker.java b/core/java/android/widget/SpellChecker.java index 9afaee3..c725b64 100644 --- a/core/java/android/widget/SpellChecker.java +++ b/core/java/android/widget/SpellChecker.java @@ -227,8 +227,7 @@ public class SpellChecker implements SpellCheckerSessionListener { for (int i = 0; i < length; i++) { final SpellParser spellParser = mSpellParsers[i]; if (spellParser.isFinished()) { - spellParser.init(start, end); - spellParser.parse(); + spellParser.parse(start, end); return; } } @@ -240,8 +239,7 @@ public class SpellChecker implements SpellCheckerSessionListener { SpellParser spellParser = new SpellParser(); mSpellParsers[length] = spellParser; - spellParser.init(start, end); - spellParser.parse(); + spellParser.parse(start, end); } private void spellCheck() { @@ -421,8 +419,11 @@ public class SpellChecker implements SpellCheckerSessionListener { private class SpellParser { private Object mRange = new Object(); - public void init(int start, int end) { - setRangeSpan((Editable) mTextView.getText(), start, end); + public void parse(int start, int end) { + if (end > start) { + setRangeSpan((Editable) mTextView.getText(), start, end); + parse(); + } } public boolean isFinished() { -- cgit v1.1