On Mon, 7 Aug 2023 08:52:01 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:
>>> > > both windows (using EmbeddedFrameBug class listed earlier) shows O100% >>> > > for primary retina screen (should be 200%). >>> > >>> > >>> > @hjohn Seems like @andy-goryachev-oracle is telling it regressed after >>> > `updateSceneState` integration as previously he mentioned >>> > [#1171 >>> > (review)](https://github.com/openjdk/jfx/pull/1171#pullrequestreview-1522452579) >>> > > getting the scales right on both monitors now (macOS) >>> > > the image is rendered with no gaps. >>> > > LGTM >>> > >>> > >>> > So, it seems scaling swing-interop fix with`stage.setRenderScaleX/Y`was >>> > solving JDK-8274932 but not JDK-8222209 whereas your fix is solving both >>> > in windows but not solving JDK-8274932 in mac.. Since this seems to be >>> > about windows-toolkit `updateSceneState` , probably it's best to take it >>> > out from this PR and let it have only swing-interop change with >>> > `stage.setRenderScaleX/Y` and remove JDK-8222209 from this PR.. What you >>> > both suggest? >>> >>> IMHO, calling `setRenderScaleX/Y` should never be done by JavaFX, unless it >>> is in response to a change in the ouput scale properties -- render scale is >>> for the user to change the render size. Calling them from JFXPanel is >>> fixing a symptom of the problem, not the actual problem. The ouput scales >>> should be correct as well (in fact, in 99% of the cases they will be the >>> same as the render scale, unless the user overrides the render scale). >>> >>> So, JavaFX should be ensuring that the output scale is correct (which will >>> then be copied to the render scale). >>> >>> This is why I was looking further, and why I recommended calling >>> `updateSceneState`, as in that way the output scales get changed, and, >>> consequently, the render scales are updated. >> >> Any idea then why it would not work in macos? Did you test it there? >> Probably windows-toolkit has a native component where it needs some updation? > >> > > > both windows (using EmbeddedFrameBug class listed earlier) shows O100% >> > > > for primary retina screen (should be 200%). >> > > >> > > >> > > @hjohn Seems like @andy-goryachev-oracle is telling it regressed after >> > > `updateSceneState` integration as previously he mentioned >> > > [#1171 >> > > (review)](https://github.com/openjdk/jfx/pull/1171#pullrequestreview-1522452579) >> > > > getting the scales right on both monitors now (macOS) >> > > > the image is rendered with no gaps. >> > > > LGTM >> > > >> > > >> > > So, it seems scaling swing-interop fix with`stage.setRenderScaleX/Y`was >> > > solving JDK-8274932 but not JDK-8222209 whereas your fix is solving both >> > > in windows but not solving JDK-8274932 in mac.. Since this seems to be >> > > about windows-toolkit `updateSceneState` , probably it's best to take it >> > > out from this PR and let it have only swing-interop change with >> > > `stage.setRenderScaleX/Y` and remove JDK-8222209 from this PR.. What you >> > > both suggest? >> > >> > >> > IMHO, calling `setRenderScaleX/Y` should never be done by JavaFX, unless >> > it is in response to a change in the ouput scale properties -- render >> > scale is for the user to change the render size. Calling them from >> > JFXPanel is fixing a symptom of the problem, not the actual problem. The >> > ouput scales should be correct as well (in fact, in 99% of the cases they >> > will be the same as the render scale, unless the user overrides the render >> > scale). >> > So, JavaFX should be ensuring that the output scale is correct (which will >> > then be copied to the render scale). >> > This is why I was looking further, and why I recommended calling >> > `updateSceneState`, as in that way the output scales get changed, and, >> > consequently, the render scales are updated. >> >> Any idea then why it would not work in macos? Did you test it there? >> Probably windows-toolkit has a native component where it needs some updation? > > Sorry, no, I don't have macos. I'm only pointing out that the original fix > was not correct, as you are updating render scale directly but that would > leave output scale wrong -- it should be updating the output scale. I've > debugged this for the move window to another screen case, but didn't try > debugging the initial window showing case. > > Are you sure it works correctly on Windows now? In other words, both output > and render scale are set correctly? If that still bugs on macos, there may > be a 3rd problem that is mac specific. > > Judging from the build failure, it does seem to work everywhere except mac, > which would point to a deeper underly... @hjohn Do you want a chance to re-review? @prsadhuk You should be good to integrate this tomorrow (or early next week). ------------- PR Comment: https://git.openjdk.org/jfx/pull/1171#issuecomment-1682884612
