glsl: fix uniform linking logic in the presence of structs

Add a enter/leave record callback so that the offset may be aligned to
the proper value. Otherwise only leaf fields are called, and the first
field needs to be aligned to the outer struct's base alignment while the
last field needs to be aligned to the inner struct's base alignment.

This removes most usage of the last field/record type values passed into
visit_field.

Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
Reviewed-by: Dave Airlie <airlied@redhat.com>

diff --git a/src/glsl/link_uniform_blocks.cpp b/src/glsl/link_uniform_blocks.cpp
index f5fc5022e1883f2b116733cb84def27e9493161d..1c175ec8bdb4474f06e724b5a82980380428d995 100644 (file)
--- a/src/glsl/link_uniform_blocks.cpp
+++ b/src/glsl/link_uniform_blocks.cpp
@@ -67,6 +67,25 @@ private:
       assert(!"Should not get here.");
    }
 
+   virtual void enter_record(const glsl_type *type, const char *name,
+                             bool row_major) {
+      this->offset = glsl_align(
+            this->offset, type->std140_base_alignment(row_major));
+   }
+
+   virtual void leave_record(const glsl_type *type, const char *name,
+                             bool row_major) {
+      /* If this is the last field of a structure, apply rule #9.  The
+       * GL_ARB_uniform_buffer_object spec says:
+       *
+       *     "The structure may have padding at the end; the base offset of
+       *     the member following the sub-structure is rounded up to the next
+       *     multiple of the base alignment of the structure."
+       */
+      this->offset = glsl_align(
+            this->offset, type->std140_base_alignment(row_major));
+   }
+
    virtual void visit_field(const glsl_type *type, const char *name,
                             bool row_major, const glsl_type *record_type,
                             bool last_field)
@@ -97,27 +116,13 @@ private:
          v->IndexName = v->Name;
       }
 
-      const unsigned alignment = record_type
-         ? record_type->std140_base_alignment(v->RowMajor)
-         : type->std140_base_alignment(v->RowMajor);
+      const unsigned alignment = type->std140_base_alignment(v->RowMajor);
       unsigned size = type->std140_size(v->RowMajor);
 
       this->offset = glsl_align(this->offset, alignment);
       v->Offset = this->offset;
 
-      /* If this is the last field of a structure, apply rule #9.  The
-       * GL_ARB_uniform_buffer_object spec says:
-       *
-       *     "The structure may have padding at the end; the base offset of
-       *     the member following the sub-structure is rounded up to the next
-       *     multiple of the base alignment of the structure."
-       *
-       * last_field won't be set if this is the last field of a UBO that is
-       * not a named instance.
-       */
       this->offset += size;
-      if (last_field)
-         this->offset = glsl_align(this->offset, 16);
 
       /* From the GL_ARB_uniform_buffer_object spec:
        *
@@ -131,16 +136,6 @@ private:
        */
       this->buffer_size = glsl_align(this->offset, 16);
    }
-
-   virtual void visit_field(const glsl_struct_field *field)
-   {
-      /* FINISHME: When support for doubles (dvec4, etc.) is added to the
-       * FINISHME: compiler, this may be incorrect for a structure in a UBO
-       * FINISHME: like struct s { struct { float f } s1; dvec4 v; };.
-       */
-      this->offset = glsl_align(this->offset,
-                                field->type->std140_base_alignment(false));
-   }
 };
 
 class count_block_size : public program_resource_visitor {

diff --git a/src/glsl/link_uniforms.cpp b/src/glsl/link_uniforms.cpp
index 3aa6e0a9c7475bf3cc528182052a542109265e5d..347f079fa42ba505b5f061027f9a5e041879ff92 100644 (file)
--- a/src/glsl/link_uniforms.cpp
+++ b/src/glsl/link_uniforms.cpp
@@ -169,6 +169,9 @@ program_resource_visitor::recursion(const glsl_type *t, char **name,
       if (record_type == NULL && t->is_record())
          record_type = t;
 
+      if (t->is_record())
+         this->enter_record(t, *name, row_major);
+
       for (unsigned i = 0; i < t->length; i++) {
         const char *field = t->fields.structure[i].name;
         size_t new_length = name_length;
@@ -208,6 +211,11 @@ program_resource_visitor::recursion(const glsl_type *t, char **name,
           */
          record_type = NULL;
       }
+
+      if (t->is_record()) {
+         (*name)[name_length] = '\0';
+         this->leave_record(t, *name, row_major);
+      }
    } else if (t->is_array() && (t->fields.array->is_record()
                                 || t->fields.array->is_interface())) {
       if (record_type == NULL && t->fields.array->is_record())
@@ -249,6 +257,16 @@ program_resource_visitor::visit_field(const glsl_struct_field *field)
    /* empty */
 }
 
+void
+program_resource_visitor::enter_record(const glsl_type *, const char *, bool)
+{
+}
+
+void
+program_resource_visitor::leave_record(const glsl_type *, const char *, bool)
+{
+}
+
 namespace {
 
 /**
@@ -526,6 +544,18 @@ private:
       assert(!"Should not get here.");
    }
 
+   virtual void enter_record(const glsl_type *type, const char *name,
+                             bool row_major) {
+      this->ubo_byte_offset = glsl_align(
+            this->ubo_byte_offset, type->std140_base_alignment(row_major));
+   }
+
+   virtual void leave_record(const glsl_type *type, const char *name,
+                             bool row_major) {
+      this->ubo_byte_offset = glsl_align(
+            this->ubo_byte_offset, type->std140_base_alignment(row_major));
+   }
+
    virtual void visit_field(const glsl_type *type, const char *name,
                             bool row_major, const glsl_type *record_type,
                             bool last_field)
@@ -590,16 +620,11 @@ private:
       if (this->ubo_block_index != -1) {
         this->uniforms[id].block_index = this->ubo_block_index;
 
-        const unsigned alignment = record_type
-           ? record_type->std140_base_alignment(row_major)
-           : type->std140_base_alignment(row_major);
+        const unsigned alignment = type->std140_base_alignment(row_major);
         this->ubo_byte_offset = glsl_align(this->ubo_byte_offset, alignment);
         this->uniforms[id].offset = this->ubo_byte_offset;
         this->ubo_byte_offset += type->std140_size(row_major);
 
-         if (last_field)
-            this->ubo_byte_offset = glsl_align(this->ubo_byte_offset, 16);
-
         if (type->is_array()) {
            this->uniforms[id].array_stride =
               glsl_align(type->fields.array->std140_size(row_major), 16);

diff --git a/src/glsl/linker.h b/src/glsl/linker.h
index 6ee58589835467f43d6216c57a073fe01b4059b1..be4da5e0aa2584209a001b8f231dba06236e36a2 100644 (file)
--- a/src/glsl/linker.h
+++ b/src/glsl/linker.h
@@ -170,6 +170,12 @@ protected:
     */
    virtual void visit_field(const glsl_struct_field *field);
 
+   virtual void enter_record(const glsl_type *type, const char *name,
+                             bool row_major);
+
+   virtual void leave_record(const glsl_type *type, const char *name,
+                             bool row_major);
+
 private:
    /**
     * \param name_length  Length of the current name \b not including the