On Fri, 27 Oct 2023 16:55:30 GMT, Kevin Rushforth <k...@openjdk.org> wrote:

>> Prasanta Sadhukhan has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Code optimize
>
> 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`.

I have tried to put in place what you suggested. Can you please take a look at 
the PR and see if it's in the right direction and suggest further.
I have few doubts

- It seems JFXPanel call EmbeddedStage interface methods and not the other way 
round, so I am not sure how to "call a method in embedded stage that would 
delegate to JFXPanel".
- It seems 
[Scene::calcEffectiveNodeOrientation](https://github.com/openjdk/jfx/blob/9b93c962f45e5cf0b3a9f1090f307603be130a0e/modules/javafx.graphics/src/main/java/javafx/scene/Scene.java#L6343)
 is private and not part of swing-interop code/package so I am not sure how to 
leverage it from JFXPanel/EmbeddedStage....I have tried setting EmbeddedStage 
orientation from JFXPanel and query it from Scene but am not sure how to call 
`calcEffectiveNodeOrientation`

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1271#discussion_r1375187803

Reply via email to