[RFC PATCH] glx: Try to reduce the number of GLX visuals

[Thomas Hellstrom]

On 09/26/2017 05:35 PM, Thomas Hellstrom wrote:
> Don't create GLX visuals that would be viewed as identical as an already
> existing GLX visual by glXChooseVisual. Multiple fbconfigs may point to
> the same GLX visual.
>
> Signed-off-by: Thomas Hellstrom <thellstrom at vmware.com>
> ---
>   glx/glxscreens.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 66 insertions(+), 1 deletion(-)
>
> diff --git a/glx/glxscreens.c b/glx/glxscreens.c
> index 99bf6dd..f07850d 100644
> --- a/glx/glxscreens.c
> +++ b/glx/glxscreens.c
> @@ -158,6 +158,55 @@ static const char GLServerExtensions[] =
>       "GL_SGIX_shadow_ambient "
>       "GL_SUN_slice_accum ";
>   
> +/**
> + * Are visuals equal in terms of glXChooseVisual?
> + *
> + * Some fields of the __GLXconfig structure are, according to the
> + * GLX spec not part of the visual structure meaning that their
> + * corresponding attribute can not be used in glXChooseVisual, nor
> + * using glXGetConfig. Eliminating those fields, check whether
> + * two __GLXconfigs would be considered equal.
> + */
> +static Bool
> +glxVisualsEqual(const __GLXconfig *c1, const __GLXconfig *c2) {
> +    /* GLX 1.4 spec */
> +    return c1->rgbBits == c2->rgbBits &&
> +	c1->indexBits == c2->indexBits &&
> +	c1->level == c2->level &&
> +	(c1->renderType & GLX_RGBA_BIT) ==
> +	(c2->renderType & GLX_RGBA_BIT) &&
> +	c1->doubleBufferMode == c2->doubleBufferMode &&
> +	c1->stereoMode == c2->stereoMode &&
> +	c1->numAuxBuffers == c2->numAuxBuffers &&
> +	c1->redBits == c2->redBits &&
> +	c1->greenBits == c2->greenBits &&
> +	c1->blueBits == c2->blueBits &&
> +	c1->alphaBits == c2->alphaBits &&
> +	c1->depthBits == c2->depthBits &&
> +	c1->stencilBits == c2->stencilBits &&
> +	c1->accumRedBits == c2->accumRedBits &&
> +	c1->accumGreenBits == c2->accumGreenBits &&
> +	c1->accumBlueBits == c2->accumBlueBits &&
> +	c1->accumAlphaBits == c2->accumAlphaBits &&
> +	c1->sampleBuffers == c2->sampleBuffers &&
> +	c1->samples == c2->samples &&
> +	c1->visualType == c2->visualType &&
> +	/* EXT_visual_rating, but NOT GLX 1.4 */
> +	c1->visualRating == c2->visualRating &&
> +	/* EXT_visual_info, but NOT GLX 1.4 */
> +	c1->transparentPixel == c2->transparentPixel &&
> +	c1->transparentRed == c2->transparentRed &&
> +	c1->transparentGreen == c2->transparentGreen &&
> +	c1->transparentBlue == c2->transparentBlue &&
> +	c1->transparentAlpha == c2->transparentAlpha &&
> +	c1->transparentIndex == c2->transparentIndex &&
> +	/* SGIX_visual_select_group */
> +	c1->visualSelectGroup == c2->visualSelectGroup &&
> +	/* ARB_framebuffer_sRGB */
> +	c1->sRGBCapable == c2->sRGBCapable;

Actually, if reducing visuals like this appears to be a good thing, we 
should probably also check the value of
the "duplicatedForComp" field.

/Thomas

> +}
> +
> +
>   static Bool
>   glxCloseScreen(ScreenPtr pScreen)
>   {
> @@ -358,9 +407,10 @@ __glXScreenInit(__GLXscreen * pGlxScreen, ScreenPtr pScreen)
>        * an existing, appropriate visual.
>        */
>       for (config = pGlxScreen->fbconfigs; config != NULL; config = config->next) {
> -        int depth;
> +        int depth, vis;
>   
>           VisualPtr visual;
> +        const __GLXconfig *config2;
>   
>           if (config->visualID != 0)
>               continue;
> @@ -393,6 +443,21 @@ __glXScreenInit(__GLXscreen * pGlxScreen, ScreenPtr pScreen)
>               continue;
>           }
>   
> +        /* We don't need to create a new visual if we already have an
> +         * identical visual. Instead we will have multiple fbconfigs
> +         * pointing to the same GLX visual.
> +         */
> +        for (vis = 0; vis < pGlxScreen->numVisuals; ++vis) {
> +            config2 = pGlxScreen->visuals[vis];
> +            if (glxVisualsEqual(config, config2)) {
> +                config->visualID = config2->visualID;
> +                break;
> +            }
> +        }
> +
> +        if (config->visualID != 0)
> +            continue;
> +
>           /* Create a new X visual for our FBconfig. */
>           visual = AddScreenVisuals(pScreen, 1, depth);
>           if (visual == NULL)