[Mesa-dev] [PATCH 10/32] glsl: Generate an interface type for uniform blocks

Ian Romanick idr at freedesktop.org
Fri Jan 25 05:59:37 PST 2013


On 01/23/2013 09:49 PM, Paul Berry wrote:
> On 22 January 2013 00:52, Ian Romanick <idr at freedesktop.org
> <mailto:idr at freedesktop.org>> wrote:
>
>     From: Ian Romanick <ian.d.romanick at intel.com
>     <mailto:ian.d.romanick at intel.com>>
>
>     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 <ian.d.romanick at intel.com
>     <mailto:ian.d.romanick at intel.com>>
>     ---
>       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());
>
>
> 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.

I'm not sure what I was thinking when I wrote that comment.  You are 
correct, and I'll update it.

> Other than that confusion, the patch looks good.
>
> Reviewed-by: Paul Berry <stereotype441 at gmail.com
> <mailto:stereotype441 at gmail.com>>
>
>     +
>     +   /* 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
>     mesa-dev at lists.freedesktop.org <mailto:mesa-dev at lists.freedesktop.org>
>     http://lists.freedesktop.org/mailman/listinfo/mesa-dev



More information about the mesa-dev mailing list