RFR: 8319844 : Text/TextFlow.hitTest() is incorrect in RTL orientation [v13]
John Hendrikx
jhendrikx at openjdk.org
Mon Feb 26 14:02:05 UTC 2024
On Wed, 21 Feb 2024 10:01:37 GMT, Karthik P K <kpk at openjdk.org> wrote:
>> In the `getHitInfo()` method of PrismTextLayout, RTL node orientation conditions were not considered, hence hit test values such as character index and insertion index values were incorrect.
>>
>> Added checks for RTL orientation of nodes and fixed the issue in `getHitInfo()` to calculate correct hit test values.
>>
>> Added system tests to validate the changes.
>
> Karthik P K has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 13 additional commits since the last revision:
>
> - Merge branch 'master' into rtl_hittest_issue
> - Code refactoring
> - Review comments
> - Fix emoji issue
> - Inline node issue fix
> - Code review changes
> - Fix issue with multiline text
> - Fix issue with RTL text within LTR text
> - Review changes
> - Fix multiline text insertion index calculation issue
> - ... and 3 more: https://git.openjdk.org/jfx/compare/579ce0f7...e3812732
Okay, I've been looking into this myself. First, I've limited myself to the simple case of LTR text with standard English text. I used 5 Text Nodes containing `The quick brown fox jumped over the lazy dog`. Using the "old" `getHitInfo(float, float)` code from JavaFX 21. This code does not use the extra 3 parameters it is given. This is what I got initially:
In the above image, there are two lines drawn below each character. The first line is the `character index % 6` returned by `TextFlow` for each possible `x` position. It's correct everywhere and returns the correct character offset. The second line is the `character index` returned for the `Text` at the same location. There are two things standing out here:
- The `Text` character index is based on the first line, even for each subsequent line. The character widths therefore don't match properly.
- The `Text` character index does not reset to dark blue for the first character (The `T` of the second `The` is colored Orange, while if the character index resets it should have been Dark Blue).
The reason this is all incorrect is because the `TextLayout` used by the `Text` is the one from its parent. So it must pass coordinates in the parents coordinate system. If fix that, I get this:
Note that one of the two problems has disappeared. The returned character index is still incorrect (The `T` of the second `The` is colored Orange, while if the character index resets it should have been Dark Blue), but the widths now match perfectly.
So, I now also fix the character index by subtracting `textRunStart` from the returned `TextLayout.Hit`, and now it looks like this:
Note that now the character index for each `T` of `The` starts as Dark Blue.
So, I used the `getHitInfo` code from JavaFX 21 + this code in `Text`:
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(runs);
curRunStart = ((TextRun) runs[runIndex]).getStart();
}
double px = x;
double py = y;
if(isSpan()) {
Point2D ppoint = localToParent(point);
px = ppoint.getX();
py = ppoint.getY();
}
TextLayout.Hit h = layout.getHitInfo((float)px, (float)py, getTextInternal(), textRunStart, curRunStart);
return new HitInfo(h.getCharIndex() - textRunStart, h.getInsertionIndex(), h.isLeading());
}
>From this "base" code I think the next step is to look how to fix mirrored text.
Here's the test program I used:
import javafx.animation.Animation;
import javafx.animation.KeyFrame;
import javafx.animation.Timeline;
import javafx.application.Application;
import javafx.application.Platform;
import javafx.geometry.Point2D;
import javafx.geometry.Point3D;
import javafx.scene.Node;
import javafx.scene.Scene;
import javafx.scene.canvas.Canvas;
import javafx.scene.canvas.GraphicsContext;
import javafx.scene.control.Label;
import javafx.scene.input.MouseEvent;
import javafx.scene.input.PickResult;
import javafx.scene.layout.StackPane;
import javafx.scene.layout.VBox;
import javafx.scene.paint.Color;
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.util.Duration;
public class TextFlowRTLSample {
public static void main(String[] args) {
Application.launch(App.class);
}
private static final Color[] COLORS = new Color[] {Color.DARKBLUE, Color.YELLOW, Color.ORANGE, Color.GREEN, Color.BLUEVIOLET, Color.RED};
public static class App extends Application {
TextFlow control = new TextFlow();
Label hitInfo2 = new Label("Empty\nEmpty");
Label pickResult = new Label("Empty");
@Override
public void start(Stage primaryStage) throws Exception {
Node[] ts = createRTLText();
Canvas canvas = new Canvas(850, 500);
control.getChildren().addAll(ts);
control.addEventHandler(MouseEvent.ANY, this::handleMouseEvent);
VBox root = new VBox(hitInfo2, pickResult, new StackPane(canvas, control));
Scene scene = new Scene(root, 850, 600);
// scene.setNodeOrientation(NodeOrientation.RIGHT_TO_LEFT);
primaryStage.setScene(scene);
primaryStage.show();
Timeline timeline = new Timeline(
new KeyFrame(Duration.seconds(0.1), e -> {
canvas.getGraphicsContext2D().clearRect(0, 0, 1000, 1000);
for(int y = 50; y < 600; y += 64) {
for(int x = 0; x < 850; x++) {
HitInfo hitTest = control.hitTest(new Point2D(x, y));
GraphicsContext g2d = canvas.getGraphicsContext2D();
g2d.setStroke(COLORS[hitTest.getCharIndex() % COLORS.length]);
g2d.strokeLine(x, y, x, y + 5);
for (Node text : control.getChildren()) {
Point2D local = text.parentToLocal(x, y);
if (text.contains(local) && text instanceof Text t) {
hitTest = t.hitTest(local);
g2d.setStroke(COLORS[hitTest.getCharIndex() % COLORS.length]);
g2d.strokeLine(x, y + 8, x, y + 13);
}
}
}
}
})
);
timeline.setCycleCount(Animation.INDEFINITE);
timeline.play();
}
protected static Text t(String text) {
Text t = new Text(text);
t.setFont(new Font(48));
return t;
}
protected Node[] createRTLText() {
return new Node[] {
t("The quick brown fox jumped over the lazy dog"),
t("The quick brown fox jumped over the lazy dog"),
t("The quick brown fox jumped over the lazy dog"),
t("The quick brown fox jumped over the lazy dog"),
t("The quick brown fox jumped over the lazy dog"),
// t("شسيبلاتنمح"),
};
}
protected void handleMouseEvent(MouseEvent ev) {
PickResult pick = ev.getPickResult();
Node n = pick.getIntersectedNode();
hitInfo2.setText("-\n-");
if (n == null) {
pickResult.setText("null");
} else {
Point3D p3 = pick.getIntersectedPoint();
Point2D p = new Point2D(p3.getX(), p3.getY());
pickResult.setText(n.getClass().getSimpleName() + "." + n.hashCode());
if (n instanceof Text t) {
HitInfo h = t.getParent() instanceof TextFlow tf ? tf.hitTest(t.localToParent(p)) : t.hitTest(p);
hitInfo2.setText("TextFlow: " + h + "\nText: " + t.hitTest(p));
}
else {
hitInfo2.setText("TextFlow: " + control.hitTest(control.sceneToLocal(new Point2D(ev.getSceneX(), ev.getSceneY()))) + "\n-");
}
}
}
}
}
-------------
PR Comment: https://git.openjdk.org/jfx/pull/1323#issuecomment-1964213756
More information about the openjfx-dev
mailing list