RFR: 8317836: FX nodes embedded in JFXPanel need to track component orientation [v7]

Kevin Rushforth kcr at openjdk.org
Mon Oct 30 12:46:46 UTC 2023


On Mon, 30 Oct 2023 07:11:54 GMT, Prasanta Sadhukhan <psadhukhan at openjdk.org> wrote:

>> FX Nodes embedded in a Swing JFXPanel does not track the component orientation and FX nodes remain unaffected when component orientation changes.
>> Fix made sure JavaFX scene embedded in a JFXPanel should inherit the value from the JFXPanel.
>
> Prasanta Sadhukhan has updated the pull request incrementally with three additional commits since the last revision:
> 
>  - Code update as per review comment
>  - Code update as per review comment
>  - Code update as per review comment

This looks good now with a couple comments. I'll so some testing.

And speaking of testing, my question about providing a regression test is still pending.

modules/javafx.graphics/src/main/java/com/sun/javafx/stage/EmbeddedWindow.java line 52:

> 50: 
> 51:     private HostInterface host;
> 52:     private NodeOrientation orientation;

Initialize this to LTR.

modules/javafx.graphics/src/main/java/com/sun/javafx/stage/EmbeddedWindow.java line 94:

> 92:         orientation = nor;
> 93:         final Scene sceneValue = getScene();
> 94:         SceneHelper.parentEffectiveOrientationInvalidated(sceneValue);

You need to check whether the value has actually changed before calling invalidated or this will cause a performance regression.

Minor: I might not bother saving `getScene()` in a local var since you only use it once and `getScene()` is short.

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

PR Review: https://git.openjdk.org/jfx/pull/1271#pullrequestreview-1704013615
PR Review Comment: https://git.openjdk.org/jfx/pull/1271#discussion_r1376147367
PR Review Comment: https://git.openjdk.org/jfx/pull/1271#discussion_r1376142021


More information about the openjfx-dev mailing list