On 01/23/2013 09:49 PM, Paul Berry wrote:
On 22 January 2013 00:52, Ian Romanick <i...@freedesktop.org
<mailto:i...@freedesktop.org>> wrote:

    From: Ian Romanick <ian.d.roman...@intel.com
    <mailto:ian.d.roman...@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.roman...@intel.com
    <mailto:ian.d.roman...@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 <stereotype...@gmail.com
<mailto:stereotype...@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@lists.freedesktop.org <mailto:mesa-dev@lists.freedesktop.org>
    http://lists.freedesktop.org/mailman/listinfo/mesa-dev

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to