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

[John Hendrikx]

On Thu, 29 Feb 2024 05:38:05 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:
> 
>   Review comments

Please add a JUnit test.  Example included.

modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java line 1031:

> 1029:         if (runs.length != 0) {
> 1030:             textRunStart = findFirstRunStart(runs);
> 1031:         }

Can you rewrite this to be:

    int textRunStart = findFirstRunStart();

The `runs` parameter doesn't need to be passed (`findFirstRunStart` can access it just as easily), and the length check can be moved inside that method as well.

modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java line 1049:

> 1047:         for (GlyphList r: runs) {
> 1048:             if (((TextRun) r).getStart() < start) {
> 1049:                 start = ((TextRun) r).getStart();

minor: you can extract a new local here instead of casting and calling `getStart` twice.

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

Changes requested by jhendrikx (Committer).

PR Review: https://git.openjdk.org/jfx/pull/1323#pullrequestreview-1908709396
PR Review Comment: https://git.openjdk.org/jfx/pull/1323#discussion_r1507478198
PR Review Comment: https://git.openjdk.org/jfx/pull/1323#discussion_r1507480047