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

[Andy Goryachev]

On Wed, 31 Jan 2024 10:24:20 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 issue with multiline text

Almost works!  At least all tests where only text segments are present: bidi, RTL, mixed text, multi line.
It does fail when a Node is added to TextFlow, will explain in a separate comment.

Inline Nodes mess up computation.  In this example, the Text.hitInfo shows bogus values anywhere in the word "trailing":

![Screenshot 2024-01-31 at 13 25 10](https://github.com/openjdk/jfx/assets/107069028/5210576b-4d92-4cc9-902d-b0ed0c5b83d2)

modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java line 513:

> 511:                             if ((x > run.getWidth() && (!isMultiRunText || run.getStart() == curRunStart)) || textWidthPrevLine > 0) {
> 512:                                 getBounds(run.getTextSpan(), textBounds);
> 513:                                 x -= (runs[0].getLocation().x - textBounds.getMinX());

suggestion: we are still in the for loop, so perhaps it makes sense to extract 
`(runs[0].getLocation().x - textBounds.getMinX());`
to a variable outside of the loop

modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java line 517:

> 515:                             for (int j = runs.length - 1; j > i; j--) {
> 516:                                 if (runs[j].getStart() != curRunStart && runs[j].getTextSpan().getText().equals(text) && !runs[j].isLinebreak()) {
> 517:                                     ltrIndex += runs[j].getLength();

runs[j] is repeated 4 times... should it be a local variable?

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

PR Review: https://git.openjdk.org/jfx/pull/1323#pullrequestreview-1854843704
PR Comment: https://git.openjdk.org/jfx/pull/1323#issuecomment-1920000439
PR Review Comment: https://git.openjdk.org/jfx/pull/1323#discussion_r1473468220
PR Review Comment: https://git.openjdk.org/jfx/pull/1323#discussion_r1473469342