On Fri, 27 Oct 2023 03:19:05 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 one > additional commit since the last revision: > > Code optimize I don't think this is the right fix. See my inline comment. Also, can you provide a test case? Automated if possible, manual if automated is not feasible. modules/javafx.swing/src/main/java/javafx/embed/swing/JFXPanel.java line 822: > 820: scene.setNodeOrientation(rtl ? > 821: > NodeOrientation.RIGHT_TO_LEFT : > 822: > NodeOrientation.LEFT_TO_RIGHT); I don't think this is the right place to fix it. The default value for `nodeOrientation` is `INHERIT`, and in case it is not overridden, we want the effective orientation to be returned as RTL or LTR. In the case where it is overridden, it would be wrong to modify it. Even when it is INHERIT, we don't want to modify the actual property value, but rather want to compute the effective orientation to take the Swing component's orientation into account. See [Scene::calcEffectiveNodeOrientation](https://github.com/openjdk/jfx/blob/9b93c962f45e5cf0b3a9f1090f307603be130a0e/modules/javafx.graphics/src/main/java/javafx/scene/Scene.java#L6343). I think you should be able to add logic to check whether the scene's window is an embedded stage, and then to call a method in embedded stage that would delegate to JFXPanel to get the component's node orientation. That way it would only alter the effective node orientation (and not the property), and would only do it when the property was `INHERIT`. ------------- Changes requested by kcr (Lead). PR Review: https://git.openjdk.org/jfx/pull/1271#pullrequestreview-1702197475 PR Review Comment: https://git.openjdk.org/jfx/pull/1271#discussion_r1374832758
