RFR: 8319844 : Text/TextFlow.hitTest() is incorrect in RTL orientation [v14]

[John Hendrikx]

On Tue, 27 Feb 2024 13:51:08 GMT, Karthik P K <kpk at openjdk.org> wrote:

>> In the `getHitInfo()` method of PrismTextLayout, RTL node orientation conditions were not considered, hence hit test values such as character index and insertion index values were incorrect.
>> 
>> Added checks for RTL orientation of nodes and  fixed the issue in `getHitInfo()` to calculate correct hit test values.
>> 
>> Added system tests to validate the changes.
>
> Karthik P K has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix repeating text node issue

So, I looked into the mirroring problem as well.  I had to remove a single line from the original `getHitInfo` code to get it to work correctly.  Here's the image that shows the positions are all correct:

![image](https://github.com/openjdk/jfx/assets/995917/2946a339-c2cd-4dcd-9ae7-95b212eb7bb8)

Here's the `getHitInfo` I'm using:


    @Override
    public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRunStart) {
        int charIndex = -1;
        int insertionIndex = -1;
        boolean leading = false;

        ensureLayout();
        int lineIndex = getLineIndex(y);
        if (lineIndex >= getLineCount()) {
            charIndex = getCharCount();
            insertionIndex = charIndex + 1;
        } else {
            if (isMirrored()) {
//                x = getMirroringWidth() - x;
            }
            TextLine line = lines[lineIndex];
            TextRun[] runs = line.getRuns();
            RectBounds bounds = line.getBounds();
            TextRun run = null;
            x -= bounds.getMinX();
            //TODO binary search
            for (int i = 0; i < runs.length; i++) {
                run = runs[i];
                if (x < run.getWidth()) break;
                if (i + 1 < runs.length) {
                    if (runs[i + 1].isLinebreak()) break;
                    x -= run.getWidth();
                }
            }
            if (run != null) {
                int[] trailing = new int[1];
                charIndex = run.getStart() + run.getOffsetAtX(x, trailing);
                leading = (trailing[0] == 0);

                insertionIndex = charIndex;
                if (getText() != null && insertionIndex < getText().length) {
                    if (!leading) {
                        BreakIterator charIterator = BreakIterator.getCharacterInstance();
                        charIterator.setText(new String(getText()));
                        int next = charIterator.following(insertionIndex);
                        if (next == BreakIterator.DONE) {
                            insertionIndex += 1;
                        } else {
                            insertionIndex = next;
                        }
                    }
                } else if (!leading) {
                    insertionIndex += 1;
                }
            } else {
                //empty line, set to line break leading
                charIndex = line.getStart();
                leading = true;
                insertionIndex = charIndex;
            }
        }
        return new Hit(charIndex, insertionIndex, leading);
    }


Note that I removed the line that mirrors the X coordinate.  The reason: mouse supplied coordinates are not mirrored -- even in a mirrored scene, they have (0,0) as the top left.

@karthikpandelu Can you have a look if this simplified solution solves all cases?  Note that I don't need the 3 extra parameters `text`, `textRunStart` and `curRunStart`, although I do use `textRunStart` in `Text#hitTest` to "fix" the char index and insertion index (the returned one will be in terms of the `TextLayout`, and substracting `textRunStart` will fix it back to insertion index relative to the `Text`).

-------------

PR Comment: https://git.openjdk.org/jfx/pull/1323#issuecomment-1966928582