diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp index 50660141c3e..5508d588cd5 100644 --- a/src/compiler/glsl/linker.cpp +++ b/src/compiler/glsl/linker.cpp @@ -3735,6 +3735,65 @@ add_shader_variable(const struct gl_context *ctx, bool use_implicit_location, int location, const glsl_type *outermost_struct_type = NULL) { + const glsl_type *interface_type = var->get_interface_type(); + + if (outermost_struct_type == NULL) { + /* Unsized (non-patch) TCS output/TES input arrays are implicitly + * sized to gl_MaxPatchVertices. Internally, we shrink them to a + * smaller size. + * + * This can cause trouble with SSO programs. Since the TCS declares + * the number of output vertices, we can always shrink TCS output + * arrays. However, the TES might not be linked with a TCS, in + * which case it won't know the size of the patch. In other words, + * the TCS and TES may disagree on the (smaller) array sizes. This + * can result in the resource names differing across stages, causing + * SSO validation failures and other cascading issues. + * + * Expanding the array size to the full gl_MaxPatchVertices fixes + * these issues. It's also what program interface queries expect, + * as that is the official size of the array. + */ + if (var->data.tess_varying_implicit_sized_array) { + type = resize_to_max_patch_vertices(ctx, type); + interface_type = resize_to_max_patch_vertices(ctx, interface_type); + } + + if (var->data.from_named_ifc_block) { + const char *interface_name = interface_type->name; + + if (interface_type->is_array()) { + /* Issue #16 of the ARB_program_interface_query spec says: + * + * "* If a variable is a member of an interface block without an + * instance name, it is enumerated using just the variable name. + * + * * If a variable is a member of an interface block with an + * instance name, it is enumerated as "BlockName.Member", where + * "BlockName" is the name of the interface block (not the + * instance name) and "Member" is the name of the variable." + * + * In particular, it indicates that it should be "BlockName", + * not "BlockName[array length]". The conformance suite and + * dEQP both require this behavior. + * + * Here, we unwrap the extra array level added by named interface + * block array lowering so we have the correct variable type. We + * also unwrap the interface type when constructing the name. + * + * We leave interface_type the same so that ES 3.x SSO pipeline + * validation can enforce the rules requiring array length to + * match on interface blocks. + */ + type = type->fields.array; + + interface_name = interface_type->fields.array->name; + } + + name = ralloc_asprintf(shProg, "%s.%s", interface_name, name); + } + } + switch (type->base_type) { case GLSL_TYPE_STRUCT: { /* The ARB_program_interface_query spec says: @@ -3766,44 +3825,6 @@ add_shader_variable(const struct gl_context *ctx, } default: { - const glsl_type *interface_type = var->get_interface_type(); - - /* Unsized (non-patch) TCS output/TES input arrays are implicitly - * sized to gl_MaxPatchVertices. Internally, we shrink them to a - * smaller size. - * - * This can cause trouble with SSO programs. Since the TCS declares - * the number of output vertices, we can always shrink TCS output - * arrays. However, the TES might not be linked with a TCS, in - * which case it won't know the size of the patch. In other words, - * the TCS and TES may disagree on the (smaller) array sizes. This - * can result in the resource names differing across stages, causing - * SSO validation failures and other cascading issues. - * - * Expanding the array size to the full gl_MaxPatchVertices fixes - * these issues. It's also what program interface queries expect, - * as that is the official size of the array. - */ - if (var->data.tess_varying_implicit_sized_array) { - type = resize_to_max_patch_vertices(ctx, type); - interface_type = resize_to_max_patch_vertices(ctx, interface_type); - } - - /* Issue #16 of the ARB_program_interface_query spec says: - * - * "* If a variable is a member of an interface block without an - * instance name, it is enumerated using just the variable name. - * - * * If a variable is a member of an interface block with an instance - * name, it is enumerated as "BlockName.Member", where "BlockName" is - * the name of the interface block (not the instance name) and - * "Member" is the name of the variable." - */ - const char *prefixed_name = (var->data.from_named_ifc_block && - !is_gl_identifier(var->name)) - ? ralloc_asprintf(shProg, "%s.%s", interface_type->name, name) - : name; - /* The ARB_program_interface_query spec says: * * "For an active variable declared as a single instance of a basic @@ -3811,8 +3832,7 @@ add_shader_variable(const struct gl_context *ctx, * from the shader source." */ gl_shader_variable *sha_v = - create_shader_variable(shProg, var, prefixed_name, type, - interface_type, + create_shader_variable(shProg, var, name, type, interface_type, use_implicit_location, location, outermost_struct_type); if (!sha_v) diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp index e4a8773e09c..1d1616b7eaa 100644 --- a/src/mesa/main/shader_query.cpp +++ b/src/mesa/main/shader_query.cpp @@ -686,31 +686,14 @@ _mesa_program_resource_find_index(struct gl_shader_program *shProg, * ambiguous in this regard. However, either name can later be passed * to glGetUniformLocation (and related APIs), so there shouldn't be any * harm in always appending "[0]" to uniform array names. - * - * Geometry shader stage has different naming convention where the 'normal' - * condition is an array, therefore for variables referenced in geometry - * stage we do not add '[0]'. - * - * Note, that TCS outputs and TES inputs should not have index appended - * either. */ static bool add_index_to_name(struct gl_program_resource *res) { - bool add_index = !((res->Type == GL_PROGRAM_INPUT && - res->StageReferences & (1 << MESA_SHADER_GEOMETRY | - 1 << MESA_SHADER_TESS_CTRL | - 1 << MESA_SHADER_TESS_EVAL)) || - (res->Type == GL_PROGRAM_OUTPUT && - res->StageReferences & 1 << MESA_SHADER_TESS_CTRL)); - /* Transform feedback varyings have array index already appended * in their names. */ - if (res->Type == GL_TRANSFORM_FEEDBACK_VARYING) - add_index = false; - - return add_index; + return res->Type != GL_TRANSFORM_FEEDBACK_VARYING; } /* Get name length of a program resource. This consists of @@ -1401,9 +1384,6 @@ validate_io(struct gl_shader_program *producer, bool valid = true; - void *name_buffer = NULL; - size_t name_buffer_size = 0; - gl_shader_variable const **outputs = (gl_shader_variable const **) calloc(producer->NumProgramResourceList, sizeof(gl_shader_variable *)); @@ -1475,52 +1455,11 @@ validate_io(struct gl_shader_program *producer, } } } else { - char *consumer_name = consumer_var->name; - - if (nonarray_stage_to_array_stage && - consumer_var->interface_type != NULL && - consumer_var->interface_type->is_array() && - !is_gl_identifier(consumer_var->name)) { - const size_t name_len = strlen(consumer_var->name); - - if (name_len >= name_buffer_size) { - free(name_buffer); - - name_buffer_size = name_len + 1; - name_buffer = malloc(name_buffer_size); - if (name_buffer == NULL) { - valid = false; - goto out; - } - } - - consumer_name = (char *) name_buffer; - - char *s = strchr(consumer_var->name, '['); - if (s == NULL) { - valid = false; - goto out; - } - - char *t = strchr(s, ']'); - if (t == NULL) { - valid = false; - goto out; - } - - assert(t[1] == '.' || t[1] == '['); - - const ptrdiff_t base_name_len = s - consumer_var->name; - - memcpy(consumer_name, consumer_var->name, base_name_len); - strcpy(consumer_name + base_name_len, t + 1); - } - for (unsigned j = 0; j < num_outputs; j++) { const gl_shader_variable *const var = outputs[j]; if (!var->explicit_location && - strcmp(consumer_name, var->name) == 0) { + strcmp(consumer_var->name, var->name) == 0) { producer_var = var; match_index = j; break; @@ -1583,21 +1522,29 @@ validate_io(struct gl_shader_program *producer, * find the producer variable that goes with the consumer variable. */ if (nonarray_stage_to_array_stage) { - if (!consumer_var->type->is_array() || - consumer_var->type->fields.array != producer_var->type) { - valid = false; - goto out; - } - if (consumer_var->interface_type != NULL) { + /* the interface is the array; underlying types should match */ + if (producer_var->type != consumer_var->type) { + valid = false; + goto out; + } + if (!consumer_var->interface_type->is_array() || consumer_var->interface_type->fields.array != producer_var->interface_type) { valid = false; goto out; } - } else if (producer_var->interface_type != NULL) { - valid = false; - goto out; + } else { + if (producer_var->interface_type != NULL) { + valid = false; + goto out; + } + + if (!consumer_var->type->is_array() || + consumer_var->type->fields.array != producer_var->type) { + valid = false; + goto out; + } } } else { if (producer_var->type != consumer_var->type) { @@ -1628,7 +1575,6 @@ validate_io(struct gl_shader_program *producer, } out: - free(name_buffer); free(outputs); return valid && num_outputs == 0; }