On Tue, 9 May 2023 05:10:25 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:
> 
>   Address code review

Just by looking at the code, the code paths that leave `insertionIndex=-1` in 
`PrismTextLayout.getHitInfo()` are on lines:
431, 450(when run==null), 472

If we can initialize insertionIndex to a positive value, then the buggy code in 
HitInfo.getInsertionIndex() will never get executed, and we can remove it later 
(in JDK-8302511).

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

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

Reply via email to