[Mesa-dev] [PATCH 06/20] mesa: Move {split, merge}_location_offset to uniforms.h

[Kenneth Graunke]

On 10/28/2011 10:42 AM, Ian Romanick wrote:
> From: Ian Romanick <ian.d.romanick at intel.com>
> 
> Also prepend _mesa_uniform_ to the names.
> 
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
...
> -static void
> -merge_location_offset(GLint *location, GLint offset)
> -{
> -   *location = (*location << 16) | offset;
> -}
...
> +/**
> + * Combine the uniform's base location and the offset
> + */
> +static inline GLint
> +_mesa_uniform_merge_location_offset(unsigned base_location, unsigned
offset)
> +{
> +   return (base_location << 16) | offset;
> +}
> +

You've clearly done more than what your commit message says here...you
changed this function's calling convention from foo(&x,y) to x=foo(x,y)
and rewrote the callers appropriately.

While I'm not opposed to that, it's worth mentioning.

> -/**
> - * Separate the uniform location and parameter offset.  See above.
> - */
> -static void
> -split_location_offset(GLint *location, GLint *offset)
> -{
> -   *offset = *location & 0xffff;
> -   *location = *location >> 16;
> -}
...
> +/**
> + * Separate the uniform base location and parameter offset
> + */
> +static inline void
> +_mesa_uniform_split_location_offset(GLint location, unsigned *base_location,
> +				    unsigned *offset)
> +{
> +   *offset = location & 0xffff;
> +   *base_location = location >> 16;
> +}

This one is even stranger.  You added a third parameter and rewrote the
existing callers to do:

_mesa_uniform_split_location_offset(location, &location, offset).

This seems rather weird and redundant at first glance.  Though, I think
I see your rationale: location is an input parameter containing
base+offset (together), while the next two are output parameters for
base and offset (separately).  The former function muddled things by
forcing you to override the combined input, which is a pain for callers
that want to keep all three.

So I suppose I'm not opposed to this either, but please do
mention/justify the change in the commit message.