[PATCH 0/1] MR10033: winex11.drv: Fix x11drv_egl_surface_swap() swap for the case of absent current context.
From: Paul Gofman <pgofman@codeweavers.com> --- dlls/winex11.drv/opengl.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/dlls/winex11.drv/opengl.c b/dlls/winex11.drv/opengl.c index 01eb3b9c356..3274cb53365 100644 --- a/dlls/winex11.drv/opengl.c +++ b/dlls/winex11.drv/opengl.c @@ -216,6 +216,7 @@ enum glx_swap_control_method static struct glx_pixel_format *pixel_formats; static int nb_pixel_formats, nb_onscreen_formats; static const struct egl_platform *egl; +static EGLContext egl_fallback_context; static BOOL (*p_egl_describe_pixel_format)( int format, struct wgl_pixel_format *pf ); /* Selects the preferred GLX swap control method for use by wglSwapIntervalEXT */ @@ -1511,7 +1512,16 @@ static BOOL x11drv_egl_surface_swap( struct opengl_drawable *base ) TRACE( "%s\n", debugstr_opengl_drawable( base ) ); - funcs->p_eglSwapBuffers( egl->display, gl->base.surface ); + if (!funcs->p_eglSwapBuffers( egl->display, gl->base.surface ) && funcs->p_eglGetError() == EGL_BAD_SURFACE + && !funcs->p_eglGetCurrentContext()) + { + if (!egl_fallback_context) + egl_fallback_context = funcs->p_eglCreateContext( egl->display, EGL_NO_CONFIG_KHR, EGL_NO_CONTEXT, NULL ); + + funcs->p_eglMakeCurrent( egl->display, gl->base.surface, gl->base.surface, egl_fallback_context ); + funcs->p_eglSwapBuffers( egl->display, gl->base.surface ); + funcs->p_eglMakeCurrent( egl->display, EGL_NO_CONTEXT, EGL_NO_SURFACE, EGL_NO_CONTEXT ); + } if (InterlockedCompareExchange( &base->client->offscreen, 0, 0 )) XFlush( gdi_display ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10033
Fixes Black Desert Online launcher showing black screen only (after switch to EGL backend). The specifics of the launcher (which is using GL directly for a part of its rendering process) is that it will do SwapBuffers always after setting current context to NULL. This behaviour was once causing problems with GLX GL backend as well but was fixed by eb5993a7c6fbc1cd9deac0dceabc8f1c76e14ba8. Now with default EGL backend all the Wine logic related to SwapBuffers works, except for eglSwapBuffers() doesn't swap anything if there is no current context or current context is not the one which is associated with the surface. That is the same with Mesa and NVidia drivers. That is different from glxSwapBuffers which doesn't have any deal with the context. This patch makes the launcher render properly (while to avoid a crash in libcef that currently misses font files and can be worked around with 'winetricks corefonts'). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10033#note_128833
Thanks for finding this, quite an unfortunate discovery though :/ The fix doesn't look ideal, I'm not sure what we can do but it will probably require a bit more thoughts, see if we can do it differently. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10033#note_128847
What is the concern? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10033#note_128869
About the change here? Well it's not great that we have to create a context here, which is then racy, and dependent on EGL_NO_CONFIG_KHR support. It then assumes that surface isn't already current in a different thread (which is not allowed), and if eglSwapBuffers requires a context then using a different context than the one which was used to draw doesn't look right. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10033#note_128870
which is then racy
We can guard with InterlockedCompareExchange and free the extra context in the unlikely case it raced. I intentionally didn't consider creating that context at initialization (overall GL or drawable) as in the majority of cases it is not needed.
dependent on EGL_NO_CONFIG_KHR support.
Might avoid using it and select config for drawable format. That would imply having this backup context per drawable but probably not a big deal?
It then assumes that surface isn't already current in a different thread (which is not allowed)
Do you have any link / info about this part by any chance, what are the EGL rules here? We can probably track / check if the surface has active context attached in some thread and skip this path then.
a different context than the one which was used to draw doesn't look right.
Why? An app is free to switch the context any time and use different contexts to draw on a surface, and then select a different one any time, even if for SwapBuffers only. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10033#note_128871
Also, I think resetting context before SwapBuffers is is exceptionally rare usage. That is not an obvious move and largely falls into unspecified area, just happens to work reliably with WGL. Maybe in this case we'd want to prefer simplicity over covering all the potential corner cases? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10033#note_128872
participants (3)
-
Paul Gofman -
Paul Gofman (＠gofman)
-
Rémi Bernon (＠rbernon)