On 10/30/2011 10:47 PM, Kenneth Graunke wrote:
On 10/28/2011 10:42 AM, Ian Romanick wrote:
From: Ian Romanick<ian.d.roman...@intel.com>
Also prepend _mesa_uniform_ to the names.
Signed-off-by: Ian Romanick<ian.d.roman...@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.
Good call.
I had a couple reasons for the second change, and I'll add them to the
commit message.
1. Having 'location' in the function mean different things in different
places was confusing and annoying to me.
2. I seem to recall that a later patch wants to use the original value
of location sometime after splitting out the index and offset.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
