----- 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
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
