[Mesa-dev] [PATCH] st/mesa: handle SNORM formats in generic CopyPixels path
airlied at gmail.com
Mon Jun 24 01:28:48 PDT 2013
On Mon, Jun 24, 2013 at 6:12 PM, Jose Fonseca <jfonseca at vmware.com> wrote:
> ----- Original Message -----
>> src/gallium/auxiliary/util/u_format.c | 14 ++++++++++++++
>> src/gallium/auxiliary/util/u_format.h | 3 +++
>> src/mesa/state_tracker/st_cb_drawpixels.c | 6 ++++++
>> 3 files changed, 23 insertions(+)
>> diff --git a/src/gallium/auxiliary/util/u_format.c
>> index 9bdc2ea..b70c108 100644
>> --- a/src/gallium/auxiliary/util/u_format.c
>> +++ b/src/gallium/auxiliary/util/u_format.c
>> @@ -131,6 +131,20 @@ util_format_is_pure_uint(enum pipe_format format)
>> return (desc->channel[i].type == UTIL_FORMAT_TYPE_UNSIGNED &&
>> desc->channel[i].pure_integer) ? TRUE : FALSE;
>> +util_format_is_snorm(enum pipe_format format)
>> + const struct util_format_description *desc =
>> + int i;
>> + i = util_format_get_first_non_void_channel(format);
>> + if (i == -1)
>> + return FALSE;
>> + return desc->channel[i].type == UTIL_FORMAT_TYPE_SIGNED &&
>> + !desc->channel[i].pure_integer &&
>> + desc->channel[i].normalized;
> This will give wrong results for mixed formats -- containing a mixture of SIGNED and UNSIGNED normalized types. You can avoid that by adding this statement
> if (desc->is_mixed) return FALSE at the start.
> I think a comment would be good too -- something like "Returns true if all non-void channels are normalized signed."
> This is not your fault, but it is disappointing to see the proliferation of redundant util_format_description() calls in these helpers. util_format_description is not cost free, and st_CopyPixels and friends keep calling it over and over again. I don't think it is hard to call util_format_description() once, and then pass the format_desc pointers around, but once we start on the other route is hard to back out. But as they say -- when you're in a hole, the first thing to do is stop digging --, so I think we should just stop adding new helper functions to util u_format that take enum format instead of util_format_description.
I hestiate to say this as no doubt I'll be wrong, but if they are all
inline helpers, the compiler should figure it out and collapse the
lookups. Of course I believe compilers should do lots of things...
More information about the mesa-dev