Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit ce642dc2 authored by Mihai Popa's avatar Mihai Popa
Browse files

Supress text replacement spans for ellipsis

When a sequence of characters belonging to a text is ellipsized, we
usually replace the first characters with ... and then all the others
with a 0 width character, knowing that they will be drawn, but will be
invisible. However, these positions could have had ReplacementSpans
attached to them, such as EmojiTypefaceSpan when the EmojiCompat library
is used. When this happened, all the ellipsized out characters were
invisible, apart from emojis, which were still drawn as usual (as they
were replacing sequences of 0 width characters).
This CL fixes this behaviour, by skipping those ReplacementSpans that
were completely included in ellipsis. Please check the bug for a more
detailed explanation.

Bug: 69802699
Test: manual testing
Test: atest FrameworksCoreTests:android.text.TextLineTest
Change-Id: If9758537948abaa0226fe6b551a703110c5457b9
parent a41a7094
Loading
Loading
Loading
Loading
+6 −3
Original line number Diff line number Diff line
@@ -258,7 +258,8 @@ public class BoringLayout extends Layout implements TextUtils.EllipsizeCallback
             */
            TextLine line = TextLine.obtain();
            line.set(paint, source, 0, source.length(), Layout.DIR_LEFT_TO_RIGHT,
                    Layout.DIRS_ALL_LEFT_TO_RIGHT, false, null);
                    Layout.DIRS_ALL_LEFT_TO_RIGHT, false, null,
                    mEllipsizedStart, mEllipsizedStart + mEllipsizedCount);
            mMax = (int) Math.ceil(line.metrics(null));
            TextLine.recycle(line);
        }
@@ -333,7 +334,7 @@ public class BoringLayout extends Layout implements TextUtils.EllipsizeCallback
            Spanned sp = (Spanned) text;
            Object[] styles = sp.getSpans(0, textLength, ParagraphStyle.class);
            if (styles.length > 0) {
                return null;  // There are some PargraphStyle spans. Not boring.
                return null;  // There are some ParagraphStyle spans. Not boring.
            }
        }

@@ -346,7 +347,9 @@ public class BoringLayout extends Layout implements TextUtils.EllipsizeCallback

        TextLine line = TextLine.obtain();
        line.set(paint, text, 0, textLength, Layout.DIR_LEFT_TO_RIGHT,
                Layout.DIRS_ALL_LEFT_TO_RIGHT, false, null);
                Layout.DIRS_ALL_LEFT_TO_RIGHT, false, null,
                0 /* ellipsisStart, 0 since text has not been ellipsized at this point */,
                0 /* ellipsisEnd, 0 since text has not been ellipsized at this point */);
        fm.width = (int) Math.ceil(line.metrics(fm));
        TextLine.recycle(line);

