[Mesa-dev] [PATCH 14/15] intel: Refactor intel_screen_make_configs

[Ian Romanick]

On 07/21/2012 05:36 PM, Chad Versace wrote:
> Transform the code from clever, obfuscated, and imperative to
> straight-forward and table-driven.

Does the output of glxinfo change?  It's somewhat advantageous for the 
IDs associated with a specific set of values to remain the same.  We 
have a lot of bug reports, for example, that list specific IDs.  It 
makes it a lot easier to reproduce the bugs that way.

> CC: Ian Romanick <idr at freedesktop.org>
> Signed-off-by: Chad Versace <chad.versace at linux.intel.com>
> ---
>   src/mesa/drivers/dri/intel/intel_screen.c | 167 +++++++++++++++++-------------
>   1 file changed, 97 insertions(+), 70 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/intel/intel_screen.c b/src/mesa/drivers/dri/intel/intel_screen.c
> index 9bb42dd..61daea7 100644
> --- a/src/mesa/drivers/dri/intel/intel_screen.c
> +++ b/src/mesa/drivers/dri/intel/intel_screen.c
> @@ -851,85 +851,112 @@ intel_detect_swizzling(struct intel_screen *screen)
>   static __DRIconfig**
>   intel_screen_make_configs(__DRIscreen *dri_screen)
>   {
> -   static const GLenum back_buffer_modes[] = {
> -       GLX_NONE, GLX_SWAP_UNDEFINED_OML, GLX_SWAP_COPY_OML
> +   struct config_params {
> +      GLenum color_format;
> +      GLenum color_type;
> +      const uint8_t depth_sizes[4];
> +      const uint8_t stencil_sizes[4];

Do you really mean const here?

> +      unsigned num_depth_stencil_sizes;
> +      unsigned num_back_buffer_modes;
> +      unsigned num_msaa_modes;
> +      bool enable_accum;
>      };
>
> -   GLenum fb_format[3];
> -   GLenum fb_type[3];
> -   uint8_t depth_bits[4], stencil_bits[4], msaa_samples_array[1];
> -   int color;
> -   __DRIconfig **configs = NULL;
> -
> -   msaa_samples_array[0] = 0;
> -
> -   fb_format[0] = GL_RGB;
> -   fb_type[0] = GL_UNSIGNED_SHORT_5_6_5;
> -
> -   fb_format[1] = GL_BGR;
> -   fb_type[1] = GL_UNSIGNED_INT_8_8_8_8_REV;
> -
> -   fb_format[2] = GL_BGRA;
> -   fb_type[2] = GL_UNSIGNED_INT_8_8_8_8_REV;
> +   static const GLenum back_buffer_modes[] = {
> +      GLX_SWAP_UNDEFINED_OML, GLX_NONE, GLX_SWAP_COPY_OML,
> +   };
>
> -   depth_bits[0] = 0;
> -   stencil_bits[0] = 0;
> +   static const uint8_t msaa_samples[] = {0};
>
> -   /* Generate a rich set of useful configs that do not include an
> -    * accumulation buffer.
> +   /* Starting with DRI2 protocol version 1.1 we can request a depth/stencil
> +    * buffer that has a different number of bits per pixel than the color
> +    * buffer.  This isn't yet supported here.
>       */
> -   for (color = 0; color < ARRAY_SIZE(fb_format); color++) {
> -      __DRIconfig **new_configs;
> -      int depth_factor;
> -
> -      /* Starting with DRI2 protocol version 1.1 we can request a depth/stencil
> -       * buffer that has a different number of bits per pixel than the color
> -       * buffer.  This isn't yet supported here.
> +   struct config_params params[] = {

It seems like this should be const instead of the pair of fields in the 
structure.

> +      /* Configs without accumulation buffer. */
> +      {
> +         .color_format = GL_RGB,
> +         .color_type = GL_UNSIGNED_SHORT_5_6_5,
> +         .depth_sizes =   {0, 16},
> +         .stencil_sizes = {0,  0},
> +         .num_depth_stencil_sizes = 2,
> +         .num_back_buffer_modes = 3,
> +         .num_msaa_modes = 1,
> +         .enable_accum = false,
> +      },
> +      {
> +         .color_format = GL_BGR,
> +         .color_type = GL_UNSIGNED_INT_8_8_8_8_REV,
> +         .depth_sizes =   {0, 24},
> +         .stencil_sizes = {0,  8},
> +         .num_depth_stencil_sizes = 2,
> +         .num_back_buffer_modes = 3,
> +         .num_msaa_modes = 1,
> +         .enable_accum = false,
> +      },
> +      {
> +         .color_format = GL_BGRA,
> +         .color_type = GL_UNSIGNED_INT_8_8_8_8_REV,
> +         .depth_sizes =   {0, 24},
> +         .stencil_sizes = {0,  8},
> +         .num_depth_stencil_sizes = 2,
> +         .num_back_buffer_modes = 3,
> +         .num_msaa_modes = 1,
> +         .enable_accum = false,
> +      },
> +
> +      /* Configs with accumulation buffer.
> +       *
> +       * We generate the minimum possible set of configs that include an
> +       * accumulation buffer.
>          */
> -      if (fb_type[color] == GL_UNSIGNED_SHORT_5_6_5) {
> -         depth_bits[1] = 16;
> -         stencil_bits[1] = 0;
> -      } else {
> -         depth_bits[1] = 24;
> -         stencil_bits[1] = 8;
> -      }
> -
> -      depth_factor = 2;
> -
> -      new_configs = driCreateConfigs(fb_format[color], fb_type[color],
> -                                     depth_bits,
> -                                     stencil_bits,
> -                                     depth_factor,
> -                                     back_buffer_modes,
> -                                     ARRAY_SIZE(back_buffer_modes),
> -                                     msaa_samples_array,
> -                                     ARRAY_SIZE(msaa_samples_array),
> -                                     false);
> -      if (configs == NULL)
> -         configs = new_configs;
> -      else
> -         configs = driConcatConfigs(configs, new_configs);
> -   }
> +      {
> +         .color_format = GL_RGB,
> +         .color_type = GL_UNSIGNED_SHORT_5_6_5,
> +         .depth_sizes =   {16},
> +         .stencil_sizes = {0},
> +         .num_depth_stencil_sizes = 1,
> +         .num_back_buffer_modes = 1,
> +         .num_msaa_modes = 1,
> +         .enable_accum = true,
> +      },
> +      {
> +         .color_format = GL_BGR,
> +         .color_type = GL_UNSIGNED_INT_8_8_8_8_REV,
> +         .depth_sizes =   {24},
> +         .stencil_sizes = {8},
> +         .num_depth_stencil_sizes = 1,
> +         .num_back_buffer_modes = 1,
> +         .num_msaa_modes = 1,
> +         .enable_accum = true,
> +      },
> +      {
> +         .color_format = GL_BGRA,
> +         .color_type = GL_UNSIGNED_INT_8_8_8_8_REV,
> +         .depth_sizes =   {24},
> +         .stencil_sizes = {8},
> +         .num_depth_stencil_sizes = 1,
> +         .num_back_buffer_modes = 1,
> +         .num_msaa_modes = 1,
> +         .enable_accum = true,
> +      },
> +   };
>
> -   /* Generate the minimum possible set of configs that include an
> -    * accumulation buffer.
> -    */
> -   for (color = 0; color < ARRAY_SIZE(fb_format); color++) {
> -      __DRIconfig **new_configs;
> +   __DRIconfig **configs = NULL;
>
> -      if (fb_type[color] == GL_UNSIGNED_SHORT_5_6_5) {
> -         depth_bits[0] = 16;
> -         stencil_bits[0] = 0;
> -      } else {
> -         depth_bits[0] = 24;
> -         stencil_bits[0] = 8;
> -      }
> +   for (int i = 0; i < ARRAY_SIZE(params); ++i) {
> +      __DRIconfig **new_configs =
> +         driCreateConfigs(params[i].color_format,
> +                          params[i].color_type,
> +                          params[i].depth_sizes,
> +                          params[i].stencil_sizes,
> +                          params[i].num_depth_stencil_sizes,
> +                          back_buffer_modes,
> +                          params[i].num_back_buffer_modes,
> +                          msaa_samples,
> +                          params[i].num_msaa_modes,
> +                          params[i].enable_accum);
>
> -      new_configs = driCreateConfigs(fb_format[color], fb_type[color],
> -                                     depth_bits, stencil_bits, 1,
> -                                     back_buffer_modes + 1, 1,
> -                                     msaa_samples_array, 1,
> -                                     true);
>         if (configs == NULL)
>            configs = new_configs;
>         else
>