From 1bbf803e3bc8eeabcc2c24b89bc30a30b2445c59 Mon Sep 17 00:00:00 2001 From: Brian Paul Date: Tue, 1 Jun 2010 09:02:07 -0600 Subject: mesa: use split_location_offset() in GetUniform() functions Commit 5d0e136eff54a34258b5adaeda4cb267831e8234 exposed a long-standing bug in the glGetUniform*() code paths. We weren't properly decoding the location parameter. Fixes fd.o bug/regression 28344 Note: this patch should go into the 7.8 branch after the above-mentioned commit. --- src/mesa/shader/uniforms.c | 101 ++++++++++++++++++++++++--------------------- 1 file changed, 54 insertions(+), 47 deletions(-) (limited to 'src/mesa/shader') diff --git a/src/mesa/shader/uniforms.c b/src/mesa/shader/uniforms.c index 69758ea73f6..b1fb90d020b 100644 --- a/src/mesa/shader/uniforms.c +++ b/src/mesa/shader/uniforms.c @@ -318,6 +318,54 @@ lookup_uniform_parameter(GLcontext *ctx, GLuint program, GLint location, } +/** + * GLGL uniform arrays and structs require special handling. + * + * The GL_ARB_shader_objects spec says that if you use + * glGetUniformLocation to get the location of an array, you CANNOT + * access other elements of the array by adding an offset to the + * returned location. For example, you must call + * glGetUniformLocation("foo[16]") if you want to set the 16th element + * of the array with glUniform(). + * + * HOWEVER, some other OpenGL drivers allow accessing array elements + * by adding an offset to the returned array location. And some apps + * seem to depend on that behaviour. + * + * Mesa's gl_uniform_list doesn't directly support this since each + * entry in the list describes one uniform variable, not one uniform + * element. We could insert dummy entries in the list for each array + * element after [0] but that causes complications elsewhere. + * + * We solve this problem by encoding two values in the location that's + * returned by glGetUniformLocation(): + * a) index into gl_uniform_list::Uniforms[] for the uniform + * b) an array/field offset (0 for simple types) + * + * These two values are encoded in the high and low halves of a GLint. + * By putting the uniform number in the high part and the offset in the + * low part, we can support the unofficial ability to index into arrays + * by adding offsets to the location value. + */ +static void +merge_location_offset(GLint *location, GLint offset) +{ + *location = (*location << 16) | offset; +} + + +/** + * Separate the uniform location and parameter offset. See above. + */ +static void +split_location_offset(GLint *location, GLint *offset) +{ + *offset = *location & 0xffff; + *location = *location >> 16; +} + + + /** * Called via ctx->Driver.GetUniformfv(). */ @@ -327,6 +375,9 @@ _mesa_get_uniformfv(GLcontext *ctx, GLuint program, GLint location, { struct gl_program *prog; GLint paramPos; + GLint offset; + + split_location_offset(&location, &offset); lookup_uniform_parameter(ctx, program, location, &prog, ¶mPos); @@ -357,6 +408,9 @@ _mesa_get_uniformiv(GLcontext *ctx, GLuint program, GLint location, { struct gl_program *prog; GLint paramPos; + GLint offset; + + split_location_offset(&location, &offset); lookup_uniform_parameter(ctx, program, location, &prog, ¶mPos); @@ -377,53 +431,6 @@ _mesa_get_uniformiv(GLcontext *ctx, GLuint program, GLint location, } -/** - * GLGL uniform arrays and structs require special handling. - * - * The GL_ARB_shader_objects spec says that if you use - * glGetUniformLocation to get the location of an array, you CANNOT - * access other elements of the array by adding an offset to the - * returned location. For example, you must call - * glGetUniformLocation("foo[16]") if you want to set the 16th element - * of the array with glUniform(). - * - * HOWEVER, some other OpenGL drivers allow accessing array elements - * by adding an offset to the returned array location. And some apps - * seem to depend on that behaviour. - * - * Mesa's gl_uniform_list doesn't directly support this since each - * entry in the list describes one uniform variable, not one uniform - * element. We could insert dummy entries in the list for each array - * element after [0] but that causes complications elsewhere. - * - * We solve this problem by encoding two values in the location that's - * returned by glGetUniformLocation(): - * a) index into gl_uniform_list::Uniforms[] for the uniform - * b) an array/field offset (0 for simple types) - * - * These two values are encoded in the high and low halves of a GLint. - * By putting the uniform number in the high part and the offset in the - * low part, we can support the unofficial ability to index into arrays - * by adding offsets to the location value. - */ -static void -merge_location_offset(GLint *location, GLint offset) -{ - *location = (*location << 16) | offset; -} - - -/** - * Seperate the uniform location and parameter offset. See above. - */ -static void -split_location_offset(GLint *location, GLint *offset) -{ - *offset = *location & 0xffff; - *location = *location >> 16; -} - - /** * Called via ctx->Driver.GetUniformLocation(). * -- cgit v1.2.3