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

Commit f0bb87b7 authored by Keisuke Kuroyanagi's avatar Keisuke Kuroyanagi
Browse files

Improve selection handle behavior for bidi text. 

A point on a direction boundary can be mapped to two offset and
one offset on a direction boundary can be mapped to two points.
Previously, paragraph's primary direction is always used for deciding
offset and coordinates; thus, handle movement around a direction
boundary is often nonintuitive.

With this CL:
1. For selection end handle, direction of character at offset - 1 is
used for deciding handle shape and position.
2. For getting offset from coordinates, previous offset is used to
minimize the offset delta and primary .
3. For getting coordinates form offset, new logic chooses primary or
secondary horizontal coordinate depending on the current run
direction and paragraph direction.
4. When a handle passes another one because it passes a direction
boundary, new logic keeps the handle at the run boundary instead of
minimizing the selection.

Bug: 19199637
Bug: 21480356
Bug: 21649994


Change-Id: I2a7e87ad08416f4bd01a5f68e006240f77d9036b
parent cc63de4f

core/java/android/text/Layout.java

+49 −6
Original line number Diff line number Diff line
@@ -799,6 +799,31 @@ public abstract class Layout { 
        return false;

    }



    /**

     * Returns the range of the run that the character at offset belongs to.

     * @param offset the offset

     * @return The range of the run

     * @hide

     */

    public long getRunRange(int offset) {

        int line = getLineForOffset(offset);

        Directions dirs = getLineDirections(line);

        if (dirs == DIRS_ALL_LEFT_TO_RIGHT || dirs == DIRS_ALL_RIGHT_TO_LEFT) {

            return TextUtils.packRangeInLong(0, getLineEnd(line));

        }

        int[] runs = dirs.mDirections;

        int lineStart = getLineStart(line);

        for (int i = 0; i < runs.length; i += 2) {

            int start = lineStart + runs[i];

            int limit = start + (runs[i+1] & RUN_LENGTH_MASK);

            if (offset >= start && offset < limit) {

                return TextUtils.packRangeInLong(start, limit);

            }

        }

        // Should happen only if the offset is "out of bounds"

        return TextUtils.packRangeInLong(0, getLineEnd(line));

    }



    private boolean primaryIsTrailingPrevious(int offset) {

        int line = getLineForOffset(offset);

        int lineStart = getLineStart(line);
@@ -886,6 +911,10 @@ public abstract class Layout { 
        return getHorizontal(offset, !trailing, clamped);

    }



    private float getHorizontal(int offset, boolean primary) {

        return primary ? getPrimaryHorizontal(offset) : getSecondaryHorizontal(offset);

    }



    private float getHorizontal(int offset, boolean trailing, boolean clamped) {

        int line = getLineForOffset(offset);
@@ -1114,6 +1143,20 @@ public abstract class Layout { 
     * closest to the specified horizontal position.

     */

    public int getOffsetForHorizontal(int line, float horiz) {

        return getOffsetForHorizontal(line, horiz, true);

    }



    /**

     * Get the character offset on the specified line whose position is

     * closest to the specified horizontal position.

     *

     * @param line the line used to find the closest offset

     * @param horiz the horizontal position used to find the closest offset

     * @param primary whether to use the primary position or secondary position to find the offset

     *

     * @hide

     */

    public int getOffsetForHorizontal(int line, float horiz, boolean primary) {

        // TODO: use Paint.getOffsetForAdvance to avoid binary search

        final int lineEndOffset = getLineEnd(line);

        final int lineStartOffset = getLineStart(line);
@@ -1133,7 +1176,7 @@ public abstract class Layout { 
                    !isRtlCharAt(lineEndOffset - 1)) + lineStartOffset;

        }

        int best = lineStartOffset;

        float bestdist = Math.abs(getPrimaryHorizontal(best) - horiz);

        float bestdist = Math.abs(getHorizontal(best, primary) - horiz);



        for (int i = 0; i < dirs.mDirections.length; i += 2) {

            int here = lineStartOffset + dirs.mDirections[i];
@@ -1149,7 +1192,7 @@ public abstract class Layout { 
                guess = (high + low) / 2;

                int adguess = getOffsetAtStartOf(guess);



                if (getPrimaryHorizontal(adguess) * swap >= horiz * swap)

                if (getHorizontal(adguess, primary) * swap >= horiz * swap)

                    high = guess;

                else

                    low = guess;
@@ -1162,9 +1205,9 @@ public abstract class Layout { 
                int aft = tl.getOffsetToLeftRightOf(low - lineStartOffset, isRtl) + lineStartOffset;

                low = tl.getOffsetToLeftRightOf(aft - lineStartOffset, !isRtl) + lineStartOffset;

                if (low >= here && low < there) {

                    float dist = Math.abs(getPrimaryHorizontal(low) - horiz);

                    float dist = Math.abs(getHorizontal(low, primary) - horiz);

                    if (aft < there) {

                        float other = Math.abs(getPrimaryHorizontal(aft) - horiz);

                        float other = Math.abs(getHorizontal(aft, primary) - horiz);



                        if (other < dist) {

                            dist = other;
@@ -1179,7 +1222,7 @@ public abstract class Layout { 
                }

            }



            float dist = Math.abs(getPrimaryHorizontal(here) - horiz);

            float dist = Math.abs(getHorizontal(here, primary) - horiz);



            if (dist < bestdist) {

                bestdist = dist;
@@ -1187,7 +1230,7 @@ public abstract class Layout { 
            }

        }



        float dist = Math.abs(getPrimaryHorizontal(max) - horiz);

        float dist = Math.abs(getHorizontal(max, primary) - horiz);



        if (dist <= bestdist) {

            bestdist = dist;

core/java/android/widget/Editor.java

+105 −22
Original line number Diff line number Diff line
@@ -4034,14 +4034,17 @@ public class Editor { 
                // Don't update drawable during dragging.

                return;

            }

            final Layout layout = mTextView.getLayout();

            if (layout == null) {

                return;

            }

            final int offset = getCurrentCursorOffset();

            final boolean isRtlCharAtOffset = mTextView.getLayout().isRtlCharAt(offset);

            final boolean isRtlCharAtOffset = isAtRtlRun(layout, offset);

            final Drawable oldDrawable = mDrawable;

            mDrawable = isRtlCharAtOffset ? mDrawableRtl : mDrawableLtr;

            mHotspotX = getHotspotX(mDrawable, isRtlCharAtOffset);

            mHorizontalGravity = getHorizontalGravity(isRtlCharAtOffset);

            final Layout layout = mTextView.getLayout();

            if (layout != null && oldDrawable != mDrawable && isShowing()) {

            if (oldDrawable != mDrawable && isShowing()) {

                // Update popup window position.

                mPositionX = getCursorHorizontalPosition(layout, offset) - mHotspotX -

                        getHorizontalOffset() + getCursorOffset();
@@ -4154,6 +4157,19 @@ public class Editor { 


        public abstract void updatePosition(float x, float y);



        protected boolean isAtRtlRun(@NonNull Layout layout, int offset) {

            return layout.isRtlCharAt(offset);

        }



        @VisibleForTesting

        public float getHorizontal(@NonNull Layout layout, int offset) {

            return layout.getPrimaryHorizontal(offset);

        }



        protected int getOffsetAtCoordinate(@NonNull Layout layout, int line, float x) {

            return mTextView.getOffsetAtCoordinate(line, x);

        }



        protected void positionAtCursorOffset(int offset, boolean parentScrolled) {

            // A HandleView relies on the layout, which may be nulled by external methods

            Layout layout = mTextView.getLayout();
@@ -4194,7 +4210,7 @@ public class Editor { 
         * @return The clamped horizontal position for the cursor.

         */

        int getCursorHorizontalPosition(Layout layout, int offset) {

            return (int) (layout.getPrimaryHorizontal(offset) - 0.5f);

            return (int) (getHorizontal(layout, offset) - 0.5f);

        }



        public void updatePosition(int parentPositionX, int parentPositionY,
@@ -4427,7 +4443,7 @@ public class Editor { 
        int getCursorHorizontalPosition(Layout layout, int offset) {

            final Drawable drawable = mCursorCount > 0 ? mCursorDrawable[0] : null;

            if (drawable != null) {

                final float horizontal = layout.getPrimaryHorizontal(offset);

                final float horizontal = getHorizontal(layout, offset);

                return clampHorizontalPosition(drawable, horizontal) + mTempRect.left;

            }

            return super.getCursorHorizontalPosition(layout, offset);
@@ -4499,10 +4515,10 @@ public class Editor { 
                    mPreviousLineTouched = mTextView.getLineAtCoordinate(y);

                }

                int currLine = getCurrentLineAdjustedForSlop(layout, mPreviousLineTouched, y);

                offset = mTextView.getOffsetAtCoordinate(currLine, x);

                offset = getOffsetAtCoordinate(layout, currLine, x);

                mPreviousLineTouched = currLine;

            } else {

                offset = mTextView.getOffsetForPosition(x, y);

                offset = -1;

            }

            positionAtCursorOffset(offset, false);

            if (mTextActionMode != null) {
@@ -4612,14 +4628,14 @@ public class Editor { 
            final int anotherHandleOffset =

                    isStartHandle() ? mTextView.getSelectionEnd() : mTextView.getSelectionStart();

            int currLine = getCurrentLineAdjustedForSlop(layout, mPreviousLineTouched, y);

            int initialOffset = mTextView.getOffsetAtCoordinate(currLine, x);

            int initialOffset = getOffsetAtCoordinate(layout, currLine, x);



            if (isStartHandle() && initialOffset >= anotherHandleOffset

                    || !isStartHandle() && initialOffset <= anotherHandleOffset) {

                // Handles have crossed, bound it to the first selected line and

                // adjust by word / char as normal.

                currLine = layout.getLineForOffset(anotherHandleOffset);

                initialOffset = mTextView.getOffsetAtCoordinate(currLine, x);

                initialOffset = getOffsetAtCoordinate(layout, currLine, x);

            }



            int offset = initialOffset;
@@ -4631,8 +4647,8 @@ public class Editor { 
            }



            final int currentOffset = getCurrentCursorOffset();

            final boolean rtlAtCurrentOffset = layout.isRtlCharAt(currentOffset);

            final boolean atRtl = layout.isRtlCharAt(offset);

            final boolean rtlAtCurrentOffset = isAtRtlRun(layout, currentOffset);

            final boolean atRtl = isAtRtlRun(layout, offset);

            final boolean isLvlBoundary = layout.isLevelBoundary(offset);



            // We can't determine if the user is expanding or shrinking the selection if they're
@@ -4689,14 +4705,15 @@ public class Editor { 


            if (isExpanding) {

                // User is increasing the selection.

                final boolean snapToWord = !mInWord

                        || (isStartHandle() ? currLine < mPrevLine : currLine > mPrevLine);

                int wordBoundary = isStartHandle() ? wordStart : wordEnd;

                final boolean snapToWord = (!mInWord

                        || (isStartHandle() ? currLine < mPrevLine : currLine > mPrevLine))

                                && atRtl == isAtRtlRun(layout, wordBoundary);

                if (snapToWord) {

                    // Sometimes words can be broken across lines (Chinese, hyphenation).

                    // We still snap to the word boundary but we only use the letters on the

                    // current line to determine if the user is far enough into the word to snap.

                    int wordBoundary = isStartHandle() ? wordStart : wordEnd;

                    if (layout != null && layout.getLineForOffset(wordBoundary) != currLine) {

                    if (layout.getLineForOffset(wordBoundary) != currLine) {

                        wordBoundary = isStartHandle() ?

                                layout.getLineStart(currLine) : layout.getLineEnd(currLine);

                    }
@@ -4717,9 +4734,9 @@ public class Editor { 
                        offset = mPreviousOffset;

                    }

                }

                if (layout != null && (isStartHandle() && offset < initialOffset)

                if ((isStartHandle() && offset < initialOffset)

                        || (!isStartHandle() && offset > initialOffset)) {

                    final float adjustedX = layout.getPrimaryHorizontal(offset);

                    final float adjustedX = getHorizontal(layout, offset);

                    mTouchWordDelta =

                            mTextView.convertToLocalHorizontalCoordinate(x) - adjustedX;

                } else {
@@ -4728,7 +4745,7 @@ public class Editor { 
                positionCursor = true;

            } else {

                final int adjustedOffset =

                        mTextView.getOffsetAtCoordinate(currLine, x - mTouchWordDelta);

                        getOffsetAtCoordinate(layout, currLine, x - mTouchWordDelta);

                final boolean shrinking = isStartHandle()

                        ? adjustedOffset > mPreviousOffset || currLine > mPrevLine

                        : adjustedOffset < mPreviousOffset || currLine < mPrevLine;
@@ -4737,9 +4754,9 @@ public class Editor { 
                    if (currLine != mPrevLine) {

                        // We're on a different line, so we'll snap to word boundaries.

                        offset = isStartHandle() ? wordStart : wordEnd;

                        if (layout != null && (isStartHandle() && offset < initialOffset)

                        if ((isStartHandle() && offset < initialOffset)

                                || (!isStartHandle() && offset > initialOffset)) {

                            final float adjustedX = layout.getPrimaryHorizontal(offset);

                            final float adjustedX = getHorizontal(layout, offset);

                            mTouchWordDelta =

                                    mTextView.convertToLocalHorizontalCoordinate(x) - adjustedX;

                        } else {
@@ -4754,7 +4771,7 @@ public class Editor { 
                    // Handle has jumped to the word boundary, and the user is moving

                    // their finger towards the handle, the delta should be updated.

                    mTouchWordDelta = mTextView.convertToLocalHorizontalCoordinate(x) -

                            layout.getPrimaryHorizontal(mPreviousOffset);

                            getHorizontal(layout, mPreviousOffset);

                }

            }
@@ -4792,9 +4809,32 @@ public class Editor { 
                    isStartHandle() ? mTextView.getSelectionEnd() : mTextView.getSelectionStart();

            if ((isStartHandle() && offset >= anotherHandleOffset)

                    || (!isStartHandle() && offset <= anotherHandleOffset)) {

                mTouchWordDelta = 0.0f;

                final Layout layout = mTextView.getLayout();

                if (layout != null && offset != anotherHandleOffset) {

                    final float horiz = getHorizontal(layout, offset);

                    final float anotherHandleHoriz = getHorizontal(layout, anotherHandleOffset,

                            !isStartHandle());

                    final float currentHoriz = getHorizontal(layout, mPreviousOffset);

                    if (currentHoriz < anotherHandleHoriz && horiz < anotherHandleHoriz

                            || currentHoriz > anotherHandleHoriz && horiz > anotherHandleHoriz) {

                        // This handle passes another one as it crossed a direction boundary.

                        // Don't minimize the selection, but keep the handle at the run boundary.

                        final int currentOffset = getCurrentCursorOffset();

                        final int offsetToGetRunRange = isStartHandle() ?

                                currentOffset : Math.max(currentOffset - 1, 0);

                        final long range = layout.getRunRange(offsetToGetRunRange);

                        if (isStartHandle()) {

                            offset = TextUtils.unpackRangeStartFromLong(range);

                        } else {

                            offset = TextUtils.unpackRangeEndFromLong(range);

                        }

                        positionAtCursorOffset(offset, false);

                        return;

                    }

                }

                // Handles can not cross and selection is at least one character.

                offset = getNextCursorOffset(anotherHandleOffset, !isStartHandle());

                mTouchWordDelta = 0.0f;

            }

            positionAtCursorOffset(offset, false);

        }
@@ -4812,6 +4852,49 @@ public class Editor { 
            }

            return nearEdge;

        }



        @Override

        protected boolean isAtRtlRun(@NonNull Layout layout, int offset) {

            final int offsetToCheck = isStartHandle() ? offset : Math.max(offset - 1, 0);

            return layout.isRtlCharAt(offsetToCheck);

        }



        @Override

        public float getHorizontal(@NonNull Layout layout, int offset) {

            return getHorizontal(layout, offset, isStartHandle());

        }



        private float getHorizontal(@NonNull Layout layout, int offset, boolean startHandle) {

            final int line = layout.getLineForOffset(offset);

            final int offsetToCheck = startHandle ? offset : Math.max(offset - 1, 0);

            final boolean isRtlChar = layout.isRtlCharAt(offsetToCheck);

            final boolean isRtlParagraph = layout.getParagraphDirection(line) == -1;

            return (isRtlChar == isRtlParagraph) ?

                    layout.getPrimaryHorizontal(offset) : layout.getSecondaryHorizontal(offset);

        }



        @Override

        protected int getOffsetAtCoordinate(@NonNull Layout layout, int line, float x) {

            final int primaryOffset = layout.getOffsetForHorizontal(line, x, true);

            if (!layout.isLevelBoundary(primaryOffset)) {

                return primaryOffset;

            }

            final int secondaryOffset = layout.getOffsetForHorizontal(line, x, false);

            final int currentOffset = getCurrentCursorOffset();

            final int primaryDiff = Math.abs(primaryOffset - currentOffset);

            final int secondaryDiff = Math.abs(secondaryOffset - currentOffset);

            if (primaryDiff < secondaryDiff) {

                return primaryOffset;

            } else if (primaryDiff > secondaryDiff) {

                return secondaryOffset;

            } else {

                final int offsetToCheck = isStartHandle() ?

                        currentOffset : Math.max(currentOffset - 1, 0);

                final boolean isRtlChar = layout.isRtlCharAt(offsetToCheck);

                final boolean isRtlParagraph = layout.getParagraphDirection(line) == -1;

                return isRtlChar == isRtlParagraph ? primaryOffset : secondaryOffset;

            }

        }

    }



    private int getCurrentLineAdjustedForSlop(Layout layout, int prevLine, float y) {

core/tests/coretests/src/android/widget/TextViewActivityTest.java

+45 −0
Original line number Diff line number Diff line
@@ -384,6 +384,51 @@ public class TextViewActivityTest extends ActivityInstrumentationTestCase2<TextV 
        onView(withId(R.id.textview)).check(hasSelection("abcd efg hijk"));

    }



    @SmallTest

    public void testSelectionHandles_bidi() throws Exception {

        final String text = "abc \u0621\u0622\u0623 def";

        onView(withId(R.id.textview)).perform(click());

        onView(withId(R.id.textview)).perform(replaceText(text));



        assertNoSelectionHandles();



        onView(withId(R.id.textview)).perform(doubleClickOnTextAtIndex(text.indexOf('\u0622')));



        onHandleView(com.android.internal.R.id.selection_start_handle)

                .check(matches(isDisplayed()));

        onHandleView(com.android.internal.R.id.selection_end_handle)

                .check(matches(isDisplayed()));



        onView(withId(R.id.textview)).check(hasSelection("\u0621\u0622\u0623"));



        final TextView textView = (TextView) getActivity().findViewById(R.id.textview);

        onHandleView(com.android.internal.R.id.selection_start_handle)

                .perform(dragHandle(textView, Handle.SELECTION_START, text.indexOf('f')));

        onView(withId(R.id.textview)).check(hasSelection("\u0621\u0622\u0623"));



        onHandleView(com.android.internal.R.id.selection_end_handle)

                .perform(dragHandle(textView, Handle.SELECTION_END, text.indexOf('a')));

        onView(withId(R.id.textview)).check(hasSelection("\u0621\u0622\u0623"));



        onHandleView(com.android.internal.R.id.selection_start_handle)

                .perform(dragHandle(textView, Handle.SELECTION_START, text.indexOf('\u0623') + 1,

                        false));

        onView(withId(R.id.textview)).check(hasSelection("\u0623"));



        onHandleView(com.android.internal.R.id.selection_start_handle)

                .perform(dragHandle(textView, Handle.SELECTION_START, text.indexOf('\u0621'),

                        false));

        onView(withId(R.id.textview)).check(hasSelection("\u0621\u0622\u0623"));



        onHandleView(com.android.internal.R.id.selection_start_handle)

                .perform(dragHandle(textView, Handle.SELECTION_START, text.indexOf('a')));

        onView(withId(R.id.textview)).check(hasSelection("abc \u0621\u0622\u0623"));



        onHandleView(com.android.internal.R.id.selection_end_handle)

                .perform(dragHandle(textView, Handle.SELECTION_END, text.length()));

        onView(withId(R.id.textview)).check(hasSelection("abc \u0621\u0622\u0623 def"));

    }



    @SmallTest

    public void testSelectionHandles_multiLine() throws Exception {

        final String text = "abcd\n" + "efg\n" + "hijk\n" + "lmn\n" + "opqr";

core/tests/coretests/src/android/widget/espresso/TextViewActions.java

+123 −19

File changed.

Preview size limit exceeded, changes collapsed.