RFR: 8342565: [TestBug] StubTextLayout [v6]

[Marius Hanl]

On Wed, 5 Mar 2025 15:54:49 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>> Changed the StubTextLayout to use product PrismTextLayout with much simplified glyph layout (via stubbed fonts).  The new layout assumes all the glyphs are squares of font size, while the bold type face produces wider glyphs (by 1 pixel).  The default font size has changed from 10 to 12 to make it closer to win/linux.
>> 
>> This brings the test environment closer to the product configuration and expands the capabilities of our headless testing pipeline, which will be useful for upcoming behavior tests.
>> 
>> Existing tests have been adjusted/reworked mainly due to default font size change.
>
> 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 24 additional commits since the last revision:
> 
>  - Merge remote-tracking branch 'origin/master' into 8342565.stub.text.layout
>  - Merge remote-tracking branch 'origin/master' into 8342565.stub.text.layout
>  - Merge remote-tracking branch 'origin/master' into 8342565.stub.text.layout
>  - review comments
>  - Merge remote-tracking branch 'origin/master' into 8342565.stub.text.layout
>  - atomic boolean
>  - Merge branch 'master' into 8342565.stub.text.layout
>  - cleanup
>  - better test
>  - cleanup
>  - ... and 14 more: https://git.openjdk.org/jfx/compare/302a3dce...3650989a

Code looks good to me, some minor convention related things from my side.

modules/javafx.controls/src/test/addExports line 38:

> 36: --add-exports=javafx.graphics/com.sun.javafx.menu=ALL-UNNAMED
> 37: --add-exports=javafx.graphics/com.sun.javafx.text=ALL-UNNAMED
> 38: 

minor: extra added newline here

modules/javafx.graphics/src/main/java/com/sun/javafx/text/GlyphLayoutManager.java line 43:

> 41: public class GlyphLayoutManager {
> 42:     private static GlyphLayout reusableGL = newInstance();
> 43:     private static final AtomicBoolean guard = new AtomicBoolean(false);

if we follow the java standard, then this should be named `GUARD`

modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayoutFactory.java line 34:

> 32: 
> 33: public class PrismTextLayoutFactory implements TextLayoutFactory {
> 34:     private static final PrismTextLayoutFactory factory = new PrismTextLayoutFactory();

Again, if we want to follow the practise this should be uppercase

modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayoutFactory.java line 37:

> 35:     /* Same strategy as GlyphLayout */
> 36:     private static final TextLayout reusableTL = factory.createLayout();
> 37:     private static final AtomicBoolean guard = new AtomicBoolean(false);

same here

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

Marked as reviewed by mhanl (Committer).

PR Review: https://git.openjdk.org/jfx/pull/1667#pullrequestreview-2664025448
PR Review Comment: https://git.openjdk.org/jfx/pull/1667#discussion_r1983109268
PR Review Comment: https://git.openjdk.org/jfx/pull/1667#discussion_r1983105350
PR Review Comment: https://git.openjdk.org/jfx/pull/1667#discussion_r1983107627
PR Review Comment: https://git.openjdk.org/jfx/pull/1667#discussion_r1983107282