glsl: encapsulate binding validation and setting

This change moves the binding layout handing code into an apply
function to be consistent with other helper functions in the ast
code, and to encapsulate the code so that when we introduce
compile time constants the code will be much cleaner.

One small downside is for unnamed interface blocks we will now
be revalidating the binding for each member its applied to.
However this seems a small sacrifice in order to have code which
is readable.

We also remove the incorrect comment in the named interface code
about propagating bindings to members which seems to have been
copied from the unnamed interface code.

Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
This commit is contained in:
Timothy Arceri 2015-11-15 00:55:29 +11:00 committed by Timothy Arceri
parent db3c36aedf
commit 64710db664

View file

@ -2536,9 +2536,10 @@ validate_stream_qualifier(YYLTYPE *loc, struct _mesa_glsl_parse_state *state,
return true; return true;
} }
static bool static void
validate_binding_qualifier(struct _mesa_glsl_parse_state *state, apply_explicit_binding(struct _mesa_glsl_parse_state *state,
YYLTYPE *loc, YYLTYPE *loc,
ir_variable *var,
const glsl_type *type, const glsl_type *type,
const ast_type_qualifier *qual) const ast_type_qualifier *qual)
{ {
@ -2546,12 +2547,12 @@ validate_binding_qualifier(struct _mesa_glsl_parse_state *state,
_mesa_glsl_error(loc, state, _mesa_glsl_error(loc, state,
"the \"binding\" qualifier only applies to uniforms and " "the \"binding\" qualifier only applies to uniforms and "
"shader storage buffer objects"); "shader storage buffer objects");
return false; return;
} }
if (qual->binding < 0) { if (qual->binding < 0) {
_mesa_glsl_error(loc, state, "binding values must be >= 0"); _mesa_glsl_error(loc, state, "binding values must be >= 0");
return false; return;
} }
const struct gl_context *const ctx = state->ctx; const struct gl_context *const ctx = state->ctx;
@ -2576,7 +2577,7 @@ validate_binding_qualifier(struct _mesa_glsl_parse_state *state,
"the maximum number of UBO binding points (%d)", "the maximum number of UBO binding points (%d)",
qual->binding, elements, qual->binding, elements,
ctx->Const.MaxUniformBufferBindings); ctx->Const.MaxUniformBufferBindings);
return false; return;
} }
/* SSBOs. From page 67 of the GLSL 4.30 specification: /* SSBOs. From page 67 of the GLSL 4.30 specification:
@ -2594,7 +2595,7 @@ validate_binding_qualifier(struct _mesa_glsl_parse_state *state,
"the maximum number of SSBO binding points (%d)", "the maximum number of SSBO binding points (%d)",
qual->binding, elements, qual->binding, elements,
ctx->Const.MaxShaderStorageBufferBindings); ctx->Const.MaxShaderStorageBufferBindings);
return false; return;
} }
} else if (base_type->is_sampler()) { } else if (base_type->is_sampler()) {
/* Samplers. From page 63 of the GLSL 4.20 specification: /* Samplers. From page 63 of the GLSL 4.20 specification:
@ -2611,7 +2612,7 @@ validate_binding_qualifier(struct _mesa_glsl_parse_state *state,
"exceeds the maximum number of texture image units " "exceeds the maximum number of texture image units "
"(%d)", qual->binding, elements, limit); "(%d)", qual->binding, elements, limit);
return false; return;
} }
} else if (base_type->contains_atomic()) { } else if (base_type->contains_atomic()) {
assert(ctx->Const.MaxAtomicBufferBindings <= MAX_COMBINED_ATOMIC_BUFFERS); assert(ctx->Const.MaxAtomicBufferBindings <= MAX_COMBINED_ATOMIC_BUFFERS);
@ -2621,7 +2622,7 @@ validate_binding_qualifier(struct _mesa_glsl_parse_state *state,
"(%d)", qual->binding, "(%d)", qual->binding,
ctx->Const.MaxAtomicBufferBindings); ctx->Const.MaxAtomicBufferBindings);
return false; return;
} }
} else if (state->is_version(420, 310) && base_type->is_image()) { } else if (state->is_version(420, 310) && base_type->is_image()) {
assert(ctx->Const.MaxImageUnits <= MAX_IMAGE_UNITS); assert(ctx->Const.MaxImageUnits <= MAX_IMAGE_UNITS);
@ -2629,17 +2630,20 @@ validate_binding_qualifier(struct _mesa_glsl_parse_state *state,
_mesa_glsl_error(loc, state, "Image binding %d exceeds the " _mesa_glsl_error(loc, state, "Image binding %d exceeds the "
" maximum number of image units (%d)", max_index, " maximum number of image units (%d)", max_index,
ctx->Const.MaxImageUnits); ctx->Const.MaxImageUnits);
return false; return;
} }
} else { } else {
_mesa_glsl_error(loc, state, _mesa_glsl_error(loc, state,
"the \"binding\" qualifier only applies to uniform " "the \"binding\" qualifier only applies to uniform "
"blocks, opaque variables, or arrays thereof"); "blocks, opaque variables, or arrays thereof");
return false; return;
} }
return true; var->data.explicit_binding = true;
var->data.binding = qual->binding;
return;
} }
@ -3041,10 +3045,8 @@ apply_layout_qualifier_to_variable(const struct ast_type_qualifier *qual,
_mesa_glsl_error(loc, state, "explicit index requires explicit location"); _mesa_glsl_error(loc, state, "explicit index requires explicit location");
} }
if (qual->flags.q.explicit_binding && if (qual->flags.q.explicit_binding) {
validate_binding_qualifier(state, loc, var->type, qual)) { apply_explicit_binding(state, loc, var, var->type, qual);
var->data.explicit_binding = true;
var->data.binding = qual->binding;
} }
if (state->stage == MESA_SHADER_GEOMETRY && if (state->stage == MESA_SHADER_GEOMETRY &&
@ -6694,8 +6696,6 @@ ast_interface_block::hir(exec_list *instructions,
num_variables, num_variables,
packing, packing,
this->block_name); 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)) { if (!state->symbols->add_interface(block_type->name, block_type, var_mode)) {
YYLTYPE loc = this->get_location(); YYLTYPE loc = this->get_location();
@ -6826,10 +6826,6 @@ ast_interface_block::hir(exec_list *instructions,
"not allowed"); "not allowed");
} }
if (this->layout.flags.q.explicit_binding)
validate_binding_qualifier(state, &loc, block_array_type,
&this->layout);
var = new(state) ir_variable(block_array_type, var = new(state) ir_variable(block_array_type,
this->instance_name, this->instance_name,
var_mode); var_mode);
@ -6891,12 +6887,10 @@ ast_interface_block::hir(exec_list *instructions,
earlier->reinit_interface_type(block_type); earlier->reinit_interface_type(block_type);
delete var; delete var;
} else { } else {
/* Propagate the "binding" keyword into this UBO's fields; if (this->layout.flags.q.explicit_binding) {
* the UBO declaration itself doesn't get an ir_variable unless it apply_explicit_binding(state, &loc, var,
* has an instance name. This is ugly. var->get_interface_type(), &this->layout);
*/ }
var->data.explicit_binding = this->layout.flags.q.explicit_binding;
var->data.binding = this->layout.binding;
var->data.stream = qual_stream; var->data.stream = qual_stream;
@ -6975,8 +6969,10 @@ ast_interface_block::hir(exec_list *instructions,
* The UBO declaration itself doesn't get an ir_variable unless it * The UBO declaration itself doesn't get an ir_variable unless it
* has an instance name. This is ugly. * has an instance name. This is ugly.
*/ */
var->data.explicit_binding = this->layout.flags.q.explicit_binding; if (this->layout.flags.q.explicit_binding) {
var->data.binding = this->layout.binding; apply_explicit_binding(state, &loc, var,
var->get_interface_type(), &this->layout);
}
if (var->type->is_unsized_array()) { if (var->type->is_unsized_array()) {
if (var->is_in_shader_storage_block()) { if (var->is_in_shader_storage_block()) {