On Tue, 27 May 2025 15:06:43 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> Please refer to >> >> https://github.com/andy-goryachev-oracle/Test/blob/main/doc/Text/LayoutInfo.md >> >> The RichTextArea control >> ([JDK-8301121](https://bugs.openjdk.org/browse/JDK-8301121)), or any custom >> control that needs non-trivial navigation within complex or wrapped text >> needs a public API to get information about text layout. >> >> This change fixes the missing functionality by adding a new public method to >> the `Text` and `TextFlow` classes.: >> >> >> /** >> * Obtains the snapshot of the current text layout information. >> * @return the layout information >> * @since 25 >> */ >> public final LayoutInfo getLayoutInfo() >> >> >> The `LayoutInfo` provides a view into the text layout within >> `Text`/`TextFlow` nodes such as: >> >> - caret information >> - text lines: offsets and bounds >> - overall layout bounds >> - text selection geometry >> - strike-through geometry >> - underline geometry >> >> >> >> >> ## WARNINGS >> >> Presently, information obtained via certain Text/TextField methods is >> incorrect with non-zero padding and borders, see >> [JDK-8341438](https://bugs.openjdk.org/browse/JDK-8341438). >> >> This PR provides correct answers in the new API, leaving the behavior of the >> existing methods unchanged (there is a compatibility risk associated with >> trying to fix [JDK-8341438](https://bugs.openjdk.org/browse/JDK-8341438) ). >> >> Also, the RTL support is out of scope for this PR, due to multiple >> pre-existing conditions, see https://bugs.openjdk.org/browse/JDK-8343557 >> >> >> ## Testing >> >> The new APIs can be visually tested using the Monkey Tester on this branch: >> https://github.com/andy-goryachev-oracle/MonkeyTest/tree/text.layout.api >> >> in the Text and TextFlow pages: >>  >> >> Two very basic headful tests have been added. >> >> >> ## See Also >> >> https://github.com/FXMisc/RichTextFX/pull/1246 > > Andy Goryachev has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 61 commits: > > - cleanup > - Merge remote-tracking branch 'origin/master' into ag.text.layout.api > - Merge remote-tracking branch 'origin/master' into ag.text.layout.api > - cleanup > - removed getStrikeThroughShape > - caret geometry > - review comments > - Merge remote-tracking branch 'origin/master' into ag.text.layout.api > - review comments > - Merge remote-tracking branch 'origin/master' into ag.text.layout.api > - ... and 51 more: https://git.openjdk.org/jfx/compare/9950d33c...de1016be The API looks good given the removal of the Text/TextFlow getStrikeTrhoughShape methods. I added one question about whether you want to consider adding a getter to CaretInfo to return the list of rectangles in a single call, but I'll leave that up to you. I pointed out what I think are a couple doc typos in TextLine info. I think this is close to being ready. I'll review the implementation and do some testing. @prrace Can you also review the API changes? modules/javafx.graphics/src/main/java/javafx/scene/text/CaretInfo.java line 52: > 50: * @return the number of segments representing the caret > 51: */ > 52: public abstract int getSegmentCount(); Do you think it's worth adding a `List<Rectangle2D>getSegments()` method in addition to (or instead of) `getSegmentCount` and `getSegmentAt`? modules/javafx.graphics/src/main/java/javafx/scene/text/TextLineInfo.java line 42: > 40: * <li> > 41: * {@code minY} - the ascent of the line (negative). > 42: * The ascent of the line is the max ascent of all fonts in the line. Shouldn't that be "glyphs" not "fonts"? modules/javafx.graphics/src/main/java/javafx/scene/text/TextLineInfo.java line 45: > 43: * <li> > 44: * {@code width} - the width of the line. > 45: * The width for the line is sum of all the run widths in the line, > it is not minor: "for the line" --> "of the line" modules/javafx.graphics/src/main/java/javafx/scene/text/TextLineInfo.java line 51: > 49: * {@code height} - the height of the line. > 50: * The height of the line is sum of the max ascent, max descent, and > 51: * max line gap of all the fonts in the line. Shouldn't that be "glyphs" not "fonts"? ------------- PR Review: https://git.openjdk.org/jfx/pull/1596#pullrequestreview-2902336289 PR Comment: https://git.openjdk.org/jfx/pull/1596#issuecomment-2946299467 PR Review Comment: https://git.openjdk.org/jfx/pull/1596#discussion_r2130374964 PR Review Comment: https://git.openjdk.org/jfx/pull/1596#discussion_r2130362230 PR Review Comment: https://git.openjdk.org/jfx/pull/1596#discussion_r2130367374 PR Review Comment: https://git.openjdk.org/jfx/pull/1596#discussion_r2130365196
