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

[Tomasz Lis]

Thank you for your comments.

GCC Warnings:

There are no warnings in "glxcmds.c" during build. I think GCC only warns
when multiple logic/bitwise operators are used in a row.

Setting context renderType from config renderType:

Invalid value of renderType in configs may be caused by DRI drivers which
got used to the fact that the parameter was irrelevant.
Note that the two cases where renderType flags are empty will give
identical results to what unpatched code would do.
If you still think glXCreateContext should fail if config renderType is
invalid, I can do this change.

drawableType setting in glXChooseFBConfig vs glXChooseVisual:
I will make a separate commit out of this. The justification is that visual
configs are on-screen configs - so they have to support drawing on a
window, by definition.


2013/7/15 Ian Romanick <idr at freedesktop.org>

> 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);
>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130716/91e33dc3/attachment-0001.html>