[PATCH 0/1] MR2032: winex11: Allow replacing either context in wglShareLists.
The deleted test is a bad test: The behavior is different on different drivers, for example, the test fails on the Windows 10 and 11 testbot machines that have AMD video cards. Furthermore, MSDN does not say that the destination context "must not" have any display lists yet but rather that it "should not" have any.[1] The Khronos OpenGL Wiki similarly advises against calling wglShareLists after the source or destination context has at least one object, but if you do, it only says that "there is a chance that wglShareLists will fail", not that it will necessarily fail.[2] Since there's no clear "right" behavior here, we can adopt the more permissive behavior that some programs expect, as long as it doesn't corrupt the context. [1] https://learn.microsoft.com/en-us/windows/win32/api/wingdi/nf-wingdi-wglshar... [2] https://www.khronos.org/opengl/wiki/Platform_specifics:_Windows#wglShareList... Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=11436 -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2032
From: Alex Henrie <alexhenrie24(a)gmail.com> The deleted test is a bad test: The behavior is different on different drivers, for example, the test fails on the Windows 10 and 11 testbot machines that have AMD video cards. Furthermore, MSDN does not say that the destination context "must not" have any display lists yet but rather that it "should not" have any.[1] The Khronos OpenGL Wiki similarly advises against calling wglShareLists after the source or destination context has at least one object, but if you do, it only says that "there is a chance that wglShareLists will fail", not that it will necessarily fail.[2] Since there's no clear "right" behavior here, we can adopt the more permissive behavior that some programs expect, as long as it doesn't corrupt the context. [1] https://learn.microsoft.com/en-us/windows/win32/api/wingdi/nf-wingdi-wglshar... [2] https://www.khronos.org/opengl/wiki/Platform_specifics:_Windows#wglShareList... Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=11436 --- dlls/opengl32/tests/opengl.c | 14 ++---------- dlls/winex11.drv/opengl.c | 41 +++++++++++++++++++----------------- 2 files changed, 24 insertions(+), 31 deletions(-) diff --git a/dlls/opengl32/tests/opengl.c b/dlls/opengl32/tests/opengl.c index 42c2626a2c2..0d0e4360442 100644 --- a/dlls/opengl32/tests/opengl.c +++ b/dlls/opengl32/tests/opengl.c @@ -541,7 +541,7 @@ static void test_setpixelformat(HDC winhdc) static void test_sharelists(HDC winhdc) { - HGLRC hglrc1, hglrc2, hglrc3; + HGLRC hglrc1, hglrc2; BOOL res; hglrc1 = wglCreateContext(winhdc); @@ -569,17 +569,7 @@ static void test_sharelists(HDC winhdc) wglDeleteContext(hglrc2); } - /* Test 3: Share display lists with a context which already shares display lists with another context. - * According to MSDN the second parameter cannot share any display lists but some buggy drivers might allow it */ - hglrc3 = wglCreateContext(winhdc); - if(hglrc3) - { - res = wglShareLists(hglrc3, hglrc1); - ok(res == FALSE, "Sharing of display lists passed for a context which already shared lists before\n"); - wglDeleteContext(hglrc3); - } - - /* Test 4: Share display lists with a 'source' context which has been made current */ + /* Test 3: Share display lists with a 'source' context which has been made current */ hglrc2 = wglCreateContext(winhdc); if(hglrc2) { diff --git a/dlls/winex11.drv/opengl.c b/dlls/winex11.drv/opengl.c index c44e9579cb6..2fa7ccff495 100644 --- a/dlls/winex11.drv/opengl.c +++ b/dlls/winex11.drv/opengl.c @@ -1927,34 +1927,37 @@ static BOOL glxdrv_wglShareLists(struct wgl_context *org, struct wgl_context *de * so there delaying context creation doesn't work. * * The new approach is to create a GLX context in wglCreateContext / wglCreateContextAttribsARB - * and when a program requests sharing we recreate the destination context if it hasn't been made - * current or when it hasn't shared display lists before. + * and when a program requests sharing we recreate the destination or source context if it + * hasn't been made current and it hasn't shared display lists before. */ - if(dest->has_been_current) + if (!dest->has_been_current && !dest->sharing) { - ERR("Could not share display lists because the destination context has already been current\n"); - return FALSE; - } - else if(dest->sharing) - { - ERR("Could not share display lists because the destination context has already shared lists\n"); - return FALSE; - } - else - { - /* Re-create the GLX context and share display lists */ + /* Re-create the destination GLX context and share display lists */ pglXDestroyContext(gdi_display, dest->ctx); dest->ctx = create_glxcontext(gdi_display, dest, org->ctx); TRACE(" re-created context (%p) for Wine context %p (%s) sharing lists with ctx %p (%s)\n", dest->ctx, dest, debugstr_fbconfig(dest->fmt->fbconfig), org->ctx, debugstr_fbconfig( org->fmt->fbconfig)); - - org->sharing = TRUE; - dest->sharing = TRUE; - return TRUE; } - return FALSE; + else if (!org->has_been_current && !org->sharing) + { + /* Re-create the source GLX context and share display lists */ + pglXDestroyContext(gdi_display, org->ctx); + org->ctx = create_glxcontext(gdi_display, org, dest->ctx); + TRACE(" re-created context (%p) for Wine context %p (%s) sharing lists with ctx %p (%s)\n", + org->ctx, org, debugstr_fbconfig(org->fmt->fbconfig), + dest->ctx, debugstr_fbconfig(dest->fmt->fbconfig)); + } + else + { + ERR("Could not share display lists because both of the contexts have already been current or shared\n"); + return FALSE; + } + + org->sharing = TRUE; + dest->sharing = TRUE; + return TRUE; } static void wglFinish(void) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2032
I can't say much about this, except that I have run into the sharing restriction on Windows XP and Windows 7 when I tried to use wined3d on Windows, with multithreaded applications, before we had CSMT. Does the application from bug 11436 work on Windows machines where the test passes? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2032#note_22023
On Fri Jan 27 08:56:05 2023 +0000, Stefan Dösinger wrote:
I can't say much about this, except that I have run into the sharing restriction on Windows XP and Windows 7 when I tried to use wined3d on Windows, with multithreaded applications, before we had CSMT. Does the application from bug 11436 work on Windows machines where the test passes? I tested a 64-bit Windows 7 machine with an Nvidia GeForce GTX 580 and it both passes the test and runs Pepakura Designer without error. Most likely this is simply because both AMD and Nvidia always permit sharing lists from a source context to a destination context that has already been made current. We can't do that in GLX, but we can instead do the equivalent operation of sharing the lists from the destination context to the source context that has not been current yet.
The patch I am proposing does the same if the context is already sharing as if the context has already been made current. That means that it permits sharing from a source context to a destination context that is already sharing (by sharing from the destination to the source instead). Nvidia doesn't allow that, but AMD does, so we're just preferring AMD behavior over Nvidia behavior, which is unlikely to break anything. However, I could easily change the patch to preserve that restriction and still fix Pepakura Designer. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2032#note_22139
On Fri Jan 27 08:56:05 2023 +0000, Alex Henrie wrote:
I tested a 64-bit Windows 7 machine with an Nvidia GeForce GTX 580 and it both passes the test and runs Pepakura Designer without error. Most likely this is simply because both AMD and Nvidia always permit sharing lists from a source context to a destination context that has already been made current. We can't do that in GLX, but we can instead do the equivalent operation of sharing the lists from the destination context to the source context that has not been current yet. The patch I am proposing does the same if the context is already sharing as if the context has already been made current. That means that it permits sharing from a source context to a destination context that is already sharing (by sharing from the destination to the source instead). Nvidia doesn't allow that, but AMD does, so we're just preferring AMD behavior over Nvidia behavior, which is unlikely to break anything. However, I could easily change the patch to preserve that restriction and still fix Pepakura Designer. I have an embarrassingly large collection of old hardware if we want to get some more data points.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2032#note_22183
participants (4)
-
Alex Henrie -
Alex Henrie (＠alexhenrie)
-
Doug Johnson (＠dougvj)
-
Stefan Dösinger