+17 −8
Original line number Diff line number Diff line
@@ -562,7 +562,9 @@ public abstract class Layout {
                // XXX: assumes there's nothing additional to be done
                canvas.drawText(buf, start, end, x, lbaseline, paint);
            } else {
                tl.set(paint, buf, start, end, dir, directions, hasTab, tabStops);
                tl.set(paint, buf, start, end, dir, directions, hasTab, tabStops,
                        getEllipsisStart(lineNum),
                        getEllipsisStart(lineNum) + getEllipsisCount(lineNum));
                if (justify) {
                    tl.justify(right - left - indentWidth);
                }
@@ -1184,7 +1186,8 @@ public abstract class Layout {
        }

        TextLine tl = TextLine.obtain();
        tl.set(mPaint, mText, start, end, dir, directions, hasTab, tabStops);
        tl.set(mPaint, mText, start, end, dir, directions, hasTab, tabStops,
                getEllipsisStart(line), getEllipsisStart(line) + getEllipsisCount(line));
        float wid = tl.measure(offset - start, trailing, null);
        TextLine.recycle(tl);

@@ -1223,7 +1226,8 @@ public abstract class Layout {
        }

        TextLine tl = TextLine.obtain();
        tl.set(mPaint, mText, start, end, dir, directions, hasTab, tabStops);
        tl.set(mPaint, mText, start, end, dir, directions, hasTab, tabStops,
                getEllipsisStart(line), getEllipsisStart(line) + getEllipsisCount(line));
        boolean[] trailings = primaryIsTrailingPreviousAllLineOffsets(line);
        if (!primary) {
            for (int offset = 0; offset < trailings.length; ++offset) {
@@ -1365,7 +1369,8 @@ public abstract class Layout {
        final TextPaint paint = mWorkPaint;
        paint.set(mPaint);
        paint.setHyphenEdit(getHyphen(line));
        tl.set(paint, mText, start, end, dir, directions, hasTabs, tabStops);
        tl.set(paint, mText, start, end, dir, directions, hasTabs, tabStops,
                getEllipsisStart(line), getEllipsisStart(line) + getEllipsisCount(line));
        if (isJustificationRequired(line)) {
            tl.justify(getJustifyWidth(line));
        }
@@ -1393,7 +1398,8 @@ public abstract class Layout {
        final TextPaint paint = mWorkPaint;
        paint.set(mPaint);
        paint.setHyphenEdit(getHyphen(line));
        tl.set(paint, mText, start, end, dir, directions, hasTabs, tabStops);
        tl.set(paint, mText, start, end, dir, directions, hasTabs, tabStops,
                getEllipsisStart(line), getEllipsisStart(line) + getEllipsisCount(line));
        if (isJustificationRequired(line)) {
            tl.justify(getJustifyWidth(line));
        }
@@ -1478,7 +1484,8 @@ public abstract class Layout {
        TextLine tl = TextLine.obtain();
        // XXX: we don't care about tabs as we just use TextLine#getOffsetToLeftRightOf here.
        tl.set(mPaint, mText, lineStartOffset, lineEndOffset, getParagraphDirection(line), dirs,
                false, null);
                false, null,
                getEllipsisStart(line), getEllipsisStart(line) + getEllipsisCount(line));
        final HorizontalMeasurementProvider horizontal =
                new HorizontalMeasurementProvider(line, primary);

@@ -1732,7 +1739,8 @@ public abstract class Layout {

        TextLine tl = TextLine.obtain();
        // XXX: we don't care about tabs
        tl.set(mPaint, mText, lineStart, lineEnd, lineDir, directions, false, null);
        tl.set(mPaint, mText, lineStart, lineEnd, lineDir, directions, false, null,
                getEllipsisStart(line), getEllipsisStart(line) + getEllipsisCount(line));
        caret = lineStart + tl.getOffsetToLeftRightOf(caret - lineStart, toLeft);
        TextLine.recycle(tl);
        return caret;
@@ -2114,7 +2122,8 @@ public abstract class Layout {
                    break;
                }
            }
            tl.set(paint, text, start, end, dir, directions, hasTabs, tabStops);
            tl.set(paint, text, start, end, dir, directions, hasTabs, tabStops,
                    0 /* ellipsisStart */, 0 /* ellipsisEnd */);
            return margin + Math.abs(tl.metrics(null));
        } finally {
            TextLine.recycle(tl);
+23 −6
Original line number Diff line number Diff line
@@ -62,6 +62,11 @@ public class TextLine {
    private Spanned mSpanned;
    private PrecomputedText mComputed;

    // The start and end of a potentially existing ellipsis on this text line.
    // We use them to filter out replacement and metric affecting spans on ellipsized away chars.
    private int mEllipsisStart;
    private int mEllipsisEnd;

    // Additional width of whitespace for justification. This value is per whitespace, thus
    // the line width will increase by mAddedWidth x (number of stretchable whitespaces).
    private float mAddedWidth;
@@ -146,11 +151,15 @@ public class TextLine {
     * @param dir the paragraph direction of this line
     * @param directions the directions information of this line
     * @param hasTabs true if the line might contain tabs
     * @param tabStops the tabStops. Can be null.
     * @param tabStops the tabStops. Can be null
     * @param ellipsisStart the start of the ellipsis relative to the line
     * @param ellipsisEnd the end of the ellipsis relative to the line. When there
     *                    is no ellipsis, this should be equal to ellipsisStart.
     */
    @VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE)
    public void set(TextPaint paint, CharSequence text, int start, int limit, int dir,
            Directions directions, boolean hasTabs, TabStops tabStops) {
            Directions directions, boolean hasTabs, TabStops tabStops,
            int ellipsisStart, int ellipsisEnd) {
        mPaint = paint;
        mText = text;
        mStart = start;
@@ -196,7 +205,8 @@ public class TextLine {
                char[] chars = mChars;
                for (int i = start, inext; i < limit; i = inext) {
                    inext = mReplacementSpanSpanSet.getNextTransition(i, limit);
                    if (mReplacementSpanSpanSet.hasSpansIntersecting(i, inext)) {
                    if (mReplacementSpanSpanSet.hasSpansIntersecting(i, inext)
                            && (i - start >= ellipsisEnd || inext - start <= ellipsisStart)) {
                        // transition into a span
                        chars[i - start] = '\ufffc';
                        for (int j = i - start + 1, e = inext - start; j < e; ++j) {
@@ -208,6 +218,9 @@ public class TextLine {
        }
        mTabs = tabStops;
        mAddedWidth = 0;

        mEllipsisStart = ellipsisStart != ellipsisEnd ? ellipsisStart : 0;
        mEllipsisEnd = ellipsisStart != ellipsisEnd ? ellipsisEnd : 0;
    }

    /**
@@ -1156,11 +1169,15 @@ public class TextLine {
            for (int j = 0; j < mMetricAffectingSpanSpanSet.numberOfSpans; j++) {
                // Both intervals [spanStarts..spanEnds] and [mStart + i..mStart + mlimit] are NOT
                // empty by construction. This special case in getSpans() explains the >= & <= tests
                if ((mMetricAffectingSpanSpanSet.spanStarts[j] >= mStart + mlimit) ||
                        (mMetricAffectingSpanSpanSet.spanEnds[j] <= mStart + i)) continue;
                if ((mMetricAffectingSpanSpanSet.spanStarts[j] >= mStart + mlimit)
                        || (mMetricAffectingSpanSpanSet.spanEnds[j] <= mStart + i)) continue;

                boolean insideEllipsis =
                        mStart + mEllipsisStart <= mMetricAffectingSpanSpanSet.spanStarts[j]
                        && mMetricAffectingSpanSpanSet.spanEnds[j] <= mStart + mEllipsisEnd;
                final MetricAffectingSpan span = mMetricAffectingSpanSpanSet.spans[j];
                if (span instanceof ReplacementSpan) {
                    replacement = (ReplacementSpan)span;
                    replacement = !insideEllipsis ? (ReplacementSpan) span : null;
                } else {
                    // We might have a replacement that uses the draw
                    // state, otherwise measure state would suffice.
+72 −2
Original line number Diff line number Diff line
@@ -21,6 +21,8 @@ import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;

import android.graphics.Canvas;
import android.graphics.Paint;
import android.graphics.Typeface;
import android.platform.test.annotations.Presubmit;
import android.support.test.InstrumentationRegistry;
@@ -28,6 +30,7 @@ import android.support.test.filters.SmallTest;
import android.support.test.filters.Suppress;
import android.support.test.runner.AndroidJUnit4;
import android.text.Layout.TabStops;
import android.text.style.ReplacementSpan;
import android.text.style.TabStopSpan;

import org.junit.Test;
@@ -43,7 +46,8 @@ public class TextLineTest {
        final TextPaint paint = new TextPaint();
        final TextLine tl = TextLine.obtain();
        tl.set(paint, line, 0, line.length(), Layout.DIR_LEFT_TO_RIGHT,
                Layout.DIRS_ALL_LEFT_TO_RIGHT, false /* hasTabs */, null /* tabStops */);
                Layout.DIRS_ALL_LEFT_TO_RIGHT, false /* hasTabs */, null /* tabStops */,
                0, 0 /* no ellipsis */);
        final float originalWidth = tl.metrics(null);
        final float expandedWidth = 2 * originalWidth;

@@ -99,7 +103,8 @@ public class TextLineTest {
        TextLine tl = TextLine.obtain();
        tl.set(paint, str, 0, str.length(),
                TextDirectionHeuristics.FIRSTSTRONG_LTR.isRtl(str, 0, str.length()) ? -1 : 1,
                layout.getLineDirections(0), tabStops != null, tabStops);
                layout.getLineDirections(0), tabStops != null, tabStops,
                0, 0 /* no ellipsis */);
        return tl;
    }

@@ -247,4 +252,69 @@ public class TextLineTest {
        assertMeasurements(tl, 5, true,
                new float[]{0.0f, -10.0f, -10.0f, -100.0f, -110.0f, -110.0f});
    }

    @Test
    public void testHandleRun_ellipsizedReplacementSpan_isSkipped() {
        final Spannable text = new SpannableStringBuilder("This is a... text");

        // Setup a replacement span that the measurement should not interact with.
        final TestReplacementSpan span = new TestReplacementSpan();
        text.setSpan(span, 9, 12, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE);

        final TextLine tl = TextLine.obtain();
        tl.set(new TextPaint(), text, 0, text.length(), 1, Layout.DIRS_ALL_LEFT_TO_RIGHT,
                false /* hasTabs */, null /* tabStops */, 9, 12);
        tl.measure(text.length(), false /* trailing */, null /* fmi */);

        assertFalse(span.mIsUsed);
    }

    @Test
    public void testHandleRun_notEllipsizedReplacementSpan_isNotSkipped() {
        final Spannable text = new SpannableStringBuilder("This is a... text");

        // Setup a replacement span that the measurement should not interact with.
        final TestReplacementSpan span = new TestReplacementSpan();
        text.setSpan(span, 1, 5, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE);

        final TextLine tl = TextLine.obtain();
        tl.set(new TextPaint(), text, 0, text.length(), 1, Layout.DIRS_ALL_LEFT_TO_RIGHT,
                false /* hasTabs */, null /* tabStops */, 9, 12);
        tl.measure(text.length(), false /* trailing */, null /* fmi */);

        assertTrue(span.mIsUsed);
    }

    @Test
    public void testHandleRun_halfEllipsizedReplacementSpan_isNotSkipped() {
        final Spannable text = new SpannableStringBuilder("This is a... text");

        // Setup a replacement span that the measurement should not interact with.
        final TestReplacementSpan span = new TestReplacementSpan();
        text.setSpan(span, 7, 11, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE);

        final TextLine tl = TextLine.obtain();
        tl.set(new TextPaint(), text, 0, text.length(), 1, Layout.DIRS_ALL_LEFT_TO_RIGHT,
                false /* hasTabs */, null /* tabStops */, 9, 12);
        tl.measure(text.length(), false /* trailing */, null /* fmi */);
        assertTrue(span.mIsUsed);
    }

    private static class TestReplacementSpan extends ReplacementSpan {
        boolean mIsUsed;

        @Override
        public int getSize(Paint paint, CharSequence text, int start, int end,
                Paint.FontMetricsInt fm) {
            mIsUsed = true;
            return 0;
        }

        @Override
        public void draw(Canvas canvas, CharSequence text, int start, int end, float x, int top,
                int y,
                int bottom, Paint paint) {
            mIsUsed = true;
        }
    }
}