[PATCH v2 0/5] MR720: winemac: Allow enumeration of non-primary display modes.
In preparation for with https://gitlab.winehq.org/wine/wine/-/merge_requests/576. I think support for changing non-primary display modes in winemac will be easier /after/ the display placement logic is moved to win32u. I also don't really know how to handle the display position on macOS. -- v2: winemac.drv: Support enumerating non-primary adapters display modes. winemac.drv: Support getting non-primary adapter current display mode. winemac.drv: Invalidate mode cache when enum flags changes. winemac.drv: Remove unnecessary macdrv_EnumDisplaySettingsEx declaration. winemac.drv: Rename display modes cache variables. https://gitlab.winehq.org/wine/wine/-/merge_requests/720
From: R��mi Bernon <rbernon(a)codeweavers.com> --- dlls/winemac.drv/display.c | 58 +++++++++++++++++++------------------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/dlls/winemac.drv/display.c b/dlls/winemac.drv/display.c index 2d7b9656103..76a9d67be20 100644 --- a/dlls/winemac.drv/display.c +++ b/dlls/winemac.drv/display.c @@ -53,10 +53,10 @@ static const WCHAR initial_mode_keyW[] = {'I','n','i','t','i','a','l',' ','D','i ' ','M','o','d','e'}; static const WCHAR pixelencodingW[] = {'P','i','x','e','l','E','n','c','o','d','i','n','g',0}; -static CFArrayRef modes; -static BOOL modes_has_8bpp, modes_has_16bpp; -static int default_mode_bpp; -static pthread_mutex_t modes_mutex = PTHREAD_MUTEX_INITIALIZER; +static CFArrayRef cached_modes; +static BOOL cached_modes_has_8bpp, cached_modes_has_16bpp; +static int cached_default_mode_bpp; +static pthread_mutex_t cached_modes_mutex = PTHREAD_MUTEX_INITIALIZER; static BOOL inited_original_display_mode; @@ -397,20 +397,20 @@ static int get_default_bpp(void) { int ret; - if (!default_mode_bpp) + if (!cached_default_mode_bpp) { CGDisplayModeRef mode = CGDisplayCopyDisplayMode(kCGDirectMainDisplay); if (mode) { - default_mode_bpp = display_mode_bits_per_pixel(mode); + cached_default_mode_bpp = display_mode_bits_per_pixel(mode); CFRelease(mode); } - if (!default_mode_bpp) - default_mode_bpp = 32; + if (!cached_default_mode_bpp) + cached_default_mode_bpp = 32; } - ret = default_mode_bpp; + ret = cached_default_mode_bpp; TRACE(" -> %d\n", ret); return ret; @@ -859,9 +859,9 @@ LONG macdrv_ChangeDisplaySettingsEx(LPCWSTR devname, LPDEVMODEW devmode, return DISP_CHANGE_FAILED; } - pthread_mutex_lock(&modes_mutex); + pthread_mutex_lock(&cached_modes_mutex); bpp = get_default_bpp(); - pthread_mutex_unlock(&modes_mutex); + pthread_mutex_unlock(&cached_modes_mutex); if ((devmode->dmFields & DM_BITSPERPEL) && devmode->dmBitsPerPel != bpp) TRACE("using default %d bpp instead of caller's request %d bpp\n", bpp, devmode->dmBitsPerPel); @@ -1012,39 +1012,39 @@ BOOL macdrv_EnumDisplaySettingsEx(LPCWSTR devname, DWORD mode, LPDEVMODEW devmod if (macdrv_get_displays(&displays, &num_displays)) goto failed; - pthread_mutex_lock(&modes_mutex); + pthread_mutex_lock(&cached_modes_mutex); - if (mode == 0 || !modes) + if (mode == 0 || !cached_modes) { - if (modes) CFRelease(modes); - modes = copy_display_modes(displays[0].displayID, (flags & EDS_RAWMODE) != 0); - modes_has_8bpp = modes_has_16bpp = FALSE; + if (cached_modes) CFRelease(cached_modes); + cached_modes = copy_display_modes(displays[0].displayID, (flags & EDS_RAWMODE) != 0); + cached_modes_has_8bpp = cached_modes_has_16bpp = FALSE; - if (modes) + if (cached_modes) { - count = CFArrayGetCount(modes); - for (i = 0; i < count && !(modes_has_8bpp && modes_has_16bpp); i++) + count = CFArrayGetCount(cached_modes); + for (i = 0; i < count && !(cached_modes_has_8bpp && cached_modes_has_16bpp); i++) { - CGDisplayModeRef mode = (CGDisplayModeRef)CFArrayGetValueAtIndex(modes, i); + CGDisplayModeRef mode = (CGDisplayModeRef)CFArrayGetValueAtIndex(cached_modes, i); int bpp = display_mode_bits_per_pixel(mode); if (bpp == 8) - modes_has_8bpp = TRUE; + cached_modes_has_8bpp = TRUE; else if (bpp == 16) - modes_has_16bpp = TRUE; + cached_modes_has_16bpp = TRUE; } } } display_mode = NULL; - if (modes) + if (cached_modes) { int default_bpp; DWORD seen_modes = 0; - count = CFArrayGetCount(modes); + count = CFArrayGetCount(cached_modes); for (i = 0; i < count; i++) { - CGDisplayModeRef candidate = (CGDisplayModeRef)CFArrayGetValueAtIndex(modes, i); + CGDisplayModeRef candidate = (CGDisplayModeRef)CFArrayGetValueAtIndex(cached_modes, i); seen_modes++; if (seen_modes > mode) @@ -1058,10 +1058,10 @@ BOOL macdrv_EnumDisplaySettingsEx(LPCWSTR devname, DWORD mode, LPDEVMODEW devmod default_bpp = get_default_bpp(); /* If all the real modes are exhausted, synthesize lower bpp modes. */ - if (!display_mode && (!modes_has_16bpp || !modes_has_8bpp)) + if (!display_mode && (!cached_modes_has_16bpp || !cached_modes_has_8bpp)) { /* We want to synthesize higher depths first. */ - int synth_bpps[] = { modes_has_16bpp ? 0 : 16, modes_has_8bpp ? 0 : 8 }; + int synth_bpps[] = { cached_modes_has_16bpp ? 0 : 16, cached_modes_has_8bpp ? 0 : 8 }; size_t synth_bpp_idx; for (synth_bpp_idx = 0; synth_bpp_idx < 2; synth_bpp_idx++) { @@ -1071,7 +1071,7 @@ BOOL macdrv_EnumDisplaySettingsEx(LPCWSTR devname, DWORD mode, LPDEVMODEW devmod for (i = 0; i < count; i++) { - CGDisplayModeRef candidate = (CGDisplayModeRef)CFArrayGetValueAtIndex(modes, i); + CGDisplayModeRef candidate = (CGDisplayModeRef)CFArrayGetValueAtIndex(cached_modes, i); /* We only synthesize modes from those having the default bpp. */ if (display_mode_bits_per_pixel(candidate) != default_bpp) continue; @@ -1092,7 +1092,7 @@ BOOL macdrv_EnumDisplaySettingsEx(LPCWSTR devname, DWORD mode, LPDEVMODEW devmod } } - pthread_mutex_unlock(&modes_mutex); + pthread_mutex_unlock(&cached_modes_mutex); if (!display_mode) goto failed; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/720
From: R��mi Bernon <rbernon(a)codeweavers.com> --- dlls/winemac.drv/display.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/dlls/winemac.drv/display.c b/dlls/winemac.drv/display.c index 76a9d67be20..69348bc9f39 100644 --- a/dlls/winemac.drv/display.c +++ b/dlls/winemac.drv/display.c @@ -46,9 +46,6 @@ struct display_mode_descriptor CFStringRef pixel_encoding; }; - -BOOL macdrv_EnumDisplaySettingsEx(LPCWSTR devname, DWORD mode, LPDEVMODEW devmode, DWORD flags); - static const WCHAR initial_mode_keyW[] = {'I','n','i','t','i','a','l',' ','D','i','s','p','l','a','y', ' ','M','o','d','e'}; static const WCHAR pixelencodingW[] = {'P','i','x','e','l','E','n','c','o','d','i','n','g',0}; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/720
From: R��mi Bernon <rbernon(a)codeweavers.com> --- dlls/winemac.drv/display.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/dlls/winemac.drv/display.c b/dlls/winemac.drv/display.c index 69348bc9f39..2034c8b21b4 100644 --- a/dlls/winemac.drv/display.c +++ b/dlls/winemac.drv/display.c @@ -51,7 +51,7 @@ static const WCHAR initial_mode_keyW[] = {'I','n','i','t','i','a','l',' ','D','i static const WCHAR pixelencodingW[] = {'P','i','x','e','l','E','n','c','o','d','i','n','g',0}; static CFArrayRef cached_modes; -static BOOL cached_modes_has_8bpp, cached_modes_has_16bpp; +static BOOL cached_modes_has_8bpp, cached_modes_has_16bpp, cached_modes_flags; static int cached_default_mode_bpp; static pthread_mutex_t cached_modes_mutex = PTHREAD_MUTEX_INITIALIZER; @@ -1011,11 +1011,12 @@ BOOL macdrv_EnumDisplaySettingsEx(LPCWSTR devname, DWORD mode, LPDEVMODEW devmod pthread_mutex_lock(&cached_modes_mutex); - if (mode == 0 || !cached_modes) + if (mode == 0 || !cached_modes || flags != cached_modes_flags) { if (cached_modes) CFRelease(cached_modes); cached_modes = copy_display_modes(displays[0].displayID, (flags & EDS_RAWMODE) != 0); cached_modes_has_8bpp = cached_modes_has_16bpp = FALSE; + cached_modes_flags = flags; if (cached_modes) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/720
From: R��mi Bernon <rbernon(a)codeweavers.com> --- dlls/winemac.drv/display.c | 35 +++++++++++++++++++++++++++-------- 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/dlls/winemac.drv/display.c b/dlls/winemac.drv/display.c index 2034c8b21b4..e451de21e4b 100644 --- a/dlls/winemac.drv/display.c +++ b/dlls/winemac.drv/display.c @@ -1142,8 +1142,10 @@ failed: BOOL macdrv_GetCurrentDisplaySettings(LPCWSTR devname, LPDEVMODEW devmode) { struct macdrv_display *displays = NULL; - int num_displays; + int i, num_displays, display_idx, min_x, min_y; CGDisplayModeRef display_mode; + CGDirectDisplayID display_id; + WCHAR *end; TRACE("%s, %p + %hu\n", debugstr_w(devname), devmode, devmode->dmSize); @@ -1152,17 +1154,33 @@ BOOL macdrv_GetCurrentDisplaySettings(LPCWSTR devname, LPDEVMODEW devmode) if (macdrv_get_displays(&displays, &num_displays)) return FALSE; - display_mode = CGDisplayCopyDisplayMode(displays[0].displayID); + display_idx = wcstol(devname + 11, &end, 10) - 1; + if (display_idx >= num_displays) + { + macdrv_free_displays(displays); + return FALSE; + } - /* We currently only report modes for the primary display, so it's at (0, 0). */ - devmode->dmPosition.x = 0; - devmode->dmPosition.y = 0; + display_id = displays[display_idx].displayID; + display_mode = CGDisplayCopyDisplayMode(display_id); + + devmode->dmPosition.x = min_x = CGRectGetMinX(displays[display_idx].frame); + devmode->dmPosition.y = min_y = CGRectGetMinY(displays[display_idx].frame); + for (i = 0; i < num_displays; ++i) + { + if (min_x > CGRectGetMinX(displays[i].frame)) + min_x = CGRectGetMinX(displays[i].frame); + if (min_y > CGRectGetMinY(displays[i].frame)) + min_y = CGRectGetMinY(displays[i].frame); + } + devmode->dmPosition.x -= min_x; + devmode->dmPosition.y -= min_y; devmode->dmFields |= DM_POSITION; - display_mode_to_devmode(displays[0].displayID, display_mode, devmode); + display_mode_to_devmode(display_id, display_mode, devmode); if (retina_enabled) { - struct display_mode_descriptor *desc = create_original_display_mode_descriptor(displays[0].displayID); + struct display_mode_descriptor *desc = create_original_display_mode_descriptor(display_id); if (display_mode_matches_descriptor(display_mode, desc)) { devmode->dmPelsWidth *= 2; @@ -1174,7 +1192,8 @@ BOOL macdrv_GetCurrentDisplaySettings(LPCWSTR devname, LPDEVMODEW devmode) CFRelease(display_mode); macdrv_free_displays(displays); - TRACE("current mode -- %dx%dx%dbpp @%d Hz", + TRACE("current mode -- %dx%d-%dx%dx%dbpp @%d Hz", + devmode->dmPosition.x, devmode->dmPosition.y, devmode->dmPelsWidth, devmode->dmPelsHeight, devmode->dmBitsPerPel, devmode->dmDisplayFrequency); if (devmode->dmDisplayOrientation) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/720
From: R��mi Bernon <rbernon(a)codeweavers.com> --- dlls/winemac.drv/display.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/dlls/winemac.drv/display.c b/dlls/winemac.drv/display.c index e451de21e4b..85c6e3d544b 100644 --- a/dlls/winemac.drv/display.c +++ b/dlls/winemac.drv/display.c @@ -51,6 +51,7 @@ static const WCHAR initial_mode_keyW[] = {'I','n','i','t','i','a','l',' ','D','i static const WCHAR pixelencodingW[] = {'P','i','x','e','l','E','n','c','o','d','i','n','g',0}; static CFArrayRef cached_modes; +static CGDirectDisplayID cached_modes_display_id; static BOOL cached_modes_has_8bpp, cached_modes_has_16bpp, cached_modes_flags; static int cached_default_mode_bpp; static pthread_mutex_t cached_modes_mutex = PTHREAD_MUTEX_INITIALIZER; @@ -998,9 +999,11 @@ BOOL macdrv_EnumDisplaySettingsEx(LPCWSTR devname, DWORD mode, LPDEVMODEW devmod struct macdrv_display *displays = NULL; int num_displays; CGDisplayModeRef display_mode; - int display_mode_bpp; + int display_mode_bpp, display_idx; BOOL synthesized = FALSE; + CGDirectDisplayID display_id; DWORD count, i; + WCHAR *end; TRACE("%s, %u, %p + %hu, %08x\n", debugstr_w(devname), mode, devmode, devmode->dmSize, flags); @@ -1009,13 +1012,23 @@ BOOL macdrv_EnumDisplaySettingsEx(LPCWSTR devname, DWORD mode, LPDEVMODEW devmod if (macdrv_get_displays(&displays, &num_displays)) goto failed; + display_idx = wcstol(devname + 11, &end, 10) - 1; + if (display_idx >= num_displays) + { + macdrv_free_displays(displays); + return FALSE; + } + + display_id = displays[display_idx].displayID; + pthread_mutex_lock(&cached_modes_mutex); - if (mode == 0 || !cached_modes || flags != cached_modes_flags) + if (mode == 0 || !cached_modes || flags != cached_modes_flags || display_id != cached_modes_display_id) { if (cached_modes) CFRelease(cached_modes); - cached_modes = copy_display_modes(displays[0].displayID, (flags & EDS_RAWMODE) != 0); + cached_modes = copy_display_modes(display_id, (flags & EDS_RAWMODE) != 0); cached_modes_has_8bpp = cached_modes_has_16bpp = FALSE; + cached_modes_display_id = display_id; cached_modes_flags = flags; if (cached_modes) @@ -1095,13 +1108,13 @@ BOOL macdrv_EnumDisplaySettingsEx(LPCWSTR devname, DWORD mode, LPDEVMODEW devmod if (!display_mode) goto failed; - display_mode_to_devmode(displays[0].displayID, display_mode, devmode); + display_mode_to_devmode(display_id, display_mode, devmode); devmode->dmBitsPerPel = display_mode_bpp; if (devmode->dmBitsPerPel) devmode->dmFields |= DM_BITSPERPEL; if (retina_enabled) { - struct display_mode_descriptor* desc = create_original_display_mode_descriptor(displays[0].displayID); + struct display_mode_descriptor* desc = create_original_display_mode_descriptor(display_id); if (display_mode_matches_descriptor(display_mode, desc)) { devmode->dmPelsWidth *= 2; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/720
On Sat Aug 27 02:30:42 2022 +0000, Zhiyi Zhang wrote:
I haven't checked. But can the displays from macdrv_get_displays() be detached? If it can, you should handle it as well. For example, when a Mac laptop is connected to an external monitor and the lid is closed. I don't think it does.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/720#note_7132
Zhiyi Zhang (@zhiyi) commented about dlls/winemac.drv/display.c:
- devmode->dmPosition.x = 0; - devmode->dmPosition.y = 0; + display_id = displays[display_idx].displayID; + display_mode = CGDisplayCopyDisplayMode(display_id); + + devmode->dmPosition.x = min_x = CGRectGetMinX(displays[display_idx].frame); + devmode->dmPosition.y = min_y = CGRectGetMinY(displays[display_idx].frame); + for (i = 0; i < num_displays; ++i) + { + if (min_x > CGRectGetMinX(displays[i].frame)) + min_x = CGRectGetMinX(displays[i].frame); + if (min_y > CGRectGetMinY(displays[i].frame)) + min_y = CGRectGetMinY(displays[i].frame); + } + devmode->dmPosition.x -= min_x; + devmode->dmPosition.y -= min_y; According to https://developer.apple.com/documentation/appkit/nsscreen/1388393-screens. The primary screen already has an origin of (0, 0). This is different compared to X11 where the primary screen may not be at (0, 0). Offsetting when unnecessary, you're making the leftmost monitor the primary. Could you recheck whether this part of the code is correct?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/720#note_7142
On Mon Aug 29 09:35:01 2022 +0000, Zhiyi Zhang wrote:
According to https://developer.apple.com/documentation/appkit/nsscreen/1388393-screens. The primary screen already has an origin of (0, 0). This is different compared to X11 where the primary screen may not be at (0, 0). Offsetting when unnecessary, you're making the leftmost monitor the primary. Could you recheck whether this part of the code is correct? Well I have no idea really, is `dmPosition` supposed to be relative to the primary screen top left corner on Windows? I assumed it was some kind of virtual desktop coordinate system and that left-most screen was with `dmPosition.x == 0`, not really anything with the primary screen.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/720#note_7148
On Mon Aug 29 12:58:28 2022 +0000, R��mi Bernon wrote:
Well I have no idea really, is `dmPosition` supposed to be relative to the primary screen top left corner on Windows? I assumed it was some kind of virtual desktop coordinate system and that left-most screen was with `dmPosition.x == 0`, not really anything with the primary screen. dmPosition is in virtual screen coordinates. So if there is a 1920x1080 monitor to the left of the primary screen, then its position should be (-1920, 0). The primary screen origin is always at (0, 0).
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/720#note_7149
participants (2)
-
Rémi Bernon -
Zhiyi Zhang (＠zhiyi)