On Mon, 30 Oct 2023 07:11:54 GMT, Prasanta Sadhukhan <psadhuk...@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
