RFR: 8346281: [Windows] RenderScale doesn't update to HiDPI changes [v2]

[Kevin Rushforth]

On Sat, 8 Nov 2025 14:17:30 GMT, Jose Pereda <jpereda at openjdk.org> wrote:

>> modules/javafx.graphics/src/main/java/com/sun/glass/ui/Screen.java line 403:
>> 
>>> 401:         for (Screen screen : oldScreens) {
>>> 402:             screen.dispose();
>>> 403:         }
>> 
>> I'm still a little puzzled as to why a change was needed in the class at all. Even with the change you made to the native Windows glass code, it seems that calling dispose for all of the old Java screen objects is what we want. Am I missing something?
>
> There is a reason for it, otherwise I wouldn't have modified `Screen`.
> 
> If you run the [test](https://bugs.openjdk.org/secure/attachment/112708/Menu_RenderScale_8346281.java) in the JBS issue, on Windows with two monitors (primary with scale > 100%), with the changes from `GlassWindow.cpp` only, without changing `Screen`, when you open the File menu of the window in the secondary display, the context menu is misplaced:
> 
> <img width="404" height="341" alt="Screenshot 2025-11-08 143811" src="https://github.com/user-attachments/assets/5844155e-c6cb-4b9a-9edb-c81b1140b8bb" />
> 
> Adding some debugging info, it can be seen that, when the application starts (without doing any external change in Windows settings), there is a DPI event, that calls `notifySettingsChanged`, at a point where there is only a `Window` instance (the one in the primary screen), instead of two. The two screens get disposed (that is, `Screen.ptr = 0`), and later on, when the secondary window is created, it gets assigned the disposed secondary screen, with this `ptr = 0`. 
> 
> When opening the `File` menu of the secondary window, another DPI event is triggered, `notifySettingsChanged` is called, but since there was no real change in the screens, the new screens are the same as the old ones (the valid  `ptr` didn't change after the event). However, the valid secondary screen is not assigned to the secondary Window, because the if test, (ptr from old screen was 0, and new screen ptr >0 doesn't match), so it keeps the old screen instance with `ptr = 0`.
> 
> This causes some unexpected issues for instance in `WinWindow::notifyMoving`, as there is a mixture of valid screens (from `Screen.getScreens()`) and invalid one in `Window::getScreen`, and the equality checks (`screen1 == screen2`) fails, ultimately providing wrong screen information for the popup.
> 
> So the change in `Screen` that I propose in this PR removes the disposal of screens if these are not assigned to a window yet.
> 
> Does that make sense?

Hmm. I see what you are saying, and can confirm the behavior. With your proposed fix, though, it seems like we will still have a situation where an in-use screen object is no longer in the list of screens. When this happens it will point to the same native screen object as one of the newly-created screens. This seems fragile.

Maybe we could still dispose all old screens, but do it in a way that allows it to later be mapped to a new screen? Or, thinking out loud, maybe "put back" the screens that didn't actually change as the result of the DPI change notification?

I'll want to think about this and take a closer look, but it will be a couple days before I can get to it.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1964#discussion_r2512140208