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

[Jose Fonseca]

----- Original Message -----
> >> +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.
> 
> The only caller of these functions now is the sp blend code and it
> saves the results to avoid frequent calls.
> 
> Most of the existing util_format_is_XXX() calls take a pipe_format
> now.  I think we should be consistent and have all of them take a
> util_format_description or pipe_format.  Either way is fine with me.
> We can revisit that later.

Sure.

> >> +
> >> +   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.
> 
> I'll fix that case.
> 
> > - FALSE for PIPE_FORMAT_R8G8_UNORM, because nr_channels == 2.
> > - FALSE for PIPE_FORMAT_R8_UNORM, because nr_channels == 1.
> 
> I don't want to return TRUE for R or RG formats.
> 
> > - 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).
> 
> Hmm, ok, I thought nr_channels indicate the logical number of color
> channels in the format, even for compressed formats.  Could I count
> the number of unique swizzles that are <= SWIZZLE_W to determine the
> number of logical channels?

No. It's the number of channels in memory (hence just meaningful for PLAIN formats)

Yes, counting the unique number of swizzles <= SWIZZLE_W should give what you're looking for.

> > - TRUE for signed and mixed signed formats such as
> > PIPE_FORMAT_R8SG8SB8UX8U_NORM (not sure if this is intended or
> > matters)
> 
> I think that's OK.  The intention of the function is to return true
> if
> the format logically stores 3 color components, but not alpha.
> 
> > 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).
> 
> Hmmm, ok.  Maybe ignore nr_channels and just check the swizzles
> as-is?

Yes, that should do it.

Jose