[PATCH 0/1] MR7847: win32u: Don't distinguish WGL_SWAP_EXCHANGE_ARB and WGL_SWAP_UNDEFINED_ARB when filtering in wglChoosePixelFormatARB().

[Paul Gofman (＠gofman)]

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7847

[PATCH 1/1] win32u: Don't distinguish WGL_SWAP_EXCHANGE_ARB and WGL_SWAP_UNDEFINED_ARB when filtering in wglChoosePixelFormatARB().

[Paul Gofman]

From: Paul Gofman <pgofman(a)codeweavers.com>

---
 dlls/opengl32/tests/opengl.c | 47 ++++++++++++++++++++++++++++++++++--
 dlls/opengl32/wgl.c          |  7 +++---
 2 files changed, 49 insertions(+), 5 deletions(-)

diff --git a/dlls/opengl32/tests/opengl.c b/dlls/opengl32/tests/opengl.c
index deecc5c67e8..899b695e518 100644
--- a/dlls/opengl32/tests/opengl.c
+++ b/dlls/opengl32/tests/opengl.c
@@ -2780,12 +2780,26 @@ static void test_wglChoosePixelFormatARB(HDC hdc)
         0
     };
 
+    static int attrib_list_swap[] =
+    {
+        WGL_SWAP_METHOD_ARB, 0,
+        WGL_SUPPORT_OPENGL_ARB, 1,
+        WGL_DRAW_TO_WINDOW_ARB, 1,
+        0
+    };
+    static int swap_methods[] =
+    {
+        WGL_SWAP_COPY_ARB,
+        WGL_SWAP_EXCHANGE_ARB,
+        WGL_SWAP_UNDEFINED_ARB,
+    };
+
     PIXELFORMATDESCRIPTOR fmt, last_fmt;
     BYTE depth, last_depth;
     UINT format_count;
     int formats[1024];
-    unsigned int i;
-    int res;
+    unsigned int test, i;
+    int res, swap_method;
 
     if (!pwglChoosePixelFormatARB)
     {
@@ -2846,6 +2860,34 @@ static void test_wglChoosePixelFormatARB(HDC hdc)
 
         winetest_pop_context();
     }
+
+    for (test = 0; test < ARRAY_SIZE(swap_methods); ++test)
+    {
+        PIXELFORMATDESCRIPTOR format = {0};
+
+        winetest_push_context("swap method %#x", swap_methods[test]);
+        format_count = 0;
+        attrib_list_swap[1] = swap_methods[test];
+        res = pwglChoosePixelFormatARB(hdc, attrib_list_swap, NULL, ARRAY_SIZE(formats), formats, &format_count);
+        ok(res, "got %d.\n", res);
+        if (swap_methods[test] != WGL_SWAP_COPY_ARB)
+            ok(format_count, "got no formats.\n");
+        trace("count %d.\n", format_count);
+        for (i = 0; i < format_count; ++i)
+        {
+            res = pwglGetPixelFormatAttribivARB(hdc, formats[i], 0, 1, attrib_list_swap, &swap_method);
+            ok(res, "got %d.\n", res);
+            ok(swap_method == swap_methods[test]
+               /* AMD */
+               || (swap_methods[test] == WGL_SWAP_EXCHANGE_ARB && swap_method == WGL_SWAP_UNDEFINED_ARB)
+               || (swap_methods[test] == WGL_SWAP_UNDEFINED_ARB && swap_method == WGL_SWAP_EXCHANGE_ARB),
+               "got %#x.\n", swap_method);
+
+            res = DescribePixelFormat(hdc, formats[i], sizeof(format), &format);
+            ok(res, "DescribePixelFormat failed, error %lu\n", GetLastError());
+        }
+        winetest_pop_context();
+    }
 }
 
 static void test_copy_context(HDC hdc)
