[prev in list] [next in list] [prev in thread] [next in thread] 

List:       wine-devel
Subject:    Re: [PATCH 3/3] opengl32: Use opengl xml registry files.
From:       Rico Schüller <kgbricola () web ! de>
Date:       2013-10-31 18:55:47
Message-ID: 5272A7B3.70601 () web ! de
[Download RAW message or body]

On 31.10.2013 11:37, Henri Verbeet wrote:
> On 31 October 2013 11:16, Rico Schüller <kgbricola@web.de> wrote:
>> I found some other issues and now I think the xml seems not use able for
>> us in the state it is now. Sorry for the noise.
>>
>> E.g. glColorSubTable is listed in the extension GL_ARB_imaging, but not
>> as GL 1.2 requirement. All linux drivers support at least 1.2 (and thus
>> don't expose GL_ARB_imaging), so there is no way to tell if we should
>> support that function or not. We would need a separate list for that ...
>>
>> Imho this works for glBlendColor which is in GL_ARB_imaging and
>> GL_VERSION_1_4, but this seems also wrong as it should be in GL 1.2.
>>
>> Did I miss anything?
>>
> If this is just for functions in GL_ARB_imaging, I think this is
> correct / intentional. The GL "imaging subset" has always been a bit
> special, and not many drivers implement it, although IIRC at least
> NVIDIA does. For all the details see section 3.6.2 of the GL 1.2.1
> spec, but the bottom line is that it's not actually available unless
> you have GL_ARB_imaging in the extension string, despite being
> described in the GL 1.2.1 spec.
> 
Yeah, should be mostly for GL_ARB_imaging... so basically using
GL_ARB_imaging is fine then and the previous GL_VERSION_1_2 was wrong.

Well there are some other issues like e.g. glVertexAttrib4ubvARB which
is added by both extensions (GL_ARB_vertex_shader and
GL_ARB_vertex_program) and I'm not sure there is one preferred over the
other. Also, there are functions which are added in two ways, e.g.
glVertexP2ui (GL_ARB_vertex_type_2_10_10_10_rev and GL_VERSION_3_3).

The attached patch would fix both (and the parser needs to add list all
versions and extensions). I think, it is fine, if we would have support
til here, as that is what's working with the gl.spec file as of now.
I'll update the complete series, if there are no other concerns.


How do we handle glNamedFramebufferParameteriEXT? It basically needs two
extensions (GL_ARB_framebuffer_no_attachments and
GL_EXT_direct_state_access), but only a comment in the gl.xml mentions
that. Do we need to take care of that case or do most drivers support
both extensions anyway together?

Cheers
Rico




["o.patch" (text/x-patch)]

commit 7798b24f6cee9f8f0847421c7cbeab3dce30a373
Author: Rico Schüller <kgbricola@web.de>
Date:   Wed Oct 30 11:48:04 2013 +0100

    opengl32: Allow multiple extensions to support the same function.

diff --git a/dlls/opengl32/wgl.c b/dlls/opengl32/wgl.c
index 361a1ec..d316d94 100644
--- a/dlls/opengl32/wgl.c
+++ b/dlls/opengl32/wgl.c
@@ -665,10 +665,8 @@ int WINAPI wglGetLayerPaletteEntries(HDC hdc,
 }
 
 /* check if the extension is present in the list */
-static BOOL has_extension( const char *list, const char *ext )
+static BOOL has_extension( const char *list, const char *ext, size_t len )
 {
-    size_t len = strlen( ext );
-
     while (list)
     {
         while (*list == ' ') list++;
@@ -688,6 +686,7 @@ static BOOL is_extension_supported(const char* extension)
 {
     const struct opengl_funcs *funcs = NtCurrentTeb()->glTable;
     const char *gl_ext_string = (const char*)glGetString(GL_EXTENSIONS);
+    size_t len;
 
     TRACE("Checking for extension '%s'\n", extension);
 
@@ -702,30 +701,35 @@ static BOOL is_extension_supported(const char* extension)
      * require the function to return NULL when an extension isn't found. For this \
                reason we check
      * if the OpenGL extension required for the function we are looking up is \
supported. */  
-    /* Check if the extension is part of the GL extension string to see if it is \
                supported. */
-    if (has_extension(gl_ext_string, extension))
-        return TRUE;
-
-    /* In general an OpenGL function starts as an ARB/EXT extension and at some \
                stage
-     * it becomes part of the core OpenGL library and can be reached without the \
                ARB/EXT
-     * suffix as well. In the extension table, these functions contain \
                GL_VERSION_major_minor.
-     * Check if we are searching for a core GL function */
-    if(strncmp(extension, "GL_VERSION_", 11) == 0)
+    while ((len = strcspn(extension, " ")) != 0)
     {
-        const GLubyte *gl_version = funcs->gl.p_glGetString(GL_VERSION);
-        const char *version = extension + 11; /* Move past 'GL_VERSION_' */
+        /* Check if the extension is part of the GL extension string to see if it is \
supported. */ +        if (has_extension(gl_ext_string, extension, len))
+            return TRUE;
 
-        if(!gl_version) {
-            ERR("No OpenGL version found!\n");
-            return FALSE;
-        }
+        /* In general an OpenGL function starts as an ARB/EXT extension and at some \
stage +         * it becomes part of the core OpenGL library and can be reached \
without the ARB/EXT +         * suffix as well. In the extension table, these \
functions contain GL_VERSION_major_minor. +         * Check if we are searching for a \
core GL function */ +        if(strncmp(extension, "GL_VERSION_", 11) == 0)
+        {
+            const GLubyte *gl_version = funcs->gl.p_glGetString(GL_VERSION);
+            const char *version = extension + 11; /* Move past 'GL_VERSION_' */
 
-        /* Compare the major/minor version numbers of the native OpenGL library and \
                what is required by the function.
-         * The gl_version string is guaranteed to have at least a major/minor and \
                sometimes it has a release number as well. */
-        if( (gl_version[0] >= version[0]) || ((gl_version[0] == version[0]) && \
                (gl_version[2] >= version[2])) ) {
-            return TRUE;
+            if(!gl_version) {
+                ERR("No OpenGL version found!\n");
+                return FALSE;
+            }
+
+            /* Compare the major/minor version numbers of the native OpenGL library \
and what is required by the function. +             * The gl_version string is \
guaranteed to have at least a major/minor and sometimes it has a release number as \
well. */ +            if( (gl_version[0] >= version[0]) || ((gl_version[0] == \
version[0]) && (gl_version[2] >= version[2])) ) { +                return TRUE;
+            }
+            WARN("The function requires OpenGL version '%c.%c' while your drivers \
only provide '%c.%c'\n", version[0], version[2], gl_version[0], gl_version[2]);  }
-        WARN("The function requires OpenGL version '%c.%c' while your drivers only \
provide '%c.%c'\n", version[0], version[2], gl_version[0], gl_version[2]); +
+        extension += len;
     }
 
     return FALSE;
@@ -1681,7 +1685,7 @@ static GLubyte *filter_extensions( const char *extensions )
             if (!(end = strchr( extensions, ' ' ))) end = extensions + strlen( \
extensions );  memcpy( p, extensions, end - extensions );
             p[end - extensions] = 0;
-            if (!has_extension( disabled, p ))
+            if (!has_extension( disabled, p , strlen( p )))
             {
                 TRACE("++ %s\n", p );
                 p += end - extensions;





[prev in list] [next in list] [prev in thread] [next in thread]