From 91a2fa1490b86b11430db6a604dc28bac1871695 Mon Sep 17 00:00:00 2001 From: Ian Romanick Date: Mon, 20 Oct 2014 16:40:50 -0700 Subject: [PATCH] mesa: Rework array error checks in validate_uniform_parameters MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Before ARB_explicit_uniform_location, Mesa's location encoding allowed locations for non-array types that had non-zero array indices. Basically, part of the location was the uniform and part was the array index. This meant that some checks had to occur for arrays and non-arrays. This is no longer possible, we the checks can be split up. Valgrind callgrind results for a trace of Tesseract: _mesa_Uniform4fv _mesa_Uniform4f _mesa_Uniform1i Before (64-bit): 50,499,557 17,487,316 686,227 After (64-bit): 50,023,791 17,274,432 684,293 _mesa_Uniform4fv _mesa_Uniform4f _mesa_Uniform1i Before (32-bit): 62,968,039 21,732,380 828,147 After (32-bit): 62,373,967 21,490,756 826,223 Signed-off-by: Ian Romanick Reviewed-by: Brian Paul Reviewed-by: Tapani Pälli --- src/mesa/main/uniform_query.cpp | 41 ++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/src/mesa/main/uniform_query.cpp b/src/mesa/main/uniform_query.cpp index 16e08d44cd6..b87dbdf50d5 100644 --- a/src/mesa/main/uniform_query.cpp +++ b/src/mesa/main/uniform_query.cpp @@ -252,27 +252,30 @@ validate_uniform_parameters(struct gl_context *ctx, struct gl_uniform_storage *const uni = shProg->UniformRemapTable[location]; - if (uni->array_elements == 0 && count > 1) { - _mesa_error(ctx, GL_INVALID_OPERATION, - "%s(count > 1 for non-array, location=%d)", - caller, location); - return NULL; - } + if (uni->array_elements == 0) { + if (count > 1) { + _mesa_error(ctx, GL_INVALID_OPERATION, + "%s(count > 1 for non-array, location=%d)", + caller, location); + return NULL; + } - /* The array index specified by the uniform location is just the uniform - * location minus the base location of of the uniform. - */ - *array_index = location - uni->remap_location; + assert((location - uni->remap_location) == 0); + *array_index = 0; + } else { + /* The array index specified by the uniform location is just the uniform + * location minus the base location of of the uniform. + */ + *array_index = location - uni->remap_location; - /* If the uniform is an array, check that array_index is in bounds. - * If not an array, check that array_index is zero. - * array_index is unsigned so no need to check for less than zero. - */ - const unsigned limit = MAX2(uni->array_elements, 1); - if (*array_index >= limit) { - _mesa_error(ctx, GL_INVALID_OPERATION, "%s(location=%d)", - caller, location); - return NULL; + /* If the uniform is an array, check that array_index is in bounds. + * array_index is unsigned so no need to check for less than zero. + */ + if (*array_index >= uni->array_elements) { + _mesa_error(ctx, GL_INVALID_OPERATION, "%s(location=%d)", + caller, location); + return NULL; + } } return uni; } -- 2.30.2