[PATCH 3/4] Generate swrast GLX extension list, rather than using a fixed list

[Jeremy Huddleston Sequoia]

I don't really like having redundant data.  Is there really a need for us to store glx_enable_bits in the DRI screen record?  It seems like we could just keep that local to __glXDRIscreenProbe.

On Jul 20, 2012, at 05:54, Jon TURNEY <jon.turney at dronecode.org.uk> wrote:

> Ensure this kind of bug doesn't occur in future, by generating the GLX extension
> list for swrast, in the same way as dri and dri2 do, rather than using a fixed
> list of GLX extensions for swrast.
> 
> We explicity select the extensions reported by swrast rather than using
> __glXInitExtensionEnableBits(), to maintain the historical behaviour, which is
> slightly different:
> 
> - GLX_SGIS_multisample is not reported on APPLE
> - GLX_SGIX_visual_select_group isn't reported
> 
> (How swrast handles GLX_MESA_copy_sub_buffer still looks a bit wonky: We always
> enable it, but then subsequently also check if the loaded driver supports it,
> and if it doesn't all glxCopySubBufferMESA() calls are just going to fail
> GLXBadDrawable as the copySubBuffer function pointer is NULL.  This probably
> isn't a practical concern.)
> 
> Signed-off-by: Jon TURNEY <jon.turney at dronecode.org.uk>
> ---
> glx/glxdriswrast.c |   36 ++++++++++++++++++++++++++++++++++++
> glx/glxscreens.c   |   14 +-------------
> 2 files changed, 37 insertions(+), 13 deletions(-)
> 
> diff --git a/glx/glxdriswrast.c b/glx/glxdriswrast.c
> index b478398..d5cfaf1 100644
> --- a/glx/glxdriswrast.c
> +++ b/glx/glxdriswrast.c
> @@ -75,6 +75,8 @@ struct __GLXDRIscreen {
>     const __DRIcopySubBufferExtension *copySubBuffer;
>     const __DRItexBufferExtension *texBuffer;
>     const __DRIconfig **driConfigs;
> +
> +    unsigned char glx_enable_bits[__GLX_EXT_BYTES];
> };
> 
> struct __GLXDRIcontext {
> @@ -434,6 +436,7 @@ __glXDRIscreenProbe(ScreenPtr pScreen)
> {
>     const char *driverName = "swrast";
>     __GLXDRIscreen *screen;
> +    size_t buffer_size;
> 
>     screen = calloc(1, sizeof *screen);
>     if (screen == NULL)
> @@ -445,6 +448,26 @@ __glXDRIscreenProbe(ScreenPtr pScreen)
>     screen->base.swapInterval = NULL;
>     screen->base.pScreen = pScreen;
> 
> +    /*
> +      Rather than calling __glXInitExtensionEnableBits, we explicitly enable a
> +      specific set of extensions here to maintain the historical behaviour, which
> +      is slightly different.
> +    */
> +    __glXEnableExtension(screen->glx_enable_bits, "GLX_ARB_multisample");
> +    __glXEnableExtension(screen->glx_enable_bits, "GLX_EXT_visual_info");
> +    __glXEnableExtension(screen->glx_enable_bits, "GLX_EXT_visual_rating");
> +    __glXEnableExtension(screen->glx_enable_bits, "GLX_EXT_import_context");
> +    __glXEnableExtension(screen->glx_enable_bits, "GLX_EXT_texture_from_pixmap");
> +    __glXEnableExtension(screen->glx_enable_bits, "GLX_OML_swap_method");
> +    __glXEnableExtension(screen->glx_enable_bits, "GLX_SGI_make_current_read");
> +#ifndef __APPLE__
> +    __glXEnableExtension(screen->glx_enable_bits, "GLX_SGIS_multisample");
> +#endif
> +    __glXEnableExtension(screen->glx_enable_bits, "GLX_SGIX_fbconfig");
> +    __glXEnableExtension(screen->glx_enable_bits, "GLX_SGIX_pbuffer");
> +    __glXEnableExtension(screen->glx_enable_bits, "GLX_SGIX_visual_select_group");
> +    __glXEnableExtension(screen->glx_enable_bits, "GLX_MESA_copy_sub_buffer");
> +
>     screen->driver = glxProbeDriver(driverName,
>                                     (void **) &screen->core,
>                                     __DRI_CORE, __DRI_CORE_VERSION,
> @@ -473,6 +496,19 @@ __glXDRIscreenProbe(ScreenPtr pScreen)
> 
>     __glXScreenInit(&screen->base, pScreen);
> 
> +    /* The first call simply determines the length of the extension string.
> +     * This allows us to allocate some memory to hold the extension string,
> +     * but it requires that we call __glXGetExtensionString a second time.
> +     */
> +    buffer_size = __glXGetExtensionString(screen->glx_enable_bits, NULL);
> +    if (buffer_size > 0) {
> +        free(screen->base.GLXextensions);
> +
> +        screen->base.GLXextensions = xnfalloc(buffer_size);
> +        (void) __glXGetExtensionString(screen->glx_enable_bits,
> +                                       screen->base.GLXextensions);
> +    }
> +
>     screen->base.GLXmajor = 1;
>     screen->base.GLXminor = 4;
> 
> diff --git a/glx/glxscreens.c b/glx/glxscreens.c
> index 037b037..3ca2105 100644
> --- a/glx/glxscreens.c
> +++ b/glx/glxscreens.c
> @@ -163,18 +163,6 @@ static const char GLServerExtensions[] =
> static char GLXServerVendorName[] = "SGI";
> unsigned glxMajorVersion = SERVER_GLX_MAJOR_VERSION;
> unsigned glxMinorVersion = SERVER_GLX_MINOR_VERSION;
> -static char GLXServerExtensions[] =
> -    "GLX_ARB_multisample "
> -    "GLX_EXT_visual_info "
> -    "GLX_EXT_visual_rating "
> -    "GLX_EXT_import_context "
> -    "GLX_EXT_texture_from_pixmap "
> -    "GLX_OML_swap_method " "GLX_SGI_make_current_read "
> -#ifndef __APPLE__
> -    "GLX_SGIS_multisample "
> -#endif
> -    "GLX_SGIX_fbconfig "
> -    "GLX_SGIX_pbuffer " "GLX_MESA_copy_sub_buffer ";
> 
> static Bool
> glxCloseScreen(ScreenPtr pScreen)
> @@ -328,7 +316,7 @@ __glXScreenInit(__GLXscreen * pGlxScreen, ScreenPtr pScreen)
>     pGlxScreen->pScreen = pScreen;
>     pGlxScreen->GLextensions = strdup(GLServerExtensions);
>     pGlxScreen->GLXvendor = strdup(GLXServerVendorName);
> -    pGlxScreen->GLXextensions = strdup(GLXServerExtensions);
> +    pGlxScreen->GLXextensions = strdup("");
> 
>     /* All GLX providers must support all of the functionality required for at
>      * least GLX 1.2.  If the provider supports a higher version, the GLXminor
> -- 
> 1.7.9
> 
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
>