On Mon, Jun 24, 2013 at 6:12 PM, Jose Fonseca <jfons...@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 >> b/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; >> } >> >> +boolean >> +util_format_is_snorm(enum pipe_format format) >> +{ >> + const struct util_format_description *desc = >> util_format_description(format); >> + 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... Dave. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev