On Fri, 4 Apr 2025 14:42:20 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
>> 
>> 
>> This PR also adds the missing `strikeThroughShape()` method to complement 
>> the existing `underlineShape()` and `rangeShape()` for consistency sake:
>> 
>> 
>>     /**
>>      * Returns the shape for the strike-through in local coordinates.
>>      *
>>      * @param start the beginning character index for the range
>>      * @param end the end character index (non-inclusive) for the range
>>      * @return an array of {@code PathElement} which can be used to create a 
>> {@code Shape}
>>      * @since 25
>>      */
>>     public final PathElement[] strikeThroughShape(int start, int end)
>> 
>> 
>> 
>> 
>> ## WARNING
>> 
>> 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) ).
>> 
>> 
>> 
>> ## 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:
>> ![Screenshot 2024-11-04 at 11 38 
>> 21](https://github.com/user-attachments/assets/2e17e55c-f819-4742-8a42-b9af2b6bac72)
>> 
>> 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 48 commits:
> 
>  - Merge remote-tracking branch 'origin/master' into ag.text.layout.api
>  - Merge remote-tracking branch 'origin/master' into ag.text.layout.api
>  - twice
>  - tests
>  - review comments
>  - Merge remote-tracking branch 'origin/master' into ag.text.layout.api
>  - Merge remote-tracking branch 'origin/master' into ag.text.layout.api
>  - review comments
>  - Merge remote-tracking branch 'origin/master' into ag.text.layout.api
>  - 25 25
>  - ... and 38 more: https://git.openjdk.org/jfx/compare/76282bcf...33cf88d8

modules/javafx.graphics/src/main/java/javafx/scene/text/CaretInfo.java line 62:

> 60:      *     {@code (index < 0 || index > getSegmentCount())}
> 61:      */
> 62:     public abstract Rectangle2D getSegmentAt(int index);

The naming style in `CaretInfo`, `LayoutInfo`, and `TextLineInfo` is a bit 
inconsistent in that it uses both `getFoo()` as well as `foo()` (both with and 
without method arguments). I think you should decide on one way or the other.

modules/javafx.graphics/src/main/java/javafx/scene/text/TextLineInfo.java line 
36:

> 34:  * @param bounds the bounds of the text line, in local coordinates:
> 35:  * <ul>
> 36:  * <li>

What do you think about indenting the `<ul>` (so that it's clearer that it 
belongs to the `@param` tag), as well as dropping the closing `</li>` and 
indent the `<li>` elements to make them stand out better:


@param bounds the bounds of the text line, in local coordinates:
       <ul>
           <li>{@code minX} - the x origin of the line (relative to the layout).
               The x origin is defined by TextAlignment of the text layout, 
always zero
               for left-aligned text.
           <li>{@code minY} - the ascent of the line (negative).
               The ascent of the line is the max ascent of all fonts in the 
line.
           ...



I find this formatting to be more readable in source code form.

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/1596#discussion_r2029802324
PR Review Comment: https://git.openjdk.org/jfx/pull/1596#discussion_r2029801455

Reply via email to