<div dir="ltr">On 22 January 2013 00:52, Ian Romanick <<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>> wrote: <div class="gmail_extra"><div class="gmail_quote"> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">From: Ian Romanick <<a href="mailto:ian.d.romanick@intel.com" target="_blank">ian.d.romanick@intel.com</a>> If the block has an instance name, add the instance name to the symbol table instead of the individual fields. Fixes the piglit test interface-name-access-without-interface-name.vert for real. Signed-off-by: Ian Romanick <<a href="mailto:ian.d.romanick@intel.com" target="_blank">ian.d.romanick@intel.com</a>> --- src/glsl/ast_to_hir.cpp | 167 ++++++++++++++++++++++++++++++++++-------------- 1 file changed, 118 insertions(+), 49 deletions(-) diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp index 575dd84..a740a3c 100644 --- a/src/glsl/ast_to_hir.cpp +++ b/src/glsl/ast_to_hir.cpp @@ -4020,7 +4020,9 @@ ast_process_structure_or_interface_block(exec_list *instructions, struct _mesa_glsl_parse_state *state, exec_list *declarations, YYLTYPE &loc, - glsl_struct_field **fields_ret) + glsl_struct_field **fields_ret, + bool is_interface, + bool block_row_major) { unsigned decl_count = 0; @@ -4062,7 +4064,32 @@ ast_process_structure_or_interface_block(exec_list *instructions, foreach_list_typed (ast_declaration, decl, link, &decl_list->declarations) { - const struct glsl_type *field_type = decl_type; + /* From the GL_ARB_uniform_buffer_object spec: + * + * "Sampler types are not allowed inside of uniform + * blocks. All other types, arrays, and structures + * allowed for uniforms are allowed within a uniform + * block." + */ + const struct glsl_type *field_type = decl_type; + + if (is_interface && field_type->contains_sampler()) { + YYLTYPE loc = decl_list->get_location(); + _mesa_glsl_error(&loc, state, + "Uniform in non-default uniform block contains sampler\n"); + } + + const struct ast_type_qualifier *const qual = + & decl_list->type->qualifier; + if (qual->flags.q.std140 || + qual->flags.q.packed || + qual->flags.q.shared) { + _mesa_glsl_error(&loc, state, + "uniform block layout qualifiers std140, packed, and " + "shared can only be applied to uniform blocks, not " + "members"); + } + if (decl->is_array) { field_type = process_array_type(&loc, decl_type, decl->array_size, state); @@ -4070,6 +4097,26 @@ ast_process_structure_or_interface_block(exec_list *instructions, fields[i].type = (field_type != NULL) ? field_type : glsl_type::error_type; fields[i].name = decl->identifier; + + if (qual->flags.q.row_major || qual->flags.q.column_major) { + if (!field_type->is_matrix() && !field_type->is_record()) { + _mesa_glsl_error(&loc, state, + "uniform block layout qualifiers row_major and " + "column_major can only be applied to matrix and " + "structure types"); + } else + validate_matrix_layout_for_type(state, &loc, field_type); + } + + if (field_type->is_matrix() || + (field_type->is_array() && field_type->fields.array->is_matrix())) { + fields[i].row_major = block_row_major; + if (qual->flags.q.row_major) + fields[i].row_major = true; + else if (qual->flags.q.column_major) + fields[i].row_major = false; + } + i++; } } @@ -4092,7 +4139,9 @@ ast_struct_specifier::hir(exec_list *instructions, state, &this->declarations, loc, - &fields); + &fields, + false, + false); const glsl_type *t = glsl_type::get_record_instance(fields, decl_count, this->name); @@ -4138,6 +4187,8 @@ ir_rvalue * ast_uniform_block::hir(exec_list *instructions, struct _mesa_glsl_parse_state *state) { + YYLTYPE loc = this->get_location(); + /* The ast_uniform_block has a list of ast_declarator_lists. We * need to turn those into ir_variables with an association * with this uniform block. @@ -4161,60 +4212,78 @@ ast_uniform_block::hir(exec_list *instructions, ubo->_Packing = ubo_packing_std140; } - unsigned int num_variables = 0; - foreach_list_typed(ast_declarator_list, decl_list, link, &declarations) { - foreach_list_const(node, &decl_list->declarations) { - num_variables++; + bool block_row_major = this->layout.flags.q.row_major; + exec_list declared_variables; + glsl_struct_field *fields; + unsigned int num_variables = + ast_process_structure_or_interface_block(&declared_variables, + state, + &this->declarations, + loc, + &fields, + true, + block_row_major); + + STATIC_ASSERT(unsigned(GLSL_INTERFACE_PACKING_STD140) + == unsigned(ubo_packing_std140)); + STATIC_ASSERT(unsigned(GLSL_INTERFACE_PACKING_SHARED) + == unsigned(ubo_packing_shared)); + STATIC_ASSERT(unsigned(GLSL_INTERFACE_PACKING_PACKED) + == unsigned(ubo_packing_packed)); + + const glsl_type *block_type = + glsl_type::get_interface_instance(fields, + num_variables, + (enum glsl_interface_packing) ubo->_Packing, + this->block_name); + + /* Since interface blocks cannot contain structure definitions, it should + * be impossible for the block to generate any instructions. + */ + assert(declared_variables.is_empty()); </blockquote><div> </div><div>I'm confused. Even if an interface block contained a structure definition, how would that generate instructions? I would have thought the reason the block wouldn't generate any instructions is because interface blocks can't contain statements. </div><div>Other than that confusion, the patch looks good. Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>> </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + + /* Page 39 (page 45 of the PDF) of section 4.3.7 in the GLSL ES 3.00 spec + * says: + * + * "If an instance name (instance-name) is used, then it puts all the + * members inside a scope within its own name space, accessed with the + * field selector ( . ) operator (analogously to structures)." + */ + if (this->instance_name) { + ir_variable *var = new(state) ir_variable(block_type, + this->instance_name, + ir_var_uniform); + + state->symbols->add_variable(var); + instructions->push_tail(var); + } else { + for (unsigned i = 0; i < num_variables; i++) { + ir_variable *var = + new(state) ir_variable(fields[i].type, + ralloc_strdup(state, fields[i].name), + ir_var_uniform); + var->uniform_block = ubo - state->uniform_blocks; + + state->symbols->add_variable(var); + instructions->push_tail(var); } } - bool block_row_major = this->layout.flags.q.row_major; - + /* FINISHME: Eventually the rest of this code needs to be moved into the + * FINISHME: linker. + */ ubo->Uniforms = rzalloc_array(state->uniform_blocks, struct gl_uniform_buffer_variable, num_variables); - foreach_list_typed(ast_declarator_list, decl_list, link, &declarations) { - exec_list declared_variables; - - decl_list->hir(&declared_variables, state); - - foreach_list_const(node, &declared_variables) { - ir_variable *var = (ir_variable *)node; - - struct gl_uniform_buffer_variable *ubo_var = - &ubo->Uniforms[ubo->NumUniforms++]; - - var->uniform_block = ubo - state->uniform_blocks; - - ubo_var->Name = ralloc_strdup(state->uniform_blocks, var->name); - ubo_var->Type = var->type; - ubo_var->Offset = 0; /* Assigned at link time. */ - - if (var->type->is_matrix() || - (var->type->is_array() && var->type->fields.array->is_matrix())) { - ubo_var->RowMajor = block_row_major; - if (decl_list->type->qualifier.flags.q.row_major) - ubo_var->RowMajor = true; - else if (decl_list->type->qualifier.flags.q.column_major) - ubo_var->RowMajor = false; - } - - /* From the GL_ARB_uniform_buffer_object spec: - * - * "Sampler types are not allowed inside of uniform - * blocks. All other types, arrays, and structures - * allowed for uniforms are allowed within a uniform - * block." - */ - if (var->type->contains_sampler()) { - YYLTYPE loc = decl_list->get_location(); - _mesa_glsl_error(&loc, state, - "Uniform in non-default uniform block contains sampler\n"); - } - } + for (unsigned i = 0; i < num_variables; i++) { + struct gl_uniform_buffer_variable *ubo_var = + &ubo->Uniforms[ubo->NumUniforms++]; - instructions->append_list(&declared_variables); + ubo_var->Name = ralloc_strdup(state->uniform_blocks, fields[i].name); + ubo_var->Type = fields[i].type; + ubo_var->Offset = 0; /* Assigned at link time. */ + ubo_var->RowMajor = fields[i].row_major; } return NULL; -- 1.7.11.7 _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div></div>