RFR: 8319844 : Text/TextFlow.hitTest() is incorrect in RTL orientation [v13]

Karthik P K kpk at openjdk.org
Mon Feb 26 10:43:06 UTC 2024


On Sat, 24 Feb 2024 23:00:43 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> Karthik P K has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 13 additional commits since the last revision:
>> 
>>  - Merge branch 'master' into rtl_hittest_issue
>>  - Code refactoring
>>  - Review comments
>>  - Fix emoji issue
>>  - Inline node issue fix
>>  - Code review changes
>>  - Fix issue with multiline text
>>  - Fix issue with RTL text within LTR text
>>  - Review changes
>>  - Fix multiline text insertion index calculation issue
>>  - ... and 3 more: https://git.openjdk.org/jfx/compare/05c327e9...e3812732
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java line 606:
> 
>> 604:             boolean addLtrIdx = run.getTextSpan().getText().length() != run.length;
>> 605:             if (r.getStart() != curRunStart && !r.isLinebreak() && addLtrIdx
>> 606:                     && r.getTextSpan().getText().equals(text)) {
> 
> I'm concerned about this check: `r.getTextSpan().getText().equals(text)` -- it seems to me that it either must be irrelevant, or it if is relevant, what if I have many `Text` nodes in a `TextFlow` that happen to have the same text? Let's say I have an English text, where I give each word its own `Text` node in a `TextFlow`.  There would be many duplicates... so I find it hard to believe this check could accomplish anything.

You are right. It fails when there are repeated text nodes. I will look into this

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1323#discussion_r1502392096


More information about the openjfx-dev mailing list