RFR: 8341670: [Text, TextFlow] Public API for Text Layout Info [v7]
Kevin Rushforth
kcr at openjdk.org
Wed Oct 23 19:02:13 UTC 2024
On Mon, 14 Oct 2024 23:31:29 GMT, Andy Goryachev <angorya at openjdk.org> wrote:
>> 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 24
>> */
>> public final LayoutInfo getLayoutInfo()
>>
>>
>> The `LayoutInfo` provides a view into the text layout within `Text`/`TextFlow` nodes such as:
>>
>> - text lines: offsets and bounds
>> - overall layout bounds
>> - text selection geometry
>> - strike-through geometry
>> - underline geometry
>> - caret information
>>
>> This PR also adds the missing `strikeThroughShape()` method to complement 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 24
>> */
>> 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).
>>
>> It is not clear whether this PR should correct the computation at least for the new APIs and keep the existing APIs as is to be fixed later, or if the fix should be applied to both sets of APIs at the same time.
>>
>>
>> ## 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 incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 13 additional commits since the last revision:
>
> - Merge remote-tracking branch 'origin/master' into ag.text.layout.api
> - remove line spacing
> - tests
> - whitespace
> - caret info
> - text line info
> - Merge remote-tracking branch 'origin/master' into ag.text.layout.api
> - convert to wrapper
> - clarify
> - javadoc
> - ... and 3 more: https://git.openjdk.org/jfx/compare/66341640...5ab4f47a
I took a quick initial pass of the API. It looks like what I would expect. I left a couple questions inline. Also, you might want to document whether the newly added methods can return null (maybe in the LayoutInfo class docs, if none of them can return null).
modules/javafx.graphics/src/main/java/javafx/scene/text/CaretInfo.java line 34:
> 32: * @since 24
> 33: */
> 34: public sealed abstract class CaretInfo permits PrismCaretInfo {
Given the API you define, is there a reason not to make this an interface?
modules/javafx.graphics/src/main/java/javafx/scene/text/LayoutInfo.java line 41:
> 39: * @since 24
> 40: */
> 41: public sealed abstract class LayoutInfo permits com.sun.javafx.text.PrismLayoutInfo {
Is there a reason to not make this an interface?
modules/javafx.graphics/src/main/java/javafx/scene/text/LayoutInfo.java line 72:
> 70: * @return the list of {@code TextLineInfo} objects
> 71: */
> 72: public abstract List<TextLineInfo> getTextLines();
Is the list immutable? If so (which I think it should be), can you specify that?
modules/javafx.graphics/src/main/java/javafx/scene/text/LayoutInfo.java line 84:
> 82:
> 83: /**
> 84: * Returns the geometry of the text selection, as an array of {@code Rectangle2D} objects,
"as an array ..." --> "as a list ...".
Is the list immutable?
-------------
PR Review: https://git.openjdk.org/jfx/pull/1596#pullrequestreview-2389771014
PR Review Comment: https://git.openjdk.org/jfx/pull/1596#discussion_r1813357495
PR Review Comment: https://git.openjdk.org/jfx/pull/1596#discussion_r1813361526
PR Review Comment: https://git.openjdk.org/jfx/pull/1596#discussion_r1813363758
PR Review Comment: https://git.openjdk.org/jfx/pull/1596#discussion_r1813369494
More information about the openjfx-dev
mailing list