RFR: 8274932: Render scales in EmbeddedWindow are not properly updated [v9]

[Kevin Rushforth]

On Mon, 7 Aug 2023 08:52:01 GMT, John Hendrikx <jhendrikx at 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