On Mon, 10 Nov 2025 22:13:11 GMT, Kevin Rushforth <[email protected]> wrote:
>> 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. @kevinrushforth did you have the chance to have a look at this? ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1964#discussion_r2602088383
