Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8319844 : Text/TextFlow.hitTest() is incorrect in RTL orientation #1323

Closed
wants to merge 17 commits into from
+1,100 −98
Closed

8319844 : Text/TextFlow.hitTest() is incorrect in RTL orientation #1323

Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
8d42bd3
Fix rtl text hittest issue
karthikpandelu Jan 9, 2024
b23e491
Code review changes
karthikpandelu Jan 11, 2024
cd270e1
Code review changes
karthikpandelu Jan 16, 2024
87255aa
Fix multiline text insertion index calculation issue
karthikpandelu Jan 17, 2024
b08de73
Review changes
karthikpandelu Jan 18, 2024
a58f754
Fix issue with RTL text within LTR text
karthikpandelu Jan 29, 2024
52ee61c
Fix issue with multiline text
karthikpandelu Jan 31, 2024
3012fae
Code review changes
karthikpandelu Feb 1, 2024
cc89f12
Inline node issue fix
karthikpandelu Feb 6, 2024
43f1f18
Fix emoji issue
karthikpandelu Feb 7, 2024
8732047
Review comments
karthikpandelu Feb 9, 2024
7228785
Code refactoring
karthikpandelu Feb 19, 2024
e381273
Merge branch 'master' into rtl_hittest_issue
karthikpandelu Feb 21, 2024
f36199f
Fix repeating text node issue
karthikpandelu Feb 27, 2024
1376441
Simplified approach
karthikpandelu Feb 28, 2024
130587c
Review comments
karthikpandelu Feb 29, 2024
7ef09bf
Add unit test
karthikpandelu Mar 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 4 additions & 6 deletions modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java
View file
Original file line number Diff line number Diff line change
@@ -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());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: we are still in the for loop, so perhaps it makes sense to extract
(runs[0].getLocation().x - textBounds.getMinX());
to a variable outside of the loop

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is that outside the loop we don't know if we need to subtract the textBound min x value and the starting location of the first run or not. That is why this is present inside the loop. Once this is done we are breaking out of the loop so this will not get called multiple times.
Let me know if you have any suggestions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are right: I missed the break in line 520

}
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();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

runs[j] is repeated 4 times... should it be a local variable?

}
}