8357594: Additional geometry-based Text/TextFlow APIs #1817

andy-goryachev-oracle · 2025-05-27T18:48:45Z

Summary

This change adds new methods to the TextFlow which work correctly in the presence of non-empty insets (borders/padding). For backward compatibility, the old buggy methods are getting deprecated (not for removal). Also, adds new methods in Text which provide missing functionality.

Problem

A number of methods in TextFlow fail to return correct values in the presence of non-empty insets (i.e. when either padding or border are set):

caretShape
hitTest
rangeShape

Additionally, the current API fail to provide strike-through shape, and account for line spacing in the range shape, a problem shared between the TextFlow and the Text classes.

Solution

The solution is two-fold:

deprecate the buggy methods (not for removal), adding explanations in their javadoc comments
add new methods that behave correctly in the presence of non-empty insets and/or implementing the missing functionality.

The proposed solution retains the buggy methods for the purposes of backward compatibility in applications which employ the workarounds, while providing new APIs with additional parameters similar to those offered by the new TextLayout API https://bugs.openjdk.org/browse/JDK-8341670

Testing

Additional visualization of the data returned by the new APIs is available in the Monkey Tester using the following branch (in the Text and TextFlow pages):

https://github.com/andy-goryachev-oracle/MonkeyTest/tree/text.insets.corrected

Progress

Change must not contain extraneous whitespace
Change requires CSR request JDK-8358000 to be approved
Commit message must refer to an issue
Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Issues

JDK-8357594: Additional geometry-based Text/TextFlow APIs (Enhancement - P4)
JDK-8358000: Additional geometry-based Text/TextFlow APIs (CSR)

Reviewers

