On 28 May 2015 at 06:51, Timothy Arceri <t_arc...@yahoo.com.au> wrote:
> ---- Matt Turner wrote ----
>
>> On Wed, May 27, 2015 at 3:53 AM, Timothy Arceri <t_arc...@yahoo.com.au>
>> wrote:
>> > Cc: Kenneth Graunke <kenn...@whitecape.org>
>> > ---
>> >
>> > 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.
he meant the missing space between if and (
Dave.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
