Skip to content

Commit d3ce6f5

Browse files
Gilles DebunneAndroid (Google) Code Review
authored andcommitted
Merge "Faster and simpler replace in SSB, take two"
2 parents 79af606 + 0249b43 commit d3ce6f5

File tree

2 files changed

+70
-90
lines changed

2 files changed

+70
-90
lines changed

core/java/android/text/SpannableStringBuilder.java

Lines changed: 63 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@ public SpannableStringBuilder(CharSequence text) {
5050
public SpannableStringBuilder(CharSequence text, int start, int end) {
5151
int srclen = end - start;
5252

53+
if (srclen < 0) throw new StringIndexOutOfBoundsException();
54+
5355
int len = ArrayUtils.idealCharArraySize(srclen + 1);
5456
mText = new char[len];
5557
mGapStart = srclen;
@@ -87,7 +89,7 @@ public SpannableStringBuilder(CharSequence text, int start, int end) {
8789
if (en > end - start)
8890
en = end - start;
8991

90-
setSpan(spans[i], st, en, fl);
92+
setSpan(false, spans[i], st, en, fl);
9193
}
9294
}
9395
}
@@ -149,7 +151,7 @@ private void moveGapTo(int where) {
149151
if (where == mGapStart)
150152
return;
151153

152-
boolean atend = (where == length());
154+
boolean atEnd = (where == length());
153155

154156
if (where < mGapStart) {
155157
int overlap = mGapStart - where;
@@ -171,7 +173,7 @@ private void moveGapTo(int where) {
171173
else if (start == where) {
172174
int flag = (mSpanFlags[i] & START_MASK) >> START_SHIFT;
173175

174-
if (flag == POINT || (atend && flag == PARAGRAPH))
176+
if (flag == POINT || (atEnd && flag == PARAGRAPH))
175177
start += mGapLength;
176178
}
177179

@@ -182,7 +184,7 @@ else if (start == where) {
182184
else if (end == where) {
183185
int flag = (mSpanFlags[i] & END_MASK);
184186

185-
if (flag == POINT || (atend && flag == PARAGRAPH))
187+
if (flag == POINT || (atEnd && flag == PARAGRAPH))
186188
end += mGapLength;
187189
}
188190

@@ -284,7 +286,7 @@ private void change(int start, int end, CharSequence tb, int tbstart, int tbend)
284286
}
285287

286288
if (st != ost || en != oen)
287-
setSpan(mSpans[i], st, en, mSpanFlags[i]);
289+
setSpan(false, mSpans[i], st, en, mSpanFlags[i]);
288290
}
289291
}
290292

@@ -305,28 +307,6 @@ private void change(int start, int end, CharSequence tb, int tbstart, int tbend)
305307

306308
TextUtils.getChars(tb, tbstart, tbend, mText, start);
307309

308-
if (tb instanceof Spanned) {
309-
Spanned sp = (Spanned) tb;
310-
Object[] spans = sp.getSpans(tbstart, tbend, Object.class);
311-
312-
for (int i = 0; i < spans.length; i++) {
313-
int st = sp.getSpanStart(spans[i]);
314-
int en = sp.getSpanEnd(spans[i]);
315-
316-
if (st < tbstart)
317-
st = tbstart;
318-
if (en > tbend)
319-
en = tbend;
320-
321-
if (getSpanStart(spans[i]) < 0) {
322-
setSpan(false, spans[i],
323-
st - tbstart + start,
324-
en - tbstart + start,
325-
sp.getSpanFlags(spans[i]));
326-
}
327-
}
328-
}
329-
330310
if (end > start) {
331311
// no need for span fixup on pure insertion
332312
boolean atEnd = (mGapStart + mGapLength == mText.length);
@@ -358,6 +338,25 @@ private void change(int start, int end, CharSequence tb, int tbstart, int tbend)
358338
}
359339
}
360340
}
341+
342+
if (tb instanceof Spanned) {
343+
Spanned sp = (Spanned) tb;
344+
Object[] spans = sp.getSpans(tbstart, tbend, Object.class);
345+
346+
for (int i = 0; i < spans.length; i++) {
347+
int st = sp.getSpanStart(spans[i]);
348+
int en = sp.getSpanEnd(spans[i]);
349+
350+
if (st < tbstart) st = tbstart;
351+
if (en > tbend) en = tbend;
352+
353+
// Add span only if this object is not yet used as a span in this string
354+
if (getSpanStart(spans[i]) < 0) {
355+
setSpan(false, spans[i], st - tbstart + start, en - tbstart + start,
356+
sp.getSpanFlags(spans[i]));
357+
}
358+
}
359+
}
361360
}
362361

