[PATCH 1/4] winemac.drv: Simplify control flow in copy_display_modes.

[Tim Clem]

Hi Dean--

The display mode enumeration patches are unrelated to the Metal issues, as far as I
know. These patches help certain applications that expect lower resolutions to be
first in the results from EnumDisplaySettingsEx, or ones that only enumerate a
fixed number of resolutions before giving up (e.g. Quake).

--Tim

March 21, 2022 3:08 PM, "Dean Greer" <gcenx83 at gmail.com> wrote:

> Do you happen to know if this also resolves
> https://bugs.winehq.org/show_bug.cgi?id=52354
> 
> On Mon, Mar 21, 2022 at 5:32 PM Tim Clem <tclem at codeweavers.com> wrote:
> 
>> Signed-off-by: Tim Clem <tclem at codeweavers.com>
>> ---
>> dlls/winemac.drv/display.c | 107 ++++++++++++++++++-------------------
>> 1 file changed, 52 insertions(+), 55 deletions(-)
>> 
>> diff --git a/dlls/winemac.drv/display.c b/dlls/winemac.drv/display.c
>> index 2ac8f32fa18a..a54e78c22376 100644
>> --- a/dlls/winemac.drv/display.c
>> +++ b/dlls/winemac.drv/display.c
>> @@ -620,74 +620,71 @@ 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);
>> + 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;
>> + CGDisplayModeRef old_mode, new_mode = (CGDisplayModeRef)CFArrayGetValueAtIndex(modes, i);
>> + BOOL old_is_original, new_is_original = display_mode_matches_descriptor(new_mode, desc);
>> CFDictionaryRef key = create_mode_dict(new_mode, new_is_original);
>> 
>> /* 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;
>> + goto better;
>> 
>> - if (pixel_encoding)
>> + pixel_encoding = CGDisplayModeCopyPixelEncoding(new_mode);
>> + if (pixel_encoding)
>> + {
>> + BOOL bpp30 = CFEqual(pixel_encoding, CFSTR(kIO30BitDirectPixels));
>> + CFRelease(pixel_encoding);
>> + if (bpp30)
>> {
>> - 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;
>> - }
>> + /* 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. */
>> + goto next;
>> }
>> + }
>> 
>> - old_mode = (CGDisplayModeRef)CFDictionaryGetValue(modes_by_size, key);
>> - if (old_mode)
>> - {
>> - BOOL old_is_original = display_mode_matches_descriptor(old_mode, desc);
>> + old_mode = (CGDisplayModeRef)CFDictionaryGetValue(modes_by_size, key);
>> + if (!old_mode)
>> + goto better;
>> 
>> - 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;
>> - }
>> - }
>> - }
>> + /* Prefer the original mode over any similar mode. */
>> + old_is_original = display_mode_matches_descriptor(old_mode, desc);
>> + if (old_is_original)
>> + goto next;
>> +
>> + /* 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)
>> + goto better;
>> + else if (!new_size_same && old_size_same)
>> + goto next;
>> + else
>> + {
>> + /* Otherwise, prefer the mode with the smaller pixel size. */
>> + if (old_width_pixels < new_width_pixels || old_height_pixels < new_height_pixels)
>> + goto next;
>> }
>> 
>> - if (better)
>> - CFDictionarySetValue(modes_by_size, key, new_mode);
>> +better:
>> + CFDictionarySetValue(modes_by_size, key, new_mode);
>> 
>> +next:
>> CFRelease(key);
>> }
>> 
>> --
>> 2.34.1