On Wed, 9 Jul 2025 15:28:08 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> ## 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: >> 1) deprecate the buggy methods (not for removal), adding explanations in >> their javadoc comments >> 2) 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 > > Andy Goryachev has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 72 commits: > > - review comments > - Merge remote-tracking branch 'origin/master' into > 8341438.text.shapes.insets > - Merge branch 'master' into 8341438.text.shapes.insets > - cleanup > - Merge branch 'master' into 8341438.text.shapes.insets > - layout info > - tests > - Merge remote-tracking branch 'origin/ag.text.layout.api' into > 8341438.text.shapes.insets > - cleanup > - Merge remote-tracking branch 'origin/master' into ag.text.layout.api > - ... and 62 more: https://git.openjdk.org/jfx/compare/639a5950...16ed27eb 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. modules/javafx.graphics/src/main/java/javafx/scene/text/TextFlow.java line 202: > 200: * @return a {@code HitInfo} representing the character index found > 201: * @since 9 > 202: * @deprecated replaced by {@code getHitInfo()} I recommend `@link` here. modules/javafx.graphics/src/main/java/javafx/scene/text/TextFlow.java line 245: > 243: * @return an array of {@code PathElement} which can be used to > create a {@code Shape} > 244: * @since 9 > 245: * @deprecated replaced by {@code getCaretShape()} `@link` (here and in. other deprecated methods). ------------- PR Review: https://git.openjdk.org/jfx/pull/1817#pullrequestreview-3002944702 PR Review Comment: https://git.openjdk.org/jfx/pull/1817#discussion_r2195889785 PR Review Comment: https://git.openjdk.org/jfx/pull/1817#discussion_r2195894529