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

[Karthik P K]

On Thu, 18 May 2023 09:27:18 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:
> 
>   Add new test. Optimizations to make the tests robust

Adding `Toolkit::firePulse` is not making the tests stable. In fact I'm getting new failure after adding this in `testTextFlowInsertionIndexUsingMultipleEmojis()` test.

TextFlowSurrogatePairInsertionIndexTest > testTextFlowInsertionIndexUsingMultipleEmojis FAILED
    java.lang.NullPointerException: Cannot read the array length because "runs" is null
        at javafx.graphics at 21-internal/javafx.scene.text.Text.getSpanBounds(Text.java:359)
        at javafx.graphics at 21-internal/javafx.scene.text.Text.doComputeGeomBounds(Text.java:1159)
        at javafx.graphics at 21-internal/javafx.scene.text.Text$1.doComputeGeomBounds(Text.java:149)
        at javafx.graphics at 21-internal/com.sun.javafx.scene.shape.TextHelper.computeGeomBoundsImpl(TextHelper.java:90)
        at javafx.graphics at 21-internal/com.sun.javafx.scene.NodeHelper.computeGeomBounds(NodeHelper.java:117)
        at javafx.graphics at 21-internal/javafx.scene.Node.updateGeomBounds(Node.java:3813)
        at javafx.graphics at 21-internal/javafx.scene.Node.getGeomBounds(Node.java:3775)
        at javafx.graphics at 21-internal/javafx.scene.Node.getLocalBounds(Node.java:3723)
        at javafx.graphics at 21-internal/javafx.scene.Node.updateTxBounds(Node.java:3877)
        at javafx.graphics at 21-internal/javafx.scene.Node.getTransformedBounds(Node.java:3669)
        at javafx.graphics at 21-internal/javafx.scene.Node.updateBounds(Node.java:777)
        at javafx.graphics at 21-internal/javafx.scene.Parent.updateBounds(Parent.java:1834)
        at javafx.graphics at 21-internal/javafx.scene.Scene$ScenePulseListener.pulse(Scene.java:2615)
        at javafx.graphics at 21-internal/com.sun.javafx.tk.Toolkit.lambda$runPulse$2(Toolkit.java:401)
        at java.base/java.security.AccessController.doPrivileged(AccessController.java:399)
        at javafx.graphics at 21-internal/com.sun.javafx.tk.Toolkit.runPulse(Toolkit.java:400)
        at javafx.graphics at 21-internal/com.sun.javafx.tk.Toolkit.firePulse(Toolkit.java:430)
        at test.robot.javafx.scene.TextFlowSurrogatePairInsertionIndexTest.testTextFlowInsertionIndexUsingMultipleEmojis(TextFlowSurrogatePairInsertionIndexTest.java:233)

Could increasing the delay time is the only solution now for the test failure seen previously and then we follow up with the approach suggested by @kevinrushforth ?

@andy-goryachev-oracle  could you please help me in understanding how `Util.runAndWait(() -> { });` help in this case? 
Also I'm using the `Util.runAndWait(() -> { });` while setting text to textflow and trigger `textSetLatch.countDown()`. Once `textSetLatch` count down to 0, then I proceed with the test.

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

PR Comment: https://git.openjdk.org/jfx/pull/1091#issuecomment-1554089905