RFR: 8304831: TextFlow.hitTest.insertionIndex incorrect with surrogate pairs [v4]

[Andy Goryachev]

On Mon, 15 May 2023 09:57:59 GMT, Karthik P K <kpk at openjdk.org> wrote:

>> Since surrogate pairs are internally considered as 2 characters and text field is null in `HitInfo` when `getInsertionIndex` is invoked from `TextFlow`, wrong insertion index was returned.
>> 
>> Updated code to calculate insertion index in `getHitInfo` method of `PrismTextLayout` class when `hitTest` of trailing side of surrogate pair is requested. Since text runs are processed in this method already, calculating the insertion index in this method looks better than calculating in `getInsertionIndex` of `HitInfo` method.
>> The latter approach also requires the text to be sent to `HitInfo` as parameter from the `hitTest` method of `TextFlow`. If the number of `Text` nodes in `TextFlow` are very large, processing all the `Text` nodes on each `hitTest` method invocation might cause performance issue. Hence implemented first approach.
>> 
>> Added system test to validate the fix.
>
> Karthik P K has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Initialize insertion index in PrismTextLayout

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

> 477:             insertionIndex = charIndex;
> 478:             if (!leading) {
> 479:                 insertionIndex += 1;

I would have initialized insertionIndex directly in all the blocks instead of adding conditional logic, but this code does seem to produce the desired result. 
For clarify, would it be possible to avoid this additional conditional logic?

Also, do existing unit tests cover the newly modified paths?

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1091#discussion_r1194033275