[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-openjfx-dev
Subject:    Re: RFR: 8274932: Render scales in EmbeddedWindow are not properly updated [v6]
From:       Kevin Rushforth <kcr () openjdk ! org>
Date:       2023-07-29 15:13:49
Message-ID: 7Ncyp0c3-e4p7THnFlZ5yOAJ8YrVaYGigiaw4PiCAmE=.649a959c-870f-412f-88e0-af24a566926c () github ! com
[Download RAW message or body]

On Fri, 28 Jul 2023 03:53:17 GMT, Prasanta Sadhukhan <psadhukhan@openjdk.org> wrote:

> > This is how I understand this works:
> > 
> > The `JFXPanel` makes use of an `EmbeddedWindow` that  is a subclass of `Window`.  \
> > This window listens to its `showing` property, and when it becomes visible will \
> > call `updateOutputScales`; this will in turn set the (correct?) render scales. 
> > Now the above fix seems to 2nd guess this logic, and overrides these values with \
> > render scales it gets from Swing/AWT (note that it didn't do this before). 
> > So, my questions:
> > 
> > - If JFXPanel never called setRenderScale before, was the JFXPanel completely \
> > broken when used on monitors that are not set at 100% scale? Did they update \
> > correctly when moved between monitors?  I get the impression that it sort of \
> > worked, except for this edge case. 
> > - If it wasn't completely broken, then why is this fix needed?  Shouldn't \
> > `Window` already detect that it has become visible (with its `showing` listener) \
> > and update the render scales using the `updateOutputScales` method?  In other \
> > words, isn't this a bug that perhaps needs to be fixed in `Window`s detection of \
> > when it should be updating the output scales?
> 
> > This is how I understand this works:
> > 
> > The `JFXPanel` makes use of an `EmbeddedWindow` that is a subclass of `Window`. \
> > This window listens to its `showing` property, and when it becomes visible will \
> > call `updateOutputScales`; this will in turn set the (correct?) render scales. 
> > Now the above fix seems to 2nd guess this logic, and overrides these values with \
> > render scales it gets from Swing/AWT (note that it didn't do this before). 
> > So, my questions:
> > 
> > * If JFXPanel never called setRenderScale before, was the JFXPanel completely \
> > broken when used on monitors that are not set at 100% scale? Did they update \
> > correctly when moved between monitors?  I get the impression that it sort of \
> > worked, except for this edge case. 
> > * If it wasn't completely broken, then why is this fix needed?  Shouldn't \
> > `Window` already detect that it has become visible (with its `showing` listener) \
> > and update the render scales using the `updateOutputScales` method?  In other \
> > words, isn't this a bug that perhaps needs to be fixed in `Window`s detection of \
> > when it should be updating the output scales?
> 
> I think it is broken...See \
> [JDK-8222209](https://bugs.openjdk.org/browse/JDK-8222209) I am not sure if it \
> might be a bug in FX Embedded `Window` s detection logic and scale updation logic, \
> might be FX windows-graphics/toolkit team can share their thoughts but I guess it's \
> not as otherwise we will get lot more issues and not just in JFXPanel. Also, \
> JFXPanel uses `GraphicsEnvironment.getLocalGraphicsEnvironment().                   \
> getDefaultScreenDevice().getDefaultConfiguration().getDefaultTransform().getScaleX()` \
> which only works for primary screen and will not work if secondary screen has \
> different scale, so that needs to be fixed in my opinion, which is being done in \
> this PR.

@prsadhuk @hjohn I'll take a closer look early next week, but I think the best way \
forward might be for Prasanta to evaluate #1189 and, if the modified fix in that PR \
is what we want to use, update _this_ PR to include the change that adds the call to \
`updateSceneState()` and removes the explicit call to `Stage::setRenderScale[XY]`. \
Then add John as a contributor to this PR using `/contributor add`. We can then close \
[JDK-8222209](https://bugs.openjdk.org/browse/JDK-8222209) as a duplicate of \
[JDK-8274932](https://bugs.openjdk.org/browse/JDK-8274932).

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

PR Comment: https://git.openjdk.org/jfx/pull/1171#issuecomment-1656755453


[prev in list] [next in list] [prev in thread] [next in thread]