On Fri, 28 Oct 2022 06:42:21 GMT, Johan Vos <j...@openjdk.org> wrote:

>> The root problem is actually broader than stated in the JBS issue. This PR 
>> now translates screencoordinates from absolute coordinates into coordinates 
>> that take the platformScale into account. 
>> The whole process is complicated by the fact that throughout our code, we 
>> use e.g. `x` and `y` without clearly stating if those are absolute, logical, 
>> screen or rendering coordinates. 
>> I believe the most consistent approach is to have the different entry points 
>> (e.g. a Glass Window or a JFXPanel) to deal with platformScale before 
>> passing screen coordinates. This is already done in the Glass approach, and 
>> this PR does the same in JFXPanel. That means some code is duplicated, but 
>> since this is only about 12 lines, and said code lives in 2 different 
>> modules, I think it's not worth the hassle of moving that into e.g. the base 
>> module.
>
> Johan Vos has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   address reviewer comments
>   Fix detection on screen, based on awtScale factors

I think this looks basically correct, although I did leave a couple questions / 
comments inline. I'll test it next week.

modules/javafx.swing/src/main/java/javafx/embed/swing/JFXPanel.java line 387:

> 385:     private Dimension2D convertSwingToFxPixel(GraphicsConfiguration g, 
> float wx, float wy) {
> 386:         float newx, newy;
> 387:         Screen screen = findScreen(getGraphicsConfiguration());

Did you mean to use the passed in graphics config, `g`? If not, you might 
remove the `g` parameter, since it is otherwise unused.

modules/javafx.swing/src/main/java/javafx/embed/swing/JFXPanel.java line 400:

> 398:             float py = screen.getPlatformY();
> 399:             newx = sx + (wx - px) * awtScaleX / pScaleX;
> 400:             newy = sy + (wy - py) * awtScaleY / pScaleY;

`px` and `py` are already (scaled) "FX" coordinates, so I'm not sure AWT scale 
should be applied to that. I might be missing something here.

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

PR: https://git.openjdk.org/jfx/pull/924

Reply via email to