On Tue Feb 13 10:57:43 2024 +0000, Alexandros Frantzis wrote:
@rbernon Thanks for the virtual-desktop branch! I have pushed my own proposed changes/fixes on top of your branch at https://gitlab.winehq.org/afrantzis/wine/-/commits/virtual-desktop. With those changes the branch works well for me. Here are some notes:
- The virtual-desktop branch successfully virtualizes the adapter
modes, but leaves the monitor rectangles as they are, which causes issues with a lot of calculation and checks (e.g., a fullscreen window will not be detected by `NtUserIsWindowRectFullScreen`). The part-12 MR was able to provide consistent monitor information/placement since the virtualization was done by the driver itself. The current WIP MR works around the issue by ignoring the host monitors and arbitrarily creates a virtual monitor with the proper rectangles for each virtual adapter. In my proposed virtual-desktop changes I have tried to solve this by automatically scaling the monitor rectangles. 2. Concerning integrating the scale factor with the DPI, the proposed approach does make sense to me, insofar as it reflects the actual (relative) DPI. However, I am skeptical because we will end up returning DPI values less than 96, and I am concerned that this may cause issues in applications in which 96 is assumed to be a minimum. As you note, the alternative would be to allow each driver to somehow get or calculate the mode scale factor and apply it internally as it sees fit (so I guess something closer to part-12 MR in this respect, at least in how the driver handles the scaling, if not in the actual mechanism to communicate the info). 3. The virtual-desktop branch uses the adapter_key to get the current settings from the registry during updates, which depends on the driver advertising adapters with the same order across processes. The Wayland protocol doesn't guarantee such ordering, but the Wayland driver mostly alleviates this since it sorts the adapters internally before sending them to win32u. 4. In the future we will need a way to associate a wl_output (by name, which is guaranteed to be cross-process consistent) to each adapter (e.g., so that if an app requests to become fullscreen on a particula monitor/adapter, we know that this corresponds to a particular wl_output and request it from the compositor). My proposed MRs implement such an association (each in a different way), but the virtual-desktop branch doesn't at the moment. We don't need to resolve this now if it's not directly needed by the approach we choose for the display mode changes, but I am mentioning it so that it's taken into account for any design evolution.
One concern (related to (3) and (4), although I think non-critical, so we can leave it for later), that hasn't been resolved in any of the MRs so far, is what happens when processes transiently disagree about the adapters when a user adds/removes an monitor (and the corresponding wl_output addition/removal has been handled in one process but not another). For the Wayland driver the desktop process is the authoritative source of display info (i.e., the one handles UpdateDisplayDevices), but ChangeDisplaySettings does force an UpdateDisplayDevices invocation in other processes. In a worst case scenario:
- Desktop process handles wl_output event and updates display devices.
- Another process, which hasn't handled the wl_output event yet, calls
ChangeDisplaySettings which may update the devices with stale information. Ideally, to avoid problems, I wouldn't want to update the display devices during a change display settings sequence, just let the process act on the existing registry information. Perhaps we can have an UpdateDisplayDevices parameter to signal when the update is part of a settings change, or I can track that manually in the driver (record it with trivial ChangeDisplaySettings handler). Another thought would be for the driver to only act on UpdateDisplayDevices in the desktop process (but I remember we tried something similar in the early Wayland driver MRs and that was causing issues).
Thanks for the feedback, I'll have a look at your fixes and think a bit about 3)/4) and the last parts of your comment.
Regarding 1), I was expecting monitor rects to be correct (as in matching the virtual mode), but maybe I missed something. There's some DPI conversion in the NtUserGetMonitorInfo which I'm not completely sure it is doing. There's also some issues with `get_win_monitor_dpi` / `get_dpi_for_window` which will need to be sorted out. I tried to break up some recursive dependency in one of the commit but I later saw that it's was not enough and triggered some user lock asserts.
Regarding 2), what about increasing the system_dpi when virtual modes are used so that the smallest mode ends up with 96dpi? Not entirely sure how to do that, and it will probably also require changing the wayland driver scaling computation.