From fbf26e109b1e96aa4eeca018198ff0b5a71e557b Mon Sep 17 00:00:00 2001 From: Brian Paul Date: Mon, 21 Jul 2008 13:58:50 -0600 Subject: mesa: assorted glsl uniform/attribute fixes Fix incorrect uniform/attribute size query results. Add missing error checking for glUniform, glUniformMatrix params Fix an array size/allocation error. --- src/mesa/shader/prog_parameter.c | 29 +------ src/mesa/shader/prog_parameter.h | 2 +- src/mesa/shader/shader_api.c | 152 +++++++++++++++++++++++++++++++--- src/mesa/shader/slang/slang_codegen.c | 21 ++++- src/mesa/shader/slang/slang_link.c | 5 +- 5 files changed, 166 insertions(+), 43 deletions(-) diff --git a/src/mesa/shader/prog_parameter.c b/src/mesa/shader/prog_parameter.c index be696385aa..d87e8f6e15 100644 --- a/src/mesa/shader/prog_parameter.c +++ b/src/mesa/shader/prog_parameter.c @@ -62,29 +62,6 @@ _mesa_free_parameter_list(struct gl_program_parameter_list *paramList) } -static GLint -_mesa_fit_type_in_vec4(GLenum type) -{ - switch (type) { - case GL_FLOAT: - case GL_INT: - return 4; - break; - case GL_FLOAT_VEC2: - case GL_INT_VEC2: - return 2; - break; - case GL_FLOAT_VEC3: - case GL_INT_VEC3: - return 1; - break; - case GL_FLOAT_VEC4: - case GL_INT_VEC4: - default: - return 1; - } -} - /** * Add a new parameter to a parameter list. * Note that parameter values are usually 4-element GLfloat vectors. @@ -294,7 +271,7 @@ _mesa_add_uniform(struct gl_program_parameter_list *paramList, } else { i = _mesa_add_parameter(paramList, PROGRAM_UNIFORM, name, - size * _mesa_fit_type_in_vec4(datatype), datatype, NULL, NULL); + size, datatype, NULL, NULL); return i; } } @@ -363,7 +340,7 @@ _mesa_add_varying(struct gl_program_parameter_list *paramList, */ GLint _mesa_add_attribute(struct gl_program_parameter_list *paramList, - const char *name, GLint size, GLint attrib) + const char *name, GLint size, GLenum datatype, GLint attrib) { GLint i = _mesa_lookup_parameter_index(paramList, -1, name); if (i >= 0) { @@ -379,7 +356,7 @@ _mesa_add_attribute(struct gl_program_parameter_list *paramList, if (size < 0) size = 4; i = _mesa_add_parameter(paramList, PROGRAM_INPUT, name, - size, GL_NONE, NULL, state); + size, datatype, NULL, state); } return i; } diff --git a/src/mesa/shader/prog_parameter.h b/src/mesa/shader/prog_parameter.h index 105f6f24de..dfb8c39ca4 100644 --- a/src/mesa/shader/prog_parameter.h +++ b/src/mesa/shader/prog_parameter.h @@ -122,7 +122,7 @@ _mesa_add_varying(struct gl_program_parameter_list *paramList, extern GLint _mesa_add_attribute(struct gl_program_parameter_list *paramList, - const char *name, GLint size, GLint attrib); + const char *name, GLint size, GLenum datatype, GLint attrib); extern GLint _mesa_add_state_reference(struct gl_program_parameter_list *paramList, diff --git a/src/mesa/shader/shader_api.c b/src/mesa/shader/shader_api.c index ee0e369d7c..e25cb63216 100644 --- a/src/mesa/shader/shader_api.c +++ b/src/mesa/shader/shader_api.c @@ -513,6 +513,7 @@ _mesa_bind_attrib_location(GLcontext *ctx, GLuint program, GLuint index, struct gl_shader_program *shProg; const GLint size = -1; /* unknown size */ GLint i, oldIndex; + GLenum datatype; shProg = _mesa_lookup_shader_program_err(ctx, program, "glBindAttribLocation"); @@ -537,13 +538,14 @@ _mesa_bind_attrib_location(GLcontext *ctx, GLuint program, GLuint index, if (shProg->LinkStatus) { /* get current index/location for the attribute */ oldIndex = _mesa_get_attrib_location(ctx, program, name); + assert(0); } else { oldIndex = -1; } /* this will replace the current value if it's already in the list */ - i = _mesa_add_attribute(shProg->Attributes, name, size, index); + i = _mesa_add_attribute(shProg->Attributes, name, size, datatype, index); if (i < 0) { _mesa_error(ctx, GL_OUT_OF_MEMORY, "glBindAttribLocation"); } @@ -710,16 +712,51 @@ _mesa_detach_shader(GLcontext *ctx, GLuint program, GLuint shader) } +static GLint +sizeof_glsl_type(GLenum type) +{ + switch (type) { + case GL_FLOAT: + case GL_INT: + case GL_BOOL: + return 1; + case GL_FLOAT_VEC2: + case GL_INT_VEC2: + case GL_BOOL_VEC2: + return 2; + case GL_FLOAT_VEC3: + case GL_INT_VEC3: + case GL_BOOL_VEC3: + return 3; + case GL_FLOAT_VEC4: + case GL_INT_VEC4: + case GL_BOOL_VEC4: + return 4; + case GL_FLOAT_MAT2: + case GL_FLOAT_MAT2x3: + case GL_FLOAT_MAT2x4: + return 8; /* two float[4] vectors */ + case GL_FLOAT_MAT3: + case GL_FLOAT_MAT3x2: + case GL_FLOAT_MAT3x4: + return 12; /* three float[4] vectors */ + case GL_FLOAT_MAT4: + case GL_FLOAT_MAT4x2: + case GL_FLOAT_MAT4x3: + return 16; /* four float[4] vectors */ + default: + _mesa_problem(NULL, "Invalid type in sizeof_glsl_type()"); + return 1; + } +} + + static void _mesa_get_active_attrib(GLcontext *ctx, GLuint program, GLuint index, GLsizei maxLength, GLsizei *length, GLint *size, GLenum *type, GLchar *nameOut) { - static const GLenum vec_types[] = { - GL_FLOAT, GL_FLOAT_VEC2, GL_FLOAT_VEC3, GL_FLOAT_VEC4 - }; struct gl_shader_program *shProg; - GLint sz; shProg = _mesa_lookup_shader_program_err(ctx, program, "glGetActiveAttrib"); if (!shProg) @@ -732,11 +769,11 @@ _mesa_get_active_attrib(GLcontext *ctx, GLuint program, GLuint index, copy_string(nameOut, maxLength, length, shProg->Attributes->Parameters[index].Name); - sz = shProg->Attributes->Parameters[index].Size; if (size) - *size = sz; + *size = shProg->Attributes->Parameters[index].Size + / sizeof_glsl_type(shProg->Attributes->Parameters[index].DataType); if (type) - *type = vec_types[sz]; /* XXX this is a temporary hack */ + *type = shProg->Attributes->Parameters[index].DataType; } @@ -779,8 +816,8 @@ _mesa_get_active_uniform(GLcontext *ctx, GLuint program, GLuint index, copy_string(nameOut, maxLength, length, prog->Parameters->Parameters[progPos].Name); if (size) - *size = prog->Parameters->Parameters[progPos].Size; - + *size = prog->Parameters->Parameters[progPos].Size + / sizeof_glsl_type(prog->Parameters->Parameters[progPos].DataType); if (type) *type = prog->Parameters->Parameters[progPos].DataType; } @@ -1172,6 +1209,36 @@ update_textures_used(struct gl_program *prog) } +/** + * Check if the type given by userType is allowed to set a uniform of the + * target type. Generally, equivalence is required, but setting Boolean + * uniforms can be done with glUniformiv or glUniformfv. + */ +static GLboolean +compatible_types(GLenum userType, GLenum targetType) +{ + if (userType == targetType) + return GL_TRUE; + + if (targetType == GL_BOOL && (userType == GL_FLOAT || userType == GL_INT)) + return GL_TRUE; + + if (targetType == GL_BOOL_VEC2 && (userType == GL_FLOAT_VEC2 || + userType == GL_INT_VEC2)) + return GL_TRUE; + + if (targetType == GL_BOOL_VEC3 && (userType == GL_FLOAT_VEC3 || + userType == GL_INT_VEC3)) + return GL_TRUE; + + if (targetType == GL_BOOL_VEC4 && (userType == GL_FLOAT_VEC4 || + userType == GL_INT_VEC4)) + return GL_TRUE; + + return GL_FALSE; +} + + /** * Set the value of a program's uniform variable. * \param program the program whose uniform to update @@ -1185,6 +1252,12 @@ static void set_program_uniform(GLcontext *ctx, struct gl_program *program, GLint location, GLenum type, GLsizei count, GLint elems, const void *values) { + if (!compatible_types(type, + program->Parameters->Parameters[location].DataType)) { + _mesa_error(ctx, GL_INVALID_OPERATION, "glUniform(type mismatch)"); + return; + } + if (program->Parameters->Parameters[location].Type == PROGRAM_SAMPLER) { /* This controls which texture unit which is used by a sampler */ GLuint texUnit, sampler; @@ -1317,19 +1390,74 @@ _mesa_uniform(GLcontext *ctx, GLint location, GLsizei count, } +static void +get_matrix_dims(GLenum type, GLint *rows, GLint *cols) +{ + switch (type) { + case GL_FLOAT_MAT2: + *rows = *cols = 2; + break; + case GL_FLOAT_MAT2x3: + *rows = 3; + *cols = 2; + break; + case GL_FLOAT_MAT2x4: + *rows = 4; + *cols = 2; + break; + case GL_FLOAT_MAT3: + *rows = 3; + *cols = 3; + break; + case GL_FLOAT_MAT3x2: + *rows = 2; + *cols = 3; + break; + case GL_FLOAT_MAT3x4: + *rows = 4; + *cols = 3; + break; + case GL_FLOAT_MAT4: + *rows = 4; + *cols = 4; + break; + case GL_FLOAT_MAT4x2: + *rows = 2; + *cols = 4; + break; + case GL_FLOAT_MAT4x3: + *rows = 3; + *cols = 4; + break; + default: + *rows = *cols = 0; + } +} + + static void set_program_uniform_matrix(GLcontext *ctx, struct gl_program *program, GLuint location, GLuint count, GLuint rows, GLuint cols, GLboolean transpose, const GLfloat *values) { + GLuint mat, row, col; + GLuint dst = location, src = 0; + GLint nr, nc; + + /* check that the number of rows, columns is correct */ + get_matrix_dims(program->Parameters->Parameters[location].DataType, &nr, &nc); + if (rows != nr || cols != nc) { + _mesa_error(ctx, GL_INVALID_OPERATION, + "glUniformMatrix(matrix size mismatch"); + return; + } + /* * Note: the _columns_ of a matrix are stored in program registers, not * the rows. So, the loops below look a little funny. * XXX could optimize this a bit... */ - GLuint mat, row, col; - GLuint dst = location, src = 0; /* loop over matrices */ for (mat = 0; mat < count; mat++) { diff --git a/src/mesa/shader/slang/slang_codegen.c b/src/mesa/shader/slang/slang_codegen.c index b8e130555d..8f5c95c06a 100644 --- a/src/mesa/shader/slang/slang_codegen.c +++ b/src/mesa/shader/slang/slang_codegen.c @@ -3059,6 +3059,23 @@ _slang_gen_operation(slang_assemble_ctx * A, slang_operation *oper) } +/** + * Compute total size of array give size of element, number of elements. + */ +static GLint +array_size(GLint baseSize, GLint arrayLen) +{ + GLint total; + if (arrayLen > 1) { + /* round up base type to multiple of 4 */ + total = ((baseSize + 3) & ~0x3) * MAX2(arrayLen, 1); + } + else { + total = baseSize; + } + return total; +} + /** * Called by compiler when a global variable has been parsed/compiled. @@ -3098,7 +3115,7 @@ _slang_codegen_global_variable(slang_assemble_ctx *A, slang_variable *var, } else if (var->type.qualifier == SLANG_QUAL_UNIFORM) { /* Uniform variable */ - const GLint totalSize = size * MAX2(var->array_len, 1); + const GLint totalSize = array_size(size, var->array_len); const GLuint swizzle = _slang_var_swizzle(totalSize, 0); if (prog) { /* user-defined uniform */ @@ -3182,7 +3199,7 @@ _slang_codegen_global_variable(slang_assemble_ctx *A, slang_variable *var, /* user-defined vertex attribute */ const GLint attr = -1; /* unknown */ GLint index = _mesa_add_attribute(prog->Attributes, varName, - size, attr); + size, datatype, attr); assert(index >= 0); store = _slang_new_ir_storage(PROGRAM_INPUT, VERT_ATTRIB_GENERIC0 + index, size); diff --git a/src/mesa/shader/slang/slang_link.c b/src/mesa/shader/slang/slang_link.c index f08b53e8b7..e9b9bbc43f 100644 --- a/src/mesa/shader/slang/slang_link.c +++ b/src/mesa/shader/slang/slang_link.c @@ -211,7 +211,6 @@ _slang_resolve_attributes(struct gl_shader_program *shProg, { GLuint i, j; GLbitfield usedAttributes; - GLint size = 4; /* XXX fix */ assert(prog->Target == GL_VERTEX_PROGRAM_ARB); @@ -253,6 +252,8 @@ _slang_resolve_attributes(struct gl_shader_program *shProg, * Start at 1 since generic attribute 0 always aliases * glVertex/position. */ + GLint size = prog->Attributes->Parameters[k].Size; + GLenum datatype = prog->Attributes->Parameters[k].DataType; for (attr = 1; attr < MAX_VERTEX_ATTRIBS; attr++) { if (((1 << attr) & usedAttributes) == 0) break; @@ -261,7 +262,7 @@ _slang_resolve_attributes(struct gl_shader_program *shProg, /* too many! XXX record error log */ return GL_FALSE; } - _mesa_add_attribute(shProg->Attributes, name, size, attr); + _mesa_add_attribute(shProg->Attributes, name, size, datatype,attr); /* set the attribute as used */ usedAttributes |= 1<