Fixed text rendering issue with spans.

Bug 6598784

The algorithm uses three imbricated loops:
- paragraphs
- span regions (called "blocks" in that description) in these
- characters in these

We can ignore the paragraphs and assume paraStart==0.

The span region loop cuts the text into blocks of text which share
the same set of MetricAffectingSpan spans applied to them. Note that
spanStart and spanEnd represent such a range, and not necessarily an
actual span range.

The third loop then iterates over the characters of these blocks, and creates
a new line (calling out() as soon as the width has been reached.

The core of the problem comes from the 'nextSpanStart' variable.
It is used to restart the block loop from a previous position in case
a line has been created that does not intersect with the current block.

However, in case the current block is larger than the width of the text,
the character loop is going to create other lines of text before we exit the
j-loop. Going back to the block loop, we reset spanStart to the nextSpanStart,
which may be too far back in the text. As a result, the same range of characters
is measured again.

The (spanStart == spanEnd) test was used to handle the case where
nextSpanStart was indeed assigned to a value different than spanEnd.

This fix simplifies this logic and removes the nextSpanStart variable:

When the created line ends before the current block (here < spanStart), we
immediately exit the character loop, re-starting the block loop from the
current position.

Patch 4: added a fix in measured to handle overlapping character ranges.

Change-Id: Ie71b3cf4018b332e335ea916fef08acb43a6679e

2 files changed, 32 insertions, 33 deletions

diff --git a/core/java/android/text/MeasuredText.java b/core/java/android/text/MeasuredText.java
index 715d1f2..445aac6 100644
--- a/core/java/android/text/MeasuredText.java
+++ b/core/java/android/text/MeasuredText.java
@@ -82,6 +82,10 @@ class MeasuredText {
         return null;
     }
 
+    void setPos(int pos) {
+        mPos = pos;
+    }
+
     /**
      * Analyzes text for bidirectional runs.  Allocates working buffers.
      */
@@ -113,7 +117,7 @@ class MeasuredText {
                 if (startInPara < 0) startInPara = 0;
                 if (endInPara > len) endInPara = len;
                 for (int j = startInPara; j < endInPara; j++) {
-                    mChars[j] = '\uFFFC';
+                    mChars[j] = '\uFFFC'; // object replacement character
                 }
             }
         }
diff --git a/core/java/android/text/StaticLayout.java b/core/java/android/text/StaticLayout.java
index 299e115..ad17cae 100644
--- a/core/java/android/text/StaticLayout.java
+++ b/core/java/android/text/StaticLayout.java
@@ -246,12 +246,17 @@ public class StaticLayout extends Layout {
             int width = firstWidth;
 
             float w = 0;
+            // here is the offset of the starting character of the line we are currently measuring
             int here = paraStart;
 
+            // ok is a character offset located after a word separator (space, tab, number...) where
+            // we would prefer to cut the current line. Equals to here when no such break was found.
             int ok = paraStart;
             float okWidth = w;
             int okAscent = 0, okDescent = 0, okTop = 0, okBottom = 0;
 
+            // fit is a character offset such that the [here, fit[ range fits in the allowed width.
+            // We will cut the line there if no ok position is found.
             int fit = paraStart;
             float fitWidth = w;
             int fitAscent = 0, fitDescent = 0, fitTop = 0, fitBottom = 0;
@@ -260,30 +265,22 @@ public class StaticLayout extends Layout {
             boolean hasTab = false;
             TabStops tabStops = null;
 
-            for (int spanStart = paraStart, spanEnd = spanStart, nextSpanStart;
-                    spanStart < paraEnd; spanStart = nextSpanStart) {
-
-                if (spanStart == spanEnd) {
-                    if (spanned == null)
-                        spanEnd = paraEnd;
-                    else
-                        spanEnd = spanned.nextSpanTransition(spanStart, paraEnd,
-                                MetricAffectingSpan.class);
+            for (int spanStart = paraStart, spanEnd; spanStart < paraEnd; spanStart = spanEnd) {
 
+                if (spanned == null) {
+                    spanEnd = paraEnd;
                     int spanLen = spanEnd - spanStart;
-                    if (spanned == null) {
-                        measured.addStyleRun(paint, spanLen, fm);
-                    } else {
-                        MetricAffectingSpan[] spans =
+                    measured.addStyleRun(paint, spanLen, fm);
+                } else {
+                    spanEnd = spanned.nextSpanTransition(spanStart, paraEnd,
+                            MetricAffectingSpan.class);
+                    int spanLen = spanEnd - spanStart;
+                    MetricAffectingSpan[] spans =
                             spanned.getSpans(spanStart, spanEnd, MetricAffectingSpan.class);
-                        spans = TextUtils.removeEmptySpans(spans, spanned,
-                                MetricAffectingSpan.class);
-                        measured.addStyleRun(paint, spans, spanLen, fm);
-                    }
+                    spans = TextUtils.removeEmptySpans(spans, spanned, MetricAffectingSpan.class);
+                    measured.addStyleRun(paint, spans, spanLen, fm);
                 }
 
-                nextSpanStart = spanEnd;
-
                 int fmTop = fm.top;
                 int fmBottom = fm.bottom;
                 int fmAscent = fm.ascent;
@@ -343,8 +340,6 @@ public class StaticLayout extends Layout {
                         w += widths[j - paraStart];
                     }
 
-                    // Log.e("text", "was " + before + " now " + w + " after " + c + " within " + width);
-
                     if (w <= width) {
                         fitWidth = w;
                         fit = j + 1;
@@ -373,7 +368,6 @@ public class StaticLayout extends Layout {
                          * except for NS (non-starters), which can be broken
                          * after but not before.
                          */
-
                         if (c == CHAR_SPACE || c == CHAR_TAB ||
                             ((c == CHAR_DOT || c == CHAR_COMMA ||
                                     c == CHAR_COLON || c == CHAR_SEMICOLON) &&
@@ -437,17 +431,9 @@ public class StaticLayout extends Layout {
                                 needMultiply, chdirs, dir, easy, bufEnd, includepad, trackpad,
                                 chs, widths, paraStart, ellipsize, ellipsizedWidth,
                                 currentTextWidth, paint, moreChars);
-                        here = endPos;
-
-                        if (here < spanStart) {
-                            // didn't output all the text for this span
-                            // we've measured the raw widths, though, so
-                            // just reset the start point
-                            j = nextSpanStart = here;
-                        } else {
-                            j = here - 1;    // continue looping
-                        }
 
+                        here = endPos;
+                        j = here - 1; // restart j-span loop from here, compensating for the j++
                         ok = fit = here;
                         w = 0;
                         fitAscent = fitDescent = fitTop = fitBottom = 0;
@@ -456,7 +442,16 @@ public class StaticLayout extends Layout {
                         if (--firstWidthLineLimit <= 0) {
                             width = restWidth;
                         }
+
+                        if (here < spanStart) {
+                            // The text was cut before the beginning of the current span range.
+                            // Exit the span loop, and get spanStart to start over from here.
+                            measured.setPos(here);
+                            spanEnd = here;
+                            break;
+                        }
                     }
+                    // FIXME This should be moved in the above else block which changes mLineCount
                     if (mLineCount >= mMaximumVisibleLineCount) {
                         break;
                     }