[Mesa-dev] [PATCH 8/8] Float fbconfigs frontend patch [2/2] Introduced new flags in __DRI_ATTRIB_RENDER_TYPE.

[Ian Romanick]

On 07/15/2013 07:28 AM, Tomasz Lis wrote:
> From: Tomasz Lis <tomasz.lis at intel.com>
>
> The patch marks __DRI_ATTRIB_FLOAT_MODE as deprecated and introduces
> nrw flags to __DRI_ATTRIB_RENDER_TYPE which are to define float modes.
> Both signed float (fbconfig_float) and unsigned (packed_float) is
> introduced as a flag. The old attribute should be set for both float
> modes.
>
> Signed-off-by: Tomasz Lis <listom at gmail.com>
> ---
>   include/GL/internal/dri_interface.h |    2 ++
>   src/egl/drivers/dri2/egl_dri2.c     |   22 +++++++++++++---------
>   src/glx/dri_common.c                |   16 +++++++++++-----
>   src/mesa/drivers/dri/common/utils.c |    6 ------
>   4 files changed, 26 insertions(+), 20 deletions(-)
>
> diff --git a/include/GL/internal/dri_interface.h b/include/GL/internal/dri_interface.h
> index 30ce175..3316fd2 100644
> --- a/include/GL/internal/dri_interface.h
> +++ b/include/GL/internal/dri_interface.h
> @@ -552,6 +552,8 @@ struct __DRIuseInvalidateExtensionRec {
>   #define __DRI_ATTRIB_RGBA_BIT			0x01	
>   #define __DRI_ATTRIB_COLOR_INDEX_BIT		0x02
>   #define __DRI_ATTRIB_LUMINANCE_BIT		0x04
> +#define __DRI_ATTRIB_FLOAT_BIT             0x08
> +#define __DRI_ATTRIB_UNSIGNED_FLOAT_BIT    0x10
>
>   /* __DRI_ATTRIB_CONFIG_CAVEAT */
>   #define __DRI_ATTRIB_SLOW_BIT			0x01
> diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
> index 1bce314..7ec1a5d 100644
> --- a/src/egl/drivers/dri2/egl_dri2.c
> +++ b/src/egl/drivers/dri2/egl_dri2.c
> @@ -75,7 +75,7 @@ EGLint dri2_to_egl_attribute_map[] = {
>      0,				/* __DRI_ATTRIB_TRANSPARENT_GREEN_VALUE */
>      0,				/* __DRI_ATTRIB_TRANSPARENT_BLUE_VALUE */
>      0,				/* __DRI_ATTRIB_TRANSPARENT_ALPHA_VALUE */
> -   0,				/* __DRI_ATTRIB_FLOAT_MODE */
> +   0,				/* __DRI_ATTRIB_FLOAT_MODE (deprecated) */
>      0,				/* __DRI_ATTRIB_RED_MASK */
>      0,				/* __DRI_ATTRIB_GREEN_MASK */
>      0,				/* __DRI_ATTRIB_BLUE_MASK */
> @@ -136,14 +136,18 @@ dri2_add_config(_EGLDisplay *disp, const __DRIconfig *dri_config, int id,
>      while (dri2_dpy->core->indexConfigAttrib(dri_config, i++, &attrib, &value)) {
>         switch (attrib) {
>         case __DRI_ATTRIB_RENDER_TYPE:
> -	 if (value & __DRI_ATTRIB_RGBA_BIT)
> -	    value = EGL_RGB_BUFFER;
> -	 else if (value & __DRI_ATTRIB_LUMINANCE_BIT)
> -	    value = EGL_LUMINANCE_BUFFER;
> -	 else
> -	    /* not valid */;
> -	 _eglSetConfigKey(&base, EGL_COLOR_BUFFER_TYPE, value);
> -	 break;	
> +         if (value & __DRI_ATTRIB_RGBA_BIT)
> +            value = EGL_RGB_BUFFER;
> +         else if (value & __DRI_ATTRIB_LUMINANCE_BIT)
> +            value = EGL_LUMINANCE_BUFFER;
> +         else if (value & __DRI_ATTRIB_FLOAT_BIT)
> +            return NULL; // Don't allow float configs for EGL
> +         else if (value & __DRI_ATTRIB_UNSIGNED_FLOAT_BIT)
> +            return NULL; // Don't allow float configs for EGL
> +         else
> +            /* not valid */;
> +         _eglSetConfigKey(&base, EGL_COLOR_BUFFER_TYPE, value);
> +         break;

There are some spurious white space changes here.

Also, I think this code could be simplified by replacing the /* not 
valid */ comment with 'return NULL;'  The existing final else case just 
looks wrong to me.

Kristian: Do you remember why that code is like that?

>
>         case __DRI_ATTRIB_CONFIG_CAVEAT:
>            if (value & __DRI_ATTRIB_NON_CONFORMANT_CONFIG)
> diff --git a/src/glx/dri_common.c b/src/glx/dri_common.c
> index 7ab303e..872f5c3 100644
> --- a/src/glx/dri_common.c
> +++ b/src/glx/dri_common.c
> @@ -281,14 +281,20 @@ driConfigEqual(const __DRIcoreExtension *core,
>      while (core->indexConfigAttrib(driConfig, i++, &attrib, &value)) {
>         switch (attrib) {
>         case __DRI_ATTRIB_RENDER_TYPE:

Lots of spurious white space changes in this hunk too.  It looks like 
there are other spurious changes too... it makes it hard to tell what is 
actually changed.

> -         glxValue = 0;
> +          glxValue = 0;
>            if (value & __DRI_ATTRIB_RGBA_BIT) {
> -            glxValue |= GLX_RGBA_BIT;
> +             glxValue |= GLX_RGBA_BIT;
> +         }
> +         if (value & __DRI_ATTRIB_COLOR_INDEX_BIT) {
> +             glxValue |= GLX_COLOR_INDEX_BIT;
> +         }
> +         if (value & __DRI_ATTRIB_FLOAT_BIT) {
> +             glxValue |= GLX_RGBA_FLOAT_BIT_ARB;
>            }
> -         else if (value & __DRI_ATTRIB_COLOR_INDEX_BIT) {
> -            glxValue |= GLX_COLOR_INDEX_BIT;
> +         if (value & __DRI_ATTRIB_UNSIGNED_FLOAT_BIT) {
> +             glxValue |= GLX_RGBA_UNSIGNED_FLOAT_BIT_EXT;
>            }
> -         if (glxValue != config->renderType)
> +         if ((glxValue & config->renderType) == 0)
>               return GL_FALSE;
>            break;
>
> diff --git a/src/mesa/drivers/dri/common/utils.c b/src/mesa/drivers/dri/common/utils.c
> index ac0773b..c9fc218 100644
> --- a/src/mesa/drivers/dri/common/utils.c
> +++ b/src/mesa/drivers/dri/common/utils.c
> @@ -366,7 +366,6 @@ static const struct { unsigned int attrib, offset; } attribMap[] = {
>       __ATTRIB(__DRI_ATTRIB_TRANSPARENT_GREEN_VALUE,	transparentGreen),
>       __ATTRIB(__DRI_ATTRIB_TRANSPARENT_BLUE_VALUE,	transparentBlue),
>       __ATTRIB(__DRI_ATTRIB_TRANSPARENT_ALPHA_VALUE,	transparentAlpha),
> -    __ATTRIB(__DRI_ATTRIB_FLOAT_MODE,			floatMode),
>       __ATTRIB(__DRI_ATTRIB_RED_MASK,			redMask),
>       __ATTRIB(__DRI_ATTRIB_GREEN_MASK,			greenMask),
>       __ATTRIB(__DRI_ATTRIB_BLUE_MASK,			blueMask),
> @@ -418,11 +417,6 @@ driGetConfigAttribIndex(const __DRIconfig *config,
>           /* XXX no return value??? */
>   	break;
>
> -    case __DRI_ATTRIB_FLOAT_MODE:
> -        /* this field is not int-sized */
> -        *value = config->modes.floatMode;
> -        break;
> -
>       default:
>           /* any other int-sized field */
>   	*value = *(unsigned int *)
>