[Mesa-dev] [PATCH 4/8] Support of RENDER_TYPE property [4/6] Handling in glXCreateContext and init_fbconfig_for_chooser.

[Ian Romanick]

On 07/15/2013 07:28 AM, Tomasz Lis wrote:
> This patch sets the correct values of renderType and drawableType
>   in glXCreateContext and init_fbconfig_for_chooser routines.
> ---
>   src/glx/glxcmds.c |   32 +++++++++++++++++++++++++++-----
>   1 file changed, 27 insertions(+), 5 deletions(-)
>
> diff --git a/src/glx/glxcmds.c b/src/glx/glxcmds.c
> index 51b2237..967dab3 100644
> --- a/src/glx/glxcmds.c
> +++ b/src/glx/glxcmds.c
> @@ -373,7 +373,25 @@ glXCreateContext(Display * dpy, XVisualInfo * vis,
>         return None;
>      }
>
> -   renderType = config->rgbMode ? GLX_RGBA_TYPE : GLX_COLOR_INDEX_TYPE;
> +   /* Choose the context render type based on DRI config values.
> +    * It is unusual to set this type from config, but we have no other choice,
> +    * as this old API does not provide renderType parameter. */
> +   if (config->renderType & GLX_RGBA_FLOAT_BIT_ARB) {

I /think/ GCC will generate a warning about this.  Something like 
'suggest parenthesis around ...".  Could you double check that and fix 
the warning if it exists?

> +       renderType = GLX_RGBA_FLOAT_TYPE_ARB;
> +   } else if (config->renderType & GLX_RGBA_UNSIGNED_FLOAT_BIT_EXT) {
> +       renderType = GLX_RGBA_UNSIGNED_FLOAT_TYPE_EXT;
> +   } else if (config->renderType & GLX_RGBA_BIT) {
> +       renderType = GLX_RGBA_TYPE;
> +   } else if (config->renderType & GLX_COLOR_INDEX_BIT) {
> +       renderType = GLX_COLOR_INDEX_TYPE;
> +   } else if (config->rgbMode == GL_TRUE) {

       } else if (config->rgbMode) {

However, I'm unsure about this.  It seems like if the renderType is not 
correctly set that something has gone seriously wrong.  Shouldn't this 
function just fail?

> +       /* If we're here, then renderType is not set correctly.
> +        * Let's use a safeguard - any TrueColor or DirectColor mode is RGB mode */
> +       renderType = GLX_RGBA_TYPE;
> +   } else {
> +       /* Safeguard - only one option left, all non-RGB modes are indexed modes */
> +       renderType = GLX_COLOR_INDEX_TYPE;
> +   }
>   #endif
>
>      return CreateContext(dpy, vis->visualid, config, shareList, allowDirect,
> @@ -860,12 +878,17 @@ init_fbconfig_for_chooser(struct glx_config * config,
>      config->visualID = (XID) GLX_DONT_CARE;
>      config->visualType = GLX_DONT_CARE;
>
> -   /* glXChooseFBConfig specifies different defaults for these two than
> +   /* glXChooseFBConfig specifies different defaults for these properties than
>       * glXChooseVisual.
>       */
>      if (fbconfig_style_tags) {
>         config->rgbMode = GL_TRUE;
>         config->doubleBufferMode = GLX_DONT_CARE;
> +      /* allow any kind of drawable, including those for off-screen buffers */
> +      config->drawableType = 0;
> +   } else {
> +       /* allow configs which support on-screen drawing */
> +       config->drawableType = GLX_WINDOW_BIT;
>      }

This feels like it should be a separate commit.  Is there some spec text 
or further justification for this?  We ought to at least have a piglit 
test case...

>      config->visualRating = GLX_DONT_CARE;
> @@ -876,9 +899,8 @@ init_fbconfig_for_chooser(struct glx_config * config,
>      config->transparentAlpha = GLX_DONT_CARE;
>      config->transparentIndex = GLX_DONT_CARE;
>
> -   config->drawableType = GLX_WINDOW_BIT;
> -   config->renderType =
> -      (config->rgbMode) ? GLX_RGBA_BIT : GLX_COLOR_INDEX_BIT;
> +   /* Set GLX_RENDER_TYPE property to not expect any flags by default. */
> +   config->renderType = 0;
>      config->xRenderable = GLX_DONT_CARE;
>      config->fbconfigID = (GLXFBConfigID) (GLX_DONT_CARE);
>
>