Enhance text docs, rename some variables for clarity, comment places in the code

for further investigation.

5 files changed, 372 insertions, 170 deletions

diff --git a/core/java/android/text/Layout.java b/core/java/android/text/Layout.java
index afc6864..1023036 100644
--- a/core/java/android/text/Layout.java
+++ b/core/java/android/text/Layout.java
@@ -39,6 +39,8 @@ import android.view.KeyEvent;
  */
 public abstract class Layout {
     private static final boolean DEBUG = false;
+    private static final ParagraphStyle[] NO_PARA_SPANS =
+        ArrayUtils.emptyArray(ParagraphStyle.class);
 
     /* package */ static final EmojiFactory EMOJI_FACTORY =
         EmojiFactory.newAvailableInstance();
@@ -57,7 +59,7 @@ public abstract class Layout {
     private RectF mEmojiRect;
 
     /**
-     * Return how wide a layout would be necessary to display the
+     * Return how wide a layout must be in order to display the
      * specified text with one line per paragraph.
      */
     public static float getDesiredWidth(CharSequence source,
@@ -66,7 +68,7 @@ public abstract class Layout {
     }
     
     /**
-     * Return how wide a layout would be necessary to display the
+     * Return how wide a layout must be in order to display the
      * specified text slice with one line per paragraph.
      */
     public static float getDesiredWidth(CharSequence source,
@@ -82,6 +84,7 @@ public abstract class Layout {
             if (next < 0)
                 next = end;
 
+            // note, omits trailing paragraph char
             float w = measureText(paint, workPaint,
                                   source, i, next, null, true, null);
 
@@ -97,10 +100,19 @@ public abstract class Layout {
     /**
      * Subclasses of Layout use this constructor to set the display text,
      * width, and other standard properties.
+     * @param text the text to render
+     * @param paint the default paint for the layout.  Styles can override
+     * various attributes of the paint.
+     * @param width the wrapping width for the text.
+     * @param align whether to left, right, or center the text.  Styles can
+     * override the alignment.
+     * @param spacingMult factor by which to scale the font size to get the
+     * default line spacing
+     * @param spacingAdd amount to add to the default line spacing
      */
     protected Layout(CharSequence text, TextPaint paint,
                      int width, Alignment align,
-                     float spacingmult, float spacingadd) {
+                     float spacingMult, float spacingAdd) {
         if (width < 0)
             throw new IllegalArgumentException("Layout: " + width + " < 0");
 
@@ -109,8 +121,8 @@ public abstract class Layout {
         mWorkPaint = new TextPaint();
         mWidth = width;
         mAlignment = align;
-        mSpacingMult = spacingmult;
-        mSpacingAdd = spacingadd;
+        mSpacingMult = spacingMult;
+        mSpacingAdd = spacingAdd;
         mSpannedText = text instanceof Spanned;
     }
 
@@ -141,10 +153,16 @@ public abstract class Layout {
     }
 
     /**
-     * Draw the specified rectangle from this Layout on the specified Canvas,
-     * with the specified path drawn between the background and the text.
+     * Draw this Layout on the specified canvas, with the highlight path drawn
+     * between the background and the text.
+     *
+     * @param c the canvas
+     * @param highlight the path of the highlight or cursor; can be null
+     * @param highlightPaint the paint for the highlight
+     * @param cursorOffsetVertical the amount to temporarily translate the
+     *        canvas while rendering the highlight
      */
-    public void draw(Canvas c, Path highlight, Paint highlightpaint,
+    public void draw(Canvas c, Path highlight, Paint highlightPaint,
                      int cursorOffsetVertical) {
         int dtop, dbottom;
 
@@ -157,13 +175,10 @@ public abstract class Layout {
             dbottom = sTempRect.bottom;
         }
 
-        TextPaint paint = mPaint;
 
         int top = 0;
-        // getLineBottom(getLineCount() -1) just calls getLineTop(getLineCount) 
         int bottom = getLineTop(getLineCount());
 
-
         if (dtop > top) {
             top = dtop;
         }
@@ -177,16 +192,19 @@ public abstract class Layout {
         int previousLineBottom = getLineTop(first);
         int previousLineEnd = getLineStart(first);
         
+        TextPaint paint = mPaint;
         CharSequence buf = mText;
+        int width = mWidth;
+        boolean spannedText = mSpannedText;
 
-        ParagraphStyle[] nospans = ArrayUtils.emptyArray(ParagraphStyle.class);
-        ParagraphStyle[] spans = nospans;
+        ParagraphStyle[] spans = NO_PARA_SPANS;
         int spanend = 0;
         int textLength = 0;
-        boolean spannedText = mSpannedText;
 
+        // First, draw LineBackgroundSpans.
+        // LineBackgroundSpans know nothing about the alignment or direction of
+        // the layout or line.  XXX: Should they?
         if (spannedText) {
-            spanend = 0;
             textLength = buf.length();
             for (int i = first; i <= last; i++) {
                 int start = previousLineEnd;
@@ -209,7 +227,7 @@ public abstract class Layout {
                 for (int n = 0; n < spans.length; n++) {
                     LineBackgroundSpan back = (LineBackgroundSpan) spans[n];
 
-                    back.drawBackground(c, paint, 0, mWidth,
+                    back.drawBackground(c, paint, 0, width,
                                        ltop, lbaseline, lbottom,
                                        buf, start, end,
                                        i);
@@ -219,7 +237,7 @@ public abstract class Layout {
             spanend = 0;
             previousLineBottom = getLineTop(first);
             previousLineEnd = getLineStart(first);
-            spans = nospans;
+            spans = NO_PARA_SPANS;
         } 
 
         // There can be a highlight even without spans if we are drawing
@@ -229,7 +247,7 @@ public abstract class Layout {
                 c.translate(0, cursorOffsetVertical);
             }
 
-            c.drawPath(highlight, highlightpaint);
+            c.drawPath(highlight, highlightPaint);
 
             if (cursorOffsetVertical != 0) {
                 c.translate(0, -cursorOffsetVertical);
@@ -238,6 +256,9 @@ public abstract class Layout {
 
         Alignment align = mAlignment;
         
+        // Next draw the lines, one at a time.
+        // the baseline is the top of the following line minus the current
+        // line's descent.
         for (int i = first; i <= last; i++) {
             int start = previousLineEnd;
 
@@ -249,21 +270,20 @@ public abstract class Layout {
             previousLineBottom = lbottom;
             int lbaseline = lbottom - getLineDescent(i);
 
-            boolean par = false;
+            boolean isFirstParaLine = false;
             if (spannedText) { 
                 if (start == 0 || buf.charAt(start - 1) == '\n') {
-                    par = true;
+                    isFirstParaLine = true;
                 }
+                // New batch of paragraph styles, compute the alignment.
+                // Last alignment style wins.
                 if (start >= spanend) {
-
                     Spanned sp = (Spanned) buf;
-
                     spanend = sp.nextSpanTransition(start, textLength,
                                                     ParagraphStyle.class);
                     spans = sp.getSpans(start, spanend, ParagraphStyle.class);
                     
                     align = mAlignment;
-                    
                     for (int n = spans.length-1; n >= 0; n--) {
                         if (spans[n] instanceof AlignmentSpan) {
                             align = ((AlignmentSpan) spans[n]).getAlignment();
@@ -277,6 +297,8 @@ public abstract class Layout {
             int left = 0;
             int right = mWidth;
 
+            // Draw all leading margin spans.  Adjust left or right according
+            // to the paragraph direction of the line.
             if (spannedText) {
                 final int length = spans.length;
                 for (int n = 0; n < length; n++) {
@@ -286,15 +308,15 @@ public abstract class Layout {
                         if (dir == DIR_RIGHT_TO_LEFT) {
                             margin.drawLeadingMargin(c, paint, right, dir, ltop,
                                                      lbaseline, lbottom, buf,
-                                                     start, end, par, this);
+                                                     start, end, isFirstParaLine, this);
                                 
-                            right -= margin.getLeadingMargin(par);
+                            right -= margin.getLeadingMargin(isFirstParaLine);
                         } else {
                             margin.drawLeadingMargin(c, paint, left, dir, ltop,
                                                      lbaseline, lbottom, buf,
-                                                     start, end, par, this);
+                                                     start, end, isFirstParaLine, this);
 
-                            boolean useMargin = par;
+                            boolean useMargin = isFirstParaLine;
                             if (margin instanceof LeadingMarginSpan.LeadingMarginSpan2) {
                                 int count = ((LeadingMarginSpan.LeadingMarginSpan2)margin).getLeadingMarginLineCount();
                                 useMargin = count > i;
@@ -305,6 +327,8 @@ public abstract class Layout {
                 }
             }
 
+            // Adjust the point at which to start rendering depending on the
+            // alignment of the paragraph.
             int x;
             if (align == Alignment.ALIGN_NORMAL) {
                 if (dir == DIR_LEFT_TO_RIGHT) {
@@ -340,6 +364,7 @@ public abstract class Layout {
                     Assert.assertTrue(dir == DIR_LEFT_TO_RIGHT);
                     Assert.assertNotNull(c);
                 }
+                // XXX: assumes there's nothing additional to be done
                 c.drawText(buf, start, end, x, lbaseline, paint);
             } else {
                 drawText(c, buf, start, end, dir, directions,
@@ -382,7 +407,7 @@ public abstract class Layout {
 
     /**
      * Increase the width of this layout to the specified width.
-     * Be careful to use this only when you know it is appropriate --
+     * Be careful to use this only when you know it is appropriate&mdash;
      * it does not cause the text to reflow to use the full new width.
      */
     public final void increaseWidthTo(int wid) {
@@ -397,7 +422,7 @@ public abstract class Layout {
      * Return the total height of this layout.
      */
     public int getHeight() {
-        return getLineTop(getLineCount());  // same as getLineBottom(getLineCount() - 1);
+        return getLineTop(getLineCount());
     }
 
     /**
@@ -439,33 +464,35 @@ public abstract class Layout {
             bounds.left = 0;     // ???
             bounds.top = getLineTop(line);
             bounds.right = mWidth;   // ???
-            bounds.bottom = getLineBottom(line);
+            bounds.bottom = getLineTop(line + 1);
         }
         return getLineBaseline(line);
     }
 
     /**
-     * Return the vertical position of the top of the specified line.
-     * If the specified line is one beyond the last line, returns the
+     * Return the vertical position of the top of the specified line
+     * (0&hellip;getLineCount()).
+     * If the specified line is equal to the line count, returns the
      * bottom of the last line.
      */
     public abstract int getLineTop(int line);
 
     /**
-     * Return the descent of the specified line.
+     * Return the descent of the specified line(0&hellip;getLineCount() - 1).
      */
     public abstract int getLineDescent(int line);
 
     /**
-     * Return the text offset of the beginning of the specified line.
-     * If the specified line is one beyond the last line, returns the
-     * end of the last line.
+     * Return the text offset of the beginning of the specified line (
+     * 0&hellip;getLineCount()). If the specified line is equal to the line
+     * count, returns the length of the text.
      */
     public abstract int getLineStart(int line);
 
     /**
-     * Returns the primary directionality of the paragraph containing
-     * the specified line.
+     * Returns the primary directionality of the paragraph containing the
+     * specified line, either 1 for left-to-right lines, or -1 for right-to-left
+     * lines (see {@link #DIR_LEFT_TO_RIGHT}, {@link #DIR_RIGHT_TO_LEFT}).
      */
     public abstract int getParagraphDirection(int line);
 
@@ -477,9 +504,11 @@ public abstract class Layout {
     public abstract boolean getLineContainsTab(int line);
 
     /**
-     * Returns an array of directionalities for the specified line.
+     * Returns the directional run information for the specified line.
      * The array alternates counts of characters in left-to-right
      * and right-to-left segments of the line.
+     *
+     * <p>NOTE: this is inadequate to support bidirectional text, and will change.
      */
     public abstract Directions getLineDirections(int line);
 
@@ -1565,6 +1594,21 @@ public abstract class Layout {
         return h;
     }
 
+    /**
+     * Measure width of a run of text on a single line that is known to all be
+     * in the same direction as the paragraph base direction. Returns the width,
+     * and the line metrics in fm if fm is not null.
+     *
+     * @param paint the paint for the text; will not be modified
+     * @param workPaint paint available for modification
+     * @param text text
+     * @param start start of the line
+     * @param end limit of the line
+     * @param fm object to return integer metrics in, can be null
+     * @param hasTabs true if it is known that the line has tabs
+     * @param tabs tab position information
+     * @return the width of the text from start to end
+     */
     /* package */ static float measureText(TextPaint paint,
                                            TextPaint workPaint,
                                            CharSequence text,
@@ -1580,37 +1624,36 @@ public abstract class Layout {
 
         int len = end - start;
 
-        int here = 0;
-        float h = 0;
-        int ab = 0, be = 0;
-        int top = 0, bot = 0;
+        int lastPos = 0;
+        float width = 0;
+        int ascent = 0, descent = 0, top = 0, bottom = 0;
 
         if (fm != null) {
             fm.ascent = 0;
             fm.descent = 0;
         }
 
-        for (int i = hasTabs ? 0 : len; i <= len; i++) {
+        for (int pos = hasTabs ? 0 : len; pos <= len; pos++) {
             int codept = 0;
             Bitmap bm = null;
 
-            if (hasTabs && i < len) {
-                codept = buf[i];
+            if (hasTabs && pos < len) {
+                codept = buf[pos];
             }
 
-            if (codept >= 0xD800 && codept <= 0xDFFF && i < len) {
-                codept = Character.codePointAt(buf, i);
+            if (codept >= 0xD800 && codept <= 0xDFFF && pos < len) {
+                codept = Character.codePointAt(buf, pos);
 
                 if (codept >= MIN_EMOJI && codept <= MAX_EMOJI) {
                     bm = EMOJI_FACTORY.getBitmapFromAndroidPua(codept);
                 }
             }
 
-            if (i == len || codept == '\t' || bm != null) {
+            if (pos == len || codept == '\t' || bm != null) {
                 workPaint.baselineShift = 0;
 
-                h += Styled.measureText(paint, workPaint, text,
-                                        start + here, start + i,
+                width += Styled.measureText(paint, workPaint, text,
+                                        start + lastPos, start + pos,
                                         fm);
 
                 if (fm != null) {
@@ -1623,60 +1666,80 @@ public abstract class Layout {
                     }
                 }
 
-                if (i != len) {
+                if (pos != len) {
                     if (bm == null) {
-                        h = nextTab(text, start, end, h, tabs);
+                        // no emoji, must have hit a tab
+                        width = nextTab(text, start, end, width, tabs);
                     } else {
+                        // This sets up workPaint with the font on the emoji
+                        // text, so that we can extract the ascent and scale.
+
+                        // We can't use the result of the previous call to
+                        // measureText because the emoji might have its own style.
+                        // We have to initialize workPaint here because if the
+                        // text is unstyled measureText might not use workPaint
+                        // at all.
                         workPaint.set(paint);
                         Styled.measureText(paint, workPaint, text,
-                                           start + i, start + i + 1, null);
+                                           start + pos, start + pos + 1, null);
 
-                        float wid = (float) bm.getWidth() *
+                        width += (float) bm.getWidth() *
                                     -workPaint.ascent() / bm.getHeight();
 
-                        h += wid;
-                        i++;
+                        // Since we had an emoji, we bump past the second half
+                        // of the surrogate pair.
+                        pos++;
                     }
                 }
 
                 if (fm != null) {
-                    if (fm.ascent < ab) {
-                        ab = fm.ascent;
+                    if (fm.ascent < ascent) {
+                        ascent = fm.ascent;
                     }
-                    if (fm.descent > be) {
-                        be = fm.descent;
+                    if (fm.descent > descent) {
+                        descent = fm.descent;
                     }
 
                     if (fm.top < top) {
                         top = fm.top;
                     }
-                    if (fm.bottom > bot) {
-                        bot = fm.bottom;
+                    if (fm.bottom > bottom) {
+                        bottom = fm.bottom;
                     }
 
-                    /*
-                     * No need to take bitmap height into account here,
-                     * since it is scaled to match the text height.
-                     */
+                    // No need to take bitmap height into account here,
+                    // since it is scaled to match the text height.
                 }
 
-                here = i + 1;
+                lastPos = pos + 1;
             }
         }
 
         if (fm != null) {
-            fm.ascent = ab;
-            fm.descent = be;
+            fm.ascent = ascent;
+            fm.descent = descent;
             fm.top = top;
-            fm.bottom = bot;
+            fm.bottom = bottom;
         }
 
         if (hasTabs)
             TextUtils.recycle(buf);
 
-        return h;
+        return width;
     }
 
+    /**
+     * Returns the position of the next tab stop after h on the line.
+     *
+     * @param text the text
+     * @param start start of the line
+     * @param end limit of the line
+     * @param h the current horizontal offset
+     * @param tabs the tabs, can be null.  If it is null, any tabs in effect
+     * on the line will be used.  If there are no tabs, a default offset
+     * will be used to compute the tab stop.
+     * @return the offset of the next tab stop.
+     */
     /* package */ static float nextTab(CharSequence text, int start, int end,
                                        float h, Object[] tabs) {
         float nh = Float.MAX_VALUE;
@@ -1747,6 +1810,16 @@ public abstract class Layout {
     public static class Directions {
         private short[] mDirections;
 
+        // The values in mDirections are the offsets from the first character
+        // in the line to the next flip in direction.  Runs at even indices
+        // are left-to-right, the others are right-to-left.  So, for example,
+        // a line that starts with a right-to-left run has 0 at mDirections[0],
+        // since the 'first' (ltr) run is zero length.
+        //
+        // The code currently assumes that each run is adjacent to the previous
+        // one, progressing in the base line direction.  This isn't sufficient
+        // to handle nested runs, for example numeric text in an rtl context
+        // in an ltr paragraph.
         /* package */ Directions(short[] dirs) {
             mDirections = dirs;
         }
diff --git a/core/java/android/text/StaticLayout.java b/core/java/android/text/StaticLayout.java
index fbf1261..6c89f92 100644
--- a/core/java/android/text/StaticLayout.java
+++ b/core/java/android/text/StaticLayout.java
@@ -1012,6 +1012,10 @@ extends Layout
         int extra;
 
         if (needMultiply) {
+            // XXX: this looks like it is using the +0.5 and the cast to int
+            // to do rounding, but this I expect this isn't doing the intended
+            // thing when spacingmult < 1.  An intended extra of, say, -1.2
+            // will get 'rounded' to -.7 and then truncated to 0.
             extra = (int) ((below - above) * (spacingmult - 1)
                            + spacingadd + 0.5);
         } else {
diff --git a/core/java/android/text/Styled.java b/core/java/android/text/Styled.java
index 0aa2004..513b2cd 100644
--- a/core/java/android/text/Styled.java
+++ b/core/java/android/text/Styled.java
@@ -13,7 +13,6 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-
 package android.text;
 
 import android.graphics.Canvas;
@@ -23,27 +22,49 @@ import android.text.style.MetricAffectingSpan;
 import android.text.style.ReplacementSpan;
 
 /**
- * This class provides static methods for drawing and measuring styled texts, like
- * {@link android.text.Spanned} object with {@link android.text.style.ReplacementSpan}.
+ * This class provides static methods for drawing and measuring styled text,
+ * like {@link android.text.Spanned} object with
+ * {@link android.text.style.ReplacementSpan}.
+ *
  * @hide
  */
 public class Styled
 {
-    private static float each(Canvas canvas,
+    /**
+     * Draws and/or measures a uniform run of text on a single line. No span of
+     * interest should start or end in the middle of this run (if not
+     * drawing, character spans that don't affect metrics can be ignored).
+     * Neither should the run direction change in the middle of the run.
+     *
+     * <p>The x position is the leading edge of the text. In a right-to-left
+     * paragraph, this will be to the right of the text to be drawn. Paint
+     * should not have an Align value other than LEFT or positioning will get
+     * confused.
+     *
+     * <p>On return, workPaint will reflect the original paint plus any
+     * modifications made by character styles on the run.
+     *
+     * <p>The returned width is signed and will be < 0 if the paragraph
+     * direction is right-to-left.
+     */
+    private static float drawUniformRun(Canvas canvas,
                               Spanned text, int start, int end,
-                              int dir, boolean reverse,
+                              int dir, boolean runIsRtl,
                               float x, int top, int y, int bottom,
                               Paint.FontMetricsInt fmi,
                               TextPaint paint,
                               TextPaint workPaint,
-                              boolean needwid) {
+                              boolean needWidth) {
 
-        boolean havewid = false;
+        boolean haveWidth = false;
         float ret = 0;
         CharacterStyle[] spans = text.getSpans(start, end, CharacterStyle.class);
 
         ReplacementSpan replacement = null;
 
+        // XXX: This shouldn't be modifying paint, only workPaint.
+        // However, the members belonging to TextPaint should have default
+        // values anyway.  Better to ensure this in the Layout constructor.
         paint.bgColor = 0;
         paint.baselineShift = 0;
         workPaint.set(paint);
@@ -65,9 +86,10 @@ public class Styled
             CharSequence tmp;
             int tmpstart, tmpend;
 
-            if (reverse) {
+            if (runIsRtl) {
                 tmp = TextUtils.getReverse(text, start, end);
                 tmpstart = 0;
+                // XXX: assumes getReverse doesn't change the length of the text
                 tmpend = end - start;
             } else {
                 tmp = text;
@@ -86,9 +108,9 @@ public class Styled
                     workPaint.setColor(workPaint.bgColor);
                     workPaint.setStyle(Paint.Style.FILL);
 
-                    if (!havewid) {
+                    if (!haveWidth) {
                         ret = workPaint.measureText(tmp, tmpstart, tmpend);
-                        havewid = true;
+                        haveWidth = true;
                     }
 
                     if (dir == Layout.DIR_RIGHT_TO_LEFT)
@@ -101,18 +123,18 @@ public class Styled
                 }
 
                 if (dir == Layout.DIR_RIGHT_TO_LEFT) {
-                    if (!havewid) {
+                    if (!haveWidth) {
                         ret = workPaint.measureText(tmp, tmpstart, tmpend);
-                        havewid = true;
+                        haveWidth = true;
                     }
 
                     canvas.drawText(tmp, tmpstart, tmpend,
                                     x - ret, y + workPaint.baselineShift, workPaint);
                 } else {
-                    if (needwid) {
-                        if (!havewid) {
+                    if (needWidth) {
+                        if (!haveWidth) {
                             ret = workPaint.measureText(tmp, tmpstart, tmpend);
-                            havewid = true;
+                            haveWidth = true;
                         }
                     }
 
@@ -120,9 +142,9 @@ public class Styled
                                     x, y + workPaint.baselineShift, workPaint);
                 }
             } else {
-                if (needwid && !havewid) {
+                if (needWidth && !haveWidth) {
                     ret = workPaint.measureText(tmp, tmpstart, tmpend);
-                    havewid = true;
+                    haveWidth = true;
                 }
             }
         } else {
@@ -145,25 +167,28 @@ public class Styled
     }
 
     /**
-     * Return the advance widths for the characters in the string.
-     * See also {@link android.graphics.Paint#getTextWidths(CharSequence, int, int, float[])}.
+     * Returns the advance widths for a uniform left-to-right run of text with
+     * no style changes in the middle of the run. If any style is replacement
+     * text, the first character will get the width of the replacement and the
+     * remaining characters will get a width of 0.
      * 
-     * @param paint The main {@link TextPaint} object.
-     * @param workPaint The {@link TextPaint} object used for temporal workspace.
-     * @param text The text to measure
-     * @param start The index of the first char to to measure
-     * @param end The end of the text slice to measure
-     * @param widths Array to receive the advance widths of the characters.
-     * Must be at least a large as (end - start).
-     * @param fmi FontMetrics information. Can be null.
-     * @return The actual number of widths returned. 
+     * @param paint the paint, will not be modified
+     * @param workPaint a paint to modify; on return will reflect the original
+     *        paint plus the effect of all spans on the run
+     * @param text the text
+     * @param start the start of the run
+     * @param end the limit of the run
+     * @param widths array to receive the advance widths of the characters. Must
+     *        be at least a large as (end - start).
+     * @param fmi FontMetrics information; can be null
+     * @return the actual number of widths returned
      */
     public static int getTextWidths(TextPaint paint,
                                     TextPaint workPaint,
                                     Spanned text, int start, int end,
                                     float[] widths, Paint.FontMetricsInt fmi) {
-        //  Keep workPaint as is so that developers reuse the workspace.
-        MetricAffectingSpan[] spans = text.getSpans(start, end, MetricAffectingSpan.class);
+        MetricAffectingSpan[] spans =
+            text.getSpans(start, end, MetricAffectingSpan.class);
 
 		ReplacementSpan replacement = null;
         workPaint.set(paint);
@@ -186,7 +211,6 @@ public class Styled
 
             if (end > start) {
                 widths[0] = wid;
-
                 for (int i = start + 1; i < end; i++)
                     widths[i - start] = 0;
             }
@@ -194,19 +218,42 @@ public class Styled
         return end - start;
     }
 
-    private static float foreach(Canvas canvas,
+    /**
+     * Renders and/or measures a directional run of text on a single line.
+     * Unlike {@link #drawUniformRun}, this can render runs that cross style
+     * boundaries.  Returns the signed advance width, if requested.
+     *
+     * <p>The x position is the leading edge of the text. In a right-to-left
+     * paragraph, this will be to the right of the text to be drawn. Paint
+     * should not have an Align value other than LEFT or positioning will get
+     * confused.
+     *
+     * <p>This optimizes for unstyled text and so workPaint might not be
+     * modified by this call.
+     *
+     * <p>The returned advance width will be < 0 if the paragraph
+     * direction is right-to-left.
+     */
+    private static float drawDirectionalRun(Canvas canvas,
                                  CharSequence text, int start, int end,
-                                 int dir, boolean reverse,
+                                 int dir, boolean runIsRtl,
                                  float x, int top, int y, int bottom,
                                  Paint.FontMetricsInt fmi,
                                  TextPaint paint,
                                  TextPaint workPaint,
                                  boolean needWidth) {
-        if (! (text instanceof Spanned)) {
+
+        // XXX: It looks like all calls to this API match dir and runIsRtl, so
+        // having both parameters is redundant and confusing.
+
+        // fast path for unstyled text
+        if (!(text instanceof Spanned)) {
             float ret = 0;
 
-            if (reverse) {
+            if (runIsRtl) {
                 CharSequence tmp = TextUtils.getReverse(text, start, end);
+                // XXX: this assumes getReverse doesn't tweak the length of
+                // the text
                 int tmpend = end - start;
 
                 if (canvas != null || needWidth)
@@ -227,15 +274,14 @@ public class Styled
                 paint.getFontMetricsInt(fmi);
             }
 
-            return ret * dir;   //Layout.DIR_RIGHT_TO_LEFT == -1
+            return ret * dir;   // Layout.DIR_RIGHT_TO_LEFT == -1
         }
         
         float ox = x;
-        int asc = 0, desc = 0;
-        int ftop = 0, fbot = 0;
+        int minAscent = 0, maxDescent = 0, minTop = 0, maxBottom = 0;
 
         Spanned sp = (Spanned) text;
-        Class division;
+        Class<?> division;
 
         if (canvas == null)
             division = MetricAffectingSpan.class;
@@ -246,20 +292,23 @@ public class Styled
         for (int i = start; i < end; i = next) {
             next = sp.nextSpanTransition(i, end, division);
 
-            x += each(canvas, sp, i, next, dir, reverse,
+            // XXX: if dir and runIsRtl were not the same, this would draw
+            // spans in the wrong order, but no one appears to call it this
+            // way.
+            x += drawUniformRun(canvas, sp, i, next, dir, runIsRtl,
                   x, top, y, bottom, fmi, paint, workPaint,
                   needWidth || next != end);
 
             if (fmi != null) {
-                if (fmi.ascent < asc)
-                    asc = fmi.ascent;
-                if (fmi.descent > desc)
-                    desc = fmi.descent;
-
-                if (fmi.top < ftop)
-                    ftop = fmi.top;
-                if (fmi.bottom > fbot)
-                    fbot = fmi.bottom;
+                if (fmi.ascent < minAscent)
+                    minAscent = fmi.ascent;
+                if (fmi.descent > maxDescent)
+                    maxDescent = fmi.descent;
+
+                if (fmi.top < minTop)
+                    minTop = fmi.top;
+                if (fmi.bottom > maxBottom)
+                    maxBottom = fmi.bottom;
             }
         }
 
@@ -267,71 +316,78 @@ public class Styled
             if (start == end) {
                 paint.getFontMetricsInt(fmi);
             } else {
-                fmi.ascent = asc;
-                fmi.descent = desc;
-                fmi.top = ftop;
-                fmi.bottom = fbot;
+                fmi.ascent = minAscent;
+                fmi.descent = maxDescent;
+                fmi.top = minTop;
+                fmi.bottom = maxBottom;
             }
         }
 
         return x - ox;
     }
 
-
+    /**
+     * Draws a unidirectional run of text on a single line, and optionally
+     * returns the signed advance.  Unlike drawDirectionalRun, the paragraph
+     * direction and run direction can be different.
+     */
     /* package */ static float drawText(Canvas canvas,
                                        CharSequence text, int start, int end,
-                                       int direction, boolean reverse,
+                                       int dir, boolean runIsRtl,
                                        float x, int top, int y, int bottom,
                                        TextPaint paint,
                                        TextPaint workPaint,
                                        boolean needWidth) {
-        if ((direction == Layout.DIR_RIGHT_TO_LEFT && !reverse) ||
-            (reverse && direction == Layout.DIR_LEFT_TO_RIGHT)) {
-            float ch = foreach(null, text, start, end, Layout.DIR_LEFT_TO_RIGHT,
-                         false, 0, 0, 0, 0, null, paint, workPaint,
-                         true);
-
-            ch *= direction;  // DIR_RIGHT_TO_LEFT == -1
-            foreach(canvas, text, start, end, -direction,
-                    reverse, x + ch, top, y, bottom, null, paint,
+        // XXX this logic is (dir == DIR_LEFT_TO_RIGHT) == runIsRtl
+        if ((dir == Layout.DIR_RIGHT_TO_LEFT && !runIsRtl) ||
+            (runIsRtl && dir == Layout.DIR_LEFT_TO_RIGHT)) {
+            // TODO: this needs the real direction
+            float ch = drawDirectionalRun(null, text, start, end,
+                    Layout.DIR_LEFT_TO_RIGHT, false, 0, 0, 0, 0, null, paint,
+                    workPaint, true);
+
+            ch *= dir;  // DIR_RIGHT_TO_LEFT == -1
+            drawDirectionalRun(canvas, text, start, end, -dir,
+                    runIsRtl, x + ch, top, y, bottom, null, paint,
                     workPaint, true);
 
             return ch;
         }
 
-        return foreach(canvas, text, start, end, direction, reverse,
+        return drawDirectionalRun(canvas, text, start, end, dir, runIsRtl,
                        x, top, y, bottom, null, paint, workPaint,
                        needWidth);
     }
     
     /**
-     * Draw the specified range of text, specified by start/end, with its origin at (x,y),
-     * in the specified Paint. The origin is interpreted based on the Align setting in the
-     * Paint.
-     *  
-     * This method considers style information in the text
-     * (e.g. Even when text is an instance of {@link android.text.Spanned}, this method
-     * correctly draws the text).
-     * See also
-     * {@link android.graphics.Canvas#drawText(CharSequence, int, int, float, float, Paint)}
-     * and
-     * {@link android.graphics.Canvas#drawRect(float, float, float, float, Paint)}.
+     * Draws a run of text on a single line, with its
+     * origin at (x,y), in the specified Paint. The origin is interpreted based
+     * on the Align setting in the Paint.
+     *
+     * This method considers style information in the text (e.g. even when text
+     * is an instance of {@link android.text.Spanned}, this method correctly
+     * draws the text). See also
+     * {@link android.graphics.Canvas#drawText(CharSequence, int, int, float,
+     * float, Paint)} and
+     * {@link android.graphics.Canvas#drawRect(float, float, float, float,
+     * Paint)}.
      * 
-     * @param canvas The target canvas.
+     * @param canvas The target canvas
      * @param text The text to be drawn
      * @param start The index of the first character in text to draw
      * @param end (end - 1) is the index of the last character in text to draw
      * @param direction The direction of the text. This must be
-     * {@link android.text.Layout#DIR_LEFT_TO_RIGHT} or
-     * {@link android.text.Layout#DIR_RIGHT_TO_LEFT}.
+     *        {@link android.text.Layout#DIR_LEFT_TO_RIGHT} or
+     *        {@link android.text.Layout#DIR_RIGHT_TO_LEFT}.
      * @param x The x-coordinate of origin for where to draw the text
      * @param top The top side of the rectangle to be drawn
      * @param y The y-coordinate of origin for where to draw the text
      * @param bottom The bottom side of the rectangle to be drawn
      * @param paint The main {@link TextPaint} object.
-     * @param workPaint The {@link TextPaint} object used for temporal workspace.
-     * @param needWidth If true, this method returns the width of drawn text.
-     * @return Width of the drawn text if needWidth is true.
+     * @param workPaint The {@link TextPaint} object used for temporal
+     *        workspace.
+     * @param needWidth If true, this method returns the width of drawn text
+     * @return Width of the drawn text if needWidth is true
      */
     public static float drawText(Canvas canvas,
                                  CharSequence text, int start, int end,
@@ -341,34 +397,37 @@ public class Styled
                                  TextPaint workPaint,
                                  boolean needWidth) {
         // For safety.
-        direction = direction >= 0 ? Layout.DIR_LEFT_TO_RIGHT : Layout.DIR_RIGHT_TO_LEFT;
-        /*
-         * Hided "reverse" parameter since it is meaningless for external developers.
-         * Kept workPaint as is so that developers reuse the workspace.
-         */
+        direction = direction >= 0 ? Layout.DIR_LEFT_TO_RIGHT
+                : Layout.DIR_RIGHT_TO_LEFT;
+
+        // Hide runIsRtl parameter since it is meaningless for external
+        // developers.
+        // XXX: the runIsRtl probably ought to be the same as direction, then
+        // this could draw rtl text.
         return drawText(canvas, text, start, end, direction, false,
                         x, top, y, bottom, paint, workPaint, needWidth);
     }
     
     /**
-     * Return the width of the text, considering style information in the text
-     * (e.g. Even when text is an instance of {@link android.text.Spanned}, this method
-     * correctly mesures the width of the text).
+     * Returns the width of a run of left-to-right text on a single line,
+     * considering style information in the text (e.g. even when text is an
+     * instance of {@link android.text.Spanned}, this method correctly measures
+     * the width of the text).
      * 
-     * @param paint The main {@link TextPaint} object.
-     * @param workPaint The {@link TextPaint} object used for temporal workspace.
-     * @param text The text to measure
-     * @param start The index of the first character to start measuring
+     * @param paint the main {@link TextPaint} object; will not be modified
+     * @param workPaint the {@link TextPaint} object available for modification;
+     *        will not necessarily be used
+     * @param text the text to measure
+     * @param start the index of the first character to start measuring
      * @param end 1 beyond the index of the last character to measure
-     * @param fmi FontMetrics information. Can be null
-     * @return The width of the text 
+     * @param fmi FontMetrics information; can be null
+     * @return The width of the text
      */
     public static float measureText(TextPaint paint,
                                     TextPaint workPaint,
                                     CharSequence text, int start, int end,
                                     Paint.FontMetricsInt fmi) {
-        // Keep workPaint as is so that developers reuse the workspace.
-        return foreach(null, text, start, end,
+        return drawDirectionalRun(null, text, start, end,
                        Layout.DIR_LEFT_TO_RIGHT, false,
                        0, 0, 0, 0, fmi, paint, workPaint, true);
     }
diff --git a/core/java/android/text/style/LeadingMarginSpan.java b/core/java/android/text/style/LeadingMarginSpan.java
index cb55329..6635ddb8 100644
--- a/core/java/android/text/style/LeadingMarginSpan.java
+++ b/core/java/android/text/style/LeadingMarginSpan.java
@@ -23,10 +23,44 @@ import android.text.Layout;
 import android.text.ParcelableSpan;
 import android.text.TextUtils;
 
+/**
+ * A paragraph style affecting the leading margin. There can be multiple leading
+ * margin spans on a single paragraph; they will be rendered in order, each
+ * adding its margin to the ones before it. The leading margin is on the right
+ * for lines in a right-to-left paragraph.
+ */
 public interface LeadingMarginSpan
 extends ParagraphStyle
 {
+    /**
+     * Returns the amount by which to adjust the leading margin. Positive values
+     * move away from the leading edge of the paragraph, negative values move
+     * towards it.
+     * 
+     * @param first true if the request is for the first line of a paragraph,
+     * false for subsequent lines
+     * @return the offset for the margin.
+     */
     public int getLeadingMargin(boolean first);
+
+    /**
+     * Renders the leading margin.  This is called before the margin has been
+     * adjusted by the value returned by {@link getLeadingMargin(boolean)}.
+     * 
+     * @param c the canvas
+     * @param p the paint. The this should be left unchanged on exit.
+     * @param x the current position of the margin
+     * @param dir the base direction of the paragraph; if negative, the margin
+     * is to the right of the text, otherwise it is to the left.
+     * @param top the top of the line
+     * @param baseline the baseline of the line
+     * @param bottom the bottom of the line
+     * @param text the text
+     * @param start the start of the line
+     * @param end the end of the line
+     * @param first true if this is the first line of its paragraph
+     * @param layout the layout containing this line
+     */
     public void drawLeadingMargin(Canvas c, Paint p,
                                   int x, int dir,
                                   int top, int baseline, int bottom,
@@ -38,14 +72,29 @@ extends ParagraphStyle
         public int getLeadingMarginLineCount();
     };
 
+    /**
+     * The standard implementation of LeadingMarginSpan, which adjusts the
+     * margin but does not do any rendering.
+     */
     public static class Standard implements LeadingMarginSpan, ParcelableSpan {
         private final int mFirst, mRest;
         
+        /**
+         * Constructor taking separate indents for the first and subsequent
+         * lines.
+         * 
+         * @param first the indent for the first line of the paragraph
+         * @param rest the indent for the remaining lines of the paragraph
+         */
         public Standard(int first, int rest) {
             mFirst = first;
             mRest = rest;
         }
 
+        /**
+         * Constructor taking an indent for all lines.
+         * @param every the indent of each line
+         */
         public Standard(int every) {
             this(every, every);
         }
diff --git a/core/java/android/text/style/TabStopSpan.java b/core/java/android/text/style/TabStopSpan.java
index e5b7644..0566428 100644
--- a/core/java/android/text/style/TabStopSpan.java
+++ b/core/java/android/text/style/TabStopSpan.java
@@ -16,14 +16,31 @@
 
 package android.text.style;
 
+/**
+ * Represents a single tab stop on a line.
+ */
 public interface TabStopSpan
 extends ParagraphStyle
 {
+    /**
+     * Returns the offset of the tab stop from the leading margin of the
+     * line.
+     * @return the offset
+     */
     public int getTabStop();
 
+    /**
+     * The default implementation of TabStopSpan.
+     */
     public static class Standard
     implements TabStopSpan
     {
+        /**
+         * Constructor.
+         *
+         * @param where the offset of the tab stop from the leading margin of
+         *        the line
+         */
         public Standard(int where) {
             mTab = where;
         }