@@ -3015,6 +3057,7 @@ START_TEST(opengl)
          * any WGL call :( On Wine this would work but not on real Windows because there can be different implementations (software, ICD, MCD).
          */
         init_functions();
+
         test_getprocaddress(hdc);
         test_deletecontext(hwnd, hdc);
         test_makecurrent(hdc);
diff --git a/dlls/opengl32/wgl.c b/dlls/opengl32/wgl.c
index 4fd55bc3791..7de0d1b1dba 100644
--- a/dlls/opengl32/wgl.c
+++ b/dlls/opengl32/wgl.c
@@ -511,7 +511,6 @@ static enum attrib_match wgl_attrib_match_criteria( int attrib )
     case WGL_NEED_PALETTE_ARB:
     case WGL_NEED_SYSTEM_PALETTE_ARB:
     case WGL_SWAP_LAYER_BUFFERS_ARB:
-    case WGL_SWAP_METHOD_ARB:
     case WGL_SHARE_DEPTH_ARB:
     case WGL_SHARE_STENCIL_ARB:
     case WGL_SHARE_ACCUM_ARB:
@@ -557,6 +556,7 @@ static enum attrib_match wgl_attrib_match_criteria( int attrib )
     case WGL_TRANSPARENT_BLUE_VALUE_ARB:
     case WGL_TRANSPARENT_ALPHA_VALUE_ARB:
     case WGL_TRANSPARENT_INDEX_VALUE_ARB:
+    case WGL_SWAP_METHOD_ARB:
         return ATTRIB_MATCH_IGNORE;
     default:
         return ATTRIB_MATCH_INVALID;
@@ -572,14 +572,15 @@ static void filter_format_array( const struct wgl_pixel_format **array,
 
     assert(match != ATTRIB_MATCH_INVALID);
 
-    if (match == ATTRIB_MATCH_IGNORE) return;
+    if (match == ATTRIB_MATCH_IGNORE && attrib != WGL_SWAP_METHOD_ARB) return;
 
     for (i = 0; i < num_formats; ++i)
     {
         if (!array[i]) continue;
         if (!wgl_pixel_format_get_attrib( array[i], attrib, &fmt_value ) ||
             (match == ATTRIB_MATCH_EXACT && fmt_value != value) ||
-            (match == ATTRIB_MATCH_MINIMUM && fmt_value < value))
+            (match == ATTRIB_MATCH_MINIMUM && fmt_value < value) ||
+            (attrib == WGL_SWAP_METHOD_ARB && ((fmt_value == WGL_SWAP_COPY_ARB) ^ (value == WGL_SWAP_COPY_ARB))))
         {
             array[i] = NULL;
         }
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/7847

[Paul Gofman (＠gofman)]

Redesign of wglChoosePixelFormatARB broke recent enough Angle / CEF in GL mode. It requests, in particular, WGL_SWAP_METHOD_ARB: WGL_SWAP_UNDEFINED_ARB for wglChoosePixelFormatARB(), previously it ended up with a format, now it doesn't get any format and then ends up failing GPU process.

What happens with such a wglChoosePixelFormatARB on Windows differs between Nvidia and AMD in details.

On AMD (at least up to date Win11 I have locally and Testbot AMD GPU machine) there are no WGL_SWAP_EXCHANGE_ARB formats at all (there are only WGL_SWAP_COPY_ARB and WGL_SWAP_UNDEFINED_ARB) but wglChoosePixelFormatARB(WGL_SWAP_METHOD_ARB: WGL_SWAP_EXCHANGE_ARB) returns all the same formats as for WGL_SWAP_UNDEFINED_ARB. That is against spec [1] which lists WGL_SWAP_METHOD_ARB as strictly matched, but that what it does on Windows. WGL_SWAP_COPY_ARB is still strictly matched. I am guessing that is maybe done intentionally in the absense of WGL_SWAP_EXCHANGE_ARB formats as probably there are some apps which break without WGL_SWAP_EXCHANGE_ARB filter returning anything.

On Nvidia, WGL_SWAP_METHOD_ARB is matched strictly but there are both WGL_SWAP_EXCHANGE_ARB and WGL_SWAP_UNDEFINED_ARB formats available.

In any case, such wglChoosePixelFormatARB(WGL_SWAP_UNDEFINED_ARB) should always succeed on Windows.

Whether we have both swap methods in reality is driver and setup dependent. In winex11 we currently end up without swap_method and that is reported as WGL_SWAP_EXCHANGE_ARB. On winewayland WGL_SWAP_UNDEFINED_ARB is reported. We could probably change winex11's WGL_SWAP_EXCHANGE_ARB to WGL_SWAP_UNDEFINED_ARB but that would leave us without WGL_SWAP_EXCHANGE_ARB formats ever found which is probably not great.

So it looks best to do what AMD does and do not filter between WGL_SWAP_UNDEFINED_ARB and WGL_SWAP_EXCHANGE_ARB in wglChoosePixelFormatARB(), even if we then also want to change something in reported swap methods in winex11.

1. https://registry.khronos.org/OpenGL/extensions/ARB/WGL_ARB_pixel_format.txt

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7847#note_101351