Kevin Rushforth (@kevinrushforth - Reviewer)
Michael Strauß (@mstr2 - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/1817/head:pull/1817
$ git checkout pull/1817

Update a local copy of the PR:
$ git checkout pull/1817
$ git pull https://git.openjdk.org/jfx.git pull/1817/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1817

View PR using the GUI difftool:
$ git pr show -t 1817

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/1817.diff

Using Webrev

Link to Webrev Comment

andy-goryachev-oracle · 2025-07-07T20:56:56Z

Have alternatives been discussed?

Yes, internally. This PR has been out for quite a while, too.

The priority here is not to break the existing applications that might have coded a workaround. We offer a new set of (better named) APIs withe the right behavior, while deprecating (not for removal) the existing ones.

Adding a system property seems a worse choice since it will break the apps and force the change.

mstr2 · 2025-07-07T21:11:13Z

Have alternatives been discussed?

Yes, internally. This PR has been out for quite a while, too.

Okay... the OpenJFX project states in CONTRIBUTING.md, section "New features / API additions":
Discuss the proposed feature on the openjfx-dev mailing list.

I'm not sure how "internal" discussions qualify. Oracle is not the only stakeholder in this project.

The priority here is not to break the existing applications that might have coded a workaround.

My question was whether this is an educated guess, or whether this problem is corroborated by data.

andy-goryachev-oracle · 2025-07-07T21:24:54Z

Point taken. Posted to the mailing list.

My question was whether this is an educated guess

It's an educated guess.

kevinrushforth

I left a couple doc comments inline. I'm left wondering if we should do the same thing in Text that you are doing in TextFlow. Namely, deprecate the following four methods, adding getXxxxx methods as replacements:

hitTest
caretShape
rangeShape
underlineShape

Unlike TextFlow, there is no functional reason we need to do it. So the question is one of consistency. I note that you are already adding one of the four, getRangeShape (to add the lineSpacing parameter). What do you think?

kevinrushforth · 2025-07-08T23:17:20Z

modules/javafx.graphics/src/main/java/javafx/scene/text/TextFlow.java

    /**
     * Maps local point to {@link HitInfo} in the content.
+     * <p>
+     * NOTE: this method may return incorrect value with non-empty border or padding.


You should be more explicit, and say that it will not take border or padding into account.

kevinrushforth · 2025-07-08T23:30:06Z

modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java

+     * <p>
+     * NOTE: this shapes returned do not include line spacing.


I would remove the "NOTE:", making it a simple statement (and probably no need for the <p> tag), since this is just what the method does.

Question: should this method be deprecated to be consistent with TextFlow?

….insets

andy-goryachev-oracle · 2025-07-09T15:24:50Z

Question: should this method be deprecated to be consistent with TextFlow?

Namely, deprecate the following four methods, adding getXxxxx methods as replacements

I am open to suggestions, but currently leaning toward leaving Text as is. As you correctly mentioned, it is not functionally required, and would only add more noise.

I feel making Text such a heavy object (with properties and all) might have been a design mistake: it would have been much better to use TextFlow everywhere, including Labels, TextFields, and TextAreas, and use Text as immutable objects similar to GlyphList to store runs of characters with the same font and attributes. Doing so might have allowed easy introduction of rich text into these controls too, but oh well.

kevinrushforth

The API and docs look good to me now. I left a minor formatting suggestion for the deprecated methods (use @link to point to the replacement methods).

Update the CSR to match, and I'll Review it.

kevinrushforth · 2025-07-09T20:12:37Z

modules/javafx.graphics/src/main/java/javafx/scene/text/TextFlow.java

     * @param point the specified point to be tested
     * @return a {@code HitInfo} representing the character index found
     * @since 9
+     * @deprecated replaced by {@code getHitInfo()}


I recommend @link here.

kevinrushforth · 2025-07-09T20:15:05Z

modules/javafx.graphics/src/main/java/javafx/scene/text/TextFlow.java

     * @param leading whether the caret is biased on the leading edge of the character
     * @return an array of {@code PathElement} which can be used to create a {@code Shape}
     * @since 9
+     * @deprecated replaced by {@code getCaretShape()}


@link (here and in. other deprecated methods).

andy-goryachev-oracle · 2025-07-09T22:26:06Z

/issue remove 8357594

openjdk · 2025-07-09T22:26:51Z

@andy-goryachev-oracle
Removing additional issue from issue list: 8357594.

andy-goryachev-oracle · 2025-07-09T22:43:11Z

Made https://bugs.openjdk.org/browse/JDK-8357594 to be the main issue, closed https://bugs.openjdk.org/browse/JDK-8341438 as a duplicate of JDK-8357594 .

kevinrushforth

LGTM

kevinrushforth · 2025-07-11T14:39:14Z

modules/javafx.graphics/src/main/java/javafx/scene/text/TextFlow.java

+    private PathElement[] getRange(int start, int end, int type, boolean accountForInsets, double lineSpacing) {
+        double dx;
+        double dy;
+        if(accountForInsets) {


Minor: space after if

andy-goryachev-oracle · 2025-07-11T22:30:15Z

/integrate

openjdk · 2025-07-11T22:31:04Z

Going to push as commit 04c5e40.
Since your change was applied there have been 2 commits pushed to the master branch:

203c049: 8357714: AudioClip.play crash on macOS when loading resource from jar
e0f8e72: 8345348: CSS media feature queries

Your commit was automatically rebased without conflicts.

openjdk · 2025-07-11T22:31:11Z

@andy-goryachev-oracle Pushed as commit 04c5e40.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

andy-goryachev-oracle added 30 commits October 8, 2024 08:59

draft api

285db28

bounds

43ac0e1

review comments

21dbe6c

javadoc

04c4abb

clarify

6869c76

convert to wrapper

2009a7e

Merge remote-tracking branch 'origin/master' into ag.text.layout.api

6b7b079

text line info

c2e26d5

caret info

88e9ee1

whitespace

17dcdd4

tests

fd84f30

remove line spacing

466bba7

Merge remote-tracking branch 'origin/master' into ag.text.layout.api

5ab4f47

Merge remote-tracking branch 'origin/master' into ag.text.layout.api

eb99008

review comments

6776d97

Merge remote-tracking branch 'origin/master' into ag.text.layout.api

9b1f99a

include line spacing

4f4e659

Merge remote-tracking branch 'origin/master' into ag.text.layout.api

dd34810

Merge remote-tracking branch 'origin/master' into ag.text.layout.api

a716e57

caret and range

75b171f

for text

029bbe6

Merge remote-tracking branch 'origin/master' into ag.text.layout.api

3925c02

Merge remote-tracking branch 'origin/master' into ag.text.layout.api

e139484

Merge remote-tracking branch 'origin/master' into ag.text.layout.api

8a5b904

Merge remote-tracking branch 'origin/master' into ag.text.layout.api

a0e59fb

line spacing

3c2976b

shorter array

5076f36

text flow test

4479f0b

text layout test

c05c790

note

7c86978

kevinrushforth reviewed Jul 8, 2025

View reviewed changes

Merge remote-tracking branch 'origin/master' into 8341438.text.shapes…

20c8190

….insets

review comments

16ed27e

kevinrushforth reviewed Jul 9, 2025

View reviewed changes

andy-goryachev-oracle added 2 commits July 9, 2025 13:31

link

d466ebc

javadoc

a2099f0

andy-goryachev-oracle changed the title ~~8341438: TextFlow: incorrect caretShape(), hitTest(), rangeShape() with non-empty padding/border~~ 8357594: Additional geometry-based Text/TextFlow APIs Jul 9, 2025

openjdk bot removed the csr Need approved CSR to integrate pull request label Jul 10, 2025

kevinrushforth self-requested a review July 11, 2025 14:24

kevinrushforth approved these changes Jul 11, 2025

View reviewed changes

andy-goryachev-oracle added 2 commits July 11, 2025 08:06

Merge branch 'master' into 8341438.text.shapes.insets

6ca8ab7

if

a283479

kevinrushforth approved these changes Jul 11, 2025

View reviewed changes

mstr2 approved these changes Jul 11, 2025

View reviewed changes

openjdk bot added the ready Ready to be integrated label Jul 11, 2025

openjdk bot added the integrated Pull request has been integrated label Jul 11, 2025

openjdk bot closed this Jul 11, 2025

openjdk bot removed ready Ready to be integrated rfr Ready for review labels Jul 11, 2025

andy-goryachev-oracle deleted the 8341438.text.shapes.insets branch July 11, 2025 22:33

		* <p>
		* NOTE: this shapes returned do not include line spacing.

8357594: Additional geometry-based Text/TextFlow APIs #1817

8357594: Additional geometry-based Text/TextFlow APIs #1817

Uh oh!

Conversation

andy-goryachev-oracle commented May 27, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Problem

Solution

Testing

Progress

Issues

Reviewers

Reviewing

Uh oh!

andy-goryachev-oracle commented Jul 7, 2025

Uh oh!

mstr2 commented Jul 7, 2025

Uh oh!

andy-goryachev-oracle commented Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kevinrushforth left a comment

Choose a reason for hiding this comment

Uh oh!

kevinrushforth Jul 8, 2025

Choose a reason for hiding this comment

Uh oh!

kevinrushforth Jul 8, 2025

Choose a reason for hiding this comment

Uh oh!

andy-goryachev-oracle commented Jul 9, 2025

Uh oh!

kevinrushforth left a comment

Choose a reason for hiding this comment

Uh oh!

kevinrushforth Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

kevinrushforth Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

andy-goryachev-oracle commented Jul 9, 2025

Uh oh!

openjdk bot commented Jul 9, 2025

Uh oh!

andy-goryachev-oracle commented Jul 9, 2025

Uh oh!

kevinrushforth left a comment

Choose a reason for hiding this comment

Uh oh!

kevinrushforth Jul 11, 2025

Choose a reason for hiding this comment

Uh oh!

andy-goryachev-oracle commented Jul 11, 2025

Uh oh!

openjdk bot commented Jul 11, 2025

Uh oh!

openjdk bot commented Jul 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

andy-goryachev-oracle commented May 27, 2025 •

edited by openjdk bot

Loading

andy-goryachev-oracle commented Jul 7, 2025 •

edited

Loading