[PATCH v3 0/2] MR3053: opengl32: Do not expose ARB_buffer_storage on wow64.
We don't properly support persistent mapping, so don't pretend to support ARB_buffer_storage. -- v3: opengl32: Do not report a GL version higher than 4.3 on wow64. https://gitlab.winehq.org/wine/wine/-/merge_requests/3053
From: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/opengl32/unix_wgl.c | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/dlls/opengl32/unix_wgl.c b/dlls/opengl32/unix_wgl.c index 6c3379b90a2..efb16464d64 100644 --- a/dlls/opengl32/unix_wgl.c +++ b/dlls/opengl32/unix_wgl.c @@ -43,6 +43,13 @@ WINE_DEFAULT_DEBUG_CHANNEL(opengl); +static const BOOL is_win64 = (sizeof(void *) > sizeof(int)); + +static BOOL is_wow64(void) +{ + return !!NtCurrentTeb()->WowTebOffset; +} + static pthread_mutex_t wgl_lock = PTHREAD_MUTEX_INITIALIZER; /* handle management */ @@ -168,7 +175,13 @@ static GLubyte *filter_extensions_list( const char *extensions, const char *disa memcpy( p, extensions, end - extensions ); p[end - extensions] = 0; - if (!has_extension( disabled, p, strlen( p ) )) + /* We do not support GL_MAP_PERSISTENT_BIT, and hence + * ARB_buffer_storage, on wow64. */ + if (is_win64 && is_wow64() && (!strcmp( p, "GL_ARB_buffer_storage" ) || !strcmp( p, "GL_EXT_buffer_storage" ))) + { + TRACE( "-- %s (disabled due to wow64)\n", p ); + } + else if (!has_extension( disabled, p, strlen( p ) )) { TRACE( "++ %s\n", p ); p += end - extensions; @@ -208,7 +221,15 @@ static GLuint *filter_extensions_index( TEB *teb, const char *disabled ) for (j = 0; j < extensions_count; ++j) { ext = (const char *)funcs->ext.p_glGetStringi( GL_EXTENSIONS, j ); - if (!has_extension( disabled, ext, strlen( ext ) )) + + /* We do not support GL_MAP_PERSISTENT_BIT, and hence + * ARB_buffer_storage, on wow64. */ + if (is_win64 && is_wow64() && (!strcmp( ext, "GL_ARB_buffer_storage" ) || !strcmp( ext, "GL_EXT_buffer_storage" ))) + { + TRACE( "-- %s (disabled due to wow64)\n", ext ); + disabled_index[i++] = j; + } + else if (!has_extension( disabled, ext, strlen( ext ) )) { TRACE( "++ %s\n", ext ); } @@ -329,7 +350,6 @@ static BOOL filter_extensions( TEB * teb, const char *extensions, GLubyte **exts else disabled = ""; } - if (!disabled[0]) return FALSE; if (extensions && !*exts_list) *exts_list = filter_extensions_list( extensions, disabled ); if (!*disabled_exts) *disabled_exts = filter_extensions_index( teb, disabled ); return (exts_list && *exts_list) || *disabled_exts; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3053
From: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/opengl32/unix_wgl.c | 53 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 48 insertions(+), 5 deletions(-) diff --git a/dlls/opengl32/unix_wgl.c b/dlls/opengl32/unix_wgl.c index efb16464d64..e58e5b5d9b5 100644 --- a/dlls/opengl32/unix_wgl.c +++ b/dlls/opengl32/unix_wgl.c @@ -412,6 +412,21 @@ static BOOL check_extension_support( TEB *teb, const char *extension, const char return FALSE; } +static void parse_gl_version( const char *gl_version, int *major, int *minor ) +{ + const char *ptr = gl_version; + + *major = atoi( ptr ); + if (*major <= 0) + ERR( "Invalid OpenGL major version %d.\n", *major ); + + while (isdigit( *ptr )) ++ptr; + if (*ptr++ != '.') + ERR( "Invalid OpenGL version string %s.\n", debugstr_a(gl_version) ); + + *minor = atoi( ptr ); +} + static void wrap_glGetIntegerv( TEB *teb, GLenum pname, GLint *data ) { const struct opengl_funcs *funcs = teb->glTable; @@ -421,6 +436,21 @@ static void wrap_glGetIntegerv( TEB *teb, GLenum pname, GLint *data ) if (pname == GL_NUM_EXTENSIONS && (disabled = disabled_extensions_index( teb ))) while (*disabled++ != ~0u) (*data)--; + + if (is_win64 && is_wow64()) + { + /* 4.4 depends on ARB_buffer_storage, which we don't support on wow64. */ + if (pname == GL_MAJOR_VERSION && *data > 4) + *data = 4; + else if (pname == GL_MINOR_VERSION) + { + GLint major; + + funcs->gl.p_glGetIntegerv( GL_MAJOR_VERSION, &major ); + if (major == 4 && *data > 3) + *data = 3; + } + } } static const GLubyte *wrap_glGetString( TEB *teb, GLenum name ) @@ -428,12 +458,25 @@ static const GLubyte *wrap_glGetString( TEB *teb, GLenum name ) const struct opengl_funcs *funcs = teb->glTable; const GLubyte *ret; - if ((ret = funcs->gl.p_glGetString( name )) && name == GL_EXTENSIONS) + if ((ret = funcs->gl.p_glGetString( name ))) { - struct wgl_handle *ptr = get_current_context_ptr( teb ); - GLubyte **extensions = &ptr->u.context->extensions; - GLuint **disabled = &ptr->u.context->disabled_exts; - if (*extensions || filter_extensions( teb, (const char *)ret, extensions, disabled )) return *extensions; + if (name == GL_EXTENSIONS) + { + struct wgl_handle *ptr = get_current_context_ptr( teb ); + GLubyte **extensions = &ptr->u.context->extensions; + GLuint **disabled = &ptr->u.context->disabled_exts; + if (*extensions || filter_extensions( teb, (const char *)ret, extensions, disabled )) return *extensions; + } + else if (name == GL_VERSION && is_win64 && is_wow64()) + { + int major, minor; + + parse_gl_version( (const char *)ret, &major, &minor ); + + /* 4.4 depends on ARB_buffer_storage, which we don't support on wow64. */ + if (major > 4 || (major == 4 && minor >= 4)) + return (const GLubyte *)"4.3"; + } } return ret; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3053
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=133824 Your paranoid android. === debian11 (32 bit report) === Report validation errors: quartz:vmr7 has no test summary line (early exit of the main process?) quartz:vmr7 has unaccounted for todo messages quartz:vmr7 has unaccounted for skip messages
On Wed Jun 14 19:41:58 2023 +0000, Rémi Bernon wrote:
Please keep the module style consistent. Oops, I copied that from wined3d and forgot to change its style to match its surroundings. Done now.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3053#note_35817
Alex Henrie (@alexhenrie) commented about dlls/opengl32/unix_wgl.c:
memcpy( p, extensions, end - extensions ); p[end - extensions] = 0;
- if (!has_extension( disabled, p, strlen( p ) )) + /* We do not support GL_MAP_PERSISTENT_BIT, and hence + * ARB_buffer_storage, on wow64. */ + if (is_win64 && is_wow64() && (!strcmp( p, "GL_ARB_buffer_storage" ) || !strcmp( p, "GL_EXT_buffer_storage" ))) + { + TRACE( "-- %s (disabled due to wow64)\n", p );
Should this be FIXME? (Is it something that we'd like to support in the future?) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3053#note_35996
On Sat Jun 17 05:02:30 2023 +0000, Alex Henrie wrote:
Should this be FIXME? (Is it something that we'd like to support in the future?) Probably a better approach is to introduce a new GL extension that will allow us to control where memory is mapped. There is such an extension in the works for Vulkan, but as far as I'm aware nobody has started one for GL yet.
We could make this a FIXME anyway, though I don't know if there's a point. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3053#note_36130
On Mon Jun 19 16:46:53 2023 +0000, Zebediah Figura wrote:
Probably a better approach is to introduce a new GL extension that will allow us to control where memory is mapped. There is such an extension in the works for Vulkan, but as far as I'm aware nobody has started one for GL yet. We could make this a FIXME anyway, though I don't know if there's a point. I just wanted to make sure that it wasn't a copy-paste mistake. Thanks for the explanation.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3053#note_36131
This appears to be languishing. If this is languishing because there's a concern that this will break applications that request 4.4 and don't actually need ARB_buffer_storage, I will offer the following points: (1) not adding this commit will break applications that *do* need ARB_buffer_storage. There's no evidence of which group is larger, but persistent mapping is not exactly a corner feature of the language; (2) even if some applications are broken in new wow64, there is still a simple workaround—viz. use old wow64; (3) currently *all* d3d applications are broken in new wow64, because if wined3d is told that ARB_buffer_storage is available then it will use it; however, wined3d is perfectly capable of doing buffer updates using non-persistent memory, even for the most recent d3d versions (potentially at a significant loss of performance, but this is still better than failing to render at all); (4) fixing this the "right" way requires adding a new extension, which is a slow process and, moreover, will never be universal, so we will need this anyway. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3053#note_36846
participants (4)
-
Alex Henrie (＠alexhenrie) -
Marvin -
Zebediah Figura
-
Zebediah Figura (＠zfigura)