[PATCH v2 1/4] winemac.drv: Simplify control flow in copy_display_modes.
Introduce a helper function to determine if a mode is preferred over another. Signed-off-by: Tim Clem <tclem(a)codeweavers.com> --- v2: As per Huw's suggestion, pull the logic into its own function. dlls/winemac.drv/display.c | 123 ++++++++++++++++++------------------- 1 file changed, 61 insertions(+), 62 deletions(-) diff --git a/dlls/winemac.drv/display.c b/dlls/winemac.drv/display.c index 2ac8f32fa18a..cc3bbc6cffcd 100644 --- a/dlls/winemac.drv/display.c +++ b/dlls/winemac.drv/display.c @@ -576,6 +576,65 @@ static CFDictionaryRef create_mode_dict(CGDisplayModeRef display_mode, BOOL is_o return ret; } + + +static BOOL mode_is_preferred(CGDisplayModeRef new_mode, CGDisplayModeRef old_mode, + struct display_mode_descriptor *original_mode_desc) +{ + CFStringRef pixel_encoding; + size_t width_points, height_points; + size_t old_width_pixels, old_height_pixels, new_width_pixels, new_height_pixels; + BOOL old_size_same, new_size_same; + + /* If a given mode is the user's default, then always list it in preference to any similar + modes that may exist. */ + if (display_mode_matches_descriptor(new_mode, original_mode_desc)) + return TRUE; + + pixel_encoding = CGDisplayModeCopyPixelEncoding(new_mode); + if (pixel_encoding) + { + BOOL bpp30 = CFEqual(pixel_encoding, CFSTR(kIO30BitDirectPixels)); + CFRelease(pixel_encoding); + if (bpp30) + { + /* This is an odd pixel encoding. It seems it's only returned + when using kCGDisplayShowDuplicateLowResolutionModes. It's + 32bpp in terms of the actual raster layout, but it's 10 + bits per component. I think that no Windows program is + likely to need it and they will probably be confused by it. + Skip it. */ + return FALSE; + } + } + + if (!old_mode) + return TRUE; + + /* Prefer the original mode over any similar mode. */ + if (display_mode_matches_descriptor(old_mode, original_mode_desc)) + return FALSE; + + /* Otherwise, prefer a mode whose pixel size equals its point size over one which + is scaled. */ + width_points = CGDisplayModeGetWidth(new_mode); + height_points = CGDisplayModeGetHeight(new_mode); + new_width_pixels = CGDisplayModeGetPixelWidth(new_mode); + new_height_pixels = CGDisplayModeGetPixelHeight(new_mode); + old_width_pixels = CGDisplayModeGetPixelWidth(old_mode); + old_height_pixels = CGDisplayModeGetPixelHeight(old_mode); + new_size_same = (new_width_pixels == width_points && new_height_pixels == height_points); + old_size_same = (old_width_pixels == width_points && old_height_pixels == height_points); + + if (new_size_same && !old_size_same) + return TRUE; + + if (!new_size_same && old_size_same) + return FALSE; + + /* Otherwise, prefer the mode with the smaller pixel size. */ + return new_width_pixels < old_width_pixels && new_height_pixels < old_height_pixels; +} #endif @@ -620,72 +679,12 @@ static CFArrayRef copy_display_modes(CGDirectDisplayID display) count = CFArrayGetCount(modes); for (i = 0; i < count; i++) { - BOOL better = TRUE; CGDisplayModeRef new_mode = (CGDisplayModeRef)CFArrayGetValueAtIndex(modes, i); BOOL new_is_original = display_mode_matches_descriptor(new_mode, desc); CFDictionaryRef key = create_mode_dict(new_mode, new_is_original); + CGDisplayModeRef old_mode = (CGDisplayModeRef)CFDictionaryGetValue(modes_by_size, key); - /* If a given mode is the user's default, then always list it in preference to any similar - modes that may exist. */ - if (new_is_original) - better = TRUE; - else - { - CFStringRef pixel_encoding = CGDisplayModeCopyPixelEncoding(new_mode); - CGDisplayModeRef old_mode; - - if (pixel_encoding) - { - BOOL bpp30 = CFEqual(pixel_encoding, CFSTR(kIO30BitDirectPixels)); - CFRelease(pixel_encoding); - if (bpp30) - { - /* This is an odd pixel encoding. It seems it's only returned - when using kCGDisplayShowDuplicateLowResolutionModes. It's - 32bpp in terms of the actual raster layout, but it's 10 - bits per component. I think that no Windows program is - likely to need it and they will probably be confused by it. - Skip it. */ - CFRelease(key); - continue; - } - } - - old_mode = (CGDisplayModeRef)CFDictionaryGetValue(modes_by_size, key); - if (old_mode) - { - BOOL old_is_original = display_mode_matches_descriptor(old_mode, desc); - - if (old_is_original) - better = FALSE; - else - { - /* Otherwise, prefer a mode whose pixel size equals its point size over one which - is scaled. */ - size_t width_points = CGDisplayModeGetWidth(new_mode); - size_t height_points = CGDisplayModeGetHeight(new_mode); - size_t new_width_pixels = CGDisplayModeGetPixelWidth(new_mode); - size_t new_height_pixels = CGDisplayModeGetPixelHeight(new_mode); - size_t old_width_pixels = CGDisplayModeGetPixelWidth(old_mode); - size_t old_height_pixels = CGDisplayModeGetPixelHeight(old_mode); - BOOL new_size_same = (new_width_pixels == width_points && new_height_pixels == height_points); - BOOL old_size_same = (old_width_pixels == width_points && old_height_pixels == height_points); - - if (new_size_same && !old_size_same) - better = TRUE; - else if (!new_size_same && old_size_same) - better = FALSE; - else - { - /* Otherwise, prefer the mode with the smaller pixel size. */ - if (old_width_pixels < new_width_pixels || old_height_pixels < new_height_pixels) - better = FALSE; - } - } - } - } - - if (better) + if (mode_is_preferred(new_mode, old_mode, desc)) CFDictionarySetValue(modes_by_size, key, new_mode); CFRelease(key); -- 2.34.1
Prefer supported display modes over similar unsupported ones. Centralizes the logic from the ChangeDisplaySettingsEx and EnumDisplaySettingsEx implementations. Signed-off-by: Tim Clem <tclem(a)codeweavers.com> --- dlls/winemac.drv/display.c | 46 +++++++++++++++++++++++--------------- 1 file changed, 28 insertions(+), 18 deletions(-) diff --git a/dlls/winemac.drv/display.c b/dlls/winemac.drv/display.c index cc3bbc6cffcd..dfc238ed6297 100644 --- a/dlls/winemac.drv/display.c +++ b/dlls/winemac.drv/display.c @@ -524,6 +524,13 @@ static int get_default_bpp(void) } +static BOOL display_mode_is_supported(CGDisplayModeRef display_mode) +{ + uint32_t io_flags = CGDisplayModeGetIOFlags(display_mode); + return (io_flags & kDisplayModeValidFlag) && (io_flags & kDisplayModeSafeFlag); +} + + #if defined(MAC_OS_X_VERSION_10_8) && MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_8 static CFDictionaryRef create_mode_dict(CGDisplayModeRef display_mode, BOOL is_original) { @@ -579,8 +586,10 @@ static CFDictionaryRef create_mode_dict(CGDisplayModeRef display_mode, BOOL is_o static BOOL mode_is_preferred(CGDisplayModeRef new_mode, CGDisplayModeRef old_mode, - struct display_mode_descriptor *original_mode_desc) + struct display_mode_descriptor *original_mode_desc, + BOOL include_unsupported) { + BOOL new_is_supported; CFStringRef pixel_encoding; size_t width_points, height_points; size_t old_width_pixels, old_height_pixels, new_width_pixels, new_height_pixels; @@ -591,6 +600,11 @@ static BOOL mode_is_preferred(CGDisplayModeRef new_mode, CGDisplayModeRef old_mo if (display_mode_matches_descriptor(new_mode, original_mode_desc)) return TRUE; + /* Skip unsupported modes unless told to do otherwise. */ + new_is_supported = display_mode_is_supported(new_mode); + if (!new_is_supported && !include_unsupported) + return FALSE; + pixel_encoding = CGDisplayModeCopyPixelEncoding(new_mode); if (pixel_encoding) { @@ -615,6 +629,10 @@ static BOOL mode_is_preferred(CGDisplayModeRef new_mode, CGDisplayModeRef old_mo if (display_mode_matches_descriptor(old_mode, original_mode_desc)) return FALSE; + /* Prefer supported modes over similar unsupported ones. */ + if (!new_is_supported && display_mode_is_supported(old_mode)) + return FALSE; + /* Otherwise, prefer a mode whose pixel size equals its point size over one which is scaled. */ width_points = CGDisplayModeGetWidth(new_mode); @@ -649,8 +667,11 @@ static BOOL mode_is_preferred(CGDisplayModeRef new_mode, CGDisplayModeRef old_mo * returned from CGDisplayCopyAllDisplayModes() without special options. * This is especially bad if that's the user's default mode, since then * no "available" mode matches the initial settings. + * + * If include_unsupported is FALSE, display modes with IO flags that + * indicate that they are invalid or unsafe are filtered. */ -static CFArrayRef copy_display_modes(CGDirectDisplayID display) +static CFArrayRef copy_display_modes(CGDirectDisplayID display, BOOL include_unsupported) { CFArrayRef modes = NULL; @@ -684,7 +705,7 @@ static CFArrayRef copy_display_modes(CGDirectDisplayID display) CFDictionaryRef key = create_mode_dict(new_mode, new_is_original); CGDisplayModeRef old_mode = (CGDisplayModeRef)CFDictionaryGetValue(modes_by_size, key); - if (mode_is_preferred(new_mode, old_mode, desc)) + if (mode_is_preferred(new_mode, old_mode, desc, include_unsupported)) CFDictionarySetValue(modes_by_size, key, new_mode); CFRelease(key); @@ -766,7 +787,7 @@ LONG CDECL macdrv_ChangeDisplaySettingsEx(LPCWSTR devname, LPDEVMODEW devmode, int num_displays; CFArrayRef display_modes; struct display_mode_descriptor* desc; - CFIndex count, i, safe, best; + CFIndex count, i, best; CGDisplayModeRef best_display_mode; uint32_t best_io_flags; BOOL best_is_original; @@ -801,7 +822,7 @@ LONG CDECL macdrv_ChangeDisplaySettingsEx(LPCWSTR devname, LPDEVMODEW devmode, if (macdrv_get_displays(&displays, &num_displays)) return DISP_CHANGE_FAILED; - display_modes = copy_display_modes(displays[0].displayID); + display_modes = copy_display_modes(displays[0].displayID, FALSE); if (!display_modes) { macdrv_free_displays(displays); @@ -825,7 +846,6 @@ LONG CDECL macdrv_ChangeDisplaySettingsEx(LPCWSTR devname, LPDEVMODEW devmode, desc = create_original_display_mode_descriptor(displays[0].displayID); - safe = -1; best_display_mode = NULL; count = CFArrayGetCount(display_modes); for (i = 0; i < count; i++) @@ -843,11 +863,6 @@ LONG CDECL macdrv_ChangeDisplaySettingsEx(LPCWSTR devname, LPDEVMODEW devmode, height *= 2; } - if (!(io_flags & kDisplayModeValidFlag) || !(io_flags & kDisplayModeSafeFlag)) - continue; - - safe++; - if (bpp != mode_bpp) continue; @@ -901,7 +916,7 @@ LONG CDECL macdrv_ChangeDisplaySettingsEx(LPCWSTR devname, LPDEVMODEW devmode, better: best_display_mode = display_mode; - best = safe; + best = i; best_io_flags = io_flags; best_is_original = is_original; } @@ -1013,7 +1028,7 @@ BOOL CDECL macdrv_EnumDisplaySettingsEx(LPCWSTR devname, DWORD mode, if (mode == 0 || !modes) { if (modes) CFRelease(modes); - modes = copy_display_modes(displays[0].displayID); + modes = copy_display_modes(displays[0].displayID, (flags & EDS_RAWMODE) != 0); modes_has_8bpp = modes_has_16bpp = FALSE; if (modes) @@ -1042,11 +1057,6 @@ BOOL CDECL macdrv_EnumDisplaySettingsEx(LPCWSTR devname, DWORD mode, { CGDisplayModeRef candidate = (CGDisplayModeRef)CFArrayGetValueAtIndex(modes, i); - io_flags = CGDisplayModeGetIOFlags(candidate); - if (!(flags & EDS_RAWMODE) && - (!(io_flags & kDisplayModeValidFlag) || !(io_flags & kDisplayModeSafeFlag))) - continue; - seen_modes++; if (seen_modes > mode) { -- 2.34.1
Signed-off-by: Huw Davies <huw(a)codeweavers.com>
Also return higher synthesized bitdepths first. Signed-off-by: Tim Clem <tclem(a)codeweavers.com> --- dlls/winemac.drv/display.c | 50 ++++++++++++++++++++++---------------- 1 file changed, 29 insertions(+), 21 deletions(-) diff --git a/dlls/winemac.drv/display.c b/dlls/winemac.drv/display.c index dfc238ed6297..d43baf6bd683 100644 --- a/dlls/winemac.drv/display.c +++ b/dlls/winemac.drv/display.c @@ -1049,7 +1049,7 @@ BOOL CDECL macdrv_EnumDisplaySettingsEx(LPCWSTR devname, DWORD mode, display_mode = NULL; if (modes) { - int default_bpp = get_default_bpp(); + int default_bpp; DWORD seen_modes = 0; count = CFArrayGetCount(modes); @@ -1064,33 +1064,41 @@ BOOL CDECL macdrv_EnumDisplaySettingsEx(LPCWSTR devname, DWORD mode, display_mode_bpp = display_mode_bits_per_pixel(display_mode); break; } + } - /* We only synthesize modes from those having the default bpp. */ - if (display_mode_bits_per_pixel(candidate) != default_bpp) - continue; + default_bpp = get_default_bpp(); - if (!modes_has_8bpp) + /* If all the real modes are exhausted, synthesize lower bpp modes. */ + if (!display_mode && (!modes_has_16bpp || !modes_has_8bpp)) + { + /* We want to synthesize higher depths first. */ + int synth_bpps[] = { modes_has_16bpp ? 0 : 16, modes_has_8bpp ? 0 : 8 }; + size_t synth_bpp_idx; + for (synth_bpp_idx = 0; synth_bpp_idx < 2; synth_bpp_idx++) { - seen_modes++; - if (seen_modes > mode) + int synth_bpp = synth_bpps[synth_bpp_idx]; + if (synth_bpp == 0) + continue; + + for (i = 0; i < count; i++) { - display_mode = (CGDisplayModeRef)CFRetain(candidate); - display_mode_bpp = 8; - synthesized = TRUE; - break; + CGDisplayModeRef candidate = (CGDisplayModeRef)CFArrayGetValueAtIndex(modes, i); + /* We only synthesize modes from those having the default bpp. */ + if (display_mode_bits_per_pixel(candidate) != default_bpp) + continue; + + seen_modes++; + if (seen_modes > mode) + { + display_mode = (CGDisplayModeRef)CFRetain(candidate); + display_mode_bpp = synth_bpp; + synthesized = TRUE; + break; + } } - } - if (!modes_has_16bpp) - { - seen_modes++; - if (seen_modes > mode) - { - display_mode = (CGDisplayModeRef)CFRetain(candidate); - display_mode_bpp = 16; - synthesized = TRUE; + if (display_mode) break; - } } } } -- 2.34.1
Signed-off-by: Huw Davies <huw(a)codeweavers.com>
Mac driver version of a8b4cf7f2d3d1fbd79308a106a84e753cdac69e8. Combined with the previous patch that reordered synthesized modes, the Mac driver implementation of EnumDisplayModesEx now returns modes in the same order as the X11 driver. Signed-off-by: Tim Clem <tclem(a)codeweavers.com> --- dlls/winemac.drv/display.c | 53 +++++++++++++++++++++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-) diff --git a/dlls/winemac.drv/display.c b/dlls/winemac.drv/display.c index d43baf6bd683..023f7b80ed83 100644 --- a/dlls/winemac.drv/display.c +++ b/dlls/winemac.drv/display.c @@ -656,6 +656,48 @@ static BOOL mode_is_preferred(CGDisplayModeRef new_mode, CGDisplayModeRef old_mo #endif +static CFComparisonResult mode_compare(const void *p1, const void *p2, void *context) +{ + CGDisplayModeRef a = (CGDisplayModeRef)p1, b = (CGDisplayModeRef)p2; + size_t a_val, b_val; + double a_refresh_rate, b_refresh_rate; + + /* Sort by bpp descending, */ + a_val = display_mode_bits_per_pixel(a); + b_val = display_mode_bits_per_pixel(b); + if (a_val < b_val) + return kCFCompareGreaterThan; + else if (a_val > b_val) + return kCFCompareLessThan; + + /* then width ascending, */ + a_val = CGDisplayModeGetWidth(a); + b_val = CGDisplayModeGetWidth(b); + if (a_val < b_val) + return kCFCompareLessThan; + else if (a_val > b_val) + return kCFCompareGreaterThan; + + /* then height ascending, */ + a_val = CGDisplayModeGetHeight(a); + b_val = CGDisplayModeGetHeight(b); + if (a_val < b_val) + return kCFCompareLessThan; + else if (a_val > b_val) + return kCFCompareGreaterThan; + + /* then refresh rate descending. */ + a_refresh_rate = CGDisplayModeGetRefreshRate(a); + b_refresh_rate = CGDisplayModeGetRefreshRate(b); + if (a_refresh_rate < b_refresh_rate) + return kCFCompareGreaterThan; + else if (a_refresh_rate > b_refresh_rate) + return kCFCompareLessThan; + + return kCFCompareEqualTo; +} + + /*********************************************************************** * copy_display_modes * @@ -725,7 +767,16 @@ static CFArrayRef copy_display_modes(CGDirectDisplayID display, BOOL include_uns #endif modes = CGDisplayCopyAllDisplayModes(display, NULL); - return modes; + if (modes) + { + CFIndex count = CFArrayGetCount(modes); + CFMutableArrayRef sorted_modes = CFArrayCreateMutableCopy(NULL, count, modes); + CFRelease(modes); + CFArraySortValues(sorted_modes, CFRangeMake(0, count), mode_compare, NULL); + return sorted_modes; + } + + return NULL; } -- 2.34.1
Signed-off-by: Huw Davies <huw(a)codeweavers.com>
participants (2)
-
Huw Davies -
Tim Clem