[Mesa-dev] [PATCH v5 08/29] glsl: fix uniform linking logic in the presence of structs

Ilia Mirkin imirkin at alum.mit.edu
Tue Feb 10 03:58:48 PST 2015


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 at alum.mit.edu>
---
I kept this separate from the other changes since it seemed self-contained.
I can fold somewhere else if desired.

 src/glsl/link_uniform_blocks.cpp | 45 ++++++++++++++++++----------------------
 src/glsl/link_uniforms.cpp       | 37 +++++++++++++++++++++++++++------
 src/glsl/linker.h                |  6 ++++++
 3 files changed, 57 insertions(+), 31 deletions(-)

diff --git a/src/glsl/link_uniform_blocks.cpp b/src/glsl/link_uniform_blocks.cpp
index f5fc502..1c175ec 100644
--- 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 3aa6e0a..347f079 100644
--- 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 6ee5858..be4da5e 100644
--- 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
-- 
2.0.5



More information about the mesa-dev mailing list