On Tue, 27 Feb 2024 13:51:08 GMT, Karthik P K <k...@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:  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
