From c123294dfe2e52443f2eff2723ef922f22972ef5 Mon Sep 17 00:00:00 2001 From: Kenneth Graunke Date: Tue, 29 Mar 2016 13:21:48 -0700 Subject: [PATCH] glsl: Return -1 for program interface query locations in many cases. We were recording locations for all variables, even ones without an explicit location set. Implement the rules from the spec, and record -1 in the resource list accordngly. Make program_resource_location stop doing math on negative values. Remove hacks that are no longer necessary now that we've stopped doing that. Fixes 4 dEQP-GLES31.functional.program_interface_query tests: - program_input.location.separable_fragment.var - program_input.location.separable_fragment.var_array - program_output.location.separable_vertex.var_array - program_output.location.separable_vertex.var_array v2: Delete more code Signed-off-by: Kenneth Graunke Reviewed-by: Timothy Arceri --- src/compiler/glsl/linker.cpp | 38 ++++++++++++++++++--- src/mesa/main/shader_query.cpp | 62 +++++----------------------------- 2 files changed, 42 insertions(+), 58 deletions(-) diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp index c0d310746f3..71dbddd8763 100644 --- a/src/compiler/glsl/linker.cpp +++ b/src/compiler/glsl/linker.cpp @@ -3497,7 +3497,8 @@ build_stageref(struct gl_shader_program *shProg, const char *name, * Create gl_shader_variable from ir_variable class. */ static gl_shader_variable * -create_shader_variable(struct gl_shader_program *shProg, const ir_variable *in) +create_shader_variable(struct gl_shader_program *shProg, + const ir_variable *in, bool use_implicit_location) { gl_shader_variable *out = ralloc(shProg, struct gl_shader_variable); if (!out) @@ -3516,8 +3517,29 @@ create_shader_variable(struct gl_shader_program *shProg, const ir_variable *in) if (!out->name) return NULL; + /* From the ARB_program_interface_query specification: + * + * "Not all active variables are assigned valid locations; the + * following variables will have an effective location of -1: + * + * * uniforms declared as atomic counters; + * + * * members of a uniform block; + * + * * built-in inputs, outputs, and uniforms (starting with "gl_"); and + * + * * inputs or outputs not declared with a "location" layout qualifier, + * except for vertex shader inputs and fragment shader outputs." + */ + if (in->type->base_type == GLSL_TYPE_ATOMIC_UINT || + is_gl_identifier(in->name) || + !(in->data.explicit_location || use_implicit_location)) { + out->location = -1; + } else { + out->location = in->data.location; + } + out->type = in->type; - out->location = in->data.location; out->index = in->data.index; out->patch = in->data.patch; out->mode = in->data.mode; @@ -3563,7 +3585,12 @@ add_interface_variables(struct gl_shader_program *shProg, if (strncmp(var->name, "gl_out_FragData", 15) == 0) continue; - gl_shader_variable *sha_v = create_shader_variable(shProg, var); + const bool vs_input_or_fs_output = + (stage == MESA_SHADER_VERTEX && var->data.mode == ir_var_shader_in) || + (stage == MESA_SHADER_FRAGMENT && var->data.mode == ir_var_shader_out); + + gl_shader_variable *sha_v = + create_shader_variable(shProg, var, vs_input_or_fs_output); if (!sha_v) return false; @@ -3597,7 +3624,8 @@ add_packed_varyings(struct gl_shader_program *shProg, int stage, GLenum type) } if (type == iface) { - gl_shader_variable *sha_v = create_shader_variable(shProg, var); + gl_shader_variable *sha_v = + create_shader_variable(shProg, var, false); if (!sha_v) return false; if (!add_program_resource(shProg, iface, sha_v, @@ -3622,7 +3650,7 @@ add_fragdata_arrays(struct gl_shader_program *shProg) ir_variable *var = node->as_variable(); if (var) { assert(var->data.mode == ir_var_shader_out); - gl_shader_variable *sha_v = create_shader_variable(shProg, var); + gl_shader_variable *sha_v = create_shader_variable(shProg, var, true); if (!sha_v) return false; if (!add_program_resource(shProg, GL_PROGRAM_OUTPUT, sha_v, diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp index caff79f39af..190f6387987 100644 --- a/src/mesa/main/shader_query.cpp +++ b/src/mesa/main/shader_query.cpp @@ -101,24 +101,6 @@ _mesa_BindAttribLocation(GLuint program, GLuint index, */ } -static bool -is_active_attrib(const gl_shader_variable *var) -{ - if (!var) - return false; - - switch (var->mode) { - case ir_var_shader_in: - return var->location != -1; - - case ir_var_system_value: - return true; - - default: - return false; - } -} - void GLAPIENTRY _mesa_GetActiveAttrib(GLuint program, GLuint desired_index, GLsizei maxLength, GLsizei * length, GLint * size, @@ -159,9 +141,6 @@ _mesa_GetActiveAttrib(GLuint program, GLuint desired_index, const gl_shader_variable *const var = RESOURCE_VAR(res); - if (!is_active_attrib(var)) - return; - const char *var_name = var->name; _mesa_copy_string(name, maxLength, length, var_name); @@ -208,19 +187,7 @@ _mesa_GetAttribLocation(GLuint program, const GLchar * name) if (!res) return -1; - GLint loc = program_resource_location(shProg, res, name, array_index); - - /* The extra check against against 0 is made because of builtin-attribute - * locations that have offset applied. Function program_resource_location - * can return built-in attribute locations < 0 and glGetAttribLocation - * cannot be used on "conventional" attributes. - * - * From page 95 of the OpenGL 3.0 spec: - * - * "If name is not an active attribute, if name is a conventional - * attribute, or if an error occurs, -1 will be returned." - */ - return (loc >= 0) ? loc : -1; + return program_resource_location(shProg, res, name, array_index); } unsigned @@ -235,8 +202,7 @@ _mesa_count_active_attribs(struct gl_shader_program *shProg) unsigned count = 0; for (unsigned j = 0; j < shProg->NumProgramResourceList; j++, res++) { if (res->Type == GL_PROGRAM_INPUT && - res->StageReferences & (1 << MESA_SHADER_VERTEX) && - is_active_attrib(RESOURCE_VAR(res))) + res->StageReferences & (1 << MESA_SHADER_VERTEX)) count++; } return count; @@ -394,19 +360,7 @@ _mesa_GetFragDataLocation(GLuint program, const GLchar *name) if (!res) return -1; - GLint loc = program_resource_location(shProg, res, name, array_index); - - /* The extra check against against 0 is made because of builtin-attribute - * locations that have offset applied. Function program_resource_location - * can return built-in attribute locations < 0 and glGetFragDataLocation - * cannot be used on "conventional" attributes. - * - * From page 95 of the OpenGL 3.0 spec: - * - * "If name is not an active attribute, if name is a conventional - * attribute, or if an error occurs, -1 will be returned." - */ - return (loc >= 0) ? loc : -1; + return program_resource_location(shProg, res, name, array_index); } const char* @@ -826,10 +780,6 @@ program_resource_location(struct gl_shader_program *shProg, struct gl_program_resource *res, const char *name, unsigned array_index) { - /* Built-in locations should report GL_INVALID_INDEX. */ - if (is_gl_identifier(name)) - return GL_INVALID_INDEX; - /* VERT_ATTRIB_GENERIC0 and FRAG_RESULT_DATA0 are decremented as these * offsets are used internally to differentiate between built-in attributes * and user-defined attributes. @@ -838,6 +788,9 @@ program_resource_location(struct gl_shader_program *shProg, case GL_PROGRAM_INPUT: { const gl_shader_variable *var = RESOURCE_VAR(res); + if (var->location == -1) + return -1; + /* If the input is an array, fail if the index is out of bounds. */ if (array_index > 0 && array_index >= var->type->length) { @@ -848,6 +801,9 @@ program_resource_location(struct gl_shader_program *shProg, VERT_ATTRIB_GENERIC0); } case GL_PROGRAM_OUTPUT: + if (RESOURCE_VAR(res)->location == -1) + return -1; + /* If the output is an array, fail if the index is out of bounds. */ if (array_index > 0 && array_index >= RESOURCE_VAR(res)->type->length) { -- 2.30.2