[Mesa-dev] [PATCH 1/2] util: add util_format_is_luminance/intensity/rgb(), etc

[Jose Fonseca]

Hi Brian,

----- Original Message -----
> From: Brian Paul <brianp at vmware.com>
> 
> ---
>  src/gallium/auxiliary/util/u_format.c |   77
>  +++++++++++++++++++++++++++++++++
>  src/gallium/auxiliary/util/u_format.h |   16 +++++++
>  2 files changed, 93 insertions(+), 0 deletions(-)
> 
> diff --git a/src/gallium/auxiliary/util/u_format.c
> b/src/gallium/auxiliary/util/u_format.c
> index 34922ab..2bd3737 100644
> --- a/src/gallium/auxiliary/util/u_format.c
> +++ b/src/gallium/auxiliary/util/u_format.c
> @@ -67,6 +67,83 @@ util_format_is_float(enum pipe_format format)
>  }
>  
>  
> +/** Test if the format contains RGB, but not alpha */
> +boolean

It might be better to change  the name to util_format_is_rgb_no_alpha, as is_rgb sounds more like a RGB colorspace inquiry.

> +util_format_is_rgb(enum pipe_format format)
> +{
> +   const struct util_format_description *desc =
> +      util_format_description(format);

For callers that call these helpers a lot, all these internal calls to util_format_description() will be inefficient. In such cases it might be preferrable for these functions to be declared inline, and take a "const struct util_format_description *desc" argument instead of "enum pipe_format format", allowing the caller will call util_format_description only once.  

> +
> +   if ((desc->colorspace == UTIL_FORMAT_COLORSPACE_RGB ||
> +        desc->colorspace == UTIL_FORMAT_COLORSPACE_SRGB) &&
> +       desc->nr_channels == 4 &&
> +       desc->swizzle[0] <= UTIL_FORMAT_SWIZZLE_W &&
> +       desc->swizzle[1] <= UTIL_FORMAT_SWIZZLE_W &&
> +       desc->swizzle[2] <= UTIL_FORMAT_SWIZZLE_W &&
> +       desc->swizzle[3] == UTIL_FORMAT_SWIZZLE_1) {
> +      return TRUE;
> +   }
> +   return FALSE;
> +}

The above logic will produce the following unexpected results:
- FALSE for PIPE_FORMAT_R8G8B8_UNORM, because nr_channels == 3.
- FALSE for PIPE_FORMAT_R8G8_UNORM, because nr_channels == 2.
- FALSE for PIPE_FORMAT_R8_UNORM, because nr_channels == 1.
- FALSE for PIPE_FORMAT_DXT1_RGB, becuase nr_channels == 1 (nr_channels is always 1 for non PLAIN formats and not really very meaningful in such format types).
- TRUE for signed and mixed signed formats such as PIPE_FORMAT_R8SG8SB8UX8U_NORM (not sure if this is intended or matters)

It might be useful to extend/adapt a test like such as src/gallium/tests/unit/u_format_compatible_test.c to ensure that this gives the expected results for all formats.

> +boolean
> +util_format_is_luminance(enum pipe_format format)
> +{
> +   const struct util_format_description *desc =
> +      util_format_description(format);
> +
> +   if ((desc->colorspace == UTIL_FORMAT_COLORSPACE_RGB ||
> +        desc->colorspace == UTIL_FORMAT_COLORSPACE_SRGB) &&
> +       desc->nr_channels == 1 &&
> +       desc->swizzle[0] == UTIL_FORMAT_SWIZZLE_X &&
> +       desc->swizzle[1] == UTIL_FORMAT_SWIZZLE_X &&
> +       desc->swizzle[2] == UTIL_FORMAT_SWIZZLE_X &&
> +       desc->swizzle[3] == UTIL_FORMAT_SWIZZLE_1) {
> +      return TRUE;
> +   }
> +   return FALSE;
> +}

This will produce TRUE for PIPE_FORMAT_LATC1_UNORM...

> +
> +boolean
> +util_format_is_luminance_alpha(enum pipe_format format)
> +{
> +   const struct util_format_description *desc =
> +      util_format_description(format);
> +
> +   if ((desc->colorspace == UTIL_FORMAT_COLORSPACE_RGB ||
> +        desc->colorspace == UTIL_FORMAT_COLORSPACE_SRGB) &&
> +       desc->nr_channels == 2 &&
> +       desc->swizzle[0] == UTIL_FORMAT_SWIZZLE_X &&
> +       desc->swizzle[1] == UTIL_FORMAT_SWIZZLE_X &&
> +       desc->swizzle[2] == UTIL_FORMAT_SWIZZLE_X &&
> +       desc->swizzle[3] == UTIL_FORMAT_SWIZZLE_Y) {
> +      return TRUE;
> +   }
> +   return FALSE;
> +}

.. but this will produce FALSE for PIPE_FORMAT_LATC2_UNORM (nr_channels != 2). 

> +
> +boolean
> +util_format_is_intensity(enum pipe_format format)
> +{
> +   const struct util_format_description *desc =
> +      util_format_description(format);
> +
> +   if ((desc->colorspace == UTIL_FORMAT_COLORSPACE_RGB ||
> +        desc->colorspace == UTIL_FORMAT_COLORSPACE_SRGB) &&
> +       desc->nr_channels == 1 &&
> +       desc->swizzle[0] == UTIL_FORMAT_SWIZZLE_X &&
> +       desc->swizzle[1] == UTIL_FORMAT_SWIZZLE_X &&
> +       desc->swizzle[2] == UTIL_FORMAT_SWIZZLE_X &&
> +       desc->swizzle[3] == UTIL_FORMAT_SWIZZLE_X) {
> +      return TRUE;
> +   }
> +   return FALSE;
> +}

This looks alright.

Jose