---- Matt Turner wrote ---- > On Wed, May 27, 2015 at 3:53 AM, Timothy Arceri <<a href="mailto:t_arceri@yahoo.com.au">t_arceri@yahoo.com.au</a>> wrote: > > Cc: Kenneth Graunke <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>> > > --- > > > > It looks like this may never have actually worked, at least there > > is no way it could have for inteface block arrays :) > > > > No piglit regressions. > > > > src/glsl/ast_to_hir.cpp | 22 ++++++++++++++-------- > > 1 file changed, 14 insertions(+), 8 deletions(-) > > > > diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp > > index 0ab664e..861bbfe 100644 > > --- a/src/glsl/ast_to_hir.cpp > > +++ b/src/glsl/ast_to_hir.cpp > > @@ -2041,10 +2041,10 @@ validate_matrix_layout_for_type(struct _mesa_glsl_parse_state *state, > > static bool > > validate_binding_qualifier(struct _mesa_glsl_parse_state *state, > > YYLTYPE *loc, > > - ir_variable *var, > > + const glsl_type *type, > > const ast_type_qualifier *qual) > > { > > - if (var->data.mode != ir_var_uniform) { > > + if (!qual->flags.q.uniform) { > > _mesa_glsl_error(loc, state, > > "the \"binding\" qualifier only applies to uniforms"); > > return false; > > @@ -2056,10 +2056,11 @@ validate_binding_qualifier(struct _mesa_glsl_parse_state *state, > > } > > > > const struct gl_context *const ctx = state->ctx; > > - unsigned elements = var->type->is_array() ? var->type->length : 1; > > + unsigned elements = type->is_array() ? type->length : 1; > > unsigned max_index = qual->binding + elements - 1; > > + const glsl_type *base_type = type->without_array(); > > > > - if (var->type->is_interface()) { > > + if (base_type->is_interface()) { > > /* UBOs. From page 60 of the GLSL 4.20 specification: > > * "If the binding point for any uniform block instance is less than zero, > > * or greater than or equal to the implementation-dependent maximum > > @@ -2077,8 +2078,7 @@ validate_binding_qualifier(struct _mesa_glsl_parse_state *state, > > ctx->Const.MaxUniformBufferBindings); > > return false; > > } > > - } else if (var->type->is_sampler() || > > - (var->type->is_array() && var->type->fields.array->is_sampler())) { > > + } else if (base_type->is_sampler()) { > > /* Samplers. From page 63 of the GLSL 4.20 specification: > > * "If the binding is less than zero, or greater than or equal to the > > * implementation-dependent maximum supported number of units, a > > @@ -2095,7 +2095,7 @@ validate_binding_qualifier(struct _mesa_glsl_parse_state *state, > > > > return false; > > } > > - } else if (var->type->contains_atomic()) { > > + } else if (base_type->contains_atomic()) { > > assert(ctx->Const.MaxAtomicBufferBindings <= MAX_COMBINED_ATOMIC_BUFFERS); > > if (unsigned(qual->binding) >= ctx->Const.MaxAtomicBufferBindings) { > > _mesa_glsl_error(loc, state, "layout(binding = %d) exceeds the " > > @@ -2651,7 +2651,7 @@ apply_type_qualifier_to_variable(const struct ast_type_qualifier *qual, > > } > > > > if (qual->flags.q.explicit_binding && > > - validate_binding_qualifier(state, loc, var, qual)) { > > + validate_binding_qualifier(state, loc, var->type, qual)) { > > var->data.explicit_binding = true; > > var->data.binding = qual->binding; > > } > > @@ -5658,6 +5658,8 @@ ast_interface_block::hir(exec_list *instructions, > > num_variables, > > packing, > > this->block_name); > > + if (this->layout.flags.q.explicit_binding) > > + validate_binding_qualifier(state, &loc, block_type, &this->layout); > > > > if (!state->symbols->add_interface(block_type->name, block_type, var_mode)) { > > YYLTYPE loc = this->get_location(); > > @@ -5744,6 +5746,10 @@ ast_interface_block::hir(exec_list *instructions, > > const glsl_type *block_array_type = > > process_array_type(&loc, block_type, this->array_specifier, state); > > > > + if(this->layout.flags.q.explicit_binding) > > Space after if The space was intentional to separate validation code from from code that creates the IR. If you still think it should go I can remove it but I think it helps with code readability.