On Wed, 17 Jan 2024 19:45:51 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
> Noticed a problem - on Windows 11 and on macOS 14.2.1 hit into shows
> different values for Text and TextFlow. This issue can be seen with pretty
> much every text, even "aaa\nbbb\ncccc". In this screenshot, the line
> corresponds to the mouse position:
>
I see this problem as well. I will check on this issue.
> modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java
> line 430:
>
>> 428: int insertionIndex = -1;
>> 429: int relIndex = 0;
>> 430: int LTRIndex = 0;
>
> minor: it's a bit confusing to have a local var start with an uppercase
> letter. maybe 'ltrIndex' ?
Changed to `ltrIndex`
> modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java
> line 510:
>
>> 508: if (run.getTextSpan() != null &&
>> run.getTextSpan().getText().equals(text)) {
>> 509: if ((x > run.getWidth() && !isMultiRunText)
>> || textWidthPrevLine > 0) {
>> 510: BaseBounds textBounds = new BoxBounds();
>
> here (and on line 544) we allocate `textBounds` inside the for loop.
> Maybe we should allocate one before line 494 and use that instance in both
> for loops to avoid gc pressure?
> The downside is that in some cases it may not be used, but I feel it'll
> better since alloc is cheap.
Agreed. I think allocating the `textBounds` before line 494 is better. Made
this change
> modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java line 1062:
>
>> 1060: } else {
>> 1061: double ptX = localToParent(x, y).getX();
>> 1062: double ptY = localToParent(x, y).getY();
>
> localToParent(x, y) computed twice, could we avoid that?
Created local variable to get Point2D value
> tests/system/src/test/java/test/robot/javafx/scene/RTLTextFlowCharacterIndexTest.java
> line 274:
>
>> 272: while (x > X_LEADING_OFFSET) {
>> 273: moveMouseOverTextFlow(x, Y_OFFSET);
>> 274: if (isLeading) {
>
> indentation?
Corrected indentation
-------------
PR Comment: https://git.openjdk.org/jfx/pull/1323#issuecomment-1897964532
PR Review Comment: https://git.openjdk.org/jfx/pull/1323#discussion_r1457049491
PR Review Comment: https://git.openjdk.org/jfx/pull/1323#discussion_r1457049303
PR Review Comment: https://git.openjdk.org/jfx/pull/1323#discussion_r1457047727
PR Review Comment: https://git.openjdk.org/jfx/pull/1323#discussion_r1457047377
