On Thu, 18 May 2023 09:27:18 GMT, Karthik P K <k...@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@21-internal/javafx.scene.text.Text.getSpanBounds(Text.java:359)
at
javafx.graphics@21-internal/javafx.scene.text.Text.doComputeGeomBounds(Text.java:1159)
at
javafx.graphics@21-internal/javafx.scene.text.Text$1.doComputeGeomBounds(Text.java:149)
at
javafx.graphics@21-internal/com.sun.javafx.scene.shape.TextHelper.computeGeomBoundsImpl(TextHelper.java:90)
at
javafx.graphics@21-internal/com.sun.javafx.scene.NodeHelper.computeGeomBounds(NodeHelper.java:117)
at
javafx.graphics@21-internal/javafx.scene.Node.updateGeomBounds(Node.java:3813)
at
javafx.graphics@21-internal/javafx.scene.Node.getGeomBounds(Node.java:3775)
at
javafx.graphics@21-internal/javafx.scene.Node.getLocalBounds(Node.java:3723)
at
javafx.graphics@21-internal/javafx.scene.Node.updateTxBounds(Node.java:3877)
at
javafx.graphics@21-internal/javafx.scene.Node.getTransformedBounds(Node.java:3669)
at
javafx.graphics@21-internal/javafx.scene.Node.updateBounds(Node.java:777)
at
javafx.graphics@21-internal/javafx.scene.Parent.updateBounds(Parent.java:1834)
at
javafx.graphics@21-internal/javafx.scene.Scene$ScenePulseListener.pulse(Scene.java:2615)
at
javafx.graphics@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@21-internal/com.sun.javafx.tk.Toolkit.runPulse(Toolkit.java:400)
at
javafx.graphics@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
