From 8d42bd3740de94a79e8770d6343996f1f8b323af Mon Sep 17 00:00:00 2001 From: Karthik P K Date: Tue, 9 Jan 2024 12:55:28 +0530 Subject: [PATCH 01/16] Fix rtl text hittest issue --- .../com/sun/javafx/text/PrismTextLayout.java | 131 ++++- .../src/main/java/javafx/scene/text/Text.java | 55 +- .../main/java/javafx/scene/text/TextFlow.java | 2 +- .../scene/RTLTextCharacterIndexTest.java | 389 +++++++++++++++ .../scene/RTLTextFlowCharacterIndexTest.java | 469 ++++++++++++++++++ 5 files changed, 1008 insertions(+), 38 deletions(-) create mode 100644 tests/system/src/test/java/test/robot/javafx/scene/RTLTextCharacterIndexTest.java create mode 100644 tests/system/src/test/java/test/robot/javafx/scene/RTLTextFlowCharacterIndexTest.java diff --git a/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java b/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java index c851167428e..614c7c21b9d 100644 --- a/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java +++ b/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java @@ -422,11 +422,15 @@ public PathElement[] getCaretShape(int offset, boolean isLeading, @Override public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRunStart) { + boolean leading = false; + boolean isMirrored = isMirrored(); // Node orientation is RTL + boolean isMultiRunText = false; int charIndex = -1; int insertionIndex = -1; - boolean leading = false; int relIndex = 0; + int LTRIndex = 0; int textWidthPrevLine = 0; + float xHitPos = x; ensureLayout(); int lineIndex = getLineIndex(y, text, curRunStart); @@ -434,25 +438,46 @@ public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRu charIndex = getCharCount(); insertionIndex = charIndex + 1; } else { - if (isMirrored()) { + boolean isTextFlow = textRunStart == -1 && curRunStart == -1; + if (isMirrored && (isTextFlow || spans == null)) { x = getMirroringWidth() - x; } TextLine line = lines[lineIndex]; TextRun[] runs = line.getRuns(); RectBounds bounds = line.getBounds(); TextRun run = null; - x -= bounds.getMinX(); //TODO binary search if (text == null || spans == null) { - for (int i = 0; i < runs.length; i++) { - run = runs[i]; - if (x < run.getWidth()) break; - if (i + 1 < runs.length) { - if (runs[i + 1].isLinebreak()) break; - x -= run.getWidth(); + // To calculate Text and TextFlow hit info + if (isMirrored) { + int runIndex = -1; + for (int i = runs.length - 1; i >=0; i--) { + run = runs[i]; + if (x < run.getWidth() && (isTextFlow || (run.getStart() == curRunStart))) { + runIndex = i; + break; + } + if (i - 1 >= 0) { + if (runs[i - 1].isLinebreak()) break; + x -= run.getWidth(); + } + } + for (int i = 0; i < runIndex; i++) { + xHitPos -= runs[i].getWidth(); + } + xHitPos -= bounds.getMinX(); + } else { + for (int i = 0; i < runs.length; i++) { + run = runs[i]; + if (x < run.getWidth()) break; + if (i + 1 < runs.length) { + if (runs[i + 1].isLinebreak()) break; + x -= run.getWidth(); + } } } } else { + // To calculate hit info of Text embedded in TextFlow for (int i = 0; i < lineIndex; i++) { for (TextRun r: lines[i].runs) { if (r.getTextSpan() != null && r.getStart() >= textRunStart && r.getTextSpan().getText().equals(text)) { @@ -460,27 +485,70 @@ public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRu } } } - int prevNodeLength = 0; - boolean isPrevNodeExcluded = false; - for (TextRun r: runs) { - if (!r.getTextSpan().getText().equals(text) || (r.getStart() < textRunStart && r.getTextSpan().getText().equals(text))) { - prevNodeLength += r.getWidth(); - continue; + + if (isMirrored) { + for (TextRun r: runs) { + if (r.getStart() != curRunStart && r.getTextSpan().getText().equals(text)) { + isMultiRunText = true; + } } - if (r.getTextSpan() != null && r.getTextSpan().getText().equals(text)) { - BaseBounds textBounds = new BoxBounds(); - getBounds(r.getTextSpan(), textBounds); - if (textBounds.getMinX() == 0 && !isPrevNodeExcluded) { - x -= prevNodeLength; - isPrevNodeExcluded = true; + + for (int i = 0; i < runs.length; i++) { + run = runs[i]; + if (run.getStart() != curRunStart && run.getTextSpan().getText().equals(text) && x > run.getWidth()) { + x -= run.getWidth(); + continue; + } + if (run.getTextSpan() != null && run.getTextSpan().getText().equals(text)) { + if ((x > run.getWidth() && !isMultiRunText) || textWidthPrevLine > 0) { + BaseBounds textBounds = new BoxBounds(); + getBounds(run.getTextSpan(), textBounds); + x -= (run.getLocation().x - textBounds.getMinX()); + break; + } + if (x > run.getWidth()) { + x -= run.getWidth(); + relIndex += run.getLength(); + continue; + } + for (int j = runs.length - 1; j >= 0; j--) { + if (runs[j].getTextSpan().getText().equals(text) && runs[j].getStart() != curRunStart) { + LTRIndex += runs[j].getLength(); + } + if (runs[j].getStart() == curRunStart) { + break; + } + } + break; + } + // For only English Text embedded in TextFlow in RTL orientation + if (!run.getTextSpan().getText().equals(text) && x > run.getWidth() && run.getStart() < curRunStart) { + x -= run.getWidth(); } - if (x > r.getWidth()) { - x -= r.getWidth(); - relIndex += r.getLength(); + } + } else { + boolean isPrevRunPresent = false; + int prevRunLength = 0; + for (TextRun r: runs) { + if (!r.getTextSpan().getText().equals(text) || (r.getStart() < textRunStart && r.getTextSpan().getText().equals(text))) { + prevRunLength += r.getWidth(); continue; } - run = r; - break; + if (r.getTextSpan() != null && r.getTextSpan().getText().equals(text)) { + BaseBounds textBounds = new BoxBounds(); + getBounds(r.getTextSpan(), textBounds); + if (textBounds.getMinX() == 0 && !isPrevRunPresent) { + x -= prevRunLength; + isPrevRunPresent = true; + } + if (x > r.getWidth()) { + x -= r.getWidth(); + relIndex += r.getLength(); + continue; + } + run = r; + break; + } } } } @@ -491,8 +559,17 @@ public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRu charIndex = run.getOffsetAtX(x, trailing); charIndex += textWidthPrevLine; charIndex += relIndex; + if (run.getLevel() % 2 != 0) { + charIndex += LTRIndex; + } } else { - charIndex = run.getStart() + run.getOffsetAtX(x, trailing); + int indexOffset; + if (isMirrored) { + indexOffset = run.getOffsetAtX(xHitPos, trailing); + } else { + indexOffset = run.getOffsetAtX(x, trailing); + } + charIndex = run.getStart() + indexOffset; } leading = (trailing[0] == 0); diff --git a/modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java b/modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java index d557a5e8878..9c23d8c7697 100644 --- a/modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java +++ b/modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java @@ -1024,16 +1024,8 @@ public final HitInfo hitTest(Point2D point) { double x = point.getX() - getX(); double y = point.getY() - getY() + getYRendering(); GlyphList[] runs = getRuns(); - int runIndex = 0; - if (runs.length != 0) { - double ptY = localToParent(x, y).getY(); - while (runIndex < runs.length - 1) { - if (ptY > runs[runIndex].getLocation().y && ptY < runs[runIndex + 1].getLocation().y) { - break; - } - runIndex++; - } - } + int runIndex = findRunIndex(x, y, runs); + int textRunStart = 0; int curRunStart = 0; if (runs.length != 0) { @@ -1044,6 +1036,49 @@ public final HitInfo hitTest(Point2D point) { return new HitInfo(h.getCharIndex(), h.getInsertionIndex(), h.isLeading()); } + private int findRunIndex(double x, double y, GlyphList[] runs) { + int runIndex = 0; + if (runs.length != 0) { + if (this.getScene().getNodeOrientation() == NodeOrientation.RIGHT_TO_LEFT) { + double yPos = y; + if (runs[runIndex].getTextSpan() == null) { + while (runIndex < runs.length - 1) { + if (x > runs[runIndex].getLocation().x && x < runs[runIndex + 1].getLocation().x + && y > runs[runIndex].getLocation().y && y < runs[runIndex].getHeight()) { + break; + } + runIndex++; + if (y > runs[runIndex].getHeight() && (runs[runIndex].getLocation().y != runs[runIndex - 1].getLocation().y)) { + y -= runs[runIndex].getHeight(); + } + } + } else { + double ptX = localToParent(x, y).getX(); + double ptY = localToParent(x, y).getY(); + while (runIndex < runs.length - 1) { + if (ptX > runs[runIndex].getLocation().x && ptX < (runs[runIndex].getLocation().x + runs[runIndex].getWidth()) && ptY >= runs[runIndex].getLocation().y + && y < runs[runIndex].getHeight()) { + break; + } + runIndex++; + if (y > runs[runIndex].getHeight() && (runs[runIndex].getLocation().y != runs[runIndex - 1].getLocation().y)) { + y -= runs[runIndex].getHeight(); + } + } + } + } else { + double ptY = localToParent(x, y).getY(); + while (runIndex < runs.length - 1) { + if (ptY > runs[runIndex].getLocation().y && ptY < runs[runIndex + 1].getLocation().y) { + break; + } + runIndex++; + } + } + } + return runIndex; + } + private PathElement[] getRange(int start, int end, int type) { int length = getTextInternal().length(); if (0 <= start && start < end && end <= length) { diff --git a/modules/javafx.graphics/src/main/java/javafx/scene/text/TextFlow.java b/modules/javafx.graphics/src/main/java/javafx/scene/text/TextFlow.java index 36a92297d92..a90d9db38e2 100644 --- a/modules/javafx.graphics/src/main/java/javafx/scene/text/TextFlow.java +++ b/modules/javafx.graphics/src/main/java/javafx/scene/text/TextFlow.java @@ -199,7 +199,7 @@ public final HitInfo hitTest(javafx.geometry.Point2D point) { TextLayout layout = getTextLayout(); double x = point.getX(); double y = point.getY(); - TextLayout.Hit h = layout.getHitInfo((float)x, (float)y, null, 0, 0); + TextLayout.Hit h = layout.getHitInfo((float)x, (float)y, null, -1, -1); return new HitInfo(h.getCharIndex(), h.getInsertionIndex(), h.isLeading()); } else { return null; diff --git a/tests/system/src/test/java/test/robot/javafx/scene/RTLTextCharacterIndexTest.java b/tests/system/src/test/java/test/robot/javafx/scene/RTLTextCharacterIndexTest.java new file mode 100644 index 00000000000..ad9909d3012 --- /dev/null +++ b/tests/system/src/test/java/test/robot/javafx/scene/RTLTextCharacterIndexTest.java @@ -0,0 +1,389 @@ +/* + * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +package test.robot.javafx.scene; + +import java.util.Random; +import java.util.concurrent.CountDownLatch; +import javafx.application.Application; +import javafx.application.Platform; +import javafx.geometry.NodeOrientation; +import javafx.geometry.Point2D; +import javafx.geometry.Point3D; +import javafx.scene.Node; +import javafx.scene.Scene; +import javafx.scene.input.MouseButton; +import javafx.scene.input.MouseEvent; +import javafx.scene.input.PickResult; +import javafx.scene.layout.VBox; +import javafx.scene.robot.Robot; +import javafx.scene.text.Font; +import javafx.scene.text.HitInfo; +import javafx.scene.text.Text; +import javafx.stage.Stage; +import javafx.stage.StageStyle; +import javafx.stage.Window; +import org.junit.After; +import org.junit.AfterClass; +import org.junit.Assert; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; +import test.util.Util; + +/* + * Test for verifying character index of Text nodes in RTL orientation. + * + * There are 6 tests in this file. + * Here, the scene node orientation is set to RTL for all the tests. + * Steps for testTextInfoForRTLEnglishText() + * 1. Create a Text node and add it to the scene using a VBox. + * 2. Add only english text to the Text node. + * 3. Move the cursor from right to left with a random + * decrement value generated in step() method. + * 4. Character index should change from highest value to lowest + * as expected. + * + * Steps for testTextInfoForRTLArabicText() + * 1. Create a Text node and add it to the scene using a VBox. + * 2. Add only arabic text to the Text node. + * 3. Move the cursor from right to left with a random + * decrement value generated in step() method. + * 3. Character index should increment as expected since it is RTL text. + * + * Steps for testTextInfoForRTLEnglishArabicText() + * 1. Create a Text node and add it to the scene using a VBox. + * 2. Add both english and arabic text to the Text node. + * 3. Move the cursor from right to left with a random + * decrement value generated in step() method. + * 4. Character index should change in decreasing order for english text + * and in increasing order for arabic text. + * + * Steps for testTextInfoForMultiLineRTLEnglishText() + * 1. Create a Text node and add it to the scene using a VBox. + * 2. Add two lines of only english text to the Text node. + * 3. Move the cursor from right to left with a random + * decrement value generated in step() method. + * 4. Character index should change in decreasing order as expected. + * + * Steps for testTextInfoForMultiLineRTLEnglishArabicText() + * 1. Create a Text node and add it to the scene using a VBox. + * 2. Add two lines of both english and arabic text to the Text node. + * 3. Move the cursor from right to left with a random + * decrement value generated in step() method. + * 4. Character index should change in decreasing order for english text + * and increasing order for arabic text. + * + * Steps for testTextInfoForMultiLineRTLArabicText() + * 1. Create a Text node and add it to the scene using a VBox. + * 2. Add two lines of only arabic text to the Text node. + * 3. Move the cursor from right to left with a random + * decrement value generated in step() method. + * 4. Character index should change in increasing order as expected. + */ + +public class RTLTextCharacterIndexTest { + static CountDownLatch startupLatch = new CountDownLatch(1); + static Random random; + static Robot robot; + static Text textOne; + static VBox vBox; + + static volatile Stage stage; + static volatile Scene scene; + + static final int WIDTH = 500; + static final int HEIGHT = 200; + + final int Y_OFFSET = 30; + final int X_LEADING_OFFSET = 10; + + boolean isLeading; + boolean textFlowIsLeading; + int charIndex; + int insertionIndex; + int textFlowCharIndex; + int textFlowInsertionIndex; + + private void mouseClick(double x, double y) { + Util.runAndWait(() -> { + Window w = scene.getWindow(); + robot.mouseMove((int) (w.getX() + scene.getX() + x), + (int) (w.getY() + scene.getY() + y)); + robot.mouseClick(MouseButton.PRIMARY); + }); + } + + private void moveMouseOverText(double x, double y) throws Exception { + mouseClick(textOne.getLayoutX() + x, + textOne.getLayoutY() / 2 + y); + } + + private void addRTLEnglishText() { + Util.runAndWait(() -> { + textOne.setText("This is text"); + textOne.setFont(new Font(48)); + vBox.getChildren().clear(); + vBox.getChildren().add(textOne); + }); + } + + private void addRTLArabicText() { + Util.runAndWait(() -> { + textOne.setText("شسيبلاتنم"); + textOne.setFont(new Font(48)); + vBox.getChildren().clear(); + vBox.getChildren().add(textOne); + }); + } + + private void addRTLEnglishArabicText() { + Util.runAndWait(() -> { + textOne.setText("Arabic:شسيبلاتنم"); + textOne.setFont(new Font(48)); + vBox.getChildren().clear(); + vBox.getChildren().add(textOne); + }); + } + + private void addMultiLineRTLEnglishText() { + Util.runAndWait(() -> { + textOne.setText("This is text\nThis is text"); + textOne.setFont(new Font(48)); + vBox.getChildren().clear(); + vBox.getChildren().add(textOne); + }); + } + + private void addMultiLineRTLEnglishArabicText() { + Util.runAndWait(() -> { + textOne.setText("Arabic:شسيبلاتنم\nArabic:شسيبلاتنم"); + textOne.setFont(new Font(48)); + vBox.getChildren().clear(); + vBox.getChildren().add(textOne); + }); + } + + private void addMultiLineRTLArabicText() { + Util.runAndWait(() -> { + textOne.setText("شسيبلاتنم شسيبلاتنم\nشسيبلاتنم شسيبلاتنم"); + textOne.setFont(new Font(48)); + vBox.getChildren().clear(); + vBox.getChildren().add(textOne); + }); + } + + @Test + public void testTextInfoForRTLEnglishText() throws Exception { + addRTLEnglishText(); + Util.waitForIdle(scene); + + int textOneLength = textOne.getText().length(); + + double x = WIDTH - X_LEADING_OFFSET; + while (x > X_LEADING_OFFSET) { + moveMouseOverText(x, 0); + if (isLeading) { + Assert.assertEquals(charIndex, insertionIndex); + } else { + Assert.assertEquals(charIndex, insertionIndex - 1); + } + Assert.assertTrue(charIndex < textOneLength); + x -= step(); + } + } + + @Test + public void testTextInfoForRTLArabicText() throws Exception { + addRTLArabicText(); + Util.waitForIdle(scene); + + int textOneLength = textOne.getText().length(); + + double x = WIDTH - X_LEADING_OFFSET; + while (x > X_LEADING_OFFSET) { + moveMouseOverText(x, 0); + if (isLeading) { + Assert.assertEquals(charIndex, insertionIndex); + } else { + Assert.assertEquals(charIndex, insertionIndex - 1); + } + Assert.assertTrue(charIndex < textOneLength); + x -= step(); + } + } + + @Test + public void testTextInfoForRTLEnglishArabicText() throws Exception { + addRTLEnglishArabicText(); + Util.waitForIdle(scene); + + int textOneLength = textOne.getText().length(); + + double x = WIDTH - X_LEADING_OFFSET; + while (x > X_LEADING_OFFSET) { + moveMouseOverText(x, 0); + if (isLeading) { + Assert.assertEquals(charIndex, insertionIndex); + } else { + Assert.assertEquals(charIndex, insertionIndex - 1); + } + Assert.assertTrue(charIndex < textOneLength); + x -= step(); + } + } + + @Test + public void testTextInfoForMultiLineRTLEnglishText() throws Exception { + addMultiLineRTLEnglishText(); + Util.waitForIdle(scene); + + int textOneLength = textOne.getText().length(); + + for (int y = 0; y < 2; y++) { + double x = WIDTH - X_LEADING_OFFSET; + while (x > X_LEADING_OFFSET) { + moveMouseOverText(x, (Y_OFFSET * (y * 2))); + if (isLeading) { + Assert.assertEquals(charIndex, insertionIndex); + } else { + Assert.assertEquals(charIndex, insertionIndex - 1); + } + Assert.assertTrue(charIndex < textOneLength); + x -= step(); + } + } + } + + @Test + public void testTextInfoForMultiLineRTLEnglishArabicText() throws Exception { + addMultiLineRTLEnglishArabicText(); + Util.waitForIdle(scene); + + int textOneLength = textOne.getText().length(); + + for (int y = 0; y < 2; y++) { + double x = WIDTH - X_LEADING_OFFSET; + while (x > X_LEADING_OFFSET) { + moveMouseOverText(x, (Y_OFFSET * (y * 2))); + if (isLeading) { + Assert.assertEquals(charIndex, insertionIndex); + } else { + Assert.assertEquals(charIndex, insertionIndex - 1); + } + Assert.assertTrue(charIndex < textOneLength); + x -= step(); + } + } + } + + @Test + public void testTextInfoForMultiLineRTLArabicText() throws Exception { + addMultiLineRTLArabicText(); + Util.waitForIdle(scene); + + int textOneLength = textOne.getText().length(); + + for (int y = 0; y < 2; y++) { + double x = WIDTH - X_LEADING_OFFSET; + while (x > X_LEADING_OFFSET) { + moveMouseOverText(x, (Y_OFFSET * (y * 2))); + if (isLeading) { + Assert.assertEquals(charIndex, insertionIndex); + } else { + Assert.assertEquals(charIndex, insertionIndex - 1); + } + Assert.assertTrue(charIndex < textOneLength); + x -= step(); + } + } + } + + private void handleTextMouseEvent(MouseEvent event) { + PickResult pick = event.getPickResult(); + Node n = pick.getIntersectedNode(); + + if (n != null && n instanceof Text t) { + Point3D p3 = pick.getIntersectedPoint(); + Point2D p = new Point2D(p3.getX(), p3.getY()); + HitInfo hitInfo = t.hitTest(p); + + isLeading = hitInfo.isLeading(); + charIndex = hitInfo.getCharIndex(); + insertionIndex = hitInfo.getInsertionIndex(); + } + } + + private double step() { + return 1.0 + random.nextDouble() * 8.0; + } + + @After + public void resetUI() { + Platform.runLater(() -> { + textOne.removeEventHandler(MouseEvent.MOUSE_PRESSED, this::handleTextMouseEvent); + }); + } + + @Before + public void setupUI() { + Platform.runLater(() -> { + textOne.addEventHandler(MouseEvent.MOUSE_PRESSED, this::handleTextMouseEvent); + }); + } + + @BeforeClass + public static void initFX() { + long seed = new Random().nextLong(); + System.out.println("seed=" + seed); + random = new Random(seed); + + Util.launch(startupLatch, TestApp.class); + } + + @AfterClass + public static void exit() { + Util.shutdown(stage); + } + + public static class TestApp extends Application { + @Override + public void start(Stage primaryStage) { + robot = new Robot(); + stage = primaryStage; + + textOne = new Text(); + vBox = new VBox(); + + scene = new Scene(vBox, WIDTH, HEIGHT); + scene.setNodeOrientation(NodeOrientation.RIGHT_TO_LEFT); + stage.setScene(scene); + stage.initStyle(StageStyle.UNDECORATED); + stage.setOnShown(event -> Platform.runLater(startupLatch::countDown)); + stage.setAlwaysOnTop(true); + stage.show(); + } + } +} diff --git a/tests/system/src/test/java/test/robot/javafx/scene/RTLTextFlowCharacterIndexTest.java b/tests/system/src/test/java/test/robot/javafx/scene/RTLTextFlowCharacterIndexTest.java new file mode 100644 index 00000000000..06c00ff25c1 --- /dev/null +++ b/tests/system/src/test/java/test/robot/javafx/scene/RTLTextFlowCharacterIndexTest.java @@ -0,0 +1,469 @@ +/* + * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +package test.robot.javafx.scene; + +import java.util.Random; +import java.util.concurrent.CountDownLatch; +import javafx.application.Application; +import javafx.application.Platform; +import javafx.geometry.NodeOrientation; +import javafx.geometry.Point2D; +import javafx.scene.Scene; +import javafx.scene.input.MouseButton; +import javafx.scene.input.MouseEvent; +import javafx.scene.layout.VBox; +import javafx.scene.robot.Robot; +import javafx.scene.text.Font; +import javafx.scene.text.HitInfo; +import javafx.scene.text.Text; +import javafx.scene.text.TextFlow; +import javafx.stage.Stage; +import javafx.stage.StageStyle; +import javafx.stage.Window; +import org.junit.After; +import org.junit.AfterClass; +import org.junit.Assert; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; +import test.util.Util; + +import javafx.scene.layout.StackPane; + +import javafx.scene.input.PickResult; +import javafx.scene.Node; +import javafx.geometry.Point3D; + +/* + * Test for verifying character index of Text nodes embedded in TextFlow in RTL orientation. + * + * There are 5 tests in this file. + * Here, the scene node orientation is set to RTL for all the tests. + * + * Steps for testTextAndTextFlowHitInfoForRTLArabicText() + * 1. Create a TextFlow. Add a Text nodes with only arabic text. + * 2. Move the cursor from right to left with a random + * decrement value generated in step() method. + * 3. Character index should change in ascending order as expected. + * + * Steps for testTextAndTextFlowHitInfoForRTLEnglishText() + * 1. Create a TextFlow. Add a Text nodes with only english text. + * 2. Move the cursor from right to left with a random + * decrement value generated in step() method. + * 3. Character index should change in descending order as expected. + * + * Steps for testTextAndTextFlowHitInfoForRTLMultipleTextNodes() + * 1. Create a TextFlow. Add two Text nodes with english and arabic text. + * 2. Move the cursor from right to left with a random + * decrement value generated in step() method. + * 3. Character index should change in decreasing order for english text + * and in increasing order for arabic text. + * + * Steps for testTextAndTextFlowHitInfoForRTLMultipleMultiLineEnglishArabicTextNodes() + * 1. Create a TextFlow. Add three Text nodes with english and arabic text. + * 2. Move the cursor from right to left with a random + * decrement value generated in step() method. + * 3. Character index should change in decreasing order for english text + * and in increasing order for arabic text. + * + * Steps for testTextAndTextFlowHitInfoForRTLMultipleMultiLineEnglishTextNodes() + * 1. Create a TextFlow. Add three Text nodes with only english text. + * 2. Move the cursor from right to left with a random + * decrement value generated in step() method. + * 3. Character index should change in decreasing order for english text. + * + * Steps for testTextAndTextFlowHitInfoForRTLMultipleMultiLineArabicTextNodes() + * 1. Create a TextFlow. Add three Text nodes with only arabic text. + * 2. Move the cursor from right to left with a random + * decrement value generated in step() method. + * 3. Character index should change in increasing order for arabic text. + * + */ + +public class RTLTextFlowCharacterIndexTest { + static CountDownLatch startupLatch = new CountDownLatch(1); + static Random random; + static Robot robot; + static TextFlow textFlow; + static Text textOne; + static Text textTwo; + static Text textThree; + static VBox vBox; + + static volatile Stage stage; + static volatile Scene scene; + + static final int WIDTH = 500; + static final int HEIGHT = 200; + + final int Y_OFFSET = 30; + final int X_LEADING_OFFSET = 10; + + boolean isLeading; + boolean textFlowIsLeading; + int charIndex; + int insertionIndex; + int textFlowCharIndex; + int textFlowInsertionIndex; + + private void mouseClick(double x, double y) { + Util.runAndWait(() -> { + Window w = scene.getWindow(); + robot.mouseMove((int) (w.getX() + scene.getX() + x), + (int) (w.getY() + scene.getY() + y)); + robot.mouseClick(MouseButton.PRIMARY); + }); + } + + private void moveMouseOverTextFlow(double x, double y) throws Exception { + mouseClick(textFlow.getLayoutX() + x, + textFlow.getLayoutY() + y); + } + + private void addRTLArabicText() { + Util.runAndWait(() -> { + textOne.setText("شسيبلاتنم"); + textOne.setFont(new Font(48)); + textFlow.getChildren().setAll(textOne); + vBox.getChildren().clear(); + vBox.getChildren().add(textFlow); + }); + } + + private void addRTLEnglishText() { + Util.runAndWait(() -> { + textOne.setText("This is text"); + textOne.setFont(new Font(48)); + textFlow.getChildren().setAll(textOne); + vBox.getChildren().clear(); + vBox.getChildren().add(textFlow); + }); + } + + private void addMultiNodeRTLEnglishArabicText() { + Util.runAndWait(() -> { + textOne.setText("Arabic:"); + textOne.setFont(new Font(48)); + textTwo.setText("شسيبلاتنم"); + textTwo.setFont(new Font(48)); + textFlow.getChildren().setAll(textOne, textTwo); + vBox.getChildren().clear(); + vBox.getChildren().add(textFlow); + }); + } + + private void addMultiLineMultiNodeRTLEnglishArabicText() { + Util.runAndWait(() -> { + textOne.setText("Arabic:"); + textOne.setFont(new Font(48)); + textTwo.setText("شسيبلاتنضصثقفغ"); + textTwo.setFont(new Font(48)); + textThree.setText("حخهعغقثصضشسيبل"); + textThree.setFont(new Font(48)); + textFlow.getChildren().setAll(textOne, textTwo, textThree); + vBox.getChildren().clear(); + vBox.getChildren().add(textFlow); + }); + } + + private void addMutliLineMultiNodeRTLEnglishText() { + Util.runAndWait(() -> { + textOne.setText("First line of text"); + textOne.setFont(new Font(48)); + textTwo.setText("Second line of text"); + textTwo.setFont(new Font(48)); + textThree.setText("Third line of text"); + textThree.setFont(new Font(48)); + textFlow.getChildren().setAll(textOne, textTwo, textThree); + vBox.getChildren().clear(); + vBox.getChildren().add(textFlow); + }); + } + + private void addMutliLineMultiNodeRTLArabicText() { + Util.runAndWait(() -> { + textOne.setText("شسيبلا تنضصثقفغ"); + textOne.setFont(new Font(48)); + textTwo.setText("حخهعغقث صضشسيبل"); + textTwo.setFont(new Font(48)); + textThree.setText("ضصثقف"); + textThree.setFont(new Font(48)); + textFlow.getChildren().setAll(textOne, textTwo, textThree); + vBox.getChildren().clear(); + vBox.getChildren().add(textFlow); + }); + } + + @Test + public void testTextAndTextFlowHitInfoForRTLArabicText() throws Exception { + addRTLArabicText(); + Util.waitForIdle(scene); + + int textOneLength = textOne.getText().length(); + + double x = WIDTH - X_LEADING_OFFSET; + while (x > X_LEADING_OFFSET) { + moveMouseOverTextFlow(x, Y_OFFSET); + if (isLeading) { + Assert.assertEquals(charIndex, insertionIndex); + } else { + Assert.assertEquals(charIndex, insertionIndex - 1); + } + if (textFlowIsLeading) { + Assert.assertEquals(textFlowCharIndex, textFlowInsertionIndex); + } else { + Assert.assertEquals(textFlowCharIndex, textFlowInsertionIndex - 1); + } + Assert.assertTrue(charIndex < textOneLength); + Assert.assertTrue(textFlowCharIndex < textOneLength); + x -= step(); + } + } + + @Test + public void testTextAndTextFlowHitInfoForRTLEnglishText() throws Exception { + addRTLEnglishText(); + Util.waitForIdle(scene); + + int textOneLength = textOne.getText().length(); + + double x = WIDTH - X_LEADING_OFFSET; + while (x > X_LEADING_OFFSET) { + moveMouseOverTextFlow(x, Y_OFFSET); + if (isLeading) { + Assert.assertEquals(charIndex, insertionIndex); + } else { + Assert.assertEquals(charIndex, insertionIndex - 1); + } + if (textFlowIsLeading) { + Assert.assertEquals(textFlowCharIndex, textFlowInsertionIndex); + } else { + Assert.assertEquals(textFlowCharIndex, textFlowInsertionIndex - 1); + } + Assert.assertTrue(charIndex < textOneLength); + Assert.assertTrue(textFlowCharIndex < textOneLength); + x -= step(); + } + } + + @Test + public void testTextAndTextFlowHitInfoForRTLMultipleTextNodes() throws Exception { + addMultiNodeRTLEnglishArabicText(); + Util.waitForIdle(scene); + + int textOneLength = textOne.getText().length(); + int textTwoLength = textTwo.getText().length(); + + double x = WIDTH - X_LEADING_OFFSET; + while (x > X_LEADING_OFFSET) { + moveMouseOverTextFlow(x, Y_OFFSET); + if (isLeading) { + Assert.assertEquals(charIndex, insertionIndex); + } else { + Assert.assertEquals(charIndex, insertionIndex - 1); + } + if (textFlowIsLeading) { + Assert.assertEquals(textFlowCharIndex, textFlowInsertionIndex); + } else { + Assert.assertEquals(textFlowCharIndex, textFlowInsertionIndex - 1); + } + Assert.assertTrue(charIndex < Math.max(textOneLength, textTwoLength)); + Assert.assertTrue(textFlowCharIndex < textOneLength + textTwoLength); + x -= step(); + } + } + + @Test + public void testTextAndTextFlowHitInfoForRTLMultipleMultiLineEnglishArabicTextNodes() throws Exception { + addMultiLineMultiNodeRTLEnglishArabicText(); + Util.waitForIdle(scene); + + int textOneLength = textOne.getText().length(); + int textTwoLength = textTwo.getText().length(); + int textThreeLength = textThree.getText().length(); + + for (int y = 0; y < 3; y++) { + double x = WIDTH - X_LEADING_OFFSET; + while (x > X_LEADING_OFFSET) { + moveMouseOverTextFlow(x, (Y_OFFSET + (Y_OFFSET * (y * 2)))); + if (isLeading) { + Assert.assertEquals(charIndex, insertionIndex); + } else { + Assert.assertEquals(charIndex, insertionIndex - 1); + } + if (textFlowIsLeading) { + Assert.assertEquals(textFlowCharIndex, textFlowInsertionIndex); + } else { + Assert.assertEquals(textFlowCharIndex, textFlowInsertionIndex - 1); + } + Assert.assertTrue(charIndex < Math.max(textThreeLength, Math.max(textOneLength, textTwoLength))); + Assert.assertTrue(textFlowCharIndex < textOneLength + textTwoLength + textThreeLength); + x -= step(); + } + } + } + + @Test + public void testTextAndTextFlowHitInfoForRTLMultipleMultiLineEnglishTextNodes() throws Exception { + addMutliLineMultiNodeRTLEnglishText(); + Util.waitForIdle(scene); + + int textOneLength = textOne.getText().length(); + int textTwoLength = textTwo.getText().length(); + int textThreeLength = textThree.getText().length(); + + for (int y = 0; y < 3; y++) { + double x = WIDTH - X_LEADING_OFFSET; + while (x > X_LEADING_OFFSET) { + moveMouseOverTextFlow(x, (Y_OFFSET + (Y_OFFSET * (y * 2)))); + if (isLeading) { + Assert.assertEquals(charIndex, insertionIndex); + } else { + Assert.assertEquals(charIndex, insertionIndex - 1); + } + if (textFlowIsLeading) { + Assert.assertEquals(textFlowCharIndex, textFlowInsertionIndex); + } else { + Assert.assertEquals(textFlowCharIndex, textFlowInsertionIndex - 1); + } + Assert.assertTrue(charIndex < Math.max(textThreeLength, Math.max(textOneLength, textTwoLength))); + Assert.assertTrue(textFlowCharIndex < textOneLength + textTwoLength + textThreeLength); + x -= step(); + } + } + } + + @Test + public void testTextAndTextFlowHitInfoForRTLMultipleMultiLineArabicTextNodes() throws Exception { + addMutliLineMultiNodeRTLArabicText(); + Util.waitForIdle(scene); + + int textOneLength = textOne.getText().length(); + int textTwoLength = textTwo.getText().length(); + int textThreeLength = textThree.getText().length(); + + for (int y = 0; y < 3; y++) { + double x = WIDTH - X_LEADING_OFFSET; + while (x > X_LEADING_OFFSET) { + moveMouseOverTextFlow(x, (Y_OFFSET + (Y_OFFSET * (y * 2)))); + if (isLeading) { + Assert.assertEquals(charIndex, insertionIndex); + } else { + Assert.assertEquals(charIndex, insertionIndex - 1); + } + if (textFlowIsLeading) { + Assert.assertEquals(textFlowCharIndex, textFlowInsertionIndex); + } else { + Assert.assertEquals(textFlowCharIndex, textFlowInsertionIndex - 1); + } + Assert.assertTrue(charIndex < Math.max(textThreeLength, Math.max(textOneLength, textTwoLength))); + Assert.assertTrue(textFlowCharIndex < textOneLength + textTwoLength + textThreeLength); + x -= step(); + } + } + } + + private void handleMouseEvent(MouseEvent event) { + PickResult pick = event.getPickResult(); + Node n = pick.getIntersectedNode(); + + if (n != null && n instanceof Text t) { + Point3D p3 = pick.getIntersectedPoint(); + Point2D p = new Point2D(p3.getX(), p3.getY()); + HitInfo hitInfo = t.hitTest(p); + + isLeading = hitInfo.isLeading(); + charIndex = hitInfo.getCharIndex(); + insertionIndex = hitInfo.getInsertionIndex(); + } + + Point2D point = new Point2D(event.getX(), event.getY()); + HitInfo textFlowHitInfo = textFlow.hitTest(point); + textFlowIsLeading = textFlowHitInfo.isLeading(); + textFlowCharIndex = textFlowHitInfo.getCharIndex(); + textFlowInsertionIndex = textFlowHitInfo.getInsertionIndex(); + } + + private double step() { + return 1.0 + random.nextDouble() * 8.0; + } + + @After + public void resetUI() { + Platform.runLater(() -> { + textFlow.removeEventHandler(MouseEvent.MOUSE_PRESSED, this::handleMouseEvent); + textOne.removeEventHandler(MouseEvent.MOUSE_PRESSED, this::handleMouseEvent); + textTwo.removeEventHandler(MouseEvent.MOUSE_PRESSED, this::handleMouseEvent); + textThree.removeEventHandler(MouseEvent.MOUSE_PRESSED, this::handleMouseEvent); + }); + } + + @Before + public void setupUI() { + Platform.runLater(() -> { + textFlow.addEventHandler(MouseEvent.MOUSE_PRESSED, this::handleMouseEvent); + textOne.addEventHandler(MouseEvent.MOUSE_PRESSED, this::handleMouseEvent); + textTwo.addEventHandler(MouseEvent.MOUSE_PRESSED, this::handleMouseEvent); + textThree.addEventHandler(MouseEvent.MOUSE_PRESSED, this::handleMouseEvent); + }); + } + + @BeforeClass + public static void initFX() { + long seed = new Random().nextLong(); + System.out.println("seed=" + seed); + random = new Random(seed); + + Util.launch(startupLatch, TestApp.class); + } + + @AfterClass + public static void exit() { + Util.shutdown(stage); + } + + public static class TestApp extends Application { + @Override + public void start(Stage primaryStage) { + robot = new Robot(); + stage = primaryStage; + + textOne = new Text(); + textTwo = new Text(); + textThree = new Text(); + textFlow = new TextFlow(); + vBox = new VBox(); + + scene = new Scene(vBox, WIDTH, HEIGHT); + scene.setNodeOrientation(NodeOrientation.RIGHT_TO_LEFT); + stage.setScene(scene); + stage.initStyle(StageStyle.UNDECORATED); + stage.setOnShown(event -> Platform.runLater(startupLatch::countDown)); + stage.setAlwaysOnTop(true); + stage.show(); + } + } +} From b23e491050b2382f6dedbbc3c3971263a8d5bdca Mon Sep 17 00:00:00 2001 From: Karthik P K Date: Thu, 11 Jan 2024 15:42:04 +0530 Subject: [PATCH 02/16] Code review changes --- .../com/sun/javafx/scene/text/TextLayout.java | 4 +- .../src/main/java/javafx/scene/text/Text.java | 2 +- .../scene/RTLTextCharacterIndexTest.java | 78 +++++------ .../scene/RTLTextFlowCharacterIndexTest.java | 123 ++++++++---------- 4 files changed, 97 insertions(+), 110 deletions(-) diff --git a/modules/javafx.graphics/src/main/java/com/sun/javafx/scene/text/TextLayout.java b/modules/javafx.graphics/src/main/java/com/sun/javafx/scene/text/TextLayout.java index 5b561fc1fb1..ffca00277b4 100644 --- a/modules/javafx.graphics/src/main/java/com/sun/javafx/scene/text/TextLayout.java +++ b/modules/javafx.graphics/src/main/java/com/sun/javafx/scene/text/TextLayout.java @@ -208,8 +208,10 @@ public Hit(int charIndex, int insertionIndex, boolean leading) { * @param text text for which HitInfo needs to be calculated. * It is expected to be null in the case of {@link javafx.scene.text.TextFlow} * and non-null in the case of {@link javafx.scene.text.Text} - * @param textRunStart Text run start position. + * @param textRunStart Start position of first Text run where hit info is requested. + * For TextFlow this value will be -1 to distinguish it from Text node. * @param curRunStart starting position of text run where hit info is requested. + * For TextFlow this value will be -1 to distinguish it from Text node. * @return returns a {@link Hit} object containing character index, insertion index and position of cursor on the character. */ public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRunStart); diff --git a/modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java b/modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java index 9c23d8c7697..42dfd906433 100644 --- a/modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java +++ b/modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java @@ -1039,7 +1039,7 @@ public final HitInfo hitTest(Point2D point) { private int findRunIndex(double x, double y, GlyphList[] runs) { int runIndex = 0; if (runs.length != 0) { - if (this.getScene().getNodeOrientation() == NodeOrientation.RIGHT_TO_LEFT) { + if (getEffectiveNodeOrientation() == NodeOrientation.RIGHT_TO_LEFT) { double yPos = y; if (runs[runIndex].getTextSpan() == null) { while (runIndex < runs.length - 1) { diff --git a/tests/system/src/test/java/test/robot/javafx/scene/RTLTextCharacterIndexTest.java b/tests/system/src/test/java/test/robot/javafx/scene/RTLTextCharacterIndexTest.java index ad9909d3012..d465ba4dfd0 100644 --- a/tests/system/src/test/java/test/robot/javafx/scene/RTLTextCharacterIndexTest.java +++ b/tests/system/src/test/java/test/robot/javafx/scene/RTLTextCharacterIndexTest.java @@ -45,12 +45,12 @@ import javafx.stage.Stage; import javafx.stage.StageStyle; import javafx.stage.Window; -import org.junit.After; -import org.junit.AfterClass; -import org.junit.Assert; -import org.junit.Before; -import org.junit.BeforeClass; -import org.junit.Test; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import test.util.Util; /* @@ -130,8 +130,8 @@ public class RTLTextCharacterIndexTest { private void mouseClick(double x, double y) { Util.runAndWait(() -> { Window w = scene.getWindow(); - robot.mouseMove((int) (w.getX() + scene.getX() + x), - (int) (w.getY() + scene.getY() + y)); + robot.mouseMove(w.getX() + scene.getX() + x, + w.getY() + scene.getY() + y); robot.mouseClick(MouseButton.PRIMARY); }); } @@ -145,8 +145,7 @@ private void addRTLEnglishText() { Util.runAndWait(() -> { textOne.setText("This is text"); textOne.setFont(new Font(48)); - vBox.getChildren().clear(); - vBox.getChildren().add(textOne); + vBox.getChildren().setAll(textOne); }); } @@ -154,8 +153,7 @@ private void addRTLArabicText() { Util.runAndWait(() -> { textOne.setText("شسيبلاتنم"); textOne.setFont(new Font(48)); - vBox.getChildren().clear(); - vBox.getChildren().add(textOne); + vBox.getChildren().setAll(textOne); }); } @@ -163,8 +161,7 @@ private void addRTLEnglishArabicText() { Util.runAndWait(() -> { textOne.setText("Arabic:شسيبلاتنم"); textOne.setFont(new Font(48)); - vBox.getChildren().clear(); - vBox.getChildren().add(textOne); + vBox.getChildren().setAll(textOne); }); } @@ -172,8 +169,7 @@ private void addMultiLineRTLEnglishText() { Util.runAndWait(() -> { textOne.setText("This is text\nThis is text"); textOne.setFont(new Font(48)); - vBox.getChildren().clear(); - vBox.getChildren().add(textOne); + vBox.getChildren().setAll(textOne); }); } @@ -181,8 +177,7 @@ private void addMultiLineRTLEnglishArabicText() { Util.runAndWait(() -> { textOne.setText("Arabic:شسيبلاتنم\nArabic:شسيبلاتنم"); textOne.setFont(new Font(48)); - vBox.getChildren().clear(); - vBox.getChildren().add(textOne); + vBox.getChildren().setAll(textOne); }); } @@ -190,8 +185,7 @@ private void addMultiLineRTLArabicText() { Util.runAndWait(() -> { textOne.setText("شسيبلاتنم شسيبلاتنم\nشسيبلاتنم شسيبلاتنم"); textOne.setFont(new Font(48)); - vBox.getChildren().clear(); - vBox.getChildren().add(textOne); + vBox.getChildren().setAll(textOne); }); } @@ -206,11 +200,11 @@ public void testTextInfoForRTLEnglishText() throws Exception { while (x > X_LEADING_OFFSET) { moveMouseOverText(x, 0); if (isLeading) { - Assert.assertEquals(charIndex, insertionIndex); + Assertions.assertEquals(charIndex, insertionIndex); } else { - Assert.assertEquals(charIndex, insertionIndex - 1); + Assertions.assertEquals(charIndex, insertionIndex - 1); } - Assert.assertTrue(charIndex < textOneLength); + Assertions.assertTrue(charIndex < textOneLength); x -= step(); } } @@ -226,11 +220,11 @@ public void testTextInfoForRTLArabicText() throws Exception { while (x > X_LEADING_OFFSET) { moveMouseOverText(x, 0); if (isLeading) { - Assert.assertEquals(charIndex, insertionIndex); + Assertions.assertEquals(charIndex, insertionIndex); } else { - Assert.assertEquals(charIndex, insertionIndex - 1); + Assertions.assertEquals(charIndex, insertionIndex - 1); } - Assert.assertTrue(charIndex < textOneLength); + Assertions.assertTrue(charIndex < textOneLength); x -= step(); } } @@ -246,11 +240,11 @@ public void testTextInfoForRTLEnglishArabicText() throws Exception { while (x > X_LEADING_OFFSET) { moveMouseOverText(x, 0); if (isLeading) { - Assert.assertEquals(charIndex, insertionIndex); + Assertions.assertEquals(charIndex, insertionIndex); } else { - Assert.assertEquals(charIndex, insertionIndex - 1); + Assertions.assertEquals(charIndex, insertionIndex - 1); } - Assert.assertTrue(charIndex < textOneLength); + Assertions.assertTrue(charIndex < textOneLength); x -= step(); } } @@ -267,11 +261,11 @@ public void testTextInfoForMultiLineRTLEnglishText() throws Exception { while (x > X_LEADING_OFFSET) { moveMouseOverText(x, (Y_OFFSET * (y * 2))); if (isLeading) { - Assert.assertEquals(charIndex, insertionIndex); + Assertions.assertEquals(charIndex, insertionIndex); } else { - Assert.assertEquals(charIndex, insertionIndex - 1); + Assertions.assertEquals(charIndex, insertionIndex - 1); } - Assert.assertTrue(charIndex < textOneLength); + Assertions.assertTrue(charIndex < textOneLength); x -= step(); } } @@ -289,11 +283,11 @@ public void testTextInfoForMultiLineRTLEnglishArabicText() throws Exception { while (x > X_LEADING_OFFSET) { moveMouseOverText(x, (Y_OFFSET * (y * 2))); if (isLeading) { - Assert.assertEquals(charIndex, insertionIndex); + Assertions.assertEquals(charIndex, insertionIndex); } else { - Assert.assertEquals(charIndex, insertionIndex - 1); + Assertions.assertEquals(charIndex, insertionIndex - 1); } - Assert.assertTrue(charIndex < textOneLength); + Assertions.assertTrue(charIndex < textOneLength); x -= step(); } } @@ -311,11 +305,11 @@ public void testTextInfoForMultiLineRTLArabicText() throws Exception { while (x > X_LEADING_OFFSET) { moveMouseOverText(x, (Y_OFFSET * (y * 2))); if (isLeading) { - Assert.assertEquals(charIndex, insertionIndex); + Assertions.assertEquals(charIndex, insertionIndex); } else { - Assert.assertEquals(charIndex, insertionIndex - 1); + Assertions.assertEquals(charIndex, insertionIndex - 1); } - Assert.assertTrue(charIndex < textOneLength); + Assertions.assertTrue(charIndex < textOneLength); x -= step(); } } @@ -340,21 +334,21 @@ private double step() { return 1.0 + random.nextDouble() * 8.0; } - @After + @AfterEach public void resetUI() { Platform.runLater(() -> { textOne.removeEventHandler(MouseEvent.MOUSE_PRESSED, this::handleTextMouseEvent); }); } - @Before + @BeforeEach public void setupUI() { Platform.runLater(() -> { textOne.addEventHandler(MouseEvent.MOUSE_PRESSED, this::handleTextMouseEvent); }); } - @BeforeClass + @BeforeAll public static void initFX() { long seed = new Random().nextLong(); System.out.println("seed=" + seed); @@ -363,7 +357,7 @@ public static void initFX() { Util.launch(startupLatch, TestApp.class); } - @AfterClass + @AfterAll public static void exit() { Util.shutdown(stage); } diff --git a/tests/system/src/test/java/test/robot/javafx/scene/RTLTextFlowCharacterIndexTest.java b/tests/system/src/test/java/test/robot/javafx/scene/RTLTextFlowCharacterIndexTest.java index 06c00ff25c1..768614888e9 100644 --- a/tests/system/src/test/java/test/robot/javafx/scene/RTLTextFlowCharacterIndexTest.java +++ b/tests/system/src/test/java/test/robot/javafx/scene/RTLTextFlowCharacterIndexTest.java @@ -31,9 +31,12 @@ import javafx.application.Platform; import javafx.geometry.NodeOrientation; import javafx.geometry.Point2D; +import javafx.geometry.Point3D; +import javafx.scene.Node; import javafx.scene.Scene; import javafx.scene.input.MouseButton; import javafx.scene.input.MouseEvent; +import javafx.scene.input.PickResult; import javafx.scene.layout.VBox; import javafx.scene.robot.Robot; import javafx.scene.text.Font; @@ -43,20 +46,14 @@ import javafx.stage.Stage; import javafx.stage.StageStyle; import javafx.stage.Window; -import org.junit.After; -import org.junit.AfterClass; -import org.junit.Assert; -import org.junit.Before; -import org.junit.BeforeClass; -import org.junit.Test; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import test.util.Util; -import javafx.scene.layout.StackPane; - -import javafx.scene.input.PickResult; -import javafx.scene.Node; -import javafx.geometry.Point3D; - /* * Test for verifying character index of Text nodes embedded in TextFlow in RTL orientation. * @@ -132,8 +129,8 @@ public class RTLTextFlowCharacterIndexTest { private void mouseClick(double x, double y) { Util.runAndWait(() -> { Window w = scene.getWindow(); - robot.mouseMove((int) (w.getX() + scene.getX() + x), - (int) (w.getY() + scene.getY() + y)); + robot.mouseMove(w.getX() + scene.getX() + x, + w.getY() + scene.getY() + y); robot.mouseClick(MouseButton.PRIMARY); }); } @@ -148,8 +145,7 @@ private void addRTLArabicText() { textOne.setText("شسيبلاتنم"); textOne.setFont(new Font(48)); textFlow.getChildren().setAll(textOne); - vBox.getChildren().clear(); - vBox.getChildren().add(textFlow); + vBox.getChildren().setAll(textFlow); }); } @@ -158,8 +154,7 @@ private void addRTLEnglishText() { textOne.setText("This is text"); textOne.setFont(new Font(48)); textFlow.getChildren().setAll(textOne); - vBox.getChildren().clear(); - vBox.getChildren().add(textFlow); + vBox.getChildren().setAll(textFlow); }); } @@ -170,8 +165,7 @@ private void addMultiNodeRTLEnglishArabicText() { textTwo.setText("شسيبلاتنم"); textTwo.setFont(new Font(48)); textFlow.getChildren().setAll(textOne, textTwo); - vBox.getChildren().clear(); - vBox.getChildren().add(textFlow); + vBox.getChildren().setAll(textFlow); }); } @@ -184,8 +178,7 @@ private void addMultiLineMultiNodeRTLEnglishArabicText() { textThree.setText("حخهعغقثصضشسيبل"); textThree.setFont(new Font(48)); textFlow.getChildren().setAll(textOne, textTwo, textThree); - vBox.getChildren().clear(); - vBox.getChildren().add(textFlow); + vBox.getChildren().setAll(textFlow); }); } @@ -198,8 +191,7 @@ private void addMutliLineMultiNodeRTLEnglishText() { textThree.setText("Third line of text"); textThree.setFont(new Font(48)); textFlow.getChildren().setAll(textOne, textTwo, textThree); - vBox.getChildren().clear(); - vBox.getChildren().add(textFlow); + vBox.getChildren().setAll(textFlow); }); } @@ -212,8 +204,7 @@ private void addMutliLineMultiNodeRTLArabicText() { textThree.setText("ضصثقف"); textThree.setFont(new Font(48)); textFlow.getChildren().setAll(textOne, textTwo, textThree); - vBox.getChildren().clear(); - vBox.getChildren().add(textFlow); + vBox.getChildren().setAll(textFlow); }); } @@ -228,17 +219,17 @@ public void testTextAndTextFlowHitInfoForRTLArabicText() throws Exception { while (x > X_LEADING_OFFSET) { moveMouseOverTextFlow(x, Y_OFFSET); if (isLeading) { - Assert.assertEquals(charIndex, insertionIndex); + Assertions.assertEquals(charIndex, insertionIndex); } else { - Assert.assertEquals(charIndex, insertionIndex - 1); + Assertions.assertEquals(charIndex, insertionIndex - 1); } if (textFlowIsLeading) { - Assert.assertEquals(textFlowCharIndex, textFlowInsertionIndex); + Assertions.assertEquals(textFlowCharIndex, textFlowInsertionIndex); } else { - Assert.assertEquals(textFlowCharIndex, textFlowInsertionIndex - 1); + Assertions.assertEquals(textFlowCharIndex, textFlowInsertionIndex - 1); } - Assert.assertTrue(charIndex < textOneLength); - Assert.assertTrue(textFlowCharIndex < textOneLength); + Assertions.assertTrue(charIndex < textOneLength); + Assertions.assertTrue(textFlowCharIndex < textOneLength); x -= step(); } } @@ -254,17 +245,17 @@ public void testTextAndTextFlowHitInfoForRTLEnglishText() throws Exception { while (x > X_LEADING_OFFSET) { moveMouseOverTextFlow(x, Y_OFFSET); if (isLeading) { - Assert.assertEquals(charIndex, insertionIndex); + Assertions.assertEquals(charIndex, insertionIndex); } else { - Assert.assertEquals(charIndex, insertionIndex - 1); + Assertions.assertEquals(charIndex, insertionIndex - 1); } if (textFlowIsLeading) { - Assert.assertEquals(textFlowCharIndex, textFlowInsertionIndex); + Assertions.assertEquals(textFlowCharIndex, textFlowInsertionIndex); } else { - Assert.assertEquals(textFlowCharIndex, textFlowInsertionIndex - 1); + Assertions.assertEquals(textFlowCharIndex, textFlowInsertionIndex - 1); } - Assert.assertTrue(charIndex < textOneLength); - Assert.assertTrue(textFlowCharIndex < textOneLength); + Assertions.assertTrue(charIndex < textOneLength); + Assertions.assertTrue(textFlowCharIndex < textOneLength); x -= step(); } } @@ -281,17 +272,17 @@ public void testTextAndTextFlowHitInfoForRTLMultipleTextNodes() throws Exception while (x > X_LEADING_OFFSET) { moveMouseOverTextFlow(x, Y_OFFSET); if (isLeading) { - Assert.assertEquals(charIndex, insertionIndex); + Assertions.assertEquals(charIndex, insertionIndex); } else { - Assert.assertEquals(charIndex, insertionIndex - 1); + Assertions.assertEquals(charIndex, insertionIndex - 1); } if (textFlowIsLeading) { - Assert.assertEquals(textFlowCharIndex, textFlowInsertionIndex); + Assertions.assertEquals(textFlowCharIndex, textFlowInsertionIndex); } else { - Assert.assertEquals(textFlowCharIndex, textFlowInsertionIndex - 1); + Assertions.assertEquals(textFlowCharIndex, textFlowInsertionIndex - 1); } - Assert.assertTrue(charIndex < Math.max(textOneLength, textTwoLength)); - Assert.assertTrue(textFlowCharIndex < textOneLength + textTwoLength); + Assertions.assertTrue(charIndex < Math.max(textOneLength, textTwoLength)); + Assertions.assertTrue(textFlowCharIndex < textOneLength + textTwoLength); x -= step(); } } @@ -310,17 +301,17 @@ public void testTextAndTextFlowHitInfoForRTLMultipleMultiLineEnglishArabicTextNo while (x > X_LEADING_OFFSET) { moveMouseOverTextFlow(x, (Y_OFFSET + (Y_OFFSET * (y * 2)))); if (isLeading) { - Assert.assertEquals(charIndex, insertionIndex); + Assertions.assertEquals(charIndex, insertionIndex); } else { - Assert.assertEquals(charIndex, insertionIndex - 1); + Assertions.assertEquals(charIndex, insertionIndex - 1); } if (textFlowIsLeading) { - Assert.assertEquals(textFlowCharIndex, textFlowInsertionIndex); + Assertions.assertEquals(textFlowCharIndex, textFlowInsertionIndex); } else { - Assert.assertEquals(textFlowCharIndex, textFlowInsertionIndex - 1); + Assertions.assertEquals(textFlowCharIndex, textFlowInsertionIndex - 1); } - Assert.assertTrue(charIndex < Math.max(textThreeLength, Math.max(textOneLength, textTwoLength))); - Assert.assertTrue(textFlowCharIndex < textOneLength + textTwoLength + textThreeLength); + Assertions.assertTrue(charIndex < Math.max(textThreeLength, Math.max(textOneLength, textTwoLength))); + Assertions.assertTrue(textFlowCharIndex < textOneLength + textTwoLength + textThreeLength); x -= step(); } } @@ -340,17 +331,17 @@ public void testTextAndTextFlowHitInfoForRTLMultipleMultiLineEnglishTextNodes() while (x > X_LEADING_OFFSET) { moveMouseOverTextFlow(x, (Y_OFFSET + (Y_OFFSET * (y * 2)))); if (isLeading) { - Assert.assertEquals(charIndex, insertionIndex); + Assertions.assertEquals(charIndex, insertionIndex); } else { - Assert.assertEquals(charIndex, insertionIndex - 1); + Assertions.assertEquals(charIndex, insertionIndex - 1); } if (textFlowIsLeading) { - Assert.assertEquals(textFlowCharIndex, textFlowInsertionIndex); + Assertions.assertEquals(textFlowCharIndex, textFlowInsertionIndex); } else { - Assert.assertEquals(textFlowCharIndex, textFlowInsertionIndex - 1); + Assertions.assertEquals(textFlowCharIndex, textFlowInsertionIndex - 1); } - Assert.assertTrue(charIndex < Math.max(textThreeLength, Math.max(textOneLength, textTwoLength))); - Assert.assertTrue(textFlowCharIndex < textOneLength + textTwoLength + textThreeLength); + Assertions.assertTrue(charIndex < Math.max(textThreeLength, Math.max(textOneLength, textTwoLength))); + Assertions.assertTrue(textFlowCharIndex < textOneLength + textTwoLength + textThreeLength); x -= step(); } } @@ -370,17 +361,17 @@ public void testTextAndTextFlowHitInfoForRTLMultipleMultiLineArabicTextNodes() t while (x > X_LEADING_OFFSET) { moveMouseOverTextFlow(x, (Y_OFFSET + (Y_OFFSET * (y * 2)))); if (isLeading) { - Assert.assertEquals(charIndex, insertionIndex); + Assertions.assertEquals(charIndex, insertionIndex); } else { - Assert.assertEquals(charIndex, insertionIndex - 1); + Assertions.assertEquals(charIndex, insertionIndex - 1); } if (textFlowIsLeading) { - Assert.assertEquals(textFlowCharIndex, textFlowInsertionIndex); + Assertions.assertEquals(textFlowCharIndex, textFlowInsertionIndex); } else { - Assert.assertEquals(textFlowCharIndex, textFlowInsertionIndex - 1); + Assertions.assertEquals(textFlowCharIndex, textFlowInsertionIndex - 1); } - Assert.assertTrue(charIndex < Math.max(textThreeLength, Math.max(textOneLength, textTwoLength))); - Assert.assertTrue(textFlowCharIndex < textOneLength + textTwoLength + textThreeLength); + Assertions.assertTrue(charIndex < Math.max(textThreeLength, Math.max(textOneLength, textTwoLength))); + Assertions.assertTrue(textFlowCharIndex < textOneLength + textTwoLength + textThreeLength); x -= step(); } } @@ -411,7 +402,7 @@ private double step() { return 1.0 + random.nextDouble() * 8.0; } - @After + @AfterEach public void resetUI() { Platform.runLater(() -> { textFlow.removeEventHandler(MouseEvent.MOUSE_PRESSED, this::handleMouseEvent); @@ -421,7 +412,7 @@ public void resetUI() { }); } - @Before + @BeforeEach public void setupUI() { Platform.runLater(() -> { textFlow.addEventHandler(MouseEvent.MOUSE_PRESSED, this::handleMouseEvent); @@ -431,7 +422,7 @@ public void setupUI() { }); } - @BeforeClass + @BeforeAll public static void initFX() { long seed = new Random().nextLong(); System.out.println("seed=" + seed); @@ -440,7 +431,7 @@ public static void initFX() { Util.launch(startupLatch, TestApp.class); } - @AfterClass + @AfterAll public static void exit() { Util.shutdown(stage); } From cd270e16ef0e75b3ad06f5b6ce99c37fec1ffb76 Mon Sep 17 00:00:00 2001 From: Karthik P K Date: Tue, 16 Jan 2024 15:57:23 +0530 Subject: [PATCH 03/16] Code review changes --- .../com/sun/javafx/scene/text/TextLayout.java | 7 +- .../com/sun/javafx/text/PrismTextLayout.java | 33 ++++++--- .../src/main/java/javafx/scene/text/Text.java | 73 +++++++++++-------- .../main/java/javafx/scene/text/TextFlow.java | 2 +- .../com/sun/javafx/pgstub/StubTextLayout.java | 2 +- .../scene/RTLTextCharacterIndexTest.java | 72 +++++++++--------- 6 files changed, 103 insertions(+), 86 deletions(-) diff --git a/modules/javafx.graphics/src/main/java/com/sun/javafx/scene/text/TextLayout.java b/modules/javafx.graphics/src/main/java/com/sun/javafx/scene/text/TextLayout.java index ffca00277b4..6a7a7943833 100644 --- a/modules/javafx.graphics/src/main/java/com/sun/javafx/scene/text/TextLayout.java +++ b/modules/javafx.graphics/src/main/java/com/sun/javafx/scene/text/TextLayout.java @@ -209,12 +209,11 @@ public Hit(int charIndex, int insertionIndex, boolean leading) { * It is expected to be null in the case of {@link javafx.scene.text.TextFlow} * and non-null in the case of {@link javafx.scene.text.Text} * @param textRunStart Start position of first Text run where hit info is requested. - * For TextFlow this value will be -1 to distinguish it from Text node. - * @param curRunStart starting position of text run where hit info is requested. - * For TextFlow this value will be -1 to distinguish it from Text node. + * @param curRunStart Start position of text run where hit info is requested. + * @param forTextFlow Indicates if the hit info is requested for TextFlow or Text node. True for TextFlow and False for Text node. * @return returns a {@link Hit} object containing character index, insertion index and position of cursor on the character. */ - public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRunStart); + public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRunStart, boolean forTextFlow); public PathElement[] getCaretShape(int offset, boolean isLeading, float x, float y); diff --git a/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java b/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java index 614c7c21b9d..264a3005101 100644 --- a/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java +++ b/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java @@ -421,10 +421,9 @@ public PathElement[] getCaretShape(int offset, boolean isLeading, } @Override - public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRunStart) { + public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRunStart, boolean forTextFlow) { boolean leading = false; boolean isMirrored = isMirrored(); // Node orientation is RTL - boolean isMultiRunText = false; int charIndex = -1; int insertionIndex = -1; int relIndex = 0; @@ -438,8 +437,7 @@ public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRu charIndex = getCharCount(); insertionIndex = charIndex + 1; } else { - boolean isTextFlow = textRunStart == -1 && curRunStart == -1; - if (isMirrored && (isTextFlow || spans == null)) { + if (isMirrored && (forTextFlow || spans == null)) { x = getMirroringWidth() - x; } TextLine line = lines[lineIndex]; @@ -448,17 +446,20 @@ public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRu TextRun run = null; //TODO binary search if (text == null || spans == null) { - // To calculate Text and TextFlow hit info + /* This code branch is used to calculate hit info of Text node + which are not embedded in TextFlow and hit info requested on TextFlow. */ if (isMirrored) { int runIndex = -1; for (int i = runs.length - 1; i >=0; i--) { run = runs[i]; - if (x < run.getWidth() && (isTextFlow || (run.getStart() == curRunStart))) { + if (x < run.getWidth() && (forTextFlow || (run.getStart() == curRunStart))) { runIndex = i; break; } if (i - 1 >= 0) { - if (runs[i - 1].isLinebreak()) break; + if (runs[i - 1].isLinebreak()) { + break; + } x -= run.getWidth(); } } @@ -469,15 +470,19 @@ public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRu } else { for (int i = 0; i < runs.length; i++) { run = runs[i]; - if (x < run.getWidth()) break; + if (x < run.getWidth()) { + break; + } if (i + 1 < runs.length) { - if (runs[i + 1].isLinebreak()) break; + if (runs[i + 1].isLinebreak()) { + break; + } x -= run.getWidth(); } } } } else { - // To calculate hit info of Text embedded in TextFlow + // This code branch is used to calculate hit info of Text node embedded in TextFlow. for (int i = 0; i < lineIndex; i++) { for (TextRun r: lines[i].runs) { if (r.getTextSpan() != null && r.getStart() >= textRunStart && r.getTextSpan().getText().equals(text)) { @@ -487,6 +492,7 @@ public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRu } if (isMirrored) { + boolean isMultiRunText = false; for (TextRun r: runs) { if (r.getStart() != curRunStart && r.getTextSpan().getText().equals(text)) { isMultiRunText = true; @@ -521,7 +527,7 @@ public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRu } break; } - // For only English Text embedded in TextFlow in RTL orientation + // This condition handles LTR Text nodes embedded in TextFlow in RTL mode. if (!run.getTextSpan().getText().equals(text) && x > run.getWidth() && run.getStart() < curRunStart) { x -= run.getWidth(); } @@ -559,7 +565,10 @@ public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRu charIndex = run.getOffsetAtX(x, trailing); charIndex += textWidthPrevLine; charIndex += relIndex; - if (run.getLevel() % 2 != 0) { + if ((run.getLevel() & 0x01) != 0) { + /* Odd level represents RTL text. + * If the RTL text has LTR text embedded, + * add the LTR index here to get effective character index */ charIndex += LTRIndex; } } else { diff --git a/modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java b/modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java index 42dfd906433..51b5ab7a271 100644 --- a/modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java +++ b/modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java @@ -1032,51 +1032,60 @@ public final HitInfo hitTest(Point2D point) { textRunStart = ((TextRun) runs[0]).getStart(); curRunStart = ((TextRun) runs[runIndex]).getStart(); } - TextLayout.Hit h = layout.getHitInfo((float)x, (float)y, getText(), textRunStart, curRunStart); + TextLayout.Hit h = layout.getHitInfo((float)x, (float)y, getText(), textRunStart, curRunStart, false); return new HitInfo(h.getCharIndex(), h.getInsertionIndex(), h.isLeading()); } private int findRunIndex(double x, double y, GlyphList[] runs) { - int runIndex = 0; - if (runs.length != 0) { - if (getEffectiveNodeOrientation() == NodeOrientation.RIGHT_TO_LEFT) { - double yPos = y; - if (runs[runIndex].getTextSpan() == null) { - while (runIndex < runs.length - 1) { - if (x > runs[runIndex].getLocation().x && x < runs[runIndex + 1].getLocation().x - && y > runs[runIndex].getLocation().y && y < runs[runIndex].getHeight()) { - break; - } - runIndex++; - if (y > runs[runIndex].getHeight() && (runs[runIndex].getLocation().y != runs[runIndex - 1].getLocation().y)) { - y -= runs[runIndex].getHeight(); - } - } - } else { - double ptX = localToParent(x, y).getX(); - double ptY = localToParent(x, y).getY(); - while (runIndex < runs.length - 1) { - if (ptX > runs[runIndex].getLocation().x && ptX < (runs[runIndex].getLocation().x + runs[runIndex].getWidth()) && ptY >= runs[runIndex].getLocation().y - && y < runs[runIndex].getHeight()) { - break; - } - runIndex++; - if (y > runs[runIndex].getHeight() && (runs[runIndex].getLocation().y != runs[runIndex - 1].getLocation().y)) { - y -= runs[runIndex].getHeight(); - } + int ix = 0; + int lastIndex = runs.length - 1; + + if (runs.length == 0) { + return ix; + } + + if (getEffectiveNodeOrientation() == NodeOrientation.RIGHT_TO_LEFT) { + if (runs[ix].getTextSpan() == null) { + while (ix < lastIndex) { + GlyphList r = runs[ix]; + if (x > r.getLocation().x && x < runs[ix + 1].getLocation().x + && y > r.getLocation().y && y < r.getHeight()) { + break; } + ix++; + y = updateY(y, ix, runs); } } else { + double ptX = localToParent(x, y).getX(); double ptY = localToParent(x, y).getY(); - while (runIndex < runs.length - 1) { - if (ptY > runs[runIndex].getLocation().y && ptY < runs[runIndex + 1].getLocation().y) { + while (ix < lastIndex) { + GlyphList r = runs[ix]; + if (ptX > r.getLocation().x && ptX < (r.getLocation().x + r.getWidth()) && ptY >= r.getLocation().y + && y < r.getHeight()) { break; } - runIndex++; + ix++; + y = updateY(y, ix, runs); + } + } + } else { + double ptY = localToParent(x, y).getY(); + while (ix < lastIndex) { + if (ptY > runs[ix].getLocation().y && ptY < runs[ix + 1].getLocation().y) { + break; } + ix++; } } - return runIndex; + return ix; + } + + private double updateY(double y, int ix, GlyphList[] runs) { + GlyphList curRun = runs[ix]; + if (y > curRun.getHeight() && (curRun.getLocation().y != runs[ix - 1].getLocation().y)) { + y -= curRun.getHeight(); + } + return y; } private PathElement[] getRange(int start, int end, int type) { diff --git a/modules/javafx.graphics/src/main/java/javafx/scene/text/TextFlow.java b/modules/javafx.graphics/src/main/java/javafx/scene/text/TextFlow.java index a90d9db38e2..ab4b5824b13 100644 --- a/modules/javafx.graphics/src/main/java/javafx/scene/text/TextFlow.java +++ b/modules/javafx.graphics/src/main/java/javafx/scene/text/TextFlow.java @@ -199,7 +199,7 @@ public final HitInfo hitTest(javafx.geometry.Point2D point) { TextLayout layout = getTextLayout(); double x = point.getX(); double y = point.getY(); - TextLayout.Hit h = layout.getHitInfo((float)x, (float)y, null, -1, -1); + TextLayout.Hit h = layout.getHitInfo((float)x, (float)y, null, 0, 0, true); return new HitInfo(h.getCharIndex(), h.getInsertionIndex(), h.isLeading()); } else { return null; diff --git a/modules/javafx.graphics/src/test/java/test/com/sun/javafx/pgstub/StubTextLayout.java b/modules/javafx.graphics/src/test/java/test/com/sun/javafx/pgstub/StubTextLayout.java index e079fa40c04..69793bb52ff 100644 --- a/modules/javafx.graphics/src/test/java/test/com/sun/javafx/pgstub/StubTextLayout.java +++ b/modules/javafx.graphics/src/test/java/test/com/sun/javafx/pgstub/StubTextLayout.java @@ -158,7 +158,7 @@ public Shape getShape(int type, TextSpan filter) { } @Override - public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRunStart) { + public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRunStart, boolean forTextFlow) { // TODO this probably needs to be entirely rewritten... if (getText() == null) { return new Hit(0, -1, true); diff --git a/tests/system/src/test/java/test/robot/javafx/scene/RTLTextCharacterIndexTest.java b/tests/system/src/test/java/test/robot/javafx/scene/RTLTextCharacterIndexTest.java index d465ba4dfd0..c74cd127293 100644 --- a/tests/system/src/test/java/test/robot/javafx/scene/RTLTextCharacterIndexTest.java +++ b/tests/system/src/test/java/test/robot/javafx/scene/RTLTextCharacterIndexTest.java @@ -108,7 +108,7 @@ public class RTLTextCharacterIndexTest { static CountDownLatch startupLatch = new CountDownLatch(1); static Random random; static Robot robot; - static Text textOne; + static Text text; static VBox vBox; static volatile Stage stage; @@ -137,55 +137,55 @@ private void mouseClick(double x, double y) { } private void moveMouseOverText(double x, double y) throws Exception { - mouseClick(textOne.getLayoutX() + x, - textOne.getLayoutY() / 2 + y); + mouseClick(text.getLayoutX() + x, + text.getLayoutY() / 2 + y); } private void addRTLEnglishText() { Util.runAndWait(() -> { - textOne.setText("This is text"); - textOne.setFont(new Font(48)); - vBox.getChildren().setAll(textOne); + text.setText("This is text"); + text.setFont(new Font(48)); + vBox.getChildren().setAll(text); }); } private void addRTLArabicText() { Util.runAndWait(() -> { - textOne.setText("شسيبلاتنم"); - textOne.setFont(new Font(48)); - vBox.getChildren().setAll(textOne); + text.setText("شسيبلاتنم"); + text.setFont(new Font(48)); + vBox.getChildren().setAll(text); }); } private void addRTLEnglishArabicText() { Util.runAndWait(() -> { - textOne.setText("Arabic:شسيبلاتنم"); - textOne.setFont(new Font(48)); - vBox.getChildren().setAll(textOne); + text.setText("Arabic:شسيبلاتنم"); + text.setFont(new Font(48)); + vBox.getChildren().setAll(text); }); } private void addMultiLineRTLEnglishText() { Util.runAndWait(() -> { - textOne.setText("This is text\nThis is text"); - textOne.setFont(new Font(48)); - vBox.getChildren().setAll(textOne); + text.setText("This is text\nThis is text"); + text.setFont(new Font(48)); + vBox.getChildren().setAll(text); }); } private void addMultiLineRTLEnglishArabicText() { Util.runAndWait(() -> { - textOne.setText("Arabic:شسيبلاتنم\nArabic:شسيبلاتنم"); - textOne.setFont(new Font(48)); - vBox.getChildren().setAll(textOne); + text.setText("Arabic:شسيبلاتنم\nArabic:شسيبلاتنم"); + text.setFont(new Font(48)); + vBox.getChildren().setAll(text); }); } private void addMultiLineRTLArabicText() { Util.runAndWait(() -> { - textOne.setText("شسيبلاتنم شسيبلاتنم\nشسيبلاتنم شسيبلاتنم"); - textOne.setFont(new Font(48)); - vBox.getChildren().setAll(textOne); + text.setText("شسيبلاتنم شسيبلاتنم\nشسيبلاتنم شسيبلاتنم"); + text.setFont(new Font(48)); + vBox.getChildren().setAll(text); }); } @@ -194,7 +194,7 @@ public void testTextInfoForRTLEnglishText() throws Exception { addRTLEnglishText(); Util.waitForIdle(scene); - int textOneLength = textOne.getText().length(); + int textLength = text.getText().length(); double x = WIDTH - X_LEADING_OFFSET; while (x > X_LEADING_OFFSET) { @@ -204,7 +204,7 @@ public void testTextInfoForRTLEnglishText() throws Exception { } else { Assertions.assertEquals(charIndex, insertionIndex - 1); } - Assertions.assertTrue(charIndex < textOneLength); + Assertions.assertTrue(charIndex < textLength); x -= step(); } } @@ -214,7 +214,7 @@ public void testTextInfoForRTLArabicText() throws Exception { addRTLArabicText(); Util.waitForIdle(scene); - int textOneLength = textOne.getText().length(); + int textLength = text.getText().length(); double x = WIDTH - X_LEADING_OFFSET; while (x > X_LEADING_OFFSET) { @@ -224,7 +224,7 @@ public void testTextInfoForRTLArabicText() throws Exception { } else { Assertions.assertEquals(charIndex, insertionIndex - 1); } - Assertions.assertTrue(charIndex < textOneLength); + Assertions.assertTrue(charIndex < textLength); x -= step(); } } @@ -234,7 +234,7 @@ public void testTextInfoForRTLEnglishArabicText() throws Exception { addRTLEnglishArabicText(); Util.waitForIdle(scene); - int textOneLength = textOne.getText().length(); + int textLength = text.getText().length(); double x = WIDTH - X_LEADING_OFFSET; while (x > X_LEADING_OFFSET) { @@ -244,7 +244,7 @@ public void testTextInfoForRTLEnglishArabicText() throws Exception { } else { Assertions.assertEquals(charIndex, insertionIndex - 1); } - Assertions.assertTrue(charIndex < textOneLength); + Assertions.assertTrue(charIndex < textLength); x -= step(); } } @@ -254,7 +254,7 @@ public void testTextInfoForMultiLineRTLEnglishText() throws Exception { addMultiLineRTLEnglishText(); Util.waitForIdle(scene); - int textOneLength = textOne.getText().length(); + int textLength = text.getText().length(); for (int y = 0; y < 2; y++) { double x = WIDTH - X_LEADING_OFFSET; @@ -265,7 +265,7 @@ public void testTextInfoForMultiLineRTLEnglishText() throws Exception { } else { Assertions.assertEquals(charIndex, insertionIndex - 1); } - Assertions.assertTrue(charIndex < textOneLength); + Assertions.assertTrue(charIndex < textLength); x -= step(); } } @@ -276,7 +276,7 @@ public void testTextInfoForMultiLineRTLEnglishArabicText() throws Exception { addMultiLineRTLEnglishArabicText(); Util.waitForIdle(scene); - int textOneLength = textOne.getText().length(); + int textLength = text.getText().length(); for (int y = 0; y < 2; y++) { double x = WIDTH - X_LEADING_OFFSET; @@ -287,7 +287,7 @@ public void testTextInfoForMultiLineRTLEnglishArabicText() throws Exception { } else { Assertions.assertEquals(charIndex, insertionIndex - 1); } - Assertions.assertTrue(charIndex < textOneLength); + Assertions.assertTrue(charIndex < textLength); x -= step(); } } @@ -298,7 +298,7 @@ public void testTextInfoForMultiLineRTLArabicText() throws Exception { addMultiLineRTLArabicText(); Util.waitForIdle(scene); - int textOneLength = textOne.getText().length(); + int textLength = text.getText().length(); for (int y = 0; y < 2; y++) { double x = WIDTH - X_LEADING_OFFSET; @@ -309,7 +309,7 @@ public void testTextInfoForMultiLineRTLArabicText() throws Exception { } else { Assertions.assertEquals(charIndex, insertionIndex - 1); } - Assertions.assertTrue(charIndex < textOneLength); + Assertions.assertTrue(charIndex < textLength); x -= step(); } } @@ -337,14 +337,14 @@ private double step() { @AfterEach public void resetUI() { Platform.runLater(() -> { - textOne.removeEventHandler(MouseEvent.MOUSE_PRESSED, this::handleTextMouseEvent); + text.removeEventHandler(MouseEvent.MOUSE_PRESSED, this::handleTextMouseEvent); }); } @BeforeEach public void setupUI() { Platform.runLater(() -> { - textOne.addEventHandler(MouseEvent.MOUSE_PRESSED, this::handleTextMouseEvent); + text.addEventHandler(MouseEvent.MOUSE_PRESSED, this::handleTextMouseEvent); }); } @@ -368,7 +368,7 @@ public void start(Stage primaryStage) { robot = new Robot(); stage = primaryStage; - textOne = new Text(); + text = new Text(); vBox = new VBox(); scene = new Scene(vBox, WIDTH, HEIGHT); From 87255aa9be11f97c4ac6edb4d9d211db2f21b15f Mon Sep 17 00:00:00 2001 From: Karthik P K Date: Wed, 17 Jan 2024 16:19:53 +0530 Subject: [PATCH 04/16] Fix multiline text insertion index calculation issue --- .../src/main/java/javafx/scene/text/Text.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java b/modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java index 51b5ab7a271..27288d4eed0 100644 --- a/modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java +++ b/modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java @@ -1048,8 +1048,10 @@ private int findRunIndex(double x, double y, GlyphList[] runs) { if (runs[ix].getTextSpan() == null) { while (ix < lastIndex) { GlyphList r = runs[ix]; - if (x > r.getLocation().x && x < runs[ix + 1].getLocation().x - && y > r.getLocation().y && y < r.getHeight()) { + GlyphList nr = runs[ix + 1]; + if ((x > r.getLocation().x && + (x < nr.getLocation().x || r.getLocation().y < nr.getLocation().y)) + && y < r.getHeight()) { break; } ix++; From b08de73c6f45428d4e9d5e72e1790e2e16b0d15e Mon Sep 17 00:00:00 2001 From: Karthik P K Date: Thu, 18 Jan 2024 13:15:20 +0530 Subject: [PATCH 05/16] Review changes --- .../java/com/sun/javafx/scene/text/TextLayout.java | 2 +- .../java/com/sun/javafx/text/PrismTextLayout.java | 13 ++++++------- .../src/main/java/javafx/scene/text/Text.java | 5 +++-- .../javafx/scene/RTLTextFlowCharacterIndexTest.java | 2 +- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/modules/javafx.graphics/src/main/java/com/sun/javafx/scene/text/TextLayout.java b/modules/javafx.graphics/src/main/java/com/sun/javafx/scene/text/TextLayout.java index 6a7a7943833..a2f38050c46 100644 --- a/modules/javafx.graphics/src/main/java/com/sun/javafx/scene/text/TextLayout.java +++ b/modules/javafx.graphics/src/main/java/com/sun/javafx/scene/text/TextLayout.java @@ -210,7 +210,7 @@ public Hit(int charIndex, int insertionIndex, boolean leading) { * and non-null in the case of {@link javafx.scene.text.Text} * @param textRunStart Start position of first Text run where hit info is requested. * @param curRunStart Start position of text run where hit info is requested. - * @param forTextFlow Indicates if the hit info is requested for TextFlow or Text node. True for TextFlow and False for Text node. + * @param forTextFlow Indicates if the hit info is requested for TextFlow or Text node. {@code true} for TextFlow and {@code false} for Text node. * @return returns a {@link Hit} object containing character index, insertion index and position of cursor on the character. */ public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRunStart, boolean forTextFlow); diff --git a/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java b/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java index 264a3005101..992eb82414e 100644 --- a/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java +++ b/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java @@ -427,7 +427,7 @@ public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRu int charIndex = -1; int insertionIndex = -1; int relIndex = 0; - int LTRIndex = 0; + int ltrIndex = 0; int textWidthPrevLine = 0; float xHitPos = x; @@ -450,7 +450,7 @@ public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRu which are not embedded in TextFlow and hit info requested on TextFlow. */ if (isMirrored) { int runIndex = -1; - for (int i = runs.length - 1; i >=0; i--) { + for (int i = runs.length - 1; i >= 0; i--) { run = runs[i]; if (x < run.getWidth() && (forTextFlow || (run.getStart() == curRunStart))) { runIndex = i; @@ -491,6 +491,7 @@ public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRu } } + BaseBounds textBounds = new BoxBounds(); if (isMirrored) { boolean isMultiRunText = false; for (TextRun r: runs) { @@ -507,7 +508,6 @@ public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRu } if (run.getTextSpan() != null && run.getTextSpan().getText().equals(text)) { if ((x > run.getWidth() && !isMultiRunText) || textWidthPrevLine > 0) { - BaseBounds textBounds = new BoxBounds(); getBounds(run.getTextSpan(), textBounds); x -= (run.getLocation().x - textBounds.getMinX()); break; @@ -518,8 +518,8 @@ public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRu continue; } for (int j = runs.length - 1; j >= 0; j--) { - if (runs[j].getTextSpan().getText().equals(text) && runs[j].getStart() != curRunStart) { - LTRIndex += runs[j].getLength(); + if (runs[j].getStart() != curRunStart && runs[j].getTextSpan().getText().equals(text)) { + ltrIndex += runs[j].getLength(); } if (runs[j].getStart() == curRunStart) { break; @@ -541,7 +541,6 @@ public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRu continue; } if (r.getTextSpan() != null && r.getTextSpan().getText().equals(text)) { - BaseBounds textBounds = new BoxBounds(); getBounds(r.getTextSpan(), textBounds); if (textBounds.getMinX() == 0 && !isPrevRunPresent) { x -= prevRunLength; @@ -569,7 +568,7 @@ public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRu /* Odd level represents RTL text. * If the RTL text has LTR text embedded, * add the LTR index here to get effective character index */ - charIndex += LTRIndex; + charIndex += ltrIndex; } } else { int indexOffset; diff --git a/modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java b/modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java index 27288d4eed0..5af11502f38 100644 --- a/modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java +++ b/modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java @@ -1058,8 +1058,9 @@ private int findRunIndex(double x, double y, GlyphList[] runs) { y = updateY(y, ix, runs); } } else { - double ptX = localToParent(x, y).getX(); - double ptY = localToParent(x, y).getY(); + Point2D ptInParent = localToParent(x, y); + double ptX = ptInParent.getX(); + double ptY = ptInParent.getY(); while (ix < lastIndex) { GlyphList r = runs[ix]; if (ptX > r.getLocation().x && ptX < (r.getLocation().x + r.getWidth()) && ptY >= r.getLocation().y diff --git a/tests/system/src/test/java/test/robot/javafx/scene/RTLTextFlowCharacterIndexTest.java b/tests/system/src/test/java/test/robot/javafx/scene/RTLTextFlowCharacterIndexTest.java index 768614888e9..91b0377857b 100644 --- a/tests/system/src/test/java/test/robot/javafx/scene/RTLTextFlowCharacterIndexTest.java +++ b/tests/system/src/test/java/test/robot/javafx/scene/RTLTextFlowCharacterIndexTest.java @@ -271,7 +271,7 @@ public void testTextAndTextFlowHitInfoForRTLMultipleTextNodes() throws Exception double x = WIDTH - X_LEADING_OFFSET; while (x > X_LEADING_OFFSET) { moveMouseOverTextFlow(x, Y_OFFSET); - if (isLeading) { + if (isLeading) { Assertions.assertEquals(charIndex, insertionIndex); } else { Assertions.assertEquals(charIndex, insertionIndex - 1); From a58f754e958c712a2a69d90efe4f20a3b9aece34 Mon Sep 17 00:00:00 2001 From: Karthik P K Date: Mon, 29 Jan 2024 12:57:38 +0530 Subject: [PATCH 06/16] Fix issue with RTL text within LTR text --- .../com/sun/javafx/text/PrismTextLayout.java | 29 +++++++------------ .../src/main/java/javafx/scene/text/Text.java | 12 +++++++- 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java b/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java index 992eb82414e..2e20edb07aa 100644 --- a/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java +++ b/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java @@ -497,33 +497,27 @@ public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRu for (TextRun r: runs) { if (r.getStart() != curRunStart && r.getTextSpan().getText().equals(text)) { isMultiRunText = true; + break; } } for (int i = 0; i < runs.length; i++) { run = runs[i]; - if (run.getStart() != curRunStart && run.getTextSpan().getText().equals(text) && x > run.getWidth()) { - x -= run.getWidth(); + if (run.getStart() != curRunStart) { + if (run.getTextSpan().getText().equals(text) && x > run.getWidth() && (run.getLevel() & 0x01) != 1) { + x -= run.getWidth(); + } continue; } if (run.getTextSpan() != null && run.getTextSpan().getText().equals(text)) { - if ((x > run.getWidth() && !isMultiRunText) || textWidthPrevLine > 0) { + if ((x > run.getWidth() && (!isMultiRunText || run.getStart() == curRunStart)) || textWidthPrevLine > 0) { getBounds(run.getTextSpan(), textBounds); x -= (run.getLocation().x - textBounds.getMinX()); - break; } - if (x > run.getWidth()) { - x -= run.getWidth(); - relIndex += run.getLength(); - continue; - } - for (int j = runs.length - 1; j >= 0; j--) { + for (int j = runs.length - 1; j > i; j--) { if (runs[j].getStart() != curRunStart && runs[j].getTextSpan().getText().equals(text)) { ltrIndex += runs[j].getLength(); } - if (runs[j].getStart() == curRunStart) { - break; - } } break; } @@ -564,12 +558,9 @@ public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRu charIndex = run.getOffsetAtX(x, trailing); charIndex += textWidthPrevLine; charIndex += relIndex; - if ((run.getLevel() & 0x01) != 0) { - /* Odd level represents RTL text. - * If the RTL text has LTR text embedded, - * add the LTR index here to get effective character index */ - charIndex += ltrIndex; - } + /* When RTL text has LTR text embedded, + * add the LTR index here to get effective character index */ + charIndex += ltrIndex; } else { int indexOffset; if (isMirrored) { diff --git a/modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java b/modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java index 5af11502f38..a085f43b91b 100644 --- a/modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java +++ b/modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java @@ -1029,13 +1029,23 @@ public final HitInfo hitTest(Point2D point) { int textRunStart = 0; int curRunStart = 0; if (runs.length != 0) { - textRunStart = ((TextRun) runs[0]).getStart(); + textRunStart = findFirstRunStart(x, y, runs); curRunStart = ((TextRun) runs[runIndex]).getStart(); } TextLayout.Hit h = layout.getHitInfo((float)x, (float)y, getText(), textRunStart, curRunStart, false); return new HitInfo(h.getCharIndex(), h.getInsertionIndex(), h.isLeading()); } + private int findFirstRunStart(double x, double y, GlyphList[] runs) { + int start = Integer.MAX_VALUE; + for (GlyphList r: runs) { + if (((TextRun) r).getStart() < start) { + start = ((TextRun) r).getStart(); + } + } + return start; + } + private int findRunIndex(double x, double y, GlyphList[] runs) { int ix = 0; int lastIndex = runs.length - 1; From 52ee61cc6b760d626874ab0004a39385e53c9b94 Mon Sep 17 00:00:00 2001 From: Karthik P K Date: Wed, 31 Jan 2024 15:49:41 +0530 Subject: [PATCH 07/16] Fix issue with multiline text --- .../main/java/com/sun/javafx/text/PrismTextLayout.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java b/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java index 2e20edb07aa..4d2bc865a1e 100644 --- a/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java +++ b/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java @@ -503,19 +503,17 @@ public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRu for (int i = 0; i < runs.length; i++) { run = runs[i]; - if (run.getStart() != curRunStart) { - if (run.getTextSpan().getText().equals(text) && x > run.getWidth() && (run.getLevel() & 0x01) != 1) { - x -= run.getWidth(); - } + if (run.getStart() != curRunStart && run.getTextSpan().getText().equals(text) && x > run.getWidth()) { + x -= run.getWidth(); continue; } if (run.getTextSpan() != null && run.getTextSpan().getText().equals(text)) { if ((x > run.getWidth() && (!isMultiRunText || run.getStart() == curRunStart)) || textWidthPrevLine > 0) { getBounds(run.getTextSpan(), textBounds); - x -= (run.getLocation().x - textBounds.getMinX()); + x -= (runs[0].getLocation().x - textBounds.getMinX()); } for (int j = runs.length - 1; j > i; j--) { - if (runs[j].getStart() != curRunStart && runs[j].getTextSpan().getText().equals(text)) { + if (runs[j].getStart() != curRunStart && runs[j].getTextSpan().getText().equals(text) && !runs[j].isLinebreak()) { ltrIndex += runs[j].getLength(); } } From 3012fae8c6f077c75b66a2db4d5b3a4a505a1f9f Mon Sep 17 00:00:00 2001 From: Karthik P K Date: Thu, 1 Feb 2024 14:46:47 +0530 Subject: [PATCH 08/16] Code review changes --- .../src/main/java/com/sun/javafx/text/PrismTextLayout.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java b/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java index 4d2bc865a1e..f19032eb7d2 100644 --- a/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java +++ b/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java @@ -513,8 +513,9 @@ public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRu x -= (runs[0].getLocation().x - textBounds.getMinX()); } for (int j = runs.length - 1; j > i; j--) { - if (runs[j].getStart() != curRunStart && runs[j].getTextSpan().getText().equals(text) && !runs[j].isLinebreak()) { - ltrIndex += runs[j].getLength(); + TextRun r = runs[j]; + if (r.getStart() != curRunStart && r.getTextSpan().getText().equals(text) && !r.isLinebreak()) { + ltrIndex += r.getLength(); } } break; From cc89f12a812726deed6dbef7fb2bd88a8959383d Mon Sep 17 00:00:00 2001 From: Karthik P K Date: Tue, 6 Feb 2024 15:58:17 +0530 Subject: [PATCH 09/16] Inline node issue fix --- .../com/sun/javafx/text/PrismTextLayout.java | 48 +++++++++++-------- 1 file changed, 28 insertions(+), 20 deletions(-) diff --git a/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java b/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java index f19032eb7d2..6d459d4329a 100644 --- a/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java +++ b/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java @@ -493,36 +493,44 @@ public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRu BaseBounds textBounds = new BoxBounds(); if (isMirrored) { - boolean isMultiRunText = false; + int runIdx = 0; for (TextRun r: runs) { - if (r.getStart() != curRunStart && r.getTextSpan().getText().equals(text)) { - isMultiRunText = true; + if (r.getStart() == curRunStart) { + run = r; break; } + runIdx++; } - for (int i = 0; i < runs.length; i++) { - run = runs[i]; - if (run.getStart() != curRunStart && run.getTextSpan().getText().equals(text) && x > run.getWidth()) { - x -= run.getWidth(); + boolean textFound = false; + for (int i = 0; i <= runIdx; i++) { + TextRun r = runs[i]; + if (r.getStart() != curRunStart && r.getTextSpan().getText().equals(text) + && x > r.getWidth() && textWidthPrevLine == 0) { + x -= r.getWidth(); + textFound = true; continue; } - if (run.getTextSpan() != null && run.getTextSpan().getText().equals(text)) { - if ((x > run.getWidth() && (!isMultiRunText || run.getStart() == curRunStart)) || textWidthPrevLine > 0) { - getBounds(run.getTextSpan(), textBounds); - x -= (runs[0].getLocation().x - textBounds.getMinX()); - } - for (int j = runs.length - 1; j > i; j--) { - TextRun r = runs[j]; - if (r.getStart() != curRunStart && r.getTextSpan().getText().equals(text) && !r.isLinebreak()) { - ltrIndex += r.getLength(); - } + if (r.getTextSpan() != null && r.getTextSpan().getText().equals(text) + && r.getStart() == curRunStart) { + if (x > r.getWidth() || textWidthPrevLine > 0) { + getBounds(r.getTextSpan(), textBounds); + x -= (run.getLocation().x - textBounds.getMinX()); } break; } - // This condition handles LTR Text nodes embedded in TextFlow in RTL mode. - if (!run.getTextSpan().getText().equals(text) && x > run.getWidth() && run.getStart() < curRunStart) { - x -= run.getWidth(); + /* This condition handles LTR Text nodes present between + a Text node containing both LTR and RTL text. */ + if (!r.getTextSpan().getText().equals(text) && textFound + && x > r.getWidth() && r.getStart() < curRunStart) { + x -= r.getWidth(); + } + } + for (int i = runs.length - 1; i > runIdx; i--) { + TextRun r = runs[i]; + if (r.getStart() != curRunStart && r.getTextSpan().getText().equals(text) + && !r.isLinebreak() && run.getLevel() != r.getLevel()) { + ltrIndex += r.getLength(); } } } else { From 43f1f186a9f9f40b207a60e3fde3ccd3fde8fc5f Mon Sep 17 00:00:00 2001 From: Karthik P K Date: Wed, 7 Feb 2024 16:10:21 +0530 Subject: [PATCH 10/16] Fix emoji issue --- .../src/main/java/com/sun/javafx/text/PrismTextLayout.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java b/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java index 6d459d4329a..a150d9f1355 100644 --- a/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java +++ b/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java @@ -528,8 +528,9 @@ public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRu } for (int i = runs.length - 1; i > runIdx; i--) { TextRun r = runs[i]; + boolean addLtrIdx = run.getTextSpan().getText().length() != run.length; if (r.getStart() != curRunStart && r.getTextSpan().getText().equals(text) - && !r.isLinebreak() && run.getLevel() != r.getLevel()) { + && !r.isLinebreak() && addLtrIdx) { ltrIndex += r.getLength(); } } From 8732047c0d5c85827a34a5725ffc93d696e39f52 Mon Sep 17 00:00:00 2001 From: Karthik P K Date: Fri, 9 Feb 2024 13:24:22 +0530 Subject: [PATCH 11/16] Review comments --- .../com/sun/javafx/text/PrismTextLayout.java | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java b/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java index a150d9f1355..7fc2b42ce4f 100644 --- a/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java +++ b/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java @@ -456,7 +456,7 @@ public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRu runIndex = i; break; } - if (i - 1 >= 0) { + if (i > 0) { if (runs[i - 1].isLinebreak()) { break; } @@ -485,7 +485,8 @@ public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRu // This code branch is used to calculate hit info of Text node embedded in TextFlow. for (int i = 0; i < lineIndex; i++) { for (TextRun r: lines[i].runs) { - if (r.getTextSpan() != null && r.getStart() >= textRunStart && r.getTextSpan().getText().equals(text)) { + if (r.getTextSpan() != null && r.getStart() >= textRunStart + && r.getTextSpan().getText().equals(text)) { textWidthPrevLine += r.getLength(); } } @@ -505,14 +506,14 @@ public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRu boolean textFound = false; for (int i = 0; i <= runIdx; i++) { TextRun r = runs[i]; - if (r.getStart() != curRunStart && r.getTextSpan().getText().equals(text) - && x > r.getWidth() && textWidthPrevLine == 0) { + if (r.getStart() != curRunStart && x > r.getWidth() && textWidthPrevLine == 0 + && r.getTextSpan().getText().equals(text)) { x -= r.getWidth(); textFound = true; continue; } - if (r.getTextSpan() != null && r.getTextSpan().getText().equals(text) - && r.getStart() == curRunStart) { + if (r.getTextSpan() != null && r.getStart() == curRunStart + && r.getTextSpan().getText().equals(text)) { if (x > r.getWidth() || textWidthPrevLine > 0) { getBounds(r.getTextSpan(), textBounds); x -= (run.getLocation().x - textBounds.getMinX()); @@ -521,16 +522,16 @@ public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRu } /* This condition handles LTR Text nodes present between a Text node containing both LTR and RTL text. */ - if (!r.getTextSpan().getText().equals(text) && textFound - && x > r.getWidth() && r.getStart() < curRunStart) { + if (textFound && x > r.getWidth() && r.getStart() < curRunStart + && !r.getTextSpan().getText().equals(text)) { x -= r.getWidth(); } } for (int i = runs.length - 1; i > runIdx; i--) { TextRun r = runs[i]; boolean addLtrIdx = run.getTextSpan().getText().length() != run.length; - if (r.getStart() != curRunStart && r.getTextSpan().getText().equals(text) - && !r.isLinebreak() && addLtrIdx) { + if (r.getStart() != curRunStart && !r.isLinebreak() && addLtrIdx + && r.getTextSpan().getText().equals(text)) { ltrIndex += r.getLength(); } } From 72287851157407fb0a36d6db76004a20db470294 Mon Sep 17 00:00:00 2001 From: Karthik P K Date: Mon, 19 Feb 2024 15:23:14 +0530 Subject: [PATCH 12/16] Code refactoring --- .../com/sun/javafx/scene/text/TextLayout.java | 3 +- .../com/sun/javafx/text/PrismTextLayout.java | 311 +++++++++--------- .../src/main/java/javafx/scene/text/Text.java | 44 +-- .../main/java/javafx/scene/text/TextFlow.java | 2 +- .../com/sun/javafx/pgstub/StubTextLayout.java | 2 +- .../scene/RTLTextCharacterIndexTest.java | 4 +- .../scene/RTLTextFlowCharacterIndexTest.java | 4 +- 7 files changed, 190 insertions(+), 180 deletions(-) diff --git a/modules/javafx.graphics/src/main/java/com/sun/javafx/scene/text/TextLayout.java b/modules/javafx.graphics/src/main/java/com/sun/javafx/scene/text/TextLayout.java index a2f38050c46..e50338a7ca9 100644 --- a/modules/javafx.graphics/src/main/java/com/sun/javafx/scene/text/TextLayout.java +++ b/modules/javafx.graphics/src/main/java/com/sun/javafx/scene/text/TextLayout.java @@ -210,10 +210,9 @@ public Hit(int charIndex, int insertionIndex, boolean leading) { * and non-null in the case of {@link javafx.scene.text.Text} * @param textRunStart Start position of first Text run where hit info is requested. * @param curRunStart Start position of text run where hit info is requested. - * @param forTextFlow Indicates if the hit info is requested for TextFlow or Text node. {@code true} for TextFlow and {@code false} for Text node. * @return returns a {@link Hit} object containing character index, insertion index and position of cursor on the character. */ - public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRunStart, boolean forTextFlow); + public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRunStart); public PathElement[] getCaretShape(int offset, boolean isLeading, float x, float y); diff --git a/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java b/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java index 7fc2b42ce4f..4c1c9bf98ca 100644 --- a/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java +++ b/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java @@ -421,9 +421,10 @@ public PathElement[] getCaretShape(int offset, boolean isLeading, } @Override - public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRunStart, boolean forTextFlow) { + public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRunStart) { boolean leading = false; boolean isMirrored = isMirrored(); // Node orientation is RTL + boolean forTextFlow = spans != null && text == null; int charIndex = -1; int insertionIndex = -1; int relIndex = 0; @@ -432,184 +433,196 @@ public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRu float xHitPos = x; ensureLayout(); - int lineIndex = getLineIndex(y, text, curRunStart); + int lineIndex = getLineIndex(y, forTextFlow, curRunStart); if (lineIndex >= getLineCount()) { charIndex = getCharCount(); insertionIndex = charIndex + 1; - } else { - if (isMirrored && (forTextFlow || spans == null)) { + return new Hit(charIndex, insertionIndex, leading); + } + + TextLine line = lines[lineIndex]; + TextRun[] runs = line.getRuns(); + RectBounds bounds = line.getBounds(); + TextRun run = null; + //TODO binary search + if (forTextFlow || spans == null) { + /* This code branch is used to calculate hit info of Text node + * which are not embedded in TextFlow and hit info requested on TextFlow. */ + if (isMirrored) { x = getMirroringWidth() - x; - } - TextLine line = lines[lineIndex]; - TextRun[] runs = line.getRuns(); - RectBounds bounds = line.getBounds(); - TextRun run = null; - //TODO binary search - if (text == null || spans == null) { - /* This code branch is used to calculate hit info of Text node - which are not embedded in TextFlow and hit info requested on TextFlow. */ - if (isMirrored) { - int runIndex = -1; - for (int i = runs.length - 1; i >= 0; i--) { - run = runs[i]; - if (x < run.getWidth() && (forTextFlow || (run.getStart() == curRunStart))) { - runIndex = i; + int runIndex = -1; + for (int i = runs.length - 1; i >= 0; i--) { + run = runs[i]; + if (x < run.getWidth() && (forTextFlow || (run.getStart() == curRunStart))) { + runIndex = i; + break; + } + if (i > 0) { + if (runs[i - 1].isLinebreak()) { break; } - if (i > 0) { - if (runs[i - 1].isLinebreak()) { - break; - } - x -= run.getWidth(); - } + x -= run.getWidth(); } - for (int i = 0; i < runIndex; i++) { - xHitPos -= runs[i].getWidth(); + } + for (int i = 0; i < runIndex; i++) { + xHitPos -= runs[i].getWidth(); + } + xHitPos -= bounds.getMinX(); + } else { + for (int i = 0; i < runs.length; i++) { + run = runs[i]; + if (x < run.getWidth()) { + break; } - xHitPos -= bounds.getMinX(); - } else { - for (int i = 0; i < runs.length; i++) { - run = runs[i]; - if (x < run.getWidth()) { + if (i + 1 < runs.length) { + if (runs[i + 1].isLinebreak()) { break; } - if (i + 1 < runs.length) { - if (runs[i + 1].isLinebreak()) { - break; - } - x -= run.getWidth(); - } + x -= run.getWidth(); } } - } else { - // This code branch is used to calculate hit info of Text node embedded in TextFlow. - for (int i = 0; i < lineIndex; i++) { - for (TextRun r: lines[i].runs) { - if (r.getTextSpan() != null && r.getStart() >= textRunStart - && r.getTextSpan().getText().equals(text)) { - textWidthPrevLine += r.getLength(); - } + } + } else { + // This code branch is used to calculate hit info of Text node embedded in TextFlow. + textWidthPrevLine = getPrevLineWidth(text, lineIndex, textRunStart); + + BaseBounds textBounds = new BoxBounds(); + if (isMirrored) { + int runIdx = 0; + for (TextRun r: runs) { + if (r.getStart() == curRunStart) { + run = r; + break; } + runIdx++; } - BaseBounds textBounds = new BoxBounds(); - if (isMirrored) { - int runIdx = 0; - for (TextRun r: runs) { - if (r.getStart() == curRunStart) { - run = r; - break; - } - runIdx++; + boolean textFound = false; + for (int i = 0; i <= runIdx; i++) { + TextRun r = runs[i]; + if (r.getStart() != curRunStart && x > r.getWidth() && textWidthPrevLine == 0 + && r.getTextSpan().getText().equals(text)) { + x -= r.getWidth(); + textFound = true; + continue; } - - boolean textFound = false; - for (int i = 0; i <= runIdx; i++) { - TextRun r = runs[i]; - if (r.getStart() != curRunStart && x > r.getWidth() && textWidthPrevLine == 0 - && r.getTextSpan().getText().equals(text)) { - x -= r.getWidth(); - textFound = true; - continue; - } - if (r.getTextSpan() != null && r.getStart() == curRunStart - && r.getTextSpan().getText().equals(text)) { - if (x > r.getWidth() || textWidthPrevLine > 0) { - getBounds(r.getTextSpan(), textBounds); - x -= (run.getLocation().x - textBounds.getMinX()); - } - break; + if (r.getTextSpan() != null && r.getStart() == curRunStart + && r.getTextSpan().getText().equals(text)) { + if (x > r.getWidth() || textWidthPrevLine > 0) { + getBounds(r.getTextSpan(), textBounds); + x -= (run.getLocation().x - textBounds.getMinX()); } + break; + } /* This condition handles LTR Text nodes present between a Text node containing both LTR and RTL text. */ - if (textFound && x > r.getWidth() && r.getStart() < curRunStart - && !r.getTextSpan().getText().equals(text)) { - x -= r.getWidth(); - } + if (textFound && x > r.getWidth() && r.getStart() < curRunStart + && !r.getTextSpan().getText().equals(text)) { + x -= r.getWidth(); } - for (int i = runs.length - 1; i > runIdx; i--) { - TextRun r = runs[i]; - boolean addLtrIdx = run.getTextSpan().getText().length() != run.length; - if (r.getStart() != curRunStart && !r.isLinebreak() && addLtrIdx - && r.getTextSpan().getText().equals(text)) { - ltrIndex += r.getLength(); - } + } + ltrIndex = getLtrTextWidthInRtlText(runs, text, curRunStart, runIdx); + } else { + boolean isPrevRunPresent = false; + int prevRunLength = 0; + for (TextRun r: runs) { + if (!r.getTextSpan().getText().equals(text) || (r.getStart() < textRunStart && r.getTextSpan().getText().equals(text))) { + prevRunLength += r.getWidth(); + continue; } - } else { - boolean isPrevRunPresent = false; - int prevRunLength = 0; - for (TextRun r: runs) { - if (!r.getTextSpan().getText().equals(text) || (r.getStart() < textRunStart && r.getTextSpan().getText().equals(text))) { - prevRunLength += r.getWidth(); - continue; + if (r.getTextSpan() != null && r.getTextSpan().getText().equals(text)) { + getBounds(r.getTextSpan(), textBounds); + if (textBounds.getMinX() == 0 && !isPrevRunPresent) { + x -= prevRunLength; + isPrevRunPresent = true; } - if (r.getTextSpan() != null && r.getTextSpan().getText().equals(text)) { - getBounds(r.getTextSpan(), textBounds); - if (textBounds.getMinX() == 0 && !isPrevRunPresent) { - x -= prevRunLength; - isPrevRunPresent = true; - } - if (x > r.getWidth()) { - x -= r.getWidth(); - relIndex += r.getLength(); - continue; - } - run = r; - break; + if (x > r.getWidth()) { + x -= r.getWidth(); + relIndex += r.getLength(); + continue; } + run = r; + break; } } } + } - if (run != null) { - int[] trailing = new int[1]; - if (text != null && spans != null) { - charIndex = run.getOffsetAtX(x, trailing); - charIndex += textWidthPrevLine; - charIndex += relIndex; - /* When RTL text has LTR text embedded, - * add the LTR index here to get effective character index */ - charIndex += ltrIndex; + if (run != null) { + int[] trailing = new int[1]; + if (forTextFlow || spans == null) { + int indexOffset; + if (isMirrored) { + indexOffset = run.getOffsetAtX(xHitPos, trailing); } else { - int indexOffset; - if (isMirrored) { - indexOffset = run.getOffsetAtX(xHitPos, trailing); + indexOffset = run.getOffsetAtX(x, trailing); + } + charIndex = run.getStart() + indexOffset; + } else { + charIndex = run.getOffsetAtX(x, trailing); + charIndex += textWidthPrevLine; + charIndex += relIndex; + /* When RTL text has LTR text embedded, + * add the LTR index here to get effective character index */ + charIndex += ltrIndex; + } + leading = (trailing[0] == 0); + + insertionIndex = charIndex; + if (insertionIndex < getText().length) { + if (!leading) { + BreakIterator charIterator = BreakIterator.getCharacterInstance(); + if (forTextFlow) { + charIterator.setText(new String(getText())); } else { - indexOffset = run.getOffsetAtX(x, trailing); + charIterator.setText(text); } - charIndex = run.getStart() + indexOffset; - } - leading = (trailing[0] == 0); - - insertionIndex = charIndex; - if (getText() != null && insertionIndex < getText().length) { - if (!leading) { - BreakIterator charIterator = BreakIterator.getCharacterInstance(); - if (text != null) { - charIterator.setText(text); - } else { - charIterator.setText(new String(getText())); - } - int next = charIterator.following(insertionIndex); - if (next == BreakIterator.DONE) { - insertionIndex += 1; - } else { - insertionIndex = next; - } + int next = charIterator.following(insertionIndex); + if (next == BreakIterator.DONE) { + insertionIndex += 1; + } else { + insertionIndex = next; } - } else if (!leading) { - insertionIndex += 1; } - } else { - //empty line, set to line break leading - charIndex = line.getStart(); - leading = true; - insertionIndex = charIndex; + } else if (!leading) { + insertionIndex += 1; } + } else { + //empty line, set to line break leading + charIndex = line.getStart(); + leading = true; + insertionIndex = charIndex; } return new Hit(charIndex, insertionIndex, leading); } + private int getLtrTextWidthInRtlText(TextRun[] runs, String text, int curRunStart, int runIdx) { + TextRun run = runs[runIdx]; + int ltrTextWidth = 0; + for (int i = runs.length - 1; i > runIdx; i--) { + TextRun r = runs[i]; + boolean addLtrIdx = run.getTextSpan().getText().length() != run.length; + if (r.getStart() != curRunStart && !r.isLinebreak() && addLtrIdx + && r.getTextSpan().getText().equals(text)) { + ltrTextWidth += r.getLength(); + } + } + return ltrTextWidth; + } + + private int getPrevLineWidth(String text, int lineIndex, int textRunStart) { + int prevLineWidth = 0; + for (int i = 0; i < lineIndex; i++) { + for (TextRun r: lines[i].getRuns()) { + if (r.getTextSpan() != null && r.getStart() >= textRunStart + && r.getTextSpan().getText().equals(text)) { + prevLineWidth += r.getLength(); + } + } + } + return prevLineWidth; + } + @Override public PathElement[] getRange(int start, int end, int type, float x, float y) { @@ -834,20 +847,18 @@ public boolean setTabSize(int spaces) { * * **************************************************************************/ - private int getLineIndex(float y, String text, int runStart) { + private int getLineIndex(float y, boolean forTextFlow, int runStart) { int index = 0; float bottom = 0; - /* Initializing textFound as true when text is null - * because when this function is called for TextFlow text parameter will be null */ - boolean textFound = (text == null); + /* Initializing textFound as true when this function is called for + * TextFlow or Text node which is not embedded in TextFlow */ + boolean textFound = (forTextFlow || spans == null); int lineCount = getLineCount(); while (index < lineCount) { if (!textFound) { - for (TextRun r : lines[index].runs) { - if (r.getTextSpan() == null || (r.getStart() == runStart && r.getTextSpan().getText().equals(text))) { - /* Span will present only for Rich Text. - * Hence making textFound as true */ + for (TextRun r : lines[index].getRuns()) { + if (r.getStart() == runStart) { textFound = true; break; } diff --git a/modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java b/modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java index a085f43b91b..92e5def7979 100644 --- a/modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java +++ b/modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java @@ -1032,7 +1032,7 @@ public final HitInfo hitTest(Point2D point) { textRunStart = findFirstRunStart(x, y, runs); curRunStart = ((TextRun) runs[runIndex]).getStart(); } - TextLayout.Hit h = layout.getHitInfo((float)x, (float)y, getText(), textRunStart, curRunStart, false); + TextLayout.Hit h = layout.getHitInfo((float)x, (float)y, getTextInternal(), textRunStart, curRunStart); return new HitInfo(h.getCharIndex(), h.getInsertionIndex(), h.isLeading()); } @@ -1047,50 +1047,50 @@ private int findFirstRunStart(double x, double y, GlyphList[] runs) { } private int findRunIndex(double x, double y, GlyphList[] runs) { - int ix = 0; + int runIdx = 0; int lastIndex = runs.length - 1; if (runs.length == 0) { - return ix; + return runIdx; } if (getEffectiveNodeOrientation() == NodeOrientation.RIGHT_TO_LEFT) { - if (runs[ix].getTextSpan() == null) { - while (ix < lastIndex) { - GlyphList r = runs[ix]; - GlyphList nr = runs[ix + 1]; - if ((x > r.getLocation().x && - (x < nr.getLocation().x || r.getLocation().y < nr.getLocation().y)) - && y < r.getHeight()) { + if (runs[runIdx].getTextSpan() == null) { + while (runIdx < lastIndex) { + GlyphList run = runs[runIdx]; + GlyphList nextRun = runs[runIdx + 1]; + if ((x > run.getLocation().x && + (x < nextRun.getLocation().x || run.getLocation().y < nextRun.getLocation().y)) + && y < run.getHeight()) { break; } - ix++; - y = updateY(y, ix, runs); + runIdx++; + y = updateY(y, runIdx, runs); } } else { Point2D ptInParent = localToParent(x, y); double ptX = ptInParent.getX(); double ptY = ptInParent.getY(); - while (ix < lastIndex) { - GlyphList r = runs[ix]; - if (ptX > r.getLocation().x && ptX < (r.getLocation().x + r.getWidth()) && ptY >= r.getLocation().y - && y < r.getHeight()) { + while (runIdx < lastIndex) { + GlyphList run = runs[runIdx]; + if (ptX > run.getLocation().x && ptX < (run.getLocation().x + run.getWidth()) && ptY >= run.getLocation().y + && y < run.getHeight()) { break; } - ix++; - y = updateY(y, ix, runs); + runIdx++; + y = updateY(y, runIdx, runs); } } } else { double ptY = localToParent(x, y).getY(); - while (ix < lastIndex) { - if (ptY > runs[ix].getLocation().y && ptY < runs[ix + 1].getLocation().y) { + while (runIdx < lastIndex) { + if (ptY > runs[runIdx].getLocation().y && ptY < runs[runIdx + 1].getLocation().y) { break; } - ix++; + runIdx++; } } - return ix; + return runIdx; } private double updateY(double y, int ix, GlyphList[] runs) { diff --git a/modules/javafx.graphics/src/main/java/javafx/scene/text/TextFlow.java b/modules/javafx.graphics/src/main/java/javafx/scene/text/TextFlow.java index ab4b5824b13..36a92297d92 100644 --- a/modules/javafx.graphics/src/main/java/javafx/scene/text/TextFlow.java +++ b/modules/javafx.graphics/src/main/java/javafx/scene/text/TextFlow.java @@ -199,7 +199,7 @@ public final HitInfo hitTest(javafx.geometry.Point2D point) { TextLayout layout = getTextLayout(); double x = point.getX(); double y = point.getY(); - TextLayout.Hit h = layout.getHitInfo((float)x, (float)y, null, 0, 0, true); + TextLayout.Hit h = layout.getHitInfo((float)x, (float)y, null, 0, 0); return new HitInfo(h.getCharIndex(), h.getInsertionIndex(), h.isLeading()); } else { return null; diff --git a/modules/javafx.graphics/src/test/java/test/com/sun/javafx/pgstub/StubTextLayout.java b/modules/javafx.graphics/src/test/java/test/com/sun/javafx/pgstub/StubTextLayout.java index 69793bb52ff..e079fa40c04 100644 --- a/modules/javafx.graphics/src/test/java/test/com/sun/javafx/pgstub/StubTextLayout.java +++ b/modules/javafx.graphics/src/test/java/test/com/sun/javafx/pgstub/StubTextLayout.java @@ -158,7 +158,7 @@ public Shape getShape(int type, TextSpan filter) { } @Override - public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRunStart, boolean forTextFlow) { + public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRunStart) { // TODO this probably needs to be entirely rewritten... if (getText() == null) { return new Hit(0, -1, true); diff --git a/tests/system/src/test/java/test/robot/javafx/scene/RTLTextCharacterIndexTest.java b/tests/system/src/test/java/test/robot/javafx/scene/RTLTextCharacterIndexTest.java index c74cd127293..22428c52dc4 100644 --- a/tests/system/src/test/java/test/robot/javafx/scene/RTLTextCharacterIndexTest.java +++ b/tests/system/src/test/java/test/robot/javafx/scene/RTLTextCharacterIndexTest.java @@ -117,8 +117,8 @@ public class RTLTextCharacterIndexTest { static final int WIDTH = 500; static final int HEIGHT = 200; - final int Y_OFFSET = 30; - final int X_LEADING_OFFSET = 10; + static final int Y_OFFSET = 30; + static final int X_LEADING_OFFSET = 10; boolean isLeading; boolean textFlowIsLeading; diff --git a/tests/system/src/test/java/test/robot/javafx/scene/RTLTextFlowCharacterIndexTest.java b/tests/system/src/test/java/test/robot/javafx/scene/RTLTextFlowCharacterIndexTest.java index 91b0377857b..51bb0df668c 100644 --- a/tests/system/src/test/java/test/robot/javafx/scene/RTLTextFlowCharacterIndexTest.java +++ b/tests/system/src/test/java/test/robot/javafx/scene/RTLTextFlowCharacterIndexTest.java @@ -116,8 +116,8 @@ public class RTLTextFlowCharacterIndexTest { static final int WIDTH = 500; static final int HEIGHT = 200; - final int Y_OFFSET = 30; - final int X_LEADING_OFFSET = 10; + static final int Y_OFFSET = 30; + static final int X_LEADING_OFFSET = 10; boolean isLeading; boolean textFlowIsLeading; From f36199fd919dd288b64867e1cce27f7063ae01aa Mon Sep 17 00:00:00 2001 From: Karthik P K Date: Tue, 27 Feb 2024 19:15:25 +0530 Subject: [PATCH 13/16] Fix repeating text node issue --- .../src/main/java/com/sun/javafx/text/PrismTextLayout.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java b/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java index 4c1c9bf98ca..976784b21f4 100644 --- a/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java +++ b/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java @@ -500,8 +500,9 @@ public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRu boolean textFound = false; for (int i = 0; i <= runIdx; i++) { TextRun r = runs[i]; + int textWidth = run.getStart() + run.getTextSpan().getText().length(); if (r.getStart() != curRunStart && x > r.getWidth() && textWidthPrevLine == 0 - && r.getTextSpan().getText().equals(text)) { + && r.getStart() < textWidth && r.getTextSpan().getText().equals(text)) { x -= r.getWidth(); textFound = true; continue; @@ -601,7 +602,8 @@ private int getLtrTextWidthInRtlText(TextRun[] runs, String text, int curRunStar int ltrTextWidth = 0; for (int i = runs.length - 1; i > runIdx; i--) { TextRun r = runs[i]; - boolean addLtrIdx = run.getTextSpan().getText().length() != run.length; + boolean addLtrIdx = run.getTextSpan().getText().length() != run.length + && (r.length + ltrTextWidth) < run.getTextSpan().getText().length(); if (r.getStart() != curRunStart && !r.isLinebreak() && addLtrIdx && r.getTextSpan().getText().equals(text)) { ltrTextWidth += r.getLength(); From 1376441b84e2d4105da0d8f3227e8999a3b9822e Mon Sep 17 00:00:00 2001 From: Karthik P K Date: Wed, 28 Feb 2024 17:41:53 +0530 Subject: [PATCH 14/16] Simplified approach --- .../com/sun/javafx/scene/text/TextLayout.java | 2 +- .../com/sun/javafx/text/PrismTextLayout.java | 244 +++--------------- .../src/main/java/javafx/scene/text/Text.java | 77 +----- .../main/java/javafx/scene/text/TextFlow.java | 2 +- .../com/sun/javafx/pgstub/StubTextLayout.java | 2 +- 5 files changed, 57 insertions(+), 270 deletions(-) diff --git a/modules/javafx.graphics/src/main/java/com/sun/javafx/scene/text/TextLayout.java b/modules/javafx.graphics/src/main/java/com/sun/javafx/scene/text/TextLayout.java index e50338a7ca9..fbc833b8c94 100644 --- a/modules/javafx.graphics/src/main/java/com/sun/javafx/scene/text/TextLayout.java +++ b/modules/javafx.graphics/src/main/java/com/sun/javafx/scene/text/TextLayout.java @@ -212,7 +212,7 @@ public Hit(int charIndex, int insertionIndex, boolean leading) { * @param curRunStart Start position of text run where hit info is requested. * @return returns a {@link Hit} object containing character index, insertion index and position of cursor on the character. */ - public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRunStart); + public Hit getHitInfo(float x, float y); public PathElement[] getCaretShape(int offset, boolean isLeading, float x, float y); diff --git a/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java b/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java index 976784b21f4..3a8a5584cd9 100644 --- a/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java +++ b/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java @@ -421,210 +421,61 @@ public PathElement[] getCaretShape(int offset, boolean isLeading, } @Override - public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRunStart) { - boolean leading = false; - boolean isMirrored = isMirrored(); // Node orientation is RTL - boolean forTextFlow = spans != null && text == null; + public Hit getHitInfo(float x, float y) { int charIndex = -1; int insertionIndex = -1; - int relIndex = 0; - int ltrIndex = 0; - int textWidthPrevLine = 0; - float xHitPos = x; + boolean leading = false; ensureLayout(); - int lineIndex = getLineIndex(y, forTextFlow, curRunStart); + int lineIndex = getLineIndex(y); if (lineIndex >= getLineCount()) { charIndex = getCharCount(); insertionIndex = charIndex + 1; - return new Hit(charIndex, insertionIndex, leading); - } - - TextLine line = lines[lineIndex]; - TextRun[] runs = line.getRuns(); - RectBounds bounds = line.getBounds(); - TextRun run = null; - //TODO binary search - if (forTextFlow || spans == null) { - /* This code branch is used to calculate hit info of Text node - * which are not embedded in TextFlow and hit info requested on TextFlow. */ - if (isMirrored) { - x = getMirroringWidth() - x; - int runIndex = -1; - for (int i = runs.length - 1; i >= 0; i--) { - run = runs[i]; - if (x < run.getWidth() && (forTextFlow || (run.getStart() == curRunStart))) { - runIndex = i; - break; - } - if (i > 0) { - if (runs[i - 1].isLinebreak()) { - break; - } - x -= run.getWidth(); - } - } - for (int i = 0; i < runIndex; i++) { - xHitPos -= runs[i].getWidth(); - } - xHitPos -= bounds.getMinX(); - } else { - for (int i = 0; i < runs.length; i++) { - run = runs[i]; - if (x < run.getWidth()) { - break; - } - if (i + 1 < runs.length) { - if (runs[i + 1].isLinebreak()) { - break; - } - x -= run.getWidth(); - } - } - } } else { - // This code branch is used to calculate hit info of Text node embedded in TextFlow. - textWidthPrevLine = getPrevLineWidth(text, lineIndex, textRunStart); - - BaseBounds textBounds = new BoxBounds(); - if (isMirrored) { - int runIdx = 0; - for (TextRun r: runs) { - if (r.getStart() == curRunStart) { - run = r; - break; - } - runIdx++; - } - - boolean textFound = false; - for (int i = 0; i <= runIdx; i++) { - TextRun r = runs[i]; - int textWidth = run.getStart() + run.getTextSpan().getText().length(); - if (r.getStart() != curRunStart && x > r.getWidth() && textWidthPrevLine == 0 - && r.getStart() < textWidth && r.getTextSpan().getText().equals(text)) { - x -= r.getWidth(); - textFound = true; - continue; - } - if (r.getTextSpan() != null && r.getStart() == curRunStart - && r.getTextSpan().getText().equals(text)) { - if (x > r.getWidth() || textWidthPrevLine > 0) { - getBounds(r.getTextSpan(), textBounds); - x -= (run.getLocation().x - textBounds.getMinX()); - } - break; - } - /* This condition handles LTR Text nodes present between - a Text node containing both LTR and RTL text. */ - if (textFound && x > r.getWidth() && r.getStart() < curRunStart - && !r.getTextSpan().getText().equals(text)) { - x -= r.getWidth(); - } - } - ltrIndex = getLtrTextWidthInRtlText(runs, text, curRunStart, runIdx); - } else { - boolean isPrevRunPresent = false; - int prevRunLength = 0; - for (TextRun r: runs) { - if (!r.getTextSpan().getText().equals(text) || (r.getStart() < textRunStart && r.getTextSpan().getText().equals(text))) { - prevRunLength += r.getWidth(); - continue; - } - if (r.getTextSpan() != null && r.getTextSpan().getText().equals(text)) { - getBounds(r.getTextSpan(), textBounds); - if (textBounds.getMinX() == 0 && !isPrevRunPresent) { - x -= prevRunLength; - isPrevRunPresent = true; - } - if (x > r.getWidth()) { - x -= r.getWidth(); - relIndex += r.getLength(); - continue; - } - run = r; - break; - } - } - } - } - - if (run != null) { - int[] trailing = new int[1]; - if (forTextFlow || spans == null) { - int indexOffset; - if (isMirrored) { - indexOffset = run.getOffsetAtX(xHitPos, trailing); - } else { - indexOffset = run.getOffsetAtX(x, trailing); + TextLine line = lines[lineIndex]; + TextRun[] runs = line.getRuns(); + RectBounds bounds = line.getBounds(); + TextRun run = null; + x -= bounds.getMinX(); + //TODO binary search + for (int i = 0; i < runs.length; i++) { + run = runs[i]; + if (x < run.getWidth()) break; + if (i + 1 < runs.length) { + if (runs[i + 1].isLinebreak()) break; + x -= run.getWidth(); } - charIndex = run.getStart() + indexOffset; - } else { - charIndex = run.getOffsetAtX(x, trailing); - charIndex += textWidthPrevLine; - charIndex += relIndex; - /* When RTL text has LTR text embedded, - * add the LTR index here to get effective character index */ - charIndex += ltrIndex; } - leading = (trailing[0] == 0); - - insertionIndex = charIndex; - if (insertionIndex < getText().length) { - if (!leading) { - BreakIterator charIterator = BreakIterator.getCharacterInstance(); - if (forTextFlow) { + if (run != null) { + int[] trailing = new int[1]; + charIndex = run.getStart() + run.getOffsetAtX(x, trailing); + leading = (trailing[0] == 0); + + insertionIndex = charIndex; + if (getText() != null && insertionIndex < getText().length) { + if (!leading) { + BreakIterator charIterator = BreakIterator.getCharacterInstance(); charIterator.setText(new String(getText())); - } else { - charIterator.setText(text); - } - int next = charIterator.following(insertionIndex); - if (next == BreakIterator.DONE) { - insertionIndex += 1; - } else { - insertionIndex = next; + int next = charIterator.following(insertionIndex); + if (next == BreakIterator.DONE) { + insertionIndex += 1; + } else { + insertionIndex = next; + } } + } else if (!leading) { + insertionIndex += 1; } - } else if (!leading) { - insertionIndex += 1; + } else { + //empty line, set to line break leading + charIndex = line.getStart(); + leading = true; + insertionIndex = charIndex; } - } else { - //empty line, set to line break leading - charIndex = line.getStart(); - leading = true; - insertionIndex = charIndex; } return new Hit(charIndex, insertionIndex, leading); } - private int getLtrTextWidthInRtlText(TextRun[] runs, String text, int curRunStart, int runIdx) { - TextRun run = runs[runIdx]; - int ltrTextWidth = 0; - for (int i = runs.length - 1; i > runIdx; i--) { - TextRun r = runs[i]; - boolean addLtrIdx = run.getTextSpan().getText().length() != run.length - && (r.length + ltrTextWidth) < run.getTextSpan().getText().length(); - if (r.getStart() != curRunStart && !r.isLinebreak() && addLtrIdx - && r.getTextSpan().getText().equals(text)) { - ltrTextWidth += r.getLength(); - } - } - return ltrTextWidth; - } - - private int getPrevLineWidth(String text, int lineIndex, int textRunStart) { - int prevLineWidth = 0; - for (int i = 0; i < lineIndex; i++) { - for (TextRun r: lines[i].getRuns()) { - if (r.getTextSpan() != null && r.getStart() >= textRunStart - && r.getTextSpan().getText().equals(text)) { - prevLineWidth += r.getLength(); - } - } - } - return prevLineWidth; - } - @Override public PathElement[] getRange(int start, int end, int type, float x, float y) { @@ -849,30 +700,15 @@ public boolean setTabSize(int spaces) { * * **************************************************************************/ - private int getLineIndex(float y, boolean forTextFlow, int runStart) { + private int getLineIndex(float y) { int index = 0; float bottom = 0; - /* Initializing textFound as true when this function is called for - * TextFlow or Text node which is not embedded in TextFlow */ - boolean textFound = (forTextFlow || spans == null); int lineCount = getLineCount(); while (index < lineCount) { - if (!textFound) { - for (TextRun r : lines[index].getRuns()) { - if (r.getStart() == runStart) { - textFound = true; - break; - } - } - } bottom += lines[index].getBounds().getHeight() + spacing; - if (index + 1 == lineCount) { - bottom -= lines[index].getLeading(); - } - if (bottom > y && textFound) { - break; - } + if (index + 1 == lineCount) bottom -= lines[index].getLeading(); + if (bottom > y) break; index++; } return index; diff --git a/modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java b/modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java index 92e5def7979..97d7520ce5e 100644 --- a/modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java +++ b/modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java @@ -1021,22 +1021,28 @@ public final BooleanProperty caretBiasProperty() { public final HitInfo hitTest(Point2D point) { if (point == null) return null; TextLayout layout = getTextLayout(); + double x = point.getX() - getX(); double y = point.getY() - getY() + getYRendering(); GlyphList[] runs = getRuns(); - int runIndex = findRunIndex(x, y, runs); - int textRunStart = 0; - int curRunStart = 0; if (runs.length != 0) { - textRunStart = findFirstRunStart(x, y, runs); - curRunStart = ((TextRun) runs[runIndex]).getStart(); + textRunStart = findFirstRunStart(runs); + } + + double px = x; + double py = y; + + if(isSpan()) { + Point2D pPoint = localToParent(point); + px = pPoint.getX(); + py = pPoint.getY(); } - TextLayout.Hit h = layout.getHitInfo((float)x, (float)y, getTextInternal(), textRunStart, curRunStart); - return new HitInfo(h.getCharIndex(), h.getInsertionIndex(), h.isLeading()); + TextLayout.Hit h = layout.getHitInfo((float)px, (float)py); + return new HitInfo(h.getCharIndex() - textRunStart, h.getInsertionIndex() - textRunStart, h.isLeading()); } - private int findFirstRunStart(double x, double y, GlyphList[] runs) { + private int findFirstRunStart(GlyphList[] runs) { int start = Integer.MAX_VALUE; for (GlyphList r: runs) { if (((TextRun) r).getStart() < start) { @@ -1046,61 +1052,6 @@ private int findFirstRunStart(double x, double y, GlyphList[] runs) { return start; } - private int findRunIndex(double x, double y, GlyphList[] runs) { - int runIdx = 0; - int lastIndex = runs.length - 1; - - if (runs.length == 0) { - return runIdx; - } - - if (getEffectiveNodeOrientation() == NodeOrientation.RIGHT_TO_LEFT) { - if (runs[runIdx].getTextSpan() == null) { - while (runIdx < lastIndex) { - GlyphList run = runs[runIdx]; - GlyphList nextRun = runs[runIdx + 1]; - if ((x > run.getLocation().x && - (x < nextRun.getLocation().x || run.getLocation().y < nextRun.getLocation().y)) - && y < run.getHeight()) { - break; - } - runIdx++; - y = updateY(y, runIdx, runs); - } - } else { - Point2D ptInParent = localToParent(x, y); - double ptX = ptInParent.getX(); - double ptY = ptInParent.getY(); - while (runIdx < lastIndex) { - GlyphList run = runs[runIdx]; - if (ptX > run.getLocation().x && ptX < (run.getLocation().x + run.getWidth()) && ptY >= run.getLocation().y - && y < run.getHeight()) { - break; - } - runIdx++; - y = updateY(y, runIdx, runs); - } - } - } else { - double ptY = localToParent(x, y).getY(); - while (runIdx < lastIndex) { - if (ptY > runs[runIdx].getLocation().y && ptY < runs[runIdx + 1].getLocation().y) { - break; - } - runIdx++; - } - } - return runIdx; - } - - private double updateY(double y, int ix, GlyphList[] runs) { - GlyphList curRun = runs[ix]; - if (y > curRun.getHeight() && (curRun.getLocation().y != runs[ix - 1].getLocation().y)) { - y -= curRun.getHeight(); - } - return y; - } - private PathElement[] getRange(int start, int end, int type) { int length = getTextInternal().length(); if (0 <= start && start < end && end <= length) { diff --git a/modules/javafx.graphics/src/main/java/javafx/scene/text/TextFlow.java b/modules/javafx.graphics/src/main/java/javafx/scene/text/TextFlow.java index 36a92297d92..aba73dd0d3c 100644 --- a/modules/javafx.graphics/src/main/java/javafx/scene/text/TextFlow.java +++ b/modules/javafx.graphics/src/main/java/javafx/scene/text/TextFlow.java @@ -199,7 +199,7 @@ public final HitInfo hitTest(javafx.geometry.Point2D point) { TextLayout layout = getTextLayout(); double x = point.getX(); double y = point.getY(); - TextLayout.Hit h = layout.getHitInfo((float)x, (float)y, null, 0, 0); + TextLayout.Hit h = layout.getHitInfo((float)x, (float)y); return new HitInfo(h.getCharIndex(), h.getInsertionIndex(), h.isLeading()); } else { return null; diff --git a/modules/javafx.graphics/src/test/java/test/com/sun/javafx/pgstub/StubTextLayout.java b/modules/javafx.graphics/src/test/java/test/com/sun/javafx/pgstub/StubTextLayout.java index e079fa40c04..4c58daa085d 100644 --- a/modules/javafx.graphics/src/test/java/test/com/sun/javafx/pgstub/StubTextLayout.java +++ b/modules/javafx.graphics/src/test/java/test/com/sun/javafx/pgstub/StubTextLayout.java @@ -158,7 +158,7 @@ public Shape getShape(int type, TextSpan filter) { } @Override - public Hit getHitInfo(float x, float y, String text, int textRunStart, int curRunStart) { + public Hit getHitInfo(float x, float y) { // TODO this probably needs to be entirely rewritten... if (getText() == null) { return new Hit(0, -1, true); From 130587c998c6cee26c6e248e58956571d0a8bfa8 Mon Sep 17 00:00:00 2001 From: Karthik P K Date: Thu, 29 Feb 2024 11:04:40 +0530 Subject: [PATCH 15/16] Review comments --- .../com/sun/javafx/scene/text/TextLayout.java | 5 ----- .../com/sun/javafx/text/PrismTextLayout.java | 17 ++++++++++++----- .../src/main/java/javafx/scene/text/Text.java | 2 +- 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/modules/javafx.graphics/src/main/java/com/sun/javafx/scene/text/TextLayout.java b/modules/javafx.graphics/src/main/java/com/sun/javafx/scene/text/TextLayout.java index fbc833b8c94..a3c6347aa17 100644 --- a/modules/javafx.graphics/src/main/java/com/sun/javafx/scene/text/TextLayout.java +++ b/modules/javafx.graphics/src/main/java/com/sun/javafx/scene/text/TextLayout.java @@ -205,11 +205,6 @@ public Hit(int charIndex, int insertionIndex, boolean leading) { * * @param x x coordinate value. * @param y y coordinate value. - * @param text text for which HitInfo needs to be calculated. - * It is expected to be null in the case of {@link javafx.scene.text.TextFlow} - * and non-null in the case of {@link javafx.scene.text.Text} - * @param textRunStart Start position of first Text run where hit info is requested. - * @param curRunStart Start position of text run where hit info is requested. * @return returns a {@link Hit} object containing character index, insertion index and position of cursor on the character. */ public Hit getHitInfo(float x, float y); diff --git a/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java b/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java index 3a8a5584cd9..7d67816821c 100644 --- a/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java +++ b/modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java @@ -437,12 +437,15 @@ public Hit getHitInfo(float x, float y) { RectBounds bounds = line.getBounds(); TextRun run = null; x -= bounds.getMinX(); - //TODO binary search for (int i = 0; i < runs.length; i++) { run = runs[i]; - if (x < run.getWidth()) break; + if (x < run.getWidth()) { + break; + } if (i + 1 < runs.length) { - if (runs[i + 1].isLinebreak()) break; + if (runs[i + 1].isLinebreak()) { + break; + } x -= run.getWidth(); } } @@ -707,8 +710,12 @@ private int getLineIndex(float y) { int lineCount = getLineCount(); while (index < lineCount) { bottom += lines[index].getBounds().getHeight() + spacing; - if (index + 1 == lineCount) bottom -= lines[index].getLeading(); - if (bottom > y) break; + if (index + 1 == lineCount) { + bottom -= lines[index].getLeading(); + } + if (bottom > y) { + break; + } index++; } return index; diff --git a/modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java b/modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java index 97d7520ce5e..173c89e1c4d 100644 --- a/modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java +++ b/modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java @@ -1033,7 +1033,7 @@ public final HitInfo hitTest(Point2D point) { double px = x; double py = y; - if(isSpan()) { + if (isSpan()) { Point2D pPoint = localToParent(point); px = pPoint.getX(); py = pPoint.getY(); From 7ef09bf5ca4cb493bb30eb94282f39f436b130c0 Mon Sep 17 00:00:00 2001 From: Karthik P K Date: Mon, 4 Mar 2024 16:28:06 +0530 Subject: [PATCH 16/16] Add unit test --- .../com/sun/javafx/scene/text/TextLayout.java | 24 +++ .../src/main/java/javafx/scene/text/Text.java | 16 +- tests/system/src/test/.classpath | 2 +- tests/system/src/test/addExports | 2 + .../com/sun/javafx/text/TextHitInfoTest.java | 191 ++++++++++++++++++ 5 files changed, 225 insertions(+), 10 deletions(-) create mode 100644 tests/system/src/test/java/test/com/sun/javafx/text/TextHitInfoTest.java diff --git a/modules/javafx.graphics/src/main/java/com/sun/javafx/scene/text/TextLayout.java b/modules/javafx.graphics/src/main/java/com/sun/javafx/scene/text/TextLayout.java index a3c6347aa17..74fdaa8e6d1 100644 --- a/modules/javafx.graphics/src/main/java/com/sun/javafx/scene/text/TextLayout.java +++ b/modules/javafx.graphics/src/main/java/com/sun/javafx/scene/text/TextLayout.java @@ -29,6 +29,8 @@ import com.sun.javafx.geom.BaseBounds; import com.sun.javafx.geom.Shape; +import java.util.Objects; + public interface TextLayout { /* Internal flags Flags */ @@ -91,6 +93,28 @@ public Hit(int charIndex, int insertionIndex, boolean leading) { public int getCharIndex() { return charIndex; } public int getInsertionIndex() { return insertionIndex; } public boolean isLeading() { return leading; } + + @Override + public int hashCode() { + return Objects.hash(charIndex, insertionIndex, leading); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) + return true; + if (obj == null) + return false; + if (getClass() != obj.getClass()) + return false; + Hit other = (Hit) obj; + return charIndex == other.charIndex && insertionIndex == other.insertionIndex && leading == other.leading; + } + + @Override + public String toString() { + return "Hit[charIndex=" + charIndex + ", insertionIndex=" + insertionIndex + ", leading=" + leading + "]"; + } } /** diff --git a/modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java b/modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java index 173c89e1c4d..288c01c9afe 100644 --- a/modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java +++ b/modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java @@ -1024,11 +1024,8 @@ public final HitInfo hitTest(Point2D point) { double x = point.getX() - getX(); double y = point.getY() - getY() + getYRendering(); - GlyphList[] runs = getRuns(); - int textRunStart = 0; - if (runs.length != 0) { - textRunStart = findFirstRunStart(runs); - } + + int textRunStart = findFirstRunStart(); double px = x; double py = y; @@ -1042,11 +1039,12 @@ public final HitInfo hitTest(Point2D point) { return new HitInfo(h.getCharIndex() - textRunStart, h.getInsertionIndex() - textRunStart, h.isLeading()); } - private int findFirstRunStart(GlyphList[] runs) { + private int findFirstRunStart() { int start = Integer.MAX_VALUE; - for (GlyphList r: runs) { - if (((TextRun) r).getStart() < start) { - start = ((TextRun) r).getStart(); + for (GlyphList r: getRuns()) { + int runStart = ((TextRun) r).getStart(); + if (runStart < start) { + start = runStart; } } return start; diff --git a/tests/system/src/test/.classpath b/tests/system/src/test/.classpath index 6760dad16e8..2ced868591c 100644 --- a/tests/system/src/test/.classpath +++ b/tests/system/src/test/.classpath @@ -14,7 +14,7 @@ - + diff --git a/tests/system/src/test/addExports b/tests/system/src/test/addExports index a4da975b9c1..8ed570ad6a2 100644 --- a/tests/system/src/test/addExports +++ b/tests/system/src/test/addExports @@ -23,6 +23,8 @@ --add-exports javafx.graphics/com.sun.javafx.image=ALL-UNNAMED --add-exports javafx.graphics/com.sun.javafx.sg.prism=ALL-UNNAMED --add-exports javafx.graphics/com.sun.javafx.scene=ALL-UNNAMED +--add-exports javafx.graphics/com.sun.javafx.scene.text=ALL-UNNAMED +--add-exports javafx.graphics/com.sun.javafx.text=ALL-UNNAMED --add-exports javafx.graphics/com.sun.javafx.tk=ALL-UNNAMED --add-exports javafx.graphics/com.sun.prism.impl=ALL-UNNAMED # diff --git a/tests/system/src/test/java/test/com/sun/javafx/text/TextHitInfoTest.java b/tests/system/src/test/java/test/com/sun/javafx/text/TextHitInfoTest.java new file mode 100644 index 00000000000..6ab2aae3426 --- /dev/null +++ b/tests/system/src/test/java/test/com/sun/javafx/text/TextHitInfoTest.java @@ -0,0 +1,191 @@ +/* + * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +package test.com.sun.javafx.text; + +import static org.junit.Assume.assumeTrue; +import static org.junit.jupiter.api.Assertions.assertEquals; + +import org.junit.jupiter.api.Test; + +import com.sun.javafx.font.PGFont; +import com.sun.javafx.geom.RectBounds; +import com.sun.javafx.scene.text.FontHelper; +import com.sun.javafx.scene.text.TextLayout.Hit; +import com.sun.javafx.scene.text.TextSpan; +import com.sun.javafx.text.PrismTextLayout; + +import javafx.scene.text.Font; + +public class TextHitInfoTest { + private final PrismTextLayout layout = new PrismTextLayout(); + private final PGFont arialFont = (PGFont) FontHelper.getNativeFont(Font.font("Arial", 12)); + + record TestSpan(String text, Object font) implements TextSpan { + @Override + public String getText() { + return text; + } + + @Override + public Object getFont() { + return font; + } + + @Override + public RectBounds getBounds() { + return null; + } + } + + @Test + void getHitInfoTest() { + assumeArialFontAvailable(); + + /* + * Empty line: + */ + + layout.setContent("", arialFont); + + // Checks that hits above the line results in first character: + assertEquals(new Hit(0, 0, true), layout.getHitInfo(0, -30)); + + // Checks before start of line: + assertEquals(new Hit(0, 0, true), layout.getHitInfo(-50, 0)); + + // Checks position of empty string: + assertEquals(new Hit(0, 0, true), layout.getHitInfo(0, 0)); + + // Checks past end of line: + assertEquals(new Hit(0, 0, true), layout.getHitInfo(250, 0)); + + // Checks that hits below the line results in last character + 1: + assertEquals(new Hit(0, 1, false), layout.getHitInfo(0, 30)); + + /* + * Single line: + */ + + layout.setContent("The quick brown fox jumps over the lazy dog", arialFont); + + // Checks that hits above the line results in first character: + assertEquals(new Hit(0, 0, true), layout.getHitInfo(0, -30)); + + // Checks before start of line: + assertEquals(new Hit(0, 0, true), layout.getHitInfo(-50, 0)); + + // Checks positions of a few characters: + assertEquals(new Hit(0, 0, true), layout.getHitInfo(0, 0)); // Start of "T" + assertEquals(new Hit(0, 1, false), layout.getHitInfo(5, 0)); // Past halfway of "T" + assertEquals(new Hit(1, 1, true), layout.getHitInfo(10, 0)); // Start of "h" + + // Checks past end of line: + assertEquals(new Hit(42, 43, false), layout.getHitInfo(250, 0)); + + // Checks that hits below the line results in last character + 1: + assertEquals(new Hit(43, 44, false), layout.getHitInfo(0, 30)); + + /* + * Multi line: + */ + + layout.setContent("The\nquick\nbrown\nfox\n", arialFont); + + // Checks that hits above the first line results in first character: + assertEquals(new Hit(0, 0, true), layout.getHitInfo(0, -30)); + + // Checks before start of first line: + assertEquals(new Hit(0, 0, true), layout.getHitInfo(-50, 0)); + + // Checks positions of a few characters on first line: + assertEquals(new Hit(0, 0, true), layout.getHitInfo(0, 0)); // Start of "T" + assertEquals(new Hit(0, 1, false), layout.getHitInfo(5, 0)); // Halfway past "T" + assertEquals(new Hit(1, 1, true), layout.getHitInfo(10, 0)); // Start of "h" + + // Checks past end of first line: + assertEquals(new Hit(2, 3, false), layout.getHitInfo(250, 0)); + + // Checks before start of second line: + assertEquals(new Hit(4, 4, true), layout.getHitInfo(-50, 15)); + + // Check second line: + assertEquals(new Hit(4, 4, true), layout.getHitInfo(0, 15)); // Start of "q" + + // Checks past end of second line: + assertEquals(new Hit(8, 9, false), layout.getHitInfo(250, 15)); + + /* + * Test with two spans: + */ + + layout.setContent(new TestSpan[] {new TestSpan("Two", arialFont), new TestSpan("Spans", arialFont)}); + + // Checks that hits above the line results in first character: + assertEquals(new Hit(0, 0, true), layout.getHitInfo(0, -30)); + + // Checks before start of line: + assertEquals(new Hit(0, 0, true), layout.getHitInfo(-50, 0)); + + // Checks positions of a few characters: + assertEquals(new Hit(0, 0, true), layout.getHitInfo(0, 0)); // Start of "T" + assertEquals(new Hit(0, 1, false), layout.getHitInfo(5, 0)); // Past halfway of "T" + assertEquals(new Hit(1, 1, true), layout.getHitInfo(10, 0)); // Start of "w" + + assertEquals(new Hit(7, 8, false), layout.getHitInfo(60, 0)); // Past halfway of "s" + + // Checks past end of line: + assertEquals(new Hit(7, 8, false), layout.getHitInfo(250, 0)); + + // Checks that hits below the line results in last character + 1: + assertEquals(new Hit(8, 9, false), layout.getHitInfo(0, 30)); + + /* + * Test with zero spans: + */ + + layout.setContent(new TestSpan[] {}); + + // Checks that hits above the line results in first character: + assertEquals(new Hit(0, 0, true), layout.getHitInfo(0, -30)); + + // Checks before start of line: + assertEquals(new Hit(0, 1, false), layout.getHitInfo(-50, 0)); + + // Checks positions of center: + assertEquals(new Hit(0, 1, false), layout.getHitInfo(0, 0)); // Start of "T" + + // Checks past end of line: + assertEquals(new Hit(0, 1, false), layout.getHitInfo(250, 0)); + + // Checks that hits below the line results in last character + 1: + assertEquals(new Hit(0, 1, false), layout.getHitInfo(0, 30)); + + } + + private void assumeArialFontAvailable() { + assumeTrue("Arial font missing", arialFont.getName().equals("Arial")); + } +}