RFR: 8304831: TextFlow.hitTest.insertionIndex incorrect with surrogate pairs [v8]
Andy Goryachev
angorya at openjdk.org
Mon May 22 17:16:58 UTC 2023
On Mon, 22 May 2023 14:21:33 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 with a new target base due to a merge or a rebase. The pull request now contains eight commits:
>
> - Merge master to resolve merge conflict
> - Stabilizing the system tests
> - Add new test. Optimizations to make the tests robust
> - Change insertion index initialization approach. Add additional test
> - Initialize insertion index in PrismTextLayout
> - Address code review
> - Fix insertion index calculation issue with emojis. Add additional test cases
> - Fix insertion index calculation issue in TextFlow
what do you people think of the number of pulses parameter? is 10 enough? is there a reasonable upper limit that's needed for any processing to settle?
another question is whether it could be made static - i.e. not require a Scene. Perhaps we could add a pulse listener to all visible Scenes? Would that be sufficient, since we only need a signal from one to indicate the pulse has been performed, and there is another and another (... up to 10 or N) pulses that basically guarantees that things are settled.
And if the layout has not settled by that time, we have a bigger issue (can we detect this? I've seen continuous layout loops in one of my programs)
-------------
PR Comment: https://git.openjdk.org/jfx/pull/1091#issuecomment-1557601430
More information about the openjfx-dev
mailing list