On 05/14/2017 01:20 PM, Timothy Arceri wrote:
On 14/05/17 01:39, Samuel Pitoiset wrote:
Mmh, this can still crash if location is < -1 or greater than the number of uniforms. How about:

Sorry my commit message went missing it should have something like:

"-1 is allowed by the spec so that inactive uniforms don't cause the app to throw an error."

In other words we need to not crash for that when running with no error enabled, but any of the paths that would normally hit an error we are free to crash on and need not waste any cpu checking for.

Okay, if the spec says this, then it's fine by me. Can you also add a spec comment directly in the code to avoid confusion with the errors path?

With that addressed, this patch is:

Reviewed-by: Samuel Pitoiset <samuel.pitoi...@gmail.com>



struct gl_uniform_storage *uni = NULL;
if (_mesa_is_no_error_enabled(ctx)) {
   if (location >= 0 && location < shProg->NumUniformRemapTable)
     uni = shProg->UniformRemapTable[location];
   if (!uni)
     return;
   ...
} else {
   ...
}

On 05/13/2017 08:31 AM, Timothy Arceri wrote:
---
  src/mesa/main/uniform_query.cpp | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/src/mesa/main/uniform_query.cpp b/src/mesa/main/uniform_query.cpp
index 0e02a76..5f38aa5 100644
--- a/src/mesa/main/uniform_query.cpp
+++ b/src/mesa/main/uniform_query.cpp
@@ -904,20 +904,23 @@ validate_uniform(GLint location, GLsizei count, const GLvoid *values,
  extern "C" void
  _mesa_uniform(GLint location, GLsizei count, const GLvoid *values,
struct gl_context *ctx, struct gl_shader_program *shProg,
                enum glsl_base_type basicType, unsigned src_components)
  {
     unsigned offset;
     int size_mul = glsl_base_type_is_64bit(basicType) ? 2 : 1;
     struct gl_uniform_storage *uni;
     if (_mesa_is_no_error_enabled(ctx)) {
+      if (location == -1)
+         return;
+
        uni = shProg->UniformRemapTable[location];
        /* The array index specified by the uniform location is just the
         * uniform location minus the base location of of the uniform.
         */
assert(uni->array_elements > 0 || location == (int)uni->remap_location);
        offset = location - uni->remap_location;
     } else {
uni = validate_uniform(location, count, values, &offset, ctx, shProg,
                               basicType, src_components);

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to