On Mon Apr 3 16:26:40 2023 +0000, Alexandros Frantzis wrote:
I'd suggest moving the add_mode change to a separate MR and splitting
it more, keeping driver changes separate and moving code around (for instance in winemac) before doing actual logic changes. https://gitlab.winehq.org/wine/wine/-/merge_requests/2568
I'm also not sure to see why you need to separate registry
initialization from current mode. I'd think that you could just have a BOOL current argument, ... Although, 'BOOL current' was my initial approach, I wanted to minimize implicit changes in other drivers, so that drivers that, e.g., don't need to use the default win32u for current settings can just update the registry settings. The other part was that the internal design of drivers makes it more straightforward to get the current mode independently of mode enumeration, hence the separate 'target' rather than a 'current' flag. In any case, these are not insurmountable concerns, so in the separate MR I have switch to the `current` flag approach, since it seems to be preferable.
I think all concerns in this thread have been addressed in the other MR. The only open question in my mind is the one stated in https://gitlab.winehq.org/wine/wine/-/merge_requests/2476#note_28444 (i.e., being able to query current win32u settings from within UpdateDisplayDevices). But that's not going to be needed until the Wayland driver needs to properly support ChangeDisplaySettings, which will happen later in the series, so we can discuss more then.