[Mesa-dev] [PATCH] glx: fill out GLX_DONT_CARE values

[Eric Engestrom]

On Saturday, 2018-08-11 17:19:04 +0100, Lionel Landwerlin wrote:
> When associating a template GLX config to a DRI one (filled out by the
> driver), we need to update the GLX_DONT_CARE fields (like yInverted
> for example). Otherwise those fields will report GLX_DONT_CARE to the
> application which is not what the driver is actually doing.
> 
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
> ---
>  src/glx/dri_common.c | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/src/glx/dri_common.c b/src/glx/dri_common.c
> index ab5d6c5bc03..c49c1a43c12 100644
> --- a/src/glx/dri_common.c
> +++ b/src/glx/dri_common.c
> @@ -339,6 +339,39 @@ driConfigEqual(const __DRIcoreExtension *core,
>     return GL_TRUE;
>  }
>  
> +static void
> +driUpdateDontCareValues(const __DRIcoreExtension *core,
> +                        struct glx_config *config,
> +                        const __DRIconfig *driConfig)
> +{
> +   unsigned int attrib, value;
> +   int i, j;
> +
> +   i = 0;
> +   while (core->indexConfigAttrib(driConfig, i++, &attrib, &value)) {

The `i++` hidden in the middle there bothers me; what about a for loop instead?

  for (int i = 0; core->indexConfigAttrib(driConfig, i, &attrib, &value); i++)

Either way:
Reviewed-by: Eric Engestrom <eric.engestrom at intel.com>

> +      switch (attrib) {
> +      case __DRI_ATTRIB_RENDER_TYPE:
> +      case __DRI_ATTRIB_CONFIG_CAVEAT:
> +      case __DRI_ATTRIB_BIND_TO_TEXTURE_TARGETS:
> +      case __DRI_ATTRIB_SWAP_METHOD:
> +         break;
> +
> +      default: {
> +         for (j = 0; j < ARRAY_SIZE(attribMap); j++)
> +            if (attribMap[j].attrib == attrib) {
> +               unsigned *glxValue =
> +                  (unsigned int *) ((char *) config + attribMap[j].offset);
> +
> +               if (*glxValue == GLX_DONT_CARE)
> +                  *glxValue = value;
> +            }
> +
> +         break;
> +      }
> +      }

The curly braces in `default:` are unnecessary, and at first glance
seeing two sets of braces closing on the same level looks like a bug.

> +   }
> +}
> +
>  static struct glx_config *
>  createDriMode(const __DRIcoreExtension * core,
>  	      struct glx_config *config, const __DRIconfig **driConfigs)
> @@ -360,6 +393,7 @@ createDriMode(const __DRIcoreExtension * core,
>  
>     driConfig->base = *config;
>     driConfig->driConfig = driConfigs[i];
> +   driUpdateDontCareValues(core, &driConfig->base, driConfig->driConfig);
>  
>     return &driConfig->base;
>  }
> -- 
> 2.18.0
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev