[Mesa-dev] [PATCH 3/3] glx: rename choose_visual(), drop const argument

[Ian Romanick]

On 09/30/2016 03:01 AM, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov at collabora.com>
> 
> The function deals with fb (style) configs, thus using "visual"
> in the name is misleading. Which in itself had led to the use of
> fbconfig_style_tags argument.
> 
> Rename the function to reflect what it does and drop the unneeded
> argument.
> 
> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
> ---
>  src/glx/glxcmds.c | 26 +++++++++++---------------
>  1 file changed, 11 insertions(+), 15 deletions(-)
> 
> diff --git a/src/glx/glxcmds.c b/src/glx/glxcmds.c
> index 8191da0..21fdd7a 100644
> --- a/src/glx/glxcmds.c
> +++ b/src/glx/glxcmds.c
> @@ -1162,8 +1162,8 @@ fbconfig_compare(struct glx_config **a, struct glx_config **b)
>  
>  /**
>   * Selects and sorts a subset of the supplied configs based on the attributes.
> - * This function forms to basis of \c glXChooseVisual, \c glXChooseFBConfig,
> - * and \c glXChooseFBConfigSGIX.
> + * This function forms to basis of \c glXChooseFBConfig and
> + * \c glXChooseFBConfigSGIX.
>   *
>   * \param configs   Array of pointers to possible configs.  The elements of
>   *                  this array that do not meet the criteria will be set to
> @@ -1171,20 +1171,16 @@ fbconfig_compare(struct glx_config **a, struct glx_config **b)
>   *                  the various visual / FBConfig selection rules.
>   * \param num_configs  Number of elements in the \c configs array.
>   * \param attribList   Attributes used select from \c configs.  This array is
> - *                     terminated by a \c None tag.  The array can either take
> - *                     the form expected by \c glXChooseVisual (where boolean
> - *                     tags do not have a value) or by \c glXChooseFBConfig
> - *                     (where every tag has a value).
> - * \param fbconfig_style_tags  Selects whether \c attribList is in
> - *                             \c glXChooseVisual style or
> - *                             \c glXChooseFBConfig style.

That is weird.  I did a lot of archaeology on this... and I can't see
that this parameter was *ever* passed anything other than GL_TRUE.  The
other places that call init_fbconfig_for_chooser and
__GlXInitializeVisualConfigFromTags also pass literal values for the
last parameter.  Not sure what I was thinking when I wrote that code...

Series is

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

I'd also review a series that converts a bunch of the GLboolean nonsense
to bool. :)

> + *                     terminated by a \c None tag.  The array is of the form
> + *                     expected by \c glXChooseFBConfig (where every tag has a
> + *                     value).
>   * \returns The number of valid elements left in \c configs.
>   *
> - * \sa glXChooseVisual, glXChooseFBConfig, glXChooseFBConfigSGIX
> + * \sa glXChooseFBConfig, glXChooseFBConfigSGIX
>   */
>  static int
> -choose_visual(struct glx_config ** configs, int num_configs,
> -              const int *attribList, GLboolean fbconfig_style_tags)
> +choose_fbconfig(struct glx_config ** configs, int num_configs,
> +              const int *attribList)
>  {
>     struct glx_config test_config;
>     int base;
> @@ -1196,10 +1192,10 @@ choose_visual(struct glx_config ** configs, int num_configs,
>      * list.
>      */
>  
> -   init_fbconfig_for_chooser(&test_config, fbconfig_style_tags);
> +   init_fbconfig_for_chooser(&test_config, GL_TRUE);
>     __glXInitializeVisualConfigFromTags(&test_config, 512,
>                                         (const INT32 *) attribList,
> -                                       GL_TRUE, fbconfig_style_tags);
> +                                       GL_TRUE, GL_TRUE);
>  
>     base = 0;
>     for (i = 0; i < num_configs; i++) {
> @@ -1613,7 +1609,7 @@ glXChooseFBConfig(Display * dpy, int screen,
>        glXGetFBConfigs(dpy, screen, &list_size);
>  
>     if ((config_list != NULL) && (list_size > 0) && (attribList != NULL)) {
> -      list_size = choose_visual(config_list, list_size, attribList, GL_TRUE);
> +      list_size = choose_fbconfig(config_list, list_size, attribList);
>        if (list_size == 0) {
>           free(config_list);
>           config_list = NULL;
>