363362
private void removeSpan(int i) {
@@ -389,7 +388,7 @@ public SpannableStringBuilder replace(int start, int end, CharSequence tb) {
389388

390389
// Documentation from interface
391390
public SpannableStringBuilder replace(final int start, final int end,
392-
CharSequence tb, int tbstart, int tbend) {
391+
CharSequence tb, int tbstart, int tbend) {
393392
int filtercount = mFilters.length;
394393
for (int i = 0; i < filtercount; i++) {
395394
CharSequence repl = mFilters[i].filter(tb, tbstart, tbend, this, start, end);
@@ -411,53 +410,26 @@ public SpannableStringBuilder replace(final int start, final int end,
411410
TextWatcher[] textWatchers = getSpans(start, start + origLen, TextWatcher.class);
412411
sendBeforeTextChanged(textWatchers, start, origLen, newLen);
413412

414-
if (origLen == 0 || newLen == 0) {
415-
change(start, end, tb, tbstart, tbend);
416-
} else {
417-
int selstart = Selection.getSelectionStart(this);
418-
int selend = Selection.getSelectionEnd(this);
419-
420-
// XXX just make the span fixups in change() do the right thing
421-
// instead of this madness!
422-
423-
checkRange("replace", start, end);
424-
moveGapTo(end);
425-
426-
if (mGapLength < 2)
427-
resizeFor(length() + 1);
428-
429-
for (int i = mSpanCount - 1; i >= 0; i--) {
430-
if (mSpanStarts[i] == mGapStart)
431-
mSpanStarts[i]++;
413+
// Try to keep the cursor / selection at the same relative position during
414+
// a text replacement. If replaced or replacement text length is zero, this
415+
// is already taken care of.
416+
boolean adjustSelection = origLen != 0 && newLen != 0;
417+
int selstart = 0;
418+
int selend = 0;
419+
if (adjustSelection) {
420+
selstart = Selection.getSelectionStart(this);
421+
selend = Selection.getSelectionEnd(this);
422+
}
432423

433-
if (mSpanEnds[i] == mGapStart)
434-
mSpanEnds[i]++;
435-
}
424+
checkRange("replace", start, end);
436425

437-
mText[mGapStart] = ' ';
438-
mGapStart++;
439-
mGapLength--;
426+
change(start, end, tb, tbstart, tbend);
440427

441-
if (mGapLength < 1) {
442-
new Exception("mGapLength < 1").printStackTrace();
443-
}
444-
445-
change(start + 1, start + 1, tb, tbstart, tbend);
446-
change(start, start + 1, "", 0, 0);
447-
change(start + newLen, start + newLen + origLen, "", 0, 0);
448-
449-
/*
450-
* Special case to keep the cursor in the same position
451-
* if it was somewhere in the middle of the replaced region.
452-
* If it was at the start or the end or crossing the whole
453-
* replacement, it should already be where it belongs.
454-
* TODO: Is there some more general mechanism that could
455-
* accomplish this?
456-
*/
428+
if (adjustSelection) {
457429
if (selstart > start && selstart < end) {
458430
long off = selstart - start;
459431

460-
off = off * newLen / (end - start);
432+
off = off * newLen / origLen;
461433
selstart = (int) off + start;
462434

463435
setSpan(false, Selection.SELECTION_START, selstart, selstart,
@@ -466,7 +438,7 @@ public SpannableStringBuilder replace(final int start, final int end,
466438
if (selend > start && selend < end) {
467439
long off = selend - start;
468440

469-
off = off * newLen / (end - start);
441+
off = off * newLen / origLen;
470442
selend = (int) off + start;
471443

472444
setSpan(false, Selection.SELECTION_END, selend, selend, Spanned.SPAN_POINT_POINT);
@@ -489,12 +461,10 @@ public void setSpan(Object what, int start, int end, int flags) {
489461
}
490462

491463
private void setSpan(boolean send, Object what, int start, int end, int flags) {
492-
int nstart = start;
493-
int nend = end;
494-
495464
checkRange("setSpan", start, end);
496465

497-
if ((flags & START_MASK) == (PARAGRAPH << START_SHIFT)) {
466+
int flagsStart = (flags & START_MASK) >> START_SHIFT;
467+
if (flagsStart == PARAGRAPH) {
498468
if (start != 0 && start != length()) {
499469
char c = charAt(start - 1);
500470

@@ -503,7 +473,8 @@ private void setSpan(boolean send, Object what, int start, int end, int flags) {
503473
}
504474
}
505475

506-
if ((flags & END_MASK) == PARAGRAPH) {
476+
int flagsEnd = flags & END_MASK;
477+
if (flagsEnd == PARAGRAPH) {
507478
if (end != 0 && end != length()) {
508479
char c = charAt(end - 1);
509480

@@ -512,26 +483,33 @@ private void setSpan(boolean send, Object what, int start, int end, int flags) {
512483
}
513484
}
514485

515-
if (flags == Spanned.SPAN_EXCLUSIVE_EXCLUSIVE && start == end) {
516-
throw new IllegalArgumentException(
517-
"SPAN_EXCLUSIVE_EXCLUSIVE spans cannot have a zero length");
486+
// 0-length Spanned.SPAN_EXCLUSIVE_EXCLUSIVE
487+
if (flagsStart == POINT && flagsEnd == MARK && start == end) {
488+
if (send) {
489+
throw new IllegalArgumentException(
490+
"SPAN_EXCLUSIVE_EXCLUSIVE spans cannot have a zero length");
491+
} else {
492+
// Silently ignore invalid spans when they are created from this class.
493+
// This avoids the duplication of the above test code before all the
494+
// calls to setSpan that are done in this class
495+
return;
496+
}
518497
}
519498

499+
int nstart = start;
500+
int nend = end;
501+
520502
if (start > mGapStart) {
521503
start += mGapLength;
522504
} else if (start == mGapStart) {
523-
int flag = (flags & START_MASK) >> START_SHIFT;
524-
525-
if (flag == POINT || (flag == PARAGRAPH && start == length()))
505+
if (flagsStart == POINT || (flagsStart == PARAGRAPH && start == length()))
526506
start += mGapLength;
527507
}
528508

529509
if (end > mGapStart) {
530510
end += mGapLength;
531511
} else if (end == mGapStart) {
532-
int flag = (flags & END_MASK);
533-
534-
if (flag == POINT || (flag == PARAGRAPH && end == length()))
512+
if (flagsEnd == POINT || (flagsEnd == PARAGRAPH && end == length()))
535513
end += mGapLength;
536514
}
537515

@@ -1231,6 +1209,7 @@ public InputFilter[] getFilters() {
12311209
private int mSpanCount;
12321210

12331211
// TODO These value are tightly related to the public SPAN_MARK/POINT values in {@link Spanned}
1212+
private static final int MARK = 1;
12341213
private static final int POINT = 2;
12351214
private static final int PARAGRAPH = 3;
12361215

core/java/android/widget/SpellChecker.java

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -227,8 +227,7 @@ public void spellCheck(int start, int end) {
227227
for (int i = 0; i < length; i++) {
228228
final SpellParser spellParser = mSpellParsers[i];
229229
if (spellParser.isFinished()) {
230-
spellParser.init(start, end);
231-
spellParser.parse();
230+
spellParser.parse(start, end);
232231
return;
233232
}
234233
}
@@ -240,8 +239,7 @@ public void spellCheck(int start, int end) {
240239

241240
SpellParser spellParser = new SpellParser();
242241
mSpellParsers[length] = spellParser;
243-
spellParser.init(start, end);
244-
spellParser.parse();
242+
spellParser.parse(start, end);
245243
}
246244

247245
private void spellCheck() {
@@ -421,8 +419,11 @@ private void createMisspelledSuggestionSpan(Editable editable, SuggestionsInfo s
421419
private class SpellParser {
422420
private Object mRange = new Object();
423421

424-
public void init(int start, int end) {
425-
setRangeSpan((Editable) mTextView.getText(), start, end);
422+
public void parse(int start, int end) {
423+
if (end > start) {
424+
setRangeSpan((Editable) mTextView.getText(), start, end);
425+
parse();
426+
}
426427
}
427428

428429
public boolean isFinished() {

0 commit comments

Comments
 (0)