So I moved the glx extension data to the end of __DRIscreenRec and added a pointer to __DRIscreenPrivateRec that points back to the __DRIscreenRec. This pointer is then passed to __glXScrEnable/DisableExtension by the driver's createScreen function. There may be binary compatibility issues as __DRIscreenRec is accessed by both libGL and the drivers. But adding new stuff to the end should be fine. Those added entries are only accessed by libGL.
If nothing else, you should bump the GLX interface version and add a comment in the structure. Other than that, I think it should be fine.
Then it may be better to move that data back to __GLXscreenConfigs and add a pointer to __DRIscreenRec that points back to __GLXscreenConfigs. Thoughts?
Hmm...__DRIscreen seems like the right place, but putting the data in __GLXscreenConfigsRec would eliminate some tests in the code. I'm leaning towards leaving it in __DRIscreen, but I could probably be swayed the other way. :)
Sorry if the formatting is funky below. Your patches show up as application/octet-stream, so Mozilla Mail doesn't put them in-line. I had to cut-n-paste to get it in.
> Index: glx/glxextensions.c
> ===================================================================
> RCS file: /cvsroot/dri/xc/xc/lib/GL/glx/glxextensions.c,v
> retrieving revision 1.3
> diff -u -r1.3 glxextensions.c
> --- glx/glxextensions.c 3 May 2003 05:19:30 -0000 1.3
> +++ glx/glxextensions.c 9 Aug 2003 20:58:53 -0000
> @@ -116,18 +116,23 @@
> { NULL }
> };
>
> +/* global list of available extensions */
> static struct glx_extension ext_list;
> static GLboolean ext_list_first_time = GL_TRUE;
> static GLuint next_bit = 0;I think that, since we can no longer add new extension strings, we can eliminate next_bit as well.
> +/* global bit-fields of available extensions and their characteristics */
> static unsigned char client_support[8];
> -static unsigned char direct_support[8];
> static unsigned char client_only[8];
> static unsigned char direct_only[8];
> +/* extensions enabled by default on all screens */
> +static unsigned char direct_support[8];
After this patch is committed, there is another change we could make here. All of these values (and __glXGLXClientExtensions) are, essentially, compile-time constants. We could then:
1. Change these to use static initializers & make them const.
2. Change default_glx_extensions to a new name, such as known_glx_extensions & make known_glx_extensions const.
3. Remove the *_support & *_only fields from known_glx_extensions.
4. Eliminate ext_list, add_extension, & __glXExtensionsCtr.
[snip]
> /**
> - * Disable a named GLX extension.
> + * Disable a named GLX extension on a given screen.
> *
> * \param name Name of the extension to disable.
> * \sa __glXEnableExtension
> */
> void
> -__glXDisableExtension( const char * name )
> +__glXScrDisableExtension( __DRIscreen *psc, const char * name )
> {
> if ( __glXGLXClientExtensions == NULL ) {
> __glXExtensionsCtr();
> - set_glx_extension( name, strlen( name ), GL_FALSE, direct_support );
> + __glXExtensionsCtrScreen(psc);
> + set_glx_extension( name, strlen( name ), GL_FALSE, psc->direct_support );
> }
I think we can relax this restriction a bit now. Querying glXGetClientString( dpy, GLX_EXTENSIONS ) used to be the condition because drivers could add new extensions. Since we've taken that ability away, can we relax the condition? Perhaps check to see if psc->effectiveGLXexts is still NULL instead?
[snip]
> @@ -451,13 +430,15 @@ > * \returns A pointer to the string. > */
The psc parameter should be documented here. The important thing to document is when it is valid for pcs to be NULL. It *appears* to me that psc will be NULL iff GLX_DIRECT_RENDERING is not defined. If that is the case, then should we have some conditionally compiled asserts? Something like:
#ifdef GLX_DIRECT_RENDERING assert( psc != NULL ); #else assert( psc == NULL ); #endif
In that case we could also conditionally compile some code in the loop. See my comments below.
> char *
> -__glXGetUsableExtensions( const char * server_string )
> +__glXGetUsableExtensions( __DRIscreen *psc, const char * server_string )
> {
> unsigned char server_support[8];
> unsigned char usable[8];
> unsigned i;
>
> __glXExtensionsCtr();
> + if ( psc )
> + __glXExtensionsCtrScreen( psc );
> __glXProcessServerString( server_string, server_support );
>
> /* An extension is supported if the client-side (i.e., libGL) supports
> @@ -466,9 +447,11 @@
> * the direct rendering driver supports it.
> */
> for ( i = 0 ; i < 8 ; i++ ) {
> - usable[i] = (client_support[i] & client_only[i])
> - | (client_support[i] & (direct_support[i] & server_support[i]))
> - | (client_support[i] & (direct_support[i] & direct_only[i]));
> + if ( psc )
> + usable[i] = client_support[i] & (client_only[i]
> + | (psc->direct_support[i] & (server_support[i] | direct_only[i])));
> + else
> + usable[i] = client_support[i] & client_only[i];
Gak. This troublesome bit of code...again. :) No matter what, server_support has to be considered. I think the else-case should be:
usable[i] = client_support[i] & (client_only[i]|server_support[i]);
This loop could be cleaned up in one of two ways. The first would be to conditionally compile the different paths based on GLX_DIRECT_RENDERING. The other would be to define a local variable called temp_direct_support and either copy the contents of psc->direct_support to it or memset it to all 0x0ff. Then there would be no need for the if-statement in the loop.
------------------------------------------------------- This SF.Net email sponsored by: Free pre-built ASP.NET sites including Data Reports, E-commerce, Portals, and Forums are available now. Download today and enter to win an XBOX or Visual Studio .NET. http://aspnet.click-url.com/go/psa00100003ave/direct;at.aspnet_072303_01/01 _______________________________________________ Dri-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/dri-devel
