[PATCH v2 0/10] MR5177: winewayland.drv: part 13.1: Basic OpenGL support
This MR introduces support for OpenGL in the Wayland driver. Since the MR started to become somewhat long I have left some things for a subsequent part 13.2: * wgl(Get)SwapIntervalEXT (which means everything is vsync-ed at the moment) * wglShareLists * wglCreateContextAttribsARB * Fix for some apps appearing translucent in certain compositors (typically when fullscreen). Missing features not planned to be part of the 13.x MRs: * Offscreen rendering through pbuffers or pixmaps, since Wayland EGL doesn't support them (perhaps we can simulate with FBOs instead?). * Front buffer rendering. Again Wayland EGL doesn't support this, and certainly not in the form needed (i.e., the surface should be available for additional rendering with GDI). In the experimental branch I implemented a workaround that seemed to work for many cases, but will need to revisit and reevaluate options. IIRC, the most common case I have seen for this is when using wined3d/gl for older (Direct)2D games, so perhaps we can find a better solution at that layer. * Child window rendering (this needs infrastructure in Wayland driver core which will enable both GL and Vulkan rendering in child windows). * Cross-process rendering. Thanks! -- v2: winewayland.drv: Handle resizing of OpenGL content. winewayland.drv: Implement wglSwapBuffers. winewayland.drv: Implement wglMakeCurrent and wglMakeContextCurrentARB. winewayland.drv: Implement OpenGL context creation. winewayland.drv: Implement wglSetPixelFormat(WINE). winewayland.drv: Implement wglDescribePixelFormat. winewayland.drv: Implement wglGetProcAddress. winewayland.drv: Implement wglGetExtensionsString{ARB,EXT}. winewayland.drv: Initialize core GL functions. winewayland.drv: Add skeleton OpenGL driver. https://gitlab.winehq.org/wine/wine/-/merge_requests/5177
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> The driver performs basic EGL initialization. --- configure | 112 +++++++++++++++++ configure.ac | 11 ++ dlls/winewayland.drv/Makefile.in | 3 +- dlls/winewayland.drv/opengl.c | 163 +++++++++++++++++++++++++ dlls/winewayland.drv/waylanddrv.h | 1 + dlls/winewayland.drv/waylanddrv_main.c | 1 + 6 files changed, 290 insertions(+), 1 deletion(-) create mode 100644 dlls/winewayland.drv/opengl.c diff --git a/configure b/configure index 5e1b3765413..9366b94afb1 100755 --- a/configure +++ b/configure @@ -702,6 +702,8 @@ INOTIFY_LIBS INOTIFY_CFLAGS PCSCLITE_LIBS PCAP_LIBS +EGL_LIBS +EGL_CFLAGS XKBREGISTRY_LIBS XKBREGISTRY_CFLAGS XKBCOMMON_LIBS @@ -1803,6 +1805,8 @@ XKBCOMMON_CFLAGS XKBCOMMON_LIBS XKBREGISTRY_CFLAGS XKBREGISTRY_LIBS +EGL_CFLAGS +EGL_LIBS INOTIFY_CFLAGS INOTIFY_LIBS DBUS_CFLAGS @@ -2629,6 +2633,8 @@ Some influential environment variables: C compiler flags for xkbregistry, overriding pkg-config XKBREGISTRY_LIBS Linker flags for xkbregistry, overriding pkg-config + EGL_CFLAGS C compiler flags for egl, overriding pkg-config + EGL_LIBS Linker flags for egl, overriding pkg-config INOTIFY_CFLAGS C compiler flags for libinotify, overriding pkg-config INOTIFY_LIBS @@ -16137,6 +16143,110 @@ fi CPPFLAGS=$ac_save_CPPFLAGS + if test "x$with_opengl" != "xno" + then + rm -f conftest.err +if ${EGL_CFLAGS:+false} : +then : + if test ${PKG_CONFIG+y} +then : + EGL_CFLAGS=`$PKG_CONFIG --cflags egl 2>conftest.err` +fi +fi + +if ${EGL_LIBS:+false} : +then : + if test ${PKG_CONFIG+y} +then : + EGL_LIBS=`$PKG_CONFIG --libs egl 2>/dev/null` +fi +fi + +EGL_LIBS=${EGL_LIBS:-"-lEGL"} +printf "%s\n" "$as_me:${as_lineno-$LINENO}: egl cflags: $EGL_CFLAGS" >&5 +printf "%s\n" "$as_me:${as_lineno-$LINENO}: egl libs: $EGL_LIBS" >&5 +if test -s conftest.err; then + printf %s "$as_me:${as_lineno-$LINENO}: egl errors: " >&5 + cat conftest.err >&5 +fi +rm -f conftest.err +ac_save_CPPFLAGS=$CPPFLAGS +CPPFLAGS="$CPPFLAGS $EGL_CFLAGS" +ac_fn_c_check_header_compile "$LINENO" "EGL/egl.h" "ac_cv_header_EGL_egl_h" "$ac_includes_default" +if test "x$ac_cv_header_EGL_egl_h" = xyes +then : + { printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking for -lEGL" >&5 +printf %s "checking for -lEGL... " >&6; } +if test ${ac_cv_lib_soname_EGL+y} +then : + printf %s "(cached) " >&6 +else $as_nop + ac_check_soname_save_LIBS=$LIBS +LIBS="-lEGL $EGL_LIBS $LIBS" + cat confdefs.h - <<_ACEOF >conftest.$ac_ext +/* end confdefs.h. */ + +/* Override any GCC internal prototype to avoid an error. + Use char because int might match the return type of a GCC + builtin and then its argument prototype would still apply. */ +char eglGetProcAddress (); +int +main (void) +{ +return eglGetProcAddress (); + ; + return 0; +} +_ACEOF +if ac_fn_c_try_link "$LINENO" +then : + case "$LIBEXT" in + dll) ac_cv_lib_soname_EGL=`$ac_cv_path_LDD conftest.exe | grep "EGL" | sed -e "s/dll.*/dll/"';2,$d'` ;; + dylib) ac_cv_lib_soname_EGL=`$OTOOL -L conftest$ac_exeext | grep "libEGL\\.[0-9A-Za-z.]*dylib" | sed -e "s/^.*\/\(libEGL\.[0-9A-Za-z.]*dylib\).*$/\1/"';2,$d'` ;; + *) ac_cv_lib_soname_EGL=`$READELF -d conftest$ac_exeext | grep "NEEDED.*libEGL\\.$LIBEXT" | sed -e "s/^.*\\[\\(libEGL\\.$LIBEXT[^ ]*\\)\\].*$/\1/"';2,$d'` + if ${ac_cv_lib_soname_EGL:+false} : +then : + ac_cv_lib_soname_EGL=`$LDD conftest$ac_exeext | grep "libEGL\\.$LIBEXT" | sed -e "s/^.*\(libEGL\.$LIBEXT[^ ]*\).*$/\1/"';2,$d'` +fi ;; + esac +else $as_nop + ac_cv_lib_soname_EGL= +fi +rm -f core conftest.err conftest.$ac_objext conftest.beam \ + conftest$ac_exeext conftest.$ac_ext + LIBS=$ac_check_soname_save_LIBS +fi +if ${ac_cv_lib_soname_EGL:+false} : +then : + { printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: not found" >&5 +printf "%s\n" "not found" >&6; } + +else $as_nop + { printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: $ac_cv_lib_soname_EGL" >&5 +printf "%s\n" "$ac_cv_lib_soname_EGL" >&6; } + +printf "%s\n" "#define SONAME_LIBEGL \"$ac_cv_lib_soname_EGL\"" >>confdefs.h + + +fi +fi + +CPPFLAGS=$ac_save_CPPFLAGS + + if test "x$with_wayland" != "x" + then + if test -z "$SONAME_LIBEGL" +then : + case "x$with_opengl" in + x) as_fn_append wine_notices "|EGL ${notice_platform}development files not found, the Wayland driver won't support OpenGL" ;; + xno) ;; + *) as_fn_error $? "EGL ${notice_platform}development files not found, the Wayland driver won't support OpenGL +This is an error since --with-opengl was requested." "$LINENO" 5 ;; +esac + +fi + fi + fi fi if test -z "$WAYLAND_CLIENT_LIBS" -o -z "$WAYLAND_SCANNER" -o -z "$XKBCOMMON_LIBS" -o -z "$XKBREGISTRY_LIBS" -o "$ac_cv_header_linux_input_h" = "no" then : @@ -23738,6 +23848,8 @@ XKBCOMMON_CFLAGS = $XKBCOMMON_CFLAGS XKBCOMMON_LIBS = $XKBCOMMON_LIBS XKBREGISTRY_CFLAGS = $XKBREGISTRY_CFLAGS XKBREGISTRY_LIBS = $XKBREGISTRY_LIBS +EGL_CFLAGS = $EGL_CFLAGS +EGL_LIBS = $EGL_LIBS PCAP_LIBS = $PCAP_LIBS PCSCLITE_LIBS = $PCSCLITE_LIBS INOTIFY_CFLAGS = $INOTIFY_CFLAGS diff --git a/configure.ac b/configure.ac index 1c59651ba1e..19ce0748d7b 100644 --- a/configure.ac +++ b/configure.ac @@ -1377,6 +1377,17 @@ then WINE_PACKAGE_FLAGS(XKBREGISTRY,[xkbregistry],,,, [AC_CHECK_HEADERS([xkbcommon/xkbregistry.h]) AC_CHECK_LIB(xkbregistry,rxkb_context_new,[:],[XKBREGISTRY_LIBS=""],[$XKBREGISTRY_LIBS])]) + if test "x$with_opengl" != "xno" + then + WINE_PACKAGE_FLAGS(EGL,[egl],[-lEGL],,, + [AC_CHECK_HEADER([EGL/egl.h], + [WINE_CHECK_SONAME(EGL,eglGetProcAddress,,,[$EGL_LIBS])])]) + if test "x$with_wayland" != "x" + then + WINE_NOTICE_WITH(opengl, [test -z "$SONAME_LIBEGL"], + [EGL ${notice_platform}development files not found, the Wayland driver won't support OpenGL]) + fi + fi fi WINE_NOTICE_WITH(wayland, [test -z "$WAYLAND_CLIENT_LIBS" -o -z "$WAYLAND_SCANNER" -o -z "$XKBCOMMON_LIBS" -o -z "$XKBREGISTRY_LIBS" -o "$ac_cv_header_linux_input_h" = "no"], [Wayland ${notice_platform}development files not found, the Wayland driver won't be supported.], diff --git a/dlls/winewayland.drv/Makefile.in b/dlls/winewayland.drv/Makefile.in index b47bdb262c0..9f9b8909c41 100644 --- a/dlls/winewayland.drv/Makefile.in +++ b/dlls/winewayland.drv/Makefile.in @@ -1,11 +1,12 @@ MODULE = winewayland.drv UNIXLIB = winewayland.so -UNIX_CFLAGS = $(WAYLAND_CLIENT_CFLAGS) $(XKBCOMMON_CFLAGS) $(XKBREGISTRY_CFLAGS) +UNIX_CFLAGS = $(EGL_CFLAGS) $(WAYLAND_CLIENT_CFLAGS) $(XKBCOMMON_CFLAGS) $(XKBREGISTRY_CFLAGS) UNIX_LIBS = -lwin32u $(WAYLAND_CLIENT_LIBS) $(XKBCOMMON_LIBS) $(XKBREGISTRY_LIBS) $(PTHREAD_LIBS) -lm SOURCES = \ display.c \ dllmain.c \ + opengl.c \ pointer-constraints-unstable-v1.xml \ relative-pointer-unstable-v1.xml \ version.rc \ diff --git a/dlls/winewayland.drv/opengl.c b/dlls/winewayland.drv/opengl.c new file mode 100644 index 00000000000..a4b92233cc8 --- /dev/null +++ b/dlls/winewayland.drv/opengl.c @@ -0,0 +1,163 @@ +/* + * Wayland OpenGL functions + * + * Copyright 2020 Alexandros Frantzis for Collabora Ltd. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#if 0 +#pragma makedep unix +#endif + +#include "config.h" + +#include <dlfcn.h> +#include <string.h> + +#include "waylanddrv.h" +#include "wine/debug.h" + +#if defined(SONAME_LIBEGL) + +WINE_DEFAULT_DEBUG_CHANNEL(waylanddrv); + +#define WL_EGL_PLATFORM 1 +#include <EGL/egl.h> +#include <EGL/eglext.h> + +#include "wine/wgl.h" +#include "wine/wgl_driver.h" + +static void *egl_handle; +static struct opengl_funcs opengl_funcs; +static EGLDisplay egl_display; + +#define DECL_FUNCPTR(f) static __typeof__(f) * p_##f = NULL +DECL_FUNCPTR(eglGetError); +DECL_FUNCPTR(eglGetPlatformDisplay); +DECL_FUNCPTR(eglGetProcAddress); +DECL_FUNCPTR(eglInitialize); +DECL_FUNCPTR(eglQueryString); +#undef DECL_FUNCPTR + +static BOOL has_extension(const char *list, const char *ext) +{ + size_t len = strlen(ext); + const char *cur = list; + + while (cur && (cur = strstr(cur, ext))) + { + if ((!cur[len] || cur[len] == ' ') && (cur == list || cur[-1] == ' ')) + return TRUE; + cur = strchr(cur, ' '); + } + + return FALSE; +} + +static void init_opengl(void) +{ + EGLint egl_version[2]; + const char *egl_client_exts; + + if (!(egl_handle = dlopen(SONAME_LIBEGL, RTLD_NOW|RTLD_GLOBAL))) + { + ERR("Failed to load %s: %s\n", SONAME_LIBEGL, dlerror()); + return; + } + +#define LOAD_FUNCPTR_DLSYM(func) \ + do { \ + if (!(p_##func = dlsym(egl_handle, #func))) \ + { ERR("Failed to load symbol %s\n", #func); goto err; } \ + } while(0) + LOAD_FUNCPTR_DLSYM(eglGetProcAddress); + LOAD_FUNCPTR_DLSYM(eglQueryString); +#undef LOAD_FUNCPTR_DLSYM + + egl_client_exts = p_eglQueryString(EGL_NO_DISPLAY, EGL_EXTENSIONS); + +#define REQUIRE_CLIENT_EXT(ext) \ + do { \ + if (!has_extension(egl_client_exts, #ext)) \ + { ERR("Failed to find required extension %s\n", #ext); goto err; } \ + } while(0) + REQUIRE_CLIENT_EXT(EGL_KHR_client_get_all_proc_addresses); + REQUIRE_CLIENT_EXT(EGL_KHR_platform_wayland); +#undef REQUIRE_CLIENT_EXT + +#define LOAD_FUNCPTR_EGL(func) \ + do { \ + if (!(p_##func = (__typeof__(func) *)p_eglGetProcAddress(#func))) \ + { ERR("Failed to load symbol %s\n", #func); goto err; } \ + } while(0) + LOAD_FUNCPTR_EGL(eglGetError); + LOAD_FUNCPTR_EGL(eglGetPlatformDisplay); + LOAD_FUNCPTR_EGL(eglInitialize); +#undef LOAD_FUNCPTR_EGL + + egl_display = p_eglGetPlatformDisplay(EGL_PLATFORM_WAYLAND_KHR, + process_wayland.wl_display, + NULL); + if (egl_display == EGL_NO_DISPLAY) + { + ERR("Failed to get EGLDisplay\n"); + goto err; + } + if (!p_eglInitialize(egl_display, &egl_version[0], &egl_version[1])) + { + ERR("Failed to initialized EGLDisplay with error %d\n", p_eglGetError()); + goto err; + } + TRACE("EGL version %u.%u\n", egl_version[0], egl_version[1]); + + return; + +err: + dlclose(egl_handle); + egl_handle = NULL; +} + +static BOOL has_opengl(void) +{ + static pthread_once_t init_once = PTHREAD_ONCE_INIT; + + return !pthread_once(&init_once, init_opengl) && egl_handle; +} + +/********************************************************************** + * WAYLAND_wine_get_wgl_driver + */ +struct opengl_funcs *WAYLAND_wine_get_wgl_driver(UINT version) +{ + if (version != WINE_WGL_DRIVER_VERSION) + { + ERR("Version mismatch, opengl32 wants %u but driver has %u\n", + version, WINE_WGL_DRIVER_VERSION); + return NULL; + } + if (!has_opengl()) return NULL; + return &opengl_funcs; +} + +#else /* No GL */ + +struct opengl_funcs *WAYLAND_wine_get_wgl_driver(UINT version) +{ + return NULL; +} + +#endif diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index f030f6fc6a0..66806503edf 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -332,5 +332,6 @@ BOOL WAYLAND_WindowPosChanging(HWND hwnd, HWND insert_after, UINT swp_flags, const RECT *window_rect, const RECT *client_rect, RECT *visible_rect, struct window_surface **surface); const struct vulkan_funcs *WAYLAND_wine_get_vulkan_driver(UINT version); +struct opengl_funcs *WAYLAND_wine_get_wgl_driver(UINT version); #endif /* __WINE_WAYLANDDRV_H */ diff --git a/dlls/winewayland.drv/waylanddrv_main.c b/dlls/winewayland.drv/waylanddrv_main.c index b60d282aacb..ca73cd4c97d 100644 --- a/dlls/winewayland.drv/waylanddrv_main.c +++ b/dlls/winewayland.drv/waylanddrv_main.c @@ -43,6 +43,7 @@ static const struct user_driver_funcs waylanddrv_funcs = .pWindowPosChanged = WAYLAND_WindowPosChanged, .pWindowPosChanging = WAYLAND_WindowPosChanging, .pwine_get_vulkan_driver = WAYLAND_wine_get_vulkan_driver, + .pwine_get_wgl_driver = WAYLAND_wine_get_wgl_driver, }; static NTSTATUS waylanddrv_unix_init(void *arg) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5177
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> --- configure | 2 +- configure.ac | 2 +- dlls/winewayland.drv/opengl.c | 22 ++++++++++++++++++++++ 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/configure b/configure index 9366b94afb1..aa6e9adac06 100755 --- a/configure +++ b/configure @@ -16235,7 +16235,7 @@ CPPFLAGS=$ac_save_CPPFLAGS if test "x$with_wayland" != "x" then - if test -z "$SONAME_LIBEGL" + if test -z "$ac_cv_lib_soname_EGL" then : case "x$with_opengl" in x) as_fn_append wine_notices "|EGL ${notice_platform}development files not found, the Wayland driver won't support OpenGL" ;; diff --git a/configure.ac b/configure.ac index 19ce0748d7b..45af0d31daa 100644 --- a/configure.ac +++ b/configure.ac @@ -1384,7 +1384,7 @@ then [WINE_CHECK_SONAME(EGL,eglGetProcAddress,,,[$EGL_LIBS])])]) if test "x$with_wayland" != "x" then - WINE_NOTICE_WITH(opengl, [test -z "$SONAME_LIBEGL"], + WINE_NOTICE_WITH(opengl, [test -z "$ac_cv_lib_soname_EGL"], [EGL ${notice_platform}development files not found, the Wayland driver won't support OpenGL]) fi fi diff --git a/dlls/winewayland.drv/opengl.c b/dlls/winewayland.drv/opengl.c index a4b92233cc8..0b09c62564d 100644 --- a/dlls/winewayland.drv/opengl.c +++ b/dlls/winewayland.drv/opengl.c @@ -45,6 +45,10 @@ static void *egl_handle; static struct opengl_funcs opengl_funcs; static EGLDisplay egl_display; +#define USE_GL_FUNC(name) #name, +static const char *opengl_func_names[] = { ALL_WGL_FUNCS }; +#undef USE_GL_FUNC + #define DECL_FUNCPTR(f) static __typeof__(f) * p_##f = NULL DECL_FUNCPTR(eglGetError); DECL_FUNCPTR(eglGetPlatformDisplay); @@ -68,6 +72,22 @@ static BOOL has_extension(const char *list, const char *ext) return FALSE; } +static BOOL init_opengl_funcs(void) +{ + unsigned int i; + + for (i = 0; i < ARRAY_SIZE(opengl_func_names); i++) + { + if (!(((void **)&opengl_funcs.gl)[i] = p_eglGetProcAddress(opengl_func_names[i]))) + { + ERR("%s not found, disabling OpenGL.\n", opengl_func_names[i]); + return FALSE; + } + } + + return TRUE; +} + static void init_opengl(void) { EGLint egl_version[2]; @@ -124,6 +144,8 @@ static void init_opengl(void) } TRACE("EGL version %u.%u\n", egl_version[0], egl_version[1]); + if (!init_opengl_funcs()) goto err; + return; err: -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5177
@rbernon v2: * Require `EGL_KHR_client_get_all_proc_addresses` and use `eglGetProcAddress` for all EGL and GL symbols. * Don't load libOpenGL explicitly (due to point above). * Require `EGL_KHR_platform_wayland` and use `eglGetPlatformDisplay` instead of `eglGetDisplay`. * Require `EGL_KHR_no_config_context` and create config-less EGL contexts. * Ensure we are telling EGL to use the Wayland native platform types (avoids some casts). * Misc. smaller fixes as noted in review comments. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5177#note_63040
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> --- dlls/winewayland.drv/opengl.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/dlls/winewayland.drv/opengl.c b/dlls/winewayland.drv/opengl.c index 0b09c62564d..9873f0345e1 100644 --- a/dlls/winewayland.drv/opengl.c +++ b/dlls/winewayland.drv/opengl.c @@ -44,6 +44,7 @@ WINE_DEFAULT_DEBUG_CHANNEL(waylanddrv); static void *egl_handle; static struct opengl_funcs opengl_funcs; static EGLDisplay egl_display; +static char wgl_extensions[4096]; #define USE_GL_FUNC(name) #name, static const char *opengl_func_names[] = { ALL_WGL_FUNCS }; @@ -57,6 +58,18 @@ DECL_FUNCPTR(eglInitialize); DECL_FUNCPTR(eglQueryString); #undef DECL_FUNCPTR +static const char *wayland_wglGetExtensionsStringARB(HDC hdc) +{ + TRACE("() returning \"%s\"\n", wgl_extensions); + return wgl_extensions; +} + +static const char *wayland_wglGetExtensionsStringEXT(void) +{ + TRACE("() returning \"%s\"\n", wgl_extensions); + return wgl_extensions; +} + static BOOL has_extension(const char *list, const char *ext) { size_t len = strlen(ext); @@ -72,6 +85,13 @@ static BOOL has_extension(const char *list, const char *ext) return FALSE; } +static void register_extension(const char *ext) +{ + if (wgl_extensions[0]) strcat(wgl_extensions, " "); + strcat(wgl_extensions, ext); + TRACE("%s\n", ext); +} + static BOOL init_opengl_funcs(void) { unsigned int i; @@ -85,6 +105,12 @@ static BOOL init_opengl_funcs(void) } } + register_extension("WGL_ARB_extensions_string"); + opengl_funcs.ext.p_wglGetExtensionsStringARB = wayland_wglGetExtensionsStringARB; + + register_extension("WGL_EXT_extensions_string"); + opengl_funcs.ext.p_wglGetExtensionsStringEXT = wayland_wglGetExtensionsStringEXT; + return TRUE; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5177
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> --- dlls/winewayland.drv/opengl.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/dlls/winewayland.drv/opengl.c b/dlls/winewayland.drv/opengl.c index 9873f0345e1..6eb97172531 100644 --- a/dlls/winewayland.drv/opengl.c +++ b/dlls/winewayland.drv/opengl.c @@ -70,6 +70,12 @@ static const char *wayland_wglGetExtensionsStringEXT(void) return wgl_extensions; } +static PROC wayland_wglGetProcAddress(LPCSTR name) +{ + if (!strncmp(name, "wgl", 3)) return NULL; + return (PROC)p_eglGetProcAddress(name); +} + static BOOL has_extension(const char *list, const char *ext) { size_t len = strlen(ext); @@ -186,6 +192,14 @@ static BOOL has_opengl(void) return !pthread_once(&init_once, init_opengl) && egl_handle; } +static struct opengl_funcs opengl_funcs = +{ + .wgl = + { + .p_wglGetProcAddress = wayland_wglGetProcAddress, + } +}; + /********************************************************************** * WAYLAND_wine_get_wgl_driver */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5177
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> Create an array of supported EGLConfigs, and use this information to populate the PIXELFORMATDESCRIPTOR. --- dlls/winewayland.drv/opengl.c | 118 ++++++++++++++++++++++++++++++++++ 1 file changed, 118 insertions(+) diff --git a/dlls/winewayland.drv/opengl.c b/dlls/winewayland.drv/opengl.c index 6eb97172531..7b7c21af05f 100644 --- a/dlls/winewayland.drv/opengl.c +++ b/dlls/winewayland.drv/opengl.c @@ -25,6 +25,7 @@ #include "config.h" #include <dlfcn.h> +#include <stdlib.h> #include <string.h> #include "waylanddrv.h" @@ -45,12 +46,16 @@ static void *egl_handle; static struct opengl_funcs opengl_funcs; static EGLDisplay egl_display; static char wgl_extensions[4096]; +static EGLConfig *egl_configs; +static int num_egl_configs; #define USE_GL_FUNC(name) #name, static const char *opengl_func_names[] = { ALL_WGL_FUNCS }; #undef USE_GL_FUNC #define DECL_FUNCPTR(f) static __typeof__(f) * p_##f = NULL +DECL_FUNCPTR(eglChooseConfig); +DECL_FUNCPTR(eglGetConfigAttrib); DECL_FUNCPTR(eglGetError); DECL_FUNCPTR(eglGetPlatformDisplay); DECL_FUNCPTR(eglGetProcAddress); @@ -58,6 +63,61 @@ DECL_FUNCPTR(eglInitialize); DECL_FUNCPTR(eglQueryString); #undef DECL_FUNCPTR +static BOOL has_opengl(void); + +static int wayland_wglDescribePixelFormat(HDC hdc, int fmt, UINT size, + PIXELFORMATDESCRIPTOR *pfd) +{ + EGLint val; + EGLConfig config; + + if (!has_opengl()) return 0; + if (!pfd) return num_egl_configs; + if (size < sizeof(*pfd)) return 0; + if (fmt <= 0 || fmt > num_egl_configs) return 0; + + config = egl_configs[fmt - 1]; + + memset(pfd, 0, sizeof(*pfd)); + pfd->nSize = sizeof(*pfd); + pfd->nVersion = 1; + pfd->dwFlags = PFD_SUPPORT_OPENGL | PFD_DRAW_TO_WINDOW | PFD_DOUBLEBUFFER | + PFD_SUPPORT_COMPOSITION; + pfd->iPixelType = PFD_TYPE_RGBA; + pfd->iLayerType = PFD_MAIN_PLANE; + + p_eglGetConfigAttrib(egl_display, config, EGL_RED_SIZE, &val); + pfd->cRedBits = val; + p_eglGetConfigAttrib(egl_display, config, EGL_GREEN_SIZE, &val); + pfd->cGreenBits = val; + p_eglGetConfigAttrib(egl_display, config, EGL_BLUE_SIZE, &val); + pfd->cBlueBits = val; + p_eglGetConfigAttrib(egl_display, config, EGL_ALPHA_SIZE, &val); + pfd->cAlphaBits = val; + p_eglGetConfigAttrib(egl_display, config, EGL_DEPTH_SIZE, &val); + pfd->cDepthBits = val; + p_eglGetConfigAttrib(egl_display, config, EGL_STENCIL_SIZE, &val); + pfd->cStencilBits = val; + /* cColorBits bits exclude alpha */ + pfd->cColorBits = pfd->cRedBits + pfd->cGreenBits + pfd->cBlueBits; + + /* Although we don't get information from EGL about the component shifts + * or the native format, the 0xARGB order is the most common. */ + pfd->cBlueShift = 0; + pfd->cGreenShift = pfd->cBlueBits; + pfd->cRedShift = pfd->cGreenBits + pfd->cBlueBits; + if (pfd->cAlphaBits) + pfd->cAlphaShift = pfd->cRedBits + pfd->cGreenBits + pfd->cBlueBits; + else + pfd->cAlphaShift = 0; + + TRACE("fmt %u color %u %u/%u/%u/%u depth %u stencil %u\n", + fmt, pfd->cColorBits, pfd->cRedBits, pfd->cGreenBits, pfd->cBlueBits, + pfd->cAlphaBits, pfd->cDepthBits, pfd->cStencilBits); + + return num_egl_configs; +} + static const char *wayland_wglGetExtensionsStringARB(HDC hdc) { TRACE("() returning \"%s\"\n", wgl_extensions); @@ -120,6 +180,60 @@ static BOOL init_opengl_funcs(void) return TRUE; } +static BOOL init_egl_configs(void) +{ + EGLint i; + const EGLint attribs[] = + { + EGL_RENDERABLE_TYPE, EGL_OPENGL_BIT, + EGL_SURFACE_TYPE, EGL_WINDOW_BIT, + EGL_NONE + }; + + p_eglChooseConfig(egl_display, attribs, NULL, 0, &num_egl_configs); + if (!(egl_configs = malloc(num_egl_configs * sizeof(*egl_configs)))) + { + ERR("Failed to allocate memory for EGL configs\n"); + return FALSE; + } + if (!p_eglChooseConfig(egl_display, attribs, egl_configs, num_egl_configs, + &num_egl_configs) || + !num_egl_configs) + { + free(egl_configs); + egl_configs = NULL; + num_egl_configs = 0; + ERR("Failed to get any configs from eglChooseConfig\n"); + return FALSE; + } + + if (TRACE_ON(waylanddrv)) + { + for (i = 0; i < num_egl_configs; i++) + { + EGLint id, type, visual_id, native, render, color, r, g, b, a, d, s; + p_eglGetConfigAttrib(egl_display, egl_configs[i], EGL_NATIVE_VISUAL_ID, &visual_id); + p_eglGetConfigAttrib(egl_display, egl_configs[i], EGL_SURFACE_TYPE, &type); + p_eglGetConfigAttrib(egl_display, egl_configs[i], EGL_RENDERABLE_TYPE, &render); + p_eglGetConfigAttrib(egl_display, egl_configs[i], EGL_CONFIG_ID, &id); + p_eglGetConfigAttrib(egl_display, egl_configs[i], EGL_NATIVE_RENDERABLE, &native); + p_eglGetConfigAttrib(egl_display, egl_configs[i], EGL_COLOR_BUFFER_TYPE, &color); + p_eglGetConfigAttrib(egl_display, egl_configs[i], EGL_RED_SIZE, &r); + p_eglGetConfigAttrib(egl_display, egl_configs[i], EGL_GREEN_SIZE, &g); + p_eglGetConfigAttrib(egl_display, egl_configs[i], EGL_BLUE_SIZE, &b); + p_eglGetConfigAttrib(egl_display, egl_configs[i], EGL_ALPHA_SIZE, &a); + p_eglGetConfigAttrib(egl_display, egl_configs[i], EGL_DEPTH_SIZE, &d); + p_eglGetConfigAttrib(egl_display, egl_configs[i], EGL_STENCIL_SIZE, &s); + TRACE("%u: config %d id %d type %x visual %d native %d render %x " + "colortype %d rgba %d,%d,%d,%d depth %u stencil %d\n", + num_egl_configs, i, id, type, visual_id, native, render, + color, r, g, b, a, d, s); + } + } + + return TRUE; +} + static void init_opengl(void) { EGLint egl_version[2]; @@ -156,6 +270,8 @@ static void init_opengl(void) if (!(p_##func = (__typeof__(func) *)p_eglGetProcAddress(#func))) \ { ERR("Failed to load symbol %s\n", #func); goto err; } \ } while(0) + LOAD_FUNCPTR_EGL(eglChooseConfig); + LOAD_FUNCPTR_EGL(eglGetConfigAttrib); LOAD_FUNCPTR_EGL(eglGetError); LOAD_FUNCPTR_EGL(eglGetPlatformDisplay); LOAD_FUNCPTR_EGL(eglInitialize); @@ -177,6 +293,7 @@ static void init_opengl(void) TRACE("EGL version %u.%u\n", egl_version[0], egl_version[1]); if (!init_opengl_funcs()) goto err; + if (!init_egl_configs()) goto err; return; @@ -196,6 +313,7 @@ static struct opengl_funcs opengl_funcs = { .wgl = { + .p_wglDescribePixelFormat = wayland_wglDescribePixelFormat, .p_wglGetProcAddress = wayland_wglGetProcAddress, } }; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5177
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> Introduce the internal wayland_gl_drawable object, which associates a window (and its backing Wayland surface) with an EGL surface. --- configure | 90 ++++++++++++++- configure.ac | 7 +- dlls/winewayland.drv/Makefile.in | 4 +- dlls/winewayland.drv/opengl.c | 182 +++++++++++++++++++++++++++++- dlls/winewayland.drv/waylanddrv.h | 6 + dlls/winewayland.drv/window.c | 1 + include/config.h.in | 3 + 7 files changed, 287 insertions(+), 6 deletions(-) diff --git a/configure b/configure index aa6e9adac06..f71b844e46d 100755 --- a/configure +++ b/configure @@ -702,6 +702,8 @@ INOTIFY_LIBS INOTIFY_CFLAGS PCSCLITE_LIBS PCAP_LIBS +WAYLAND_EGL_LIBS +WAYLAND_EGL_CFLAGS EGL_LIBS EGL_CFLAGS XKBREGISTRY_LIBS @@ -1807,6 +1809,8 @@ XKBREGISTRY_CFLAGS XKBREGISTRY_LIBS EGL_CFLAGS EGL_LIBS +WAYLAND_EGL_CFLAGS +WAYLAND_EGL_LIBS INOTIFY_CFLAGS INOTIFY_LIBS DBUS_CFLAGS @@ -2635,6 +2639,10 @@ Some influential environment variables: Linker flags for xkbregistry, overriding pkg-config EGL_CFLAGS C compiler flags for egl, overriding pkg-config EGL_LIBS Linker flags for egl, overriding pkg-config + WAYLAND_EGL_CFLAGS + C compiler flags for wayland-egl, overriding pkg-config + WAYLAND_EGL_LIBS + Linker flags for wayland-egl, overriding pkg-config INOTIFY_CFLAGS C compiler flags for libinotify, overriding pkg-config INOTIFY_LIBS @@ -16231,11 +16239,89 @@ printf "%s\n" "#define SONAME_LIBEGL \"$ac_cv_lib_soname_EGL\"" >>confdefs.h fi fi +CPPFLAGS=$ac_save_CPPFLAGS + + rm -f conftest.err +if ${WAYLAND_EGL_CFLAGS:+false} : +then : + if test ${PKG_CONFIG+y} +then : + WAYLAND_EGL_CFLAGS=`$PKG_CONFIG --cflags wayland-egl 2>conftest.err` +fi +fi + +if ${WAYLAND_EGL_LIBS:+false} : +then : + if test ${PKG_CONFIG+y} +then : + WAYLAND_EGL_LIBS=`$PKG_CONFIG --libs wayland-egl 2>/dev/null` +fi +fi + + +printf "%s\n" "$as_me:${as_lineno-$LINENO}: wayland-egl cflags: $WAYLAND_EGL_CFLAGS" >&5 +printf "%s\n" "$as_me:${as_lineno-$LINENO}: wayland-egl libs: $WAYLAND_EGL_LIBS" >&5 +if test -s conftest.err; then + printf %s "$as_me:${as_lineno-$LINENO}: wayland-egl errors: " >&5 + cat conftest.err >&5 +fi +rm -f conftest.err +ac_save_CPPFLAGS=$CPPFLAGS +CPPFLAGS="$CPPFLAGS $WAYLAND_EGL_CFLAGS" +ac_fn_c_check_header_compile "$LINENO" "wayland-egl.h" "ac_cv_header_wayland_egl_h" "$ac_includes_default" +if test "x$ac_cv_header_wayland_egl_h" = xyes +then : + { printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking for wl_egl_window_create in -lwayland-egl" >&5 +printf %s "checking for wl_egl_window_create in -lwayland-egl... " >&6; } +if test ${ac_cv_lib_wayland_egl_wl_egl_window_create+y} +then : + printf %s "(cached) " >&6 +else $as_nop + ac_check_lib_save_LIBS=$LIBS +LIBS="-lwayland-egl $WAYLAND_EGL_LIBS $LIBS" +cat confdefs.h - <<_ACEOF >conftest.$ac_ext +/* end confdefs.h. */ + +/* Override any GCC internal prototype to avoid an error. + Use char because int might match the return type of a GCC + builtin and then its argument prototype would still apply. */ +char wl_egl_window_create (); +int +main (void) +{ +return wl_egl_window_create (); + ; + return 0; +} +_ACEOF +if ac_fn_c_try_link "$LINENO" +then : + ac_cv_lib_wayland_egl_wl_egl_window_create=yes +else $as_nop + ac_cv_lib_wayland_egl_wl_egl_window_create=no +fi +rm -f core conftest.err conftest.$ac_objext conftest.beam \ + conftest$ac_exeext conftest.$ac_ext +LIBS=$ac_check_lib_save_LIBS +fi +{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: $ac_cv_lib_wayland_egl_wl_egl_window_create" >&5 +printf "%s\n" "$ac_cv_lib_wayland_egl_wl_egl_window_create" >&6; } +if test "x$ac_cv_lib_wayland_egl_wl_egl_window_create" = xyes +then : + +printf "%s\n" "#define HAVE_LIBWAYLAND_EGL 1" >>confdefs.h + +else $as_nop + WAYLAND_EGL_LIBS="" +fi + +fi + CPPFLAGS=$ac_save_CPPFLAGS if test "x$with_wayland" != "x" then - if test -z "$ac_cv_lib_soname_EGL" + if test -z "$ac_cv_lib_soname_EGL" -o -z "$HAVE_LIBWAYLAND_EGL" then : case "x$with_opengl" in x) as_fn_append wine_notices "|EGL ${notice_platform}development files not found, the Wayland driver won't support OpenGL" ;; @@ -23850,6 +23936,8 @@ XKBREGISTRY_CFLAGS = $XKBREGISTRY_CFLAGS XKBREGISTRY_LIBS = $XKBREGISTRY_LIBS EGL_CFLAGS = $EGL_CFLAGS EGL_LIBS = $EGL_LIBS +WAYLAND_EGL_CFLAGS = $WAYLAND_EGL_CFLAGS +WAYLAND_EGL_LIBS = $WAYLAND_EGL_LIBS PCAP_LIBS = $PCAP_LIBS PCSCLITE_LIBS = $PCSCLITE_LIBS INOTIFY_CFLAGS = $INOTIFY_CFLAGS diff --git a/configure.ac b/configure.ac index 45af0d31daa..ebbfa764e74 100644 --- a/configure.ac +++ b/configure.ac @@ -1382,9 +1382,14 @@ then WINE_PACKAGE_FLAGS(EGL,[egl],[-lEGL],,, [AC_CHECK_HEADER([EGL/egl.h], [WINE_CHECK_SONAME(EGL,eglGetProcAddress,,,[$EGL_LIBS])])]) + WINE_PACKAGE_FLAGS(WAYLAND_EGL,[wayland-egl],,,, + [AC_CHECK_HEADER([wayland-egl.h], + [AC_CHECK_LIB(wayland-egl,wl_egl_window_create, + [AC_DEFINE(HAVE_LIBWAYLAND_EGL, 1, [Define if we have the wayland-egl development environment])], + [WAYLAND_EGL_LIBS=""],[$WAYLAND_EGL_LIBS])])]) if test "x$with_wayland" != "x" then - WINE_NOTICE_WITH(opengl, [test -z "$ac_cv_lib_soname_EGL"], + WINE_NOTICE_WITH(opengl, [test -z "$ac_cv_lib_soname_EGL" -o -z "$HAVE_LIBWAYLAND_EGL"], [EGL ${notice_platform}development files not found, the Wayland driver won't support OpenGL]) fi fi diff --git a/dlls/winewayland.drv/Makefile.in b/dlls/winewayland.drv/Makefile.in index 9f9b8909c41..9ad1ad6889d 100644 --- a/dlls/winewayland.drv/Makefile.in +++ b/dlls/winewayland.drv/Makefile.in @@ -1,7 +1,7 @@ MODULE = winewayland.drv UNIXLIB = winewayland.so -UNIX_CFLAGS = $(EGL_CFLAGS) $(WAYLAND_CLIENT_CFLAGS) $(XKBCOMMON_CFLAGS) $(XKBREGISTRY_CFLAGS) -UNIX_LIBS = -lwin32u $(WAYLAND_CLIENT_LIBS) $(XKBCOMMON_LIBS) $(XKBREGISTRY_LIBS) $(PTHREAD_LIBS) -lm +UNIX_CFLAGS = $(EGL_CFLAGS) $(WAYLAND_CLIENT_CFLAGS) $(WAYLAND_EGL_CFLAGS) $(XKBCOMMON_CFLAGS) $(XKBREGISTRY_CFLAGS) +UNIX_LIBS = -lwin32u $(WAYLAND_CLIENT_LIBS) $(WAYLAND_EGL_LIBS) $(XKBCOMMON_LIBS) $(XKBREGISTRY_LIBS) $(PTHREAD_LIBS) -lm SOURCES = \ display.c \ diff --git a/dlls/winewayland.drv/opengl.c b/dlls/winewayland.drv/opengl.c index 7b7c21af05f..c54deabf8ff 100644 --- a/dlls/winewayland.drv/opengl.c +++ b/dlls/winewayland.drv/opengl.c @@ -31,11 +31,11 @@ #include "waylanddrv.h" #include "wine/debug.h" -#if defined(SONAME_LIBEGL) +#if defined(SONAME_LIBEGL) && defined(HAVE_LIBWAYLAND_EGL) WINE_DEFAULT_DEBUG_CHANNEL(waylanddrv); -#define WL_EGL_PLATFORM 1 +#include <wayland-egl.h> #include <EGL/egl.h> #include <EGL/eglext.h> @@ -55,6 +55,8 @@ static const char *opengl_func_names[] = { ALL_WGL_FUNCS }; #define DECL_FUNCPTR(f) static __typeof__(f) * p_##f = NULL DECL_FUNCPTR(eglChooseConfig); +DECL_FUNCPTR(eglCreateWindowSurface); +DECL_FUNCPTR(eglDestroySurface); DECL_FUNCPTR(eglGetConfigAttrib); DECL_FUNCPTR(eglGetError); DECL_FUNCPTR(eglGetPlatformDisplay); @@ -63,6 +65,153 @@ DECL_FUNCPTR(eglInitialize); DECL_FUNCPTR(eglQueryString); #undef DECL_FUNCPTR +static pthread_mutex_t gl_object_mutex = PTHREAD_MUTEX_INITIALIZER; +static struct list gl_drawables = LIST_INIT(gl_drawables); + +struct wayland_gl_drawable +{ + struct list entry; + LONG ref; + HWND hwnd; + struct wayland_client_surface *client; + struct wl_egl_window *wl_egl_window; + EGLSurface surface; +}; + +static void wayland_gl_drawable_release(struct wayland_gl_drawable *gl) +{ + if (InterlockedDecrement(&gl->ref)) return; + if (gl->surface) p_eglDestroySurface(egl_display, gl->surface); + if (gl->wl_egl_window) wl_egl_window_destroy(gl->wl_egl_window); + if (gl->client) + { + HWND hwnd = wl_surface_get_user_data(gl->client->wl_surface); + struct wayland_surface *wayland_surface = wayland_surface_lock_hwnd(hwnd); + + if (wayland_client_surface_release(gl->client) && wayland_surface) + wayland_surface->client = NULL; + + if (wayland_surface) pthread_mutex_unlock(&wayland_surface->mutex); + } + + free(gl); +} + +static struct wayland_gl_drawable *wayland_gl_drawable_create(HWND hwnd, int format) +{ + struct wayland_gl_drawable *gl; + struct wayland_surface *wayland_surface; + int client_width = 0, client_height = 0; + + TRACE("hwnd=%p format=%d\n", hwnd, format); + + gl = calloc(1, sizeof(*gl)); + if (!gl) return NULL; + + gl->ref = 1; + gl->hwnd = hwnd; + + /* Get the client surface for the HWND. If don't have a wayland surface + * (e.g., HWND_MESSAGE windows) just create a dummy surface to act as the + * target render surface. */ + if ((wayland_surface = wayland_surface_lock_hwnd(hwnd))) + { + gl->client = wayland_surface_get_client(wayland_surface); + client_width = wayland_surface->window.client_rect.right - + wayland_surface->window.client_rect.left; + client_height = wayland_surface->window.client_rect.bottom - + wayland_surface->window.client_rect.top; + if (client_width == 0 || client_height == 0) + client_width = client_height = 1; + pthread_mutex_unlock(&wayland_surface->mutex); + } + else if ((wayland_surface = wayland_surface_create(0))) + { + gl->client = wayland_surface_get_client(wayland_surface); + client_width = client_height = 1; + /* It's fine to destroy the wayland surface, the client surface + * can safely outlive it. */ + wayland_surface_destroy(wayland_surface); + } + if (!gl->client) goto err; + + gl->wl_egl_window = wl_egl_window_create(gl->client->wl_surface, + client_width, client_height); + if (!gl->wl_egl_window) + { + ERR("Failed to create wl_egl_window\n"); + goto err; + } + + gl->surface = p_eglCreateWindowSurface(egl_display, egl_configs[format - 1], + gl->wl_egl_window, NULL); + if (!gl->surface) + { + ERR("Failed to create EGL surface\n"); + goto err; + } + + TRACE("hwnd=%p egl_surface=%p\n", gl->hwnd, gl->surface); + + return gl; + +err: + wayland_gl_drawable_release(gl); + return NULL; +} + +static void wayland_update_gl_drawable(HWND hwnd, struct wayland_gl_drawable *new) +{ + struct wayland_gl_drawable *gl, *old = NULL; + + pthread_mutex_lock(&gl_object_mutex); + + LIST_FOR_EACH_ENTRY(gl, &gl_drawables, struct wayland_gl_drawable, entry) + { + if (gl->hwnd != hwnd) continue; + list_remove(&gl->entry); + old = gl; + break; + } + if (new) list_add_head(&gl_drawables, &new->entry); + + pthread_mutex_unlock(&gl_object_mutex); + + if (old) wayland_gl_drawable_release(old); +} + +static BOOL set_pixel_format(HDC hdc, int format, BOOL internal) +{ + HWND hwnd = NtUserWindowFromDC(hdc); + struct wayland_gl_drawable *gl; + int prev = 0; + + if (!hwnd || hwnd == NtUserGetDesktopWindow()) + { + WARN("not a proper window DC %p/%p\n", hdc, hwnd); + return FALSE; + } + if (format < 0 || format >= num_egl_configs) + { + WARN("Invalid format %d\n", format); + return FALSE; + } + TRACE("%p/%p format %d\n", hdc, hwnd, format); + + /* Even for internal pixel format fail setting it if the app has already set a + * different pixel format. Let wined3d create a backup GL context instead. + * Switching pixel format involves drawable recreation and is much more expensive + * than blitting from backup context. */ + if ((prev = win32u_get_window_pixel_format(hwnd))) + return prev == format; + + if (!(gl = wayland_gl_drawable_create(hwnd, format))) return FALSE; + wayland_update_gl_drawable(hwnd, gl); + win32u_set_window_pixel_format(hwnd, format, internal); + + return TRUE; +} + static BOOL has_opengl(void); static int wayland_wglDescribePixelFormat(HDC hdc, int fmt, UINT size, @@ -136,6 +285,17 @@ static PROC wayland_wglGetProcAddress(LPCSTR name) return (PROC)p_eglGetProcAddress(name); } +static BOOL wayland_wglSetPixelFormat(HDC hdc, int format, + const PIXELFORMATDESCRIPTOR *pfd) +{ + return set_pixel_format(hdc, format, FALSE); +} + +static BOOL wayland_wglSetPixelFormatWINE(HDC hdc, int format) +{ + return set_pixel_format(hdc, format, TRUE); +} + static BOOL has_extension(const char *list, const char *ext) { size_t len = strlen(ext); @@ -177,6 +337,9 @@ static BOOL init_opengl_funcs(void) register_extension("WGL_EXT_extensions_string"); opengl_funcs.ext.p_wglGetExtensionsStringEXT = wayland_wglGetExtensionsStringEXT; + register_extension("WGL_WINE_pixel_format_passthrough"); + opengl_funcs.ext.p_wglSetPixelFormatWINE = wayland_wglSetPixelFormatWINE; + return TRUE; } @@ -271,6 +434,8 @@ static void init_opengl(void) { ERR("Failed to load symbol %s\n", #func); goto err; } \ } while(0) LOAD_FUNCPTR_EGL(eglChooseConfig); + LOAD_FUNCPTR_EGL(eglCreateWindowSurface); + LOAD_FUNCPTR_EGL(eglDestroySurface); LOAD_FUNCPTR_EGL(eglGetConfigAttrib); LOAD_FUNCPTR_EGL(eglGetError); LOAD_FUNCPTR_EGL(eglGetPlatformDisplay); @@ -315,6 +480,7 @@ static struct opengl_funcs opengl_funcs = { .p_wglDescribePixelFormat = wayland_wglDescribePixelFormat, .p_wglGetProcAddress = wayland_wglGetProcAddress, + .p_wglSetPixelFormat = wayland_wglSetPixelFormat, } }; @@ -333,6 +499,14 @@ struct opengl_funcs *WAYLAND_wine_get_wgl_driver(UINT version) return &opengl_funcs; } +/********************************************************************** + * wayland_destroy_gl_drawable + */ +void wayland_destroy_gl_drawable(HWND hwnd) +{ + wayland_update_gl_drawable(hwnd, NULL); +} + #else /* No GL */ struct opengl_funcs *WAYLAND_wine_get_wgl_driver(UINT version) @@ -340,4 +514,8 @@ struct opengl_funcs *WAYLAND_wine_get_wgl_driver(UINT version) return NULL; } +void wayland_destroy_gl_drawable(HWND hwnd) +{ +} + #endif diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index 66806503edf..7da33dc9f18 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -292,6 +292,12 @@ void wayland_pointer_init(struct wl_pointer *wl_pointer); void wayland_pointer_deinit(void); void wayland_pointer_clear_constraint(void); +/********************************************************************** + * OpenGL + */ + +void wayland_destroy_gl_drawable(HWND hwnd); + /********************************************************************** * Helpers */ diff --git a/dlls/winewayland.drv/window.c b/dlls/winewayland.drv/window.c index ac5da371e5c..a02363411ea 100644 --- a/dlls/winewayland.drv/window.c +++ b/dlls/winewayland.drv/window.c @@ -402,6 +402,7 @@ void WAYLAND_DestroyWindow(HWND hwnd) if (!(data = wayland_win_data_get(hwnd))) return; wayland_win_data_destroy(data); + wayland_destroy_gl_drawable(hwnd); } /*********************************************************************** diff --git a/include/config.h.in b/include/config.h.in index a910b2c85f1..8f4d5e575ce 100644 --- a/include/config.h.in +++ b/include/config.h.in @@ -132,6 +132,9 @@ /* Define to 1 if you have the `unwind' library (-lunwind). */ #undef HAVE_LIBUNWIND +/* Define if we have the wayland-egl development environment */ +#undef HAVE_LIBWAYLAND_EGL + /* Define if you have the X Shape extension */ #undef HAVE_LIBXSHAPE -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5177
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> Each WGL context is backed by a config-less EGL context. We require the EGL_KHR_no_config_context extension. --- dlls/winewayland.drv/opengl.c | 96 ++++++++++++++++++++++++++++++++++- 1 file changed, 95 insertions(+), 1 deletion(-) diff --git a/dlls/winewayland.drv/opengl.c b/dlls/winewayland.drv/opengl.c index c54deabf8ff..f0db6cbbfd2 100644 --- a/dlls/winewayland.drv/opengl.c +++ b/dlls/winewayland.drv/opengl.c @@ -54,8 +54,11 @@ static const char *opengl_func_names[] = { ALL_WGL_FUNCS }; #undef USE_GL_FUNC #define DECL_FUNCPTR(f) static __typeof__(f) * p_##f = NULL +DECL_FUNCPTR(eglBindAPI); DECL_FUNCPTR(eglChooseConfig); +DECL_FUNCPTR(eglCreateContext); DECL_FUNCPTR(eglCreateWindowSurface); +DECL_FUNCPTR(eglDestroyContext); DECL_FUNCPTR(eglDestroySurface); DECL_FUNCPTR(eglGetConfigAttrib); DECL_FUNCPTR(eglGetError); @@ -67,6 +70,7 @@ DECL_FUNCPTR(eglQueryString); static pthread_mutex_t gl_object_mutex = PTHREAD_MUTEX_INITIALIZER; static struct list gl_drawables = LIST_INIT(gl_drawables); +static struct list gl_contexts = LIST_INIT(gl_contexts); struct wayland_gl_drawable { @@ -78,6 +82,30 @@ struct wayland_gl_drawable EGLSurface surface; }; +struct wgl_context +{ + struct list entry; + EGLConfig config; + EGLContext context; +}; + +static struct wayland_gl_drawable *wayland_gl_drawable_get(HWND hwnd) +{ + struct wayland_gl_drawable *gl, *ret = NULL; + + pthread_mutex_lock(&gl_object_mutex); + LIST_FOR_EACH_ENTRY(gl, &gl_drawables, struct wayland_gl_drawable, entry) + { + if (gl->hwnd != hwnd) continue; + InterlockedIncrement(&gl->ref); + ret = gl; + break; + } + pthread_mutex_unlock(&gl_object_mutex); + + return ret; +} + static void wayland_gl_drawable_release(struct wayland_gl_drawable *gl) { if (InterlockedDecrement(&gl->ref)) return; @@ -212,6 +240,57 @@ static BOOL set_pixel_format(HDC hdc, int format, BOOL internal) return TRUE; } +static struct wgl_context *create_context(HDC hdc) +{ + struct wayland_gl_drawable *gl; + struct wgl_context *ctx; + + if (!(gl = wayland_gl_drawable_get(NtUserWindowFromDC(hdc)))) return NULL; + + if (!(ctx = calloc(1, sizeof(*ctx)))) + { + ERR("Failed to allocate memory for GL context\n"); + goto out; + } + + ctx->context = p_eglCreateContext(egl_display, EGL_NO_CONFIG_KHR, + EGL_NO_CONTEXT, NULL); + + pthread_mutex_lock(&gl_object_mutex); + list_add_head(&gl_contexts, &ctx->entry); + pthread_mutex_unlock(&gl_object_mutex); + + TRACE("ctx=%p egl_context=%p\n", ctx, ctx->context); + +out: + wayland_gl_drawable_release(gl); + return ctx; +} + +static BOOL wayland_wglCopyContext(struct wgl_context *src, + struct wgl_context *dst, UINT mask) +{ + FIXME("%p -> %p mask %#x unsupported\n", src, dst, mask); + return FALSE; +} + +static struct wgl_context *wayland_wglCreateContext(HDC hdc) +{ + TRACE("hdc=%p\n", hdc); + p_eglBindAPI(EGL_OPENGL_API); + return create_context(hdc); +} + +static BOOL wayland_wglDeleteContext(struct wgl_context *ctx) +{ + pthread_mutex_lock(&gl_object_mutex); + list_remove(&ctx->entry); + pthread_mutex_unlock(&gl_object_mutex); + p_eglDestroyContext(egl_display, ctx->context); + free(ctx); + return TRUE; +} + static BOOL has_opengl(void); static int wayland_wglDescribePixelFormat(HDC hdc, int fmt, UINT size, @@ -400,7 +479,7 @@ static BOOL init_egl_configs(void) static void init_opengl(void) { EGLint egl_version[2]; - const char *egl_client_exts; + const char *egl_client_exts, *egl_exts; if (!(egl_handle = dlopen(SONAME_LIBEGL, RTLD_NOW|RTLD_GLOBAL))) { @@ -433,8 +512,11 @@ static void init_opengl(void) if (!(p_##func = (__typeof__(func) *)p_eglGetProcAddress(#func))) \ { ERR("Failed to load symbol %s\n", #func); goto err; } \ } while(0) + LOAD_FUNCPTR_EGL(eglBindAPI); LOAD_FUNCPTR_EGL(eglChooseConfig); + LOAD_FUNCPTR_EGL(eglCreateContext); LOAD_FUNCPTR_EGL(eglCreateWindowSurface); + LOAD_FUNCPTR_EGL(eglDestroyContext); LOAD_FUNCPTR_EGL(eglDestroySurface); LOAD_FUNCPTR_EGL(eglGetConfigAttrib); LOAD_FUNCPTR_EGL(eglGetError); @@ -457,6 +539,15 @@ static void init_opengl(void) } TRACE("EGL version %u.%u\n", egl_version[0], egl_version[1]); + egl_exts = p_eglQueryString(egl_display, EGL_EXTENSIONS); +#define REQUIRE_EXT(ext) \ + do { \ + if (!has_extension(egl_exts, #ext)) \ + { ERR("Failed to find required extension %s\n", #ext); goto err; } \ + } while(0) + REQUIRE_EXT(EGL_KHR_no_config_context); +#undef REQUIRE_EXT + if (!init_opengl_funcs()) goto err; if (!init_egl_configs()) goto err; @@ -478,6 +569,9 @@ static struct opengl_funcs opengl_funcs = { .wgl = { + .p_wglCopyContext = wayland_wglCopyContext, + .p_wglCreateContext = wayland_wglCreateContext, + .p_wglDeleteContext = wayland_wglDeleteContext, .p_wglDescribePixelFormat = wayland_wglDescribePixelFormat, .p_wglGetProcAddress = wayland_wglGetProcAddress, .p_wglSetPixelFormat = wayland_wglSetPixelFormat, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5177
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> --- dlls/winewayland.drv/opengl.c | 122 +++++++++++++++++++++++++++++++++- 1 file changed, 121 insertions(+), 1 deletion(-) diff --git a/dlls/winewayland.drv/opengl.c b/dlls/winewayland.drv/opengl.c index f0db6cbbfd2..8c1336fffcb 100644 --- a/dlls/winewayland.drv/opengl.c +++ b/dlls/winewayland.drv/opengl.c @@ -65,6 +65,7 @@ DECL_FUNCPTR(eglGetError); DECL_FUNCPTR(eglGetPlatformDisplay); DECL_FUNCPTR(eglGetProcAddress); DECL_FUNCPTR(eglInitialize); +DECL_FUNCPTR(eglMakeCurrent); DECL_FUNCPTR(eglQueryString); #undef DECL_FUNCPTR @@ -87,6 +88,7 @@ struct wgl_context struct list entry; EGLConfig config; EGLContext context; + struct wayland_gl_drawable *draw, *read; }; static struct wayland_gl_drawable *wayland_gl_drawable_get(HWND hwnd) @@ -188,9 +190,43 @@ err: return NULL; } +static inline void wayland_gl_drawable_array_add(struct wl_array *drawables, + struct wayland_gl_drawable *gl) +{ + struct wayland_gl_drawable **elem; + + if ((elem = wl_array_add(drawables, sizeof(gl)))) *elem = gl; +} + +static void update_context_drawables(struct wayland_gl_drawable *new, + struct wayland_gl_drawable *old, + struct wl_array *release) +{ + struct wgl_context *ctx; + + LIST_FOR_EACH_ENTRY(ctx, &gl_contexts, struct wgl_context, entry) + { + if (ctx->draw == old) + { + wayland_gl_drawable_array_add(release, ctx->draw); + InterlockedIncrement(&new->ref); + ctx->draw = new; + } + if (ctx->read == old) + { + wayland_gl_drawable_array_add(release, ctx->read); + InterlockedIncrement(&new->ref); + ctx->read = new; + } + } +} + static void wayland_update_gl_drawable(HWND hwnd, struct wayland_gl_drawable *new) { struct wayland_gl_drawable *gl, *old = NULL; + struct wl_array release; + + wl_array_init(&release); pthread_mutex_lock(&gl_object_mutex); @@ -202,10 +238,61 @@ static void wayland_update_gl_drawable(HWND hwnd, struct wayland_gl_drawable *ne break; } if (new) list_add_head(&gl_drawables, &new->entry); + if (old && new) update_context_drawables(new, old, &release); + if (old) wayland_gl_drawable_array_add(&release, old); + + pthread_mutex_unlock(&gl_object_mutex); + + /* Release the drawables outside the gl_object_mutex lock, since the release + * process may acquire the wayland_surface lock and lead to a deadlock. */ + wl_array_for_each(gl, &release) wayland_gl_drawable_release(gl); + wl_array_release(&release); +} + +static BOOL wgl_context_make_current(struct wgl_context *ctx, HWND draw_hwnd, + HWND read_hwnd) +{ + BOOL ret; + struct wayland_gl_drawable *draw_gl = NULL, *read_gl = NULL; + struct wayland_gl_drawable *release[2]; + int i; + + draw_gl = wayland_gl_drawable_get(draw_hwnd); + read_gl = wayland_gl_drawable_get(read_hwnd); + + TRACE("%p/%p context %p surface %p/%p\n", + draw_hwnd, read_hwnd, ctx->context, + draw_gl ? draw_gl->surface : NULL, + read_gl ? read_gl->surface : NULL); + + pthread_mutex_lock(&gl_object_mutex); + + ret = p_eglMakeCurrent(egl_display, + draw_gl ? draw_gl->surface : NULL, + read_gl ? read_gl->surface : NULL, + ctx->context); + if (ret) + { + release[0] = ctx->draw; + release[1] = ctx->read; + ctx->draw = draw_gl; + ctx->read = read_gl; + NtCurrentTeb()->glContext = ctx; + } + else + { + release[0] = draw_gl; + release[1] = read_gl; + } pthread_mutex_unlock(&gl_object_mutex); - if (old) wayland_gl_drawable_release(old); + /* Release the drawables outside the gl_object_mutex lock, since the release + * process may acquire the wayland_surface lock and lead to a deadlock. */ + for (i = 0; i < ARRAY_SIZE(release); ++i) + if (release[i]) wayland_gl_drawable_release(release[i]); + + return ret; } static BOOL set_pixel_format(HDC hdc, int format, BOOL internal) @@ -287,6 +374,8 @@ static BOOL wayland_wglDeleteContext(struct wgl_context *ctx) list_remove(&ctx->entry); pthread_mutex_unlock(&gl_object_mutex); p_eglDestroyContext(egl_display, ctx->context); + if (ctx->draw) wayland_gl_drawable_release(ctx->draw); + if (ctx->read) wayland_gl_drawable_release(ctx->read); free(ctx); return TRUE; } @@ -364,6 +453,31 @@ static PROC wayland_wglGetProcAddress(LPCSTR name) return (PROC)p_eglGetProcAddress(name); } +static BOOL wayland_wglMakeContextCurrentARB(HDC draw_hdc, HDC read_hdc, + struct wgl_context *ctx) +{ + BOOL ret; + + TRACE("draw_hdc=%p read_hdc=%p ctx=%p\n", draw_hdc, read_hdc, ctx); + + if (!ctx) + { + p_eglMakeCurrent(egl_display, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT); + NtCurrentTeb()->glContext = NULL; + return TRUE; + } + + ret = wgl_context_make_current(ctx, NtUserWindowFromDC(draw_hdc), NtUserWindowFromDC(read_hdc)); + if (!ret) RtlSetLastWin32Error(ERROR_INVALID_HANDLE); + + return ret; +} + +static BOOL wayland_wglMakeCurrent(HDC hdc, struct wgl_context *ctx) +{ + return wayland_wglMakeContextCurrentARB(hdc, hdc, ctx); +} + static BOOL wayland_wglSetPixelFormat(HDC hdc, int format, const PIXELFORMATDESCRIPTOR *pfd) { @@ -419,6 +533,10 @@ static BOOL init_opengl_funcs(void) register_extension("WGL_WINE_pixel_format_passthrough"); opengl_funcs.ext.p_wglSetPixelFormatWINE = wayland_wglSetPixelFormatWINE; + register_extension("WGL_ARB_make_current_read"); + opengl_funcs.ext.p_wglGetCurrentReadDCARB = (void *)1; /* never called */ + opengl_funcs.ext.p_wglMakeContextCurrentARB = wayland_wglMakeContextCurrentARB; + return TRUE; } @@ -522,6 +640,7 @@ static void init_opengl(void) LOAD_FUNCPTR_EGL(eglGetError); LOAD_FUNCPTR_EGL(eglGetPlatformDisplay); LOAD_FUNCPTR_EGL(eglInitialize); + LOAD_FUNCPTR_EGL(eglMakeCurrent); #undef LOAD_FUNCPTR_EGL egl_display = p_eglGetPlatformDisplay(EGL_PLATFORM_WAYLAND_KHR, @@ -574,6 +693,7 @@ static struct opengl_funcs opengl_funcs = .p_wglDeleteContext = wayland_wglDeleteContext, .p_wglDescribePixelFormat = wayland_wglDescribePixelFormat, .p_wglGetProcAddress = wayland_wglGetProcAddress, + .p_wglMakeCurrent = wayland_wglMakeCurrent, .p_wglSetPixelFormat = wayland_wglSetPixelFormat, } }; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5177
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> --- dlls/winewayland.drv/opengl.c | 91 ++++++++++++++++++++++++++++++++--- 1 file changed, 83 insertions(+), 8 deletions(-) diff --git a/dlls/winewayland.drv/opengl.c b/dlls/winewayland.drv/opengl.c index 8c1336fffcb..70334658296 100644 --- a/dlls/winewayland.drv/opengl.c +++ b/dlls/winewayland.drv/opengl.c @@ -67,6 +67,7 @@ DECL_FUNCPTR(eglGetProcAddress); DECL_FUNCPTR(eglInitialize); DECL_FUNCPTR(eglMakeCurrent); DECL_FUNCPTR(eglQueryString); +DECL_FUNCPTR(eglSwapBuffers); #undef DECL_FUNCPTR static pthread_mutex_t gl_object_mutex = PTHREAD_MUTEX_INITIALIZER; @@ -88,7 +89,7 @@ struct wgl_context struct list entry; EGLConfig config; EGLContext context; - struct wayland_gl_drawable *draw, *read; + struct wayland_gl_drawable *draw, *read, *new_draw, *new_read; }; static struct wayland_gl_drawable *wayland_gl_drawable_get(HWND hwnd) @@ -206,17 +207,17 @@ static void update_context_drawables(struct wayland_gl_drawable *new, LIST_FOR_EACH_ENTRY(ctx, &gl_contexts, struct wgl_context, entry) { - if (ctx->draw == old) + if (ctx->draw == old || ctx->new_draw == old) { - wayland_gl_drawable_array_add(release, ctx->draw); + if (ctx->new_draw) wayland_gl_drawable_array_add(release, ctx->new_draw); InterlockedIncrement(&new->ref); - ctx->draw = new; + ctx->new_draw = new; } - if (ctx->read == old) + if (ctx->read == old || ctx->new_read == old) { - wayland_gl_drawable_array_add(release, ctx->read); + if (ctx->new_read) wayland_gl_drawable_array_add(release, ctx->new_read); InterlockedIncrement(&new->ref); - ctx->read = new; + ctx->new_read = new; } } } @@ -249,12 +250,32 @@ static void wayland_update_gl_drawable(HWND hwnd, struct wayland_gl_drawable *ne wl_array_release(&release); } +static void wayland_gl_drawable_sync_surface_state(struct wayland_gl_drawable *gl) +{ + struct wayland_surface *wayland_surface; + + if (!(wayland_surface = wayland_surface_lock_hwnd(gl->hwnd))) return; + + wayland_surface_ensure_contents(wayland_surface); + + /* Handle any processed configure request, to ensure the related + * surface state is applied by the compositor. */ + if (wayland_surface->processing.serial && + wayland_surface->processing.processed && + wayland_surface_reconfigure(wayland_surface)) + { + wl_surface_commit(wayland_surface->wl_surface); + } + + pthread_mutex_unlock(&wayland_surface->mutex); +} + static BOOL wgl_context_make_current(struct wgl_context *ctx, HWND draw_hwnd, HWND read_hwnd) { BOOL ret; struct wayland_gl_drawable *draw_gl = NULL, *read_gl = NULL; - struct wayland_gl_drawable *release[2]; + struct wayland_gl_drawable *release[4]; int i; draw_gl = wayland_gl_drawable_get(draw_hwnd); @@ -275,14 +296,18 @@ static BOOL wgl_context_make_current(struct wgl_context *ctx, HWND draw_hwnd, { release[0] = ctx->draw; release[1] = ctx->read; + release[2] = ctx->new_draw; + release[3] = ctx->new_read; ctx->draw = draw_gl; ctx->read = read_gl; + ctx->new_draw = ctx->new_read = NULL; NtCurrentTeb()->glContext = ctx; } else { release[0] = draw_gl; release[1] = read_gl; + release[2] = release[3] = NULL; } pthread_mutex_unlock(&gl_object_mutex); @@ -295,6 +320,35 @@ static BOOL wgl_context_make_current(struct wgl_context *ctx, HWND draw_hwnd, return ret; } +static void wgl_context_refresh(struct wgl_context *ctx) +{ + BOOL refresh = FALSE; + struct wayland_gl_drawable *old_draw = NULL, *old_read = NULL; + + pthread_mutex_lock(&gl_object_mutex); + + if (ctx->new_draw) + { + old_draw = ctx->draw; + ctx->draw = ctx->new_draw; + ctx->new_draw = NULL; + refresh = TRUE; + } + if (ctx->new_read) + { + old_read = ctx->read; + ctx->read = ctx->new_read; + ctx->new_read = NULL; + refresh = TRUE; + } + if (refresh) p_eglMakeCurrent(egl_display, ctx->draw, ctx->read, ctx->context); + + pthread_mutex_unlock(&gl_object_mutex); + + if (old_draw) wayland_gl_drawable_release(old_draw); + if (old_read) wayland_gl_drawable_release(old_read); +} + static BOOL set_pixel_format(HDC hdc, int format, BOOL internal) { HWND hwnd = NtUserWindowFromDC(hdc); @@ -376,6 +430,8 @@ static BOOL wayland_wglDeleteContext(struct wgl_context *ctx) p_eglDestroyContext(egl_display, ctx->context); if (ctx->draw) wayland_gl_drawable_release(ctx->draw); if (ctx->read) wayland_gl_drawable_release(ctx->read); + if (ctx->new_draw) wayland_gl_drawable_release(ctx->new_draw); + if (ctx->new_read) wayland_gl_drawable_release(ctx->new_read); free(ctx); return TRUE; } @@ -489,6 +545,23 @@ static BOOL wayland_wglSetPixelFormatWINE(HDC hdc, int format) return set_pixel_format(hdc, format, TRUE); } +static BOOL wayland_wglSwapBuffers(HDC hdc) +{ + struct wgl_context *ctx = NtCurrentTeb()->glContext; + HWND hwnd = NtUserWindowFromDC(hdc); + struct wayland_gl_drawable *gl; + + if (!(gl = wayland_gl_drawable_get(hwnd))) return FALSE; + + if (ctx) wgl_context_refresh(ctx); + wayland_gl_drawable_sync_surface_state(gl); + p_eglSwapBuffers(egl_display, gl->surface); + + wayland_gl_drawable_release(gl); + + return TRUE; +} + static BOOL has_extension(const char *list, const char *ext) { size_t len = strlen(ext); @@ -641,6 +714,7 @@ static void init_opengl(void) LOAD_FUNCPTR_EGL(eglGetPlatformDisplay); LOAD_FUNCPTR_EGL(eglInitialize); LOAD_FUNCPTR_EGL(eglMakeCurrent); + LOAD_FUNCPTR_EGL(eglSwapBuffers); #undef LOAD_FUNCPTR_EGL egl_display = p_eglGetPlatformDisplay(EGL_PLATFORM_WAYLAND_KHR, @@ -695,6 +769,7 @@ static struct opengl_funcs opengl_funcs = .p_wglGetProcAddress = wayland_wglGetProcAddress, .p_wglMakeCurrent = wayland_wglMakeCurrent, .p_wglSetPixelFormat = wayland_wglSetPixelFormat, + .p_wglSwapBuffers = wayland_wglSwapBuffers, } }; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5177
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> Wayland surfaces don't have an inherent native size that the EGL implementation can track, so we need to explicitly tell EGL about changes in the native size with wl_egl_window_resize. Since the resize can be triggered outside the GL render thread, and wl_egl_window_resize is not thread safe (with respect to other EGL/GL calls), we cannot call it directly at will. Instead we mark the wayland_gl_drawable as resized, and actually call the wl_egl_window_resize function from the thread in which the respective drawable is current. Note that the first EGL/GL operation that requires a new backbuffer latches whatever native size we have reported, until the next eglSwapBuffers. In order to ensure the current native size is applied as soon as possible (to avoid glitches), we check for and apply resizes at a few extra points where a new backbuffer may be required (e.g., glClear, eglMakeCurrent). --- dlls/winewayland.drv/opengl.c | 59 ++++++++++++++++++++++++++ dlls/winewayland.drv/wayland_surface.c | 2 + dlls/winewayland.drv/waylanddrv.h | 1 + 3 files changed, 62 insertions(+) diff --git a/dlls/winewayland.drv/opengl.c b/dlls/winewayland.drv/opengl.c index 70334658296..994499f8820 100644 --- a/dlls/winewayland.drv/opengl.c +++ b/dlls/winewayland.drv/opengl.c @@ -68,6 +68,7 @@ DECL_FUNCPTR(eglInitialize); DECL_FUNCPTR(eglMakeCurrent); DECL_FUNCPTR(eglQueryString); DECL_FUNCPTR(eglSwapBuffers); +DECL_FUNCPTR(glClear); #undef DECL_FUNCPTR static pthread_mutex_t gl_object_mutex = PTHREAD_MUTEX_INITIALIZER; @@ -82,6 +83,7 @@ struct wayland_gl_drawable struct wayland_client_surface *client; struct wl_egl_window *wl_egl_window; EGLSurface surface; + LONG resized; }; struct wgl_context @@ -250,6 +252,28 @@ static void wayland_update_gl_drawable(HWND hwnd, struct wayland_gl_drawable *ne wl_array_release(&release); } +static void wayland_gl_drawable_sync_size(struct wayland_gl_drawable *gl) +{ + int client_width, client_height; + struct wayland_surface *wayland_surface; + + if (InterlockedCompareExchange(&gl->resized, FALSE, TRUE)) + { + if (!(wayland_surface = wayland_surface_lock_hwnd(gl->hwnd))) return; + + client_width = wayland_surface->window.client_rect.right - + wayland_surface->window.client_rect.left; + client_height = wayland_surface->window.client_rect.bottom - + wayland_surface->window.client_rect.top; + if (client_width == 0 || client_height == 0) + client_width = client_height = 1; + + wl_egl_window_resize(gl->wl_egl_window, client_width, client_height, 0, 0); + + pthread_mutex_unlock(&wayland_surface->mutex); + } +} + static void wayland_gl_drawable_sync_surface_state(struct wayland_gl_drawable *gl) { struct wayland_surface *wayland_surface; @@ -286,6 +310,10 @@ static BOOL wgl_context_make_current(struct wgl_context *ctx, HWND draw_hwnd, draw_gl ? draw_gl->surface : NULL, read_gl ? read_gl->surface : NULL); + /* Since making an EGL surface current may latch the native size, + * perform any pending resizes before calling it. */ + if (draw_gl) wayland_gl_drawable_sync_size(draw_gl); + pthread_mutex_lock(&gl_object_mutex); ret = p_eglMakeCurrent(egl_display, @@ -408,6 +436,15 @@ out: return ctx; } +void wayland_glClear(GLbitfield mask) +{ + struct wgl_context *ctx = NtCurrentTeb()->glContext; + /* Since glClear is one of the operations that may latch the native size, + * perform any pending resizes before calling it. */ + if (ctx && ctx->draw) wayland_gl_drawable_sync_size(ctx->draw); + p_glClear(mask); +} + static BOOL wayland_wglCopyContext(struct wgl_context *src, struct wgl_context *dst, UINT mask) { @@ -556,6 +593,7 @@ static BOOL wayland_wglSwapBuffers(HDC hdc) if (ctx) wgl_context_refresh(ctx); wayland_gl_drawable_sync_surface_state(gl); p_eglSwapBuffers(egl_display, gl->surface); + wayland_gl_drawable_sync_size(gl); wayland_gl_drawable_release(gl); @@ -597,6 +635,9 @@ static BOOL init_opengl_funcs(void) } } + p_glClear = opengl_funcs.gl.p_glClear; + opengl_funcs.gl.p_glClear = wayland_glClear; + register_extension("WGL_ARB_extensions_string"); opengl_funcs.ext.p_wglGetExtensionsStringARB = wayland_wglGetExtensionsStringARB; @@ -796,6 +837,20 @@ void wayland_destroy_gl_drawable(HWND hwnd) wayland_update_gl_drawable(hwnd, NULL); } +/********************************************************************** + * wayland_resize_gl_drawable + */ +void wayland_resize_gl_drawable(HWND hwnd) +{ + struct wayland_gl_drawable *gl; + + if (!(gl = wayland_gl_drawable_get(hwnd))) return; + /* wl_egl_window_resize is not thread safe, so we just mark the + * drawable as resized and perform the resize in the proper thread. */ + InterlockedExchange(&gl->resized, TRUE); + wayland_gl_drawable_release(gl); +} + #else /* No GL */ struct opengl_funcs *WAYLAND_wine_get_wgl_driver(UINT version) @@ -807,4 +862,8 @@ void wayland_destroy_gl_drawable(HWND hwnd) { } +void wayland_resize_gl_drawable(HWND hwnd) +{ +} + #endif diff --git a/dlls/winewayland.drv/wayland_surface.c b/dlls/winewayland.drv/wayland_surface.c index a955f3688c5..e212e74362a 100644 --- a/dlls/winewayland.drv/wayland_surface.c +++ b/dlls/winewayland.drv/wayland_surface.c @@ -519,6 +519,8 @@ static void wayland_surface_reconfigure_client(struct wayland_surface *surface) } wl_surface_commit(surface->client->wl_surface); + + wayland_resize_gl_drawable(surface->hwnd); } /********************************************************************** diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index 7da33dc9f18..7cf813a1ca3 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -297,6 +297,7 @@ void wayland_pointer_clear_constraint(void); */ void wayland_destroy_gl_drawable(HWND hwnd); +void wayland_resize_gl_drawable(HWND hwnd); /********************************************************************** * Helpers -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5177
I don't think `WINE_NOTICE_WITH` causes a failure, only a message?
My understanding is that it causes a failure if the check fails AND the specified flag has been explicitly requested by the user. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5177#note_63048
On Thu Feb 29 10:52:18 2024 +0000, Alexandros Frantzis wrote:
changed this line in [version 2 of the diff](/wine/wine/-/merge_requests/5177/diffs?diff_id=102341&start_sha=1382876a78655d9a7fe6200e1c995ae631cafa24#87db583be5c13c1f7b3c958b10e03d67b6a2ca06_1395_1392) Done.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5177#note_63049
On Wed Feb 28 13:24:35 2024 +0000, Alexandros Frantzis wrote:
Ack. Done.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5177#note_63050
On Wed Feb 28 13:56:36 2024 +0000, Rémi Bernon wrote:
This is indeed an oversight, EGL_CFLAGS should be added to UNIX_CFLAGS in the Wayland driver. Yes. I guess the assumption here is that in case of such setups, the dynamic linking runtime environment would also include the proper paths, but of course it's not guaranteed. You can probably ignore this part of my comment. I had in mind that SONAME might include the full path of the libraries, but it does not, so wherever pkg-config ends up locating the libraries it will end up with the same SONAME (which still needs to later be loadable without a path, but that's probably expected). Added EGL_CFLAGS.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5177#note_63051
On Thu Feb 29 10:52:34 2024 +0000, Alexandros Frantzis wrote:
changed this line in [version 2 of the diff](/wine/wine/-/merge_requests/5177/diffs?diff_id=102341&start_sha=1382876a78655d9a7fe6200e1c995ae631cafa24#2fc4f8acb340ee1b9eb3fadf8b3568bd11af1989_713_719) Done.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5177#note_63052
On Thu Feb 29 10:52:47 2024 +0000, Alexandros Frantzis wrote:
changed this line in [version 2 of the diff](/wine/wine/-/merge_requests/5177/diffs?diff_id=102341&start_sha=1382876a78655d9a7fe6200e1c995ae631cafa24#2fc4f8acb340ee1b9eb3fadf8b3568bd11af1989_747_747) Done.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5177#note_63053
On Wed Feb 28 13:24:38 2024 +0000, Alexandros Frantzis wrote:
This should be fine (and very nice) to use with Mesa, but I can't find much direct information about NVIDIA and AMD proprietary support, and that's what's been holding me back a bit. Searching again I was able to find a few references to this extension in various relatively old online reports for the proprietary drivers, so I guess let's go with it and hope for the best :) Done.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5177#note_63054
On Thu Feb 29 10:53:17 2024 +0000, Alexandros Frantzis wrote:
changed this line in [version 2 of the diff](/wine/wine/-/merge_requests/5177/diffs?diff_id=102341&start_sha=1382876a78655d9a7fe6200e1c995ae631cafa24#2fc4f8acb340ee1b9eb3fadf8b3568bd11af1989_551_546) Done.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5177#note_63055
On Thu Feb 29 10:53:09 2024 +0000, Alexandros Frantzis wrote:
changed this line in [version 2 of the diff](/wine/wine/-/merge_requests/5177/diffs?diff_id=102341&start_sha=1382876a78655d9a7fe6200e1c995ae631cafa24#2fc4f8acb340ee1b9eb3fadf8b3568bd11af1989_182_180) (I am missing some background here)
Is what you are describing going to apply only in the pixel format reset/mismatch case, which (according to the comment) is currently handled by "wined3d creating a backup context instead"? Or are you considering this for the general case? If the latter, wouldn't there be significant performance implications? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5177#note_63057
Rémi Bernon (@rbernon) commented about dlls/winewayland.drv/opengl.c:
+#if defined(SONAME_LIBEGL) + +WINE_DEFAULT_DEBUG_CHANNEL(waylanddrv); + +#define WL_EGL_PLATFORM 1 +#include <EGL/egl.h> +#include <EGL/eglext.h> + +#include "wine/wgl.h" +#include "wine/wgl_driver.h" + +static void *egl_handle; +static struct opengl_funcs opengl_funcs; +static EGLDisplay egl_display; + +#define DECL_FUNCPTR(f) static __typeof__(f) * p_##f = NULL This is `typeof` everywhere else in Wine, same for the other `__typeof__` below.
Also you don't need the `= NULL`. ```suggestion:-0+0 #define DECL_FUNCPTR(f) static typeof(f) * p_##f ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5177#note_63095
Rémi Bernon (@rbernon) commented about dlls/winewayland.drv/opengl.c:
+#undef LOAD_FUNCPTR_DLSYM + + egl_client_exts = p_eglQueryString(EGL_NO_DISPLAY, EGL_EXTENSIONS); + +#define REQUIRE_CLIENT_EXT(ext) \ + do { \ + if (!has_extension(egl_client_exts, #ext)) \ + { ERR("Failed to find required extension %s\n", #ext); goto err; } \ + } while(0) + REQUIRE_CLIENT_EXT(EGL_KHR_client_get_all_proc_addresses); + REQUIRE_CLIENT_EXT(EGL_KHR_platform_wayland); +#undef REQUIRE_CLIENT_EXT + +#define LOAD_FUNCPTR_EGL(func) \ + do { \ + if (!(p_##func = (__typeof__(func) *)p_eglGetProcAddress(#func))) \ Though this could probably just be a `(void *)` cast.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5177#note_63096
Rémi Bernon (@rbernon) commented about configure:
if test "x$with_wayland" != "x" then - if test -z "$SONAME_LIBEGL"
This should probably be squashed in the commit before? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5177#note_63097
Rémi Bernon (@rbernon) commented about dlls/winewayland.drv/opengl.c:
+ pfd->iLayerType = PFD_MAIN_PLANE; + + p_eglGetConfigAttrib(egl_display, config, EGL_RED_SIZE, &val); + pfd->cRedBits = val; + p_eglGetConfigAttrib(egl_display, config, EGL_GREEN_SIZE, &val); + pfd->cGreenBits = val; + p_eglGetConfigAttrib(egl_display, config, EGL_BLUE_SIZE, &val); + pfd->cBlueBits = val; + p_eglGetConfigAttrib(egl_display, config, EGL_ALPHA_SIZE, &val); + pfd->cAlphaBits = val; + p_eglGetConfigAttrib(egl_display, config, EGL_DEPTH_SIZE, &val); + pfd->cDepthBits = val; + p_eglGetConfigAttrib(egl_display, config, EGL_STENCIL_SIZE, &val); + pfd->cStencilBits = val; + /* cColorBits bits exclude alpha */ + pfd->cColorBits = pfd->cRedBits + pfd->cGreenBits + pfd->cBlueBits; Is that really the case? It doesn't look like so in the other drivers?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5177#note_63098
Rémi Bernon (@rbernon) commented about dlls/winewayland.drv/opengl.c:
+ struct wayland_gl_drawable *release[2]; + int i; + + draw_gl = wayland_gl_drawable_get(draw_hwnd); + read_gl = wayland_gl_drawable_get(read_hwnd); + + TRACE("%p/%p context %p surface %p/%p\n", + draw_hwnd, read_hwnd, ctx->context, + draw_gl ? draw_gl->surface : NULL, + read_gl ? read_gl->surface : NULL); + + pthread_mutex_lock(&gl_object_mutex); + + ret = p_eglMakeCurrent(egl_display, + draw_gl ? draw_gl->surface : NULL, + read_gl ? read_gl->surface : NULL, Maybe this should be EGL_NO_SURFACE instead of NULL?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5177#note_63099
Rémi Bernon (@rbernon) commented about dlls/winewayland.drv/opengl.c:
+ ctx->draw = draw_gl; + ctx->read = read_gl; + NtCurrentTeb()->glContext = ctx; + } + else + { + release[0] = draw_gl; + release[1] = read_gl; + } + + pthread_mutex_unlock(&gl_object_mutex); + + /* Release the drawables outside the gl_object_mutex lock, since the release + * process may acquire the wayland_surface lock and lead to a deadlock. */ + for (i = 0; i < ARRAY_SIZE(release); ++i) + if (release[i]) wayland_gl_drawable_release(release[i]); Would be just as simple with two ifs instead of a loop, do you expect to have a bigger array? What about even naming them old_draw / old_read?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5177#note_63100
Rémi Bernon (@rbernon) commented about dlls/winewayland.drv/opengl.c:
return NULL; }
+static inline void wayland_gl_drawable_array_add(struct wl_array *drawables, + struct wayland_gl_drawable *gl) +{ + struct wayland_gl_drawable **elem; + + if ((elem = wl_array_add(drawables, sizeof(gl)))) *elem = gl;
Instead of an allocating array that could fail, you could just add another linked list entry, say `release_entry` or `tmp_entry`, to the struct wayland_gl_drawable and use it to link them in a release list. You can then also do the same to link drawables in a local release list in wgl_context_make_current. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5177#note_63101
On Thu Feb 29 19:21:00 2024 +0000, Rémi Bernon wrote:
Is that really the case? It doesn't look like so in the other drivers? The [MSDN documentation](https://learn.microsoft.com/en-us/windows/win32/api/wingdi/ns-wingdi-pixelfo...) is very explicit about the meaning of this field (emphasis mine):
`cColorBits`
Specifies the number of color bitplanes in each color buffer. For RGBA pixel types, it is the size of the color buffer, **excluding the alpha bitplanes**. For color-index pixels, it is the size of the color-index buffer.
I checked the history of the respective line in winex11 and couldn't find an explicit justification for diverging from the specification. Perhaps the actual Win32 behavior is diverging from the spec? Or perhaps that's something particular about the use of this field in OpenGL? Or could it be an oversight that's gone unnoticed because it doesn't really matter in most cases? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5177#note_63183
On Thu Feb 29 19:21:02 2024 +0000, Rémi Bernon wrote:
Would be just as simple with two ifs instead of a loop, do you expect to have a bigger array? What about even naming them old_draw / old_read? Yes, the array grows to 4 members in the next commit (second to last in this MR). We could still go with 4 explicit statements if you prefer.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5177#note_63184
On Thu Feb 29 19:21:04 2024 +0000, Rémi Bernon wrote:
Instead of an allocating array that could fail, you could just add another linked list entry, say `release_entry` or `tmp_entry`, to the struct wayland_gl_drawable and use it to link them in a release list. You can then also do the same to link drawables in a local release list in wgl_context_make_current. The problem with the (invasive) linked list approach is that it doesn't allow us to mark the same drawable for release multiple times (the simplest case when this is needed is when \`ctx-\>draw == ctx-\>read == old\`). We could combine the release list with an extra field in `struct wayland_gl_drawable` to hold the release count, though.
If the main concern is the allocations, perhaps we can get away with using a large enough stack array based solution? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5177#note_63185
On Fri Mar 1 12:17:13 2024 +0000, Alexandros Frantzis wrote:
The [MSDN documentation](https://learn.microsoft.com/en-us/windows/win32/api/wingdi/ns-wingdi-pixelfo...) is very explicit about the meaning of this field (emphasis mine):
`cColorBits`
Specifies the number of color bitplanes in each color buffer. For RGBA pixel types, it is the size of the color buffer, **excluding the alpha bitplanes**. For color-index pixels, it is the size of the color-index buffer. I checked the history of the respective line in winex11 and couldn't find an explicit justification for diverging from the specification. Perhaps the actual Win32 behavior is diverging from the spec? Or perhaps that's something particular about the use of this field in OpenGL? Or could it be an oversight that's gone unnoticed because it doesn't really matter in most cases? It's probably driver dependent, and the documentation being simply incorrect. See for instance this run of wglinfo on actual Windows machines with physical GPUs: https://testbot.winehq.org/JobDetails.pl?Key=143604&f101=exe64.report&f102=e...
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5177#note_63187
On Fri Mar 1 12:43:33 2024 +0000, Alexandros Frantzis wrote:
Yes, the array grows to 4 members in the next commit (second to last in this MR). We could still go with 4 explicit statements if you prefer. Yeah I saw that after, forgot to delete the comment.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5177#note_63188
On Fri Mar 1 12:17:14 2024 +0000, Alexandros Frantzis wrote:
The problem with the (invasive) linked list approach is that it doesn't allow us to mark the same drawable for release multiple times (the simplest case when this is needed is when \`ctx-\>draw == ctx-\>read == old\`). We could combine the release list with an extra field in `struct wayland_gl_drawable` to hold the release count, though. If the main concern is the allocations, perhaps we can get away with using a large enough stack array based solution? You can initialize the list entries on creation, and always list_remove + list_add_tail to make sure it's not added twice.
What I don't like very much is the use of some non-Wine helper, it strongly couples the code to wayland libraries and makes it more difficult to refactor later. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5177#note_63189
On Fri Mar 1 12:46:39 2024 +0000, Rémi Bernon wrote:
You can initialize the list entries on creation, and always list_remove + list_add_tail to make sure it's not added twice. What I don't like very much is the use of some non-Wine helper, it strongly couples the code to wayland libraries and makes it more difficult to refactor later. Also, as you're removing the surfaces from the global list, maybe you don't even need a separate entry, and instead can use the same to link them into the "release" surface list? I'm not completely sure I follow all the references there though so maybe I'm missing something.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5177#note_63195
On Fri Mar 1 14:12:32 2024 +0000, Rémi Bernon wrote:
Also, as you're removing the surfaces from the global list, maybe you don't even need a separate entry, and instead can use the same to link them into the "release" surface list? I'm not completely sure I follow all the references there though so maybe I'm missing something. As I was writing my reply below, a concern that's been forming in my mind with the list based approach is that we are making various assumptions about the states we can and cannot get into and which I feel: 1. make the code more fragile (since these assumptions may be invalidated) 2. they are somewhat complex to think through, so I don't feel super confident about the correctness in the first place.
An array based approach (or any approach that has local only release state) seems to me to be much more straightforward.
Also, as you're removing the surfaces from the global list, maybe you don't even need a separate entry, and instead can use the same to link them into the "release" surface list? I'm not completely sure I follow all the references there though so maybe I'm missing something.
I think that would be valid to do *if* we use the release list only for the `update_context_drawables` case (but read below). For `wgl_context_make_current` that's invalid since we are not removing the drawables from the global list. Even for `update_context_drawables` it's not obviously correct, since in the second to last commit there is: ``` update_context_drawables(new, old, release) { if (ctx->draw == old || ctx->new_draw == old) { if (ctx->new_draw) wayland_gl_drawable_array_add(release, ctx->new_draw); ... } ... } ``` This blurs the situation a bit, since theoretically we could release `new_draw (!= old)` because `draw == old`, but I *think* that's not a case we can actually get into. That is, I think that any valid `new_draw` is guaranteed to have been the most recent one for the HWND and thus the one we just released (thus `== old`)
You can initialize the list entries on creation, and always list_remove + list_add_tail to make sure it's not added twice.
Sure, that would be required to ensure they are added to the release list properly, but the issue is that we do want to release the drawables multiple times since they have been acquired multiple times (e.g., ctx->new_draw and ctx->new_read). That's why I mentioned that with this approach we also need some kind of release count. I am not particularly fond of including fields in the structs for such particular use cases that seem to be more of an "external" concern, but all that aside, it dawned on me that another danger of this approach is that it's potentially thread unsafe. The per-drawable "release" state needs to be accessed outside of the drawable lock (by design): ``` lock(); list_remove(&drawable->release_entry); /* or possibly reuse drawable->entry */ list_add_tail(&release_list, &drawable->release_entry); ++drawable->release_count; unlock(); /* This needs to occur outside of lock, so other threads could in theory mess up with drawable->release_entry|count. */ LIST_FOR_EACH_ENTRY(drawable, &release_list ...) while (drawable->release_count--) wayland_gl_drawable_release(drawable); ```
What I don't like very much is the use of some non-Wine helper, it strongly couples the code to wayland libraries and makes it more difficult to refactor later.
That's a fair point, although perhaps the solution here is for Wine to grow such a utility :) A simple growable array is generally useful, and from a quick look around the codebase there seem to be several `realloc`-based sequences that would be candidates for it. If you feel that this could be a way forward I am happy to iterate on such a utility (header only like `wine/list.h`), but otherwise I can also get rid of `wl_array` and create some simple ad-hoc solution within `opengl.c`. (I think an even better utility would be a mixed stack/heap based dynamic array (optionally starts on the stack to avoid allocation for the most common cases, moves to heap on the first reallocation), but perhaps that's a step too far.) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5177#note_63210
On Fri Mar 1 12:42:51 2024 +0000, Rémi Bernon wrote:
It's probably driver dependent, and the documentation being simply incorrect. See for instance this run of wglinfo on actual Windows machines with physical GPUs: https://testbot.winehq.org/JobDetails.pl?Key=143604&f101=exe64.report&f102=e... Thanks, I will update the code to match reality (rather than docs) then :)
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5177#note_63211
On Fri Mar 1 15:53:00 2024 +0000, Alexandros Frantzis wrote:
As I was writing my reply below, a concern that's been forming in my mind with the list based approach is that we are making various assumptions about the states we can and cannot get into and which I feel: 1. make the code more fragile (since these assumptions may be invalidated) 2. they are somewhat complex to think through, so I don't feel super confident about the correctness in the first place. An array based approach (or any approach that has local only release state) seems to me to be much more straightforward.
Also, as you're removing the surfaces from the global list, maybe you don't even need a separate entry, and instead can use the same to link them into the "release" surface list? I'm not completely sure I follow all the references there though so maybe I'm missing something. I think that would be valid to do *if* we use the release list only for the `update_context_drawables` case (but read below). For `wgl_context_make_current` that's invalid since we are not removing the drawables from the global list. Even for `update_context_drawables` it's not obviously correct, since in the second to last commit there is:
update_context_drawables(new, old, release) { if (ctx->draw == old || ctx->new_draw == old) { if (ctx->new_draw) wayland_gl_drawable_array_add(release, ctx->new_draw); ... } ... }This blurs the situation a bit, since theoretically we could release `new_draw (!= old)` because `draw == old`, but I *think* that's not a case we can actually get into. That is, I think that any valid `new_draw` is guaranteed to have been the most recent one for the HWND and thus the one we just released (thus `== old`)
You can initialize the list entries on creation, and always list_remove + list_add_tail to make sure it's not added twice. Sure, that would be required to ensure they are added to the release list properly, but the issue is that we do want to release the drawables multiple times since they have been acquired multiple times (e.g., ctx->new_draw and ctx->new_read). That's why I mentioned that with this approach we also need some kind of release count. I am not particularly fond of including fields in the structs for such particular use cases that seem to be more of an "external" concern, but all that aside, it dawned on me that another danger of this approach is that it's potentially thread unsafe. The per-drawable "release" state needs to be accessed outside of the drawable lock (by design):
lock(); list_remove(&drawable->release_entry); /* or possibly reuse drawable->entry */ list_add_tail(&release_list, &drawable->release_entry); ++drawable->release_count; unlock(); /* This needs to occur outside of lock, so other threads could in theory mess up with drawable->release_entry|count. */ LIST_FOR_EACH_ENTRY(drawable, &release_list ...) while (drawable->release_count--) wayland_gl_drawable_release(drawable);What I don't like very much is the use of some non-Wine helper, it strongly couples the code to wayland libraries and makes it more difficult to refactor later. That's a fair point, although perhaps the solution here is for Wine to grow such a utility :) A simple growable array is generally useful, and from a quick look around the codebase there seem to be several `realloc`-based sequences that would be candidates for it. If you feel that this could be a way forward I am happy to iterate on such a utility (header only like `wine/list.h`), but otherwise I can also get rid of `wl_array` and create some simple ad-hoc solution within `opengl.c`. (I think an even better utility would be a mixed stack/heap based dynamic array (optionally starts on the stack to avoid allocation for the most common cases, moves to heap on the first reallocation), but perhaps that's a step too far.) Maybe it would be a bit simpler if the contexts didn't keep a reference on their surfaces. It doesn't look very useful as you are actually replacing the pointers in the contexts when the drawables are destroyed.
Though this is actually true for the new_read/new_draw ones, and there's arguably still the read/draw pointers which aren't replaced until the next swap, but perhaps you could move these "destroyed but still in use" drawables in a separate global list if they need to be kept alive for some reason. Or maybe find a different way, because the four drawable pointers per context, the references and global locking feels quite complicated and difficult to follow. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5177#note_63214
participants (3)
-
Alexandros Frantzis -
Alexandros Frantzis (＠afrantzis)
-
Rémi Bernon