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

[Tim Clem]

Signed-off-by: Tim Clem 
---
 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);
         }

[PATCH 2/4] winemac.drv: Add a flag to filter unsupported modes in copy_display_modes.

[Tim Clem]

Prefer supported display modes over similar unsupported ones.
Centralizes the logic from the ChangeDisplaySettingsEx and
EnumDisplaySettingsEx implementations.

Signed-off-by: Tim Clem 
---
 dlls/winemac.drv/display.c | 42 +++++++++++++++++++++++---------------
 1 file changed, 26 insertions(+), 16 deletions(-)

diff --git a/dlls/winemac.drv/display.c b/dlls/winemac.drv/display.c
index a54e78c22376..44daf4f6a51b 100644
--- a/dlls/winemac.drv/display.c
+++ b/dlls/winemac.drv/display.c
@@ -579,6 +579,13 @@ static CFDictionaryRef create_mode_dict(CGDisplayModeRef display_mode, BOOL is_o
 #endif

+static BOOL display_mode_is_supported(CGDisplayModeRef display_mode)
+{
+    uint32_t io_flags = CGDisplayModeGetIOFlags(display_mode);
+    return (io_flags & kDisplayModeValidFlag) && (io_flags & kDisplayModeSafeFlag);
+}
+
+
 /***********************************************************************
  *              copy_display_modes
  *
@@ -590,8 +597,11 @@ static CFDictionaryRef create_mode_dict(CGDisplayModeRef display_mode, BOOL is_o
  * 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;

@@ -621,6 +631,7 @@ static CFArrayRef copy_display_modes(CGDirectDisplayID display)
         for (i = 0; i < count; i++)
         {
             CFStringRef pixel_encoding;
+            BOOL old_is_supported, new_is_supported;
             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;
@@ -633,6 +644,11 @@ static CFArrayRef copy_display_modes(CGDirectDisplayID display)
             if (new_is_original)
                 goto better;

+            /* Skip unsupported modes unless told to do otherwise. */
+            new_is_supported = display_mode_is_supported(new_mode);
+            if (!new_is_supported && !include_unsupported)
+                goto next;
+
             pixel_encoding = CGDisplayModeCopyPixelEncoding(new_mode);
             if (pixel_encoding)
             {
@@ -659,6 +675,11 @@ static CFArrayRef copy_display_modes(CGDirectDisplayID display)
             if (old_is_original)
                 goto next;

+            /* Prefer supported modes over similar unsupported ones. */
+            old_is_supported = display_mode_is_supported(old_mode);
+            if (old_is_supported && !new_is_supported)
+                goto next;
+
             /* Otherwise, prefer a mode whose pixel size equals its point size over one which
             is scaled. */
             width_points = CGDisplayModeGetWidth(new_mode);
@@ -764,7 +785,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;
@@ -799,7 +820,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);
@@ -823,7 +844,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++)
@@ -841,11 +861,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;

@@ -899,7 +914,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;
     }
@@ -1011,7 +1026,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)
@@ -1040,11 +1055,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)
                 {

[PATCH 3/4] winemac.drv: Move synthetic display modes after the real ones.

[Tim Clem]

Also return higher synthesized bitdepths first.

Signed-off-by: Tim Clem 
---
 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 44daf4f6a51b..180c0e17938c 100644
--- a/dlls/winemac.drv/display.c
+++ b/dlls/winemac.drv/display.c
@@ -1047,7 +1047,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);
@@ -1062,33 +1062,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;
-                    }
                 }
             }
         }

[PATCH 4/4] winemac.drv: Sort display modes.

[Tim Clem]

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 
---
 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 180c0e17938c..9e0ac3182501 100644
--- a/dlls/winemac.drv/display.c
+++ b/dlls/winemac.drv/display.c
@@ -579,6 +579,48 @@ static CFDictionaryRef create_mode_dict(CGDisplayModeRef display_mode, BOOL is_o
 #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;
+}
+
+
 static BOOL display_mode_is_supported(CGDisplayModeRef display_mode)
 {
     uint32_t io_flags = CGDisplayModeGetIOFlags(display_mode);
@@ -723,7 +765,16 @@ next:
 #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;
 }

[Dean Greer]

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  wrote:
> Signed-off-by: Tim Clem
>
> 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

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=110874

Your paranoid android.

=== debian11 (build log) ===

Task: Patch failed to apply

=== debian11 (build log) ===

Task: Patch failed to apply

[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"  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  wrote:
> > Signed-off-by: Tim Clem
> >
> > 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

[Huw Davies]

On Mon, Mar 21, 2022 at 02:32:20PM -0700, Tim Clem wrote:
> Signed-off-by: Tim Clem
>
> dlls/winemac.drv/display.c | 107 ++++++++++++++++++-------------------
>  1 file changed, 52 insertions(+), 55 deletions(-)

Hi Tim,

I'm not convinced that adding two goto labels is an improvement.

I'd suggest moving the bulk of the code inside the loop to
a helper that returns "better".

Huw.