From: Niklas Haas <git@haasn.xyz>
Date: Sun, 16 Dec 2018 04:40:14 +0000 (+0100)
Subject: glsl: fix block member alignment validation for vec3
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=d9bd3b1cb874c44e4250c28605031ddf1c520cd1;p=mesa.git

glsl: fix block member alignment validation for vec3

Section 7.6.2.2 (Standard Uniform Block Layout) of the GL spec says:

    The base offset of the first member of a structure is taken from the
    aligned offset of the structure itself. The base offset of all other
    structure members is derived by taking the offset of the last basic
    machine unit consumed by the previous member and adding one.

The current code does not reflect this last sentence - it effectively
instead aligns up the next offset up to the alignment of the previous
member. This causes an issue in exactly one case:

layout(std140) uniform block {
    layout(offset=0) vec3 var1;
    layout(offset=12) float var2;
};

As per section 7.6.2.1 (Uniform Buffer Object Storage) and elsewhere, a
vec3 consumes 3 floats, i.e. 12 basic machine units. Therefore, `var1`
in the example above consumes units 0-11, with 12 being the first
available offset afterwards. However, before this commit, mesa
incorrectly assumes `var2` must start at offset=16 when using explicit
offsets, which results in a compile-time error. Without explicit
offsets, the shaders actually work fine, indicating that mesa is already
correctly aligning these fields internally. (Just not in the code that
handles explicit buffer offset parsing)

This patch should fix piglit tests:
ssbo-explicit-offset-vec3.vert
ubo-explicit-offset-vec3.vert

Signed-off-by: Niklas Haas <git@haasn.xyz>
Reviewed-by: Ilia Mirkin <imirkin@alum.mit.edu>
---

diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
index 67a5a8c0509..620153e6a34 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -7397,7 +7397,7 @@ ast_process_struct_or_iface_block_members(exec_list *instructions,
                                       "alignment of %s", field_type->name);
                   }
                   fields[i].offset = qual_offset;
-                  next_offset = glsl_align(qual_offset + size, align);
+                  next_offset = qual_offset + size;
                } else {
                   _mesa_glsl_error(&loc, state, "offset can only be used "
                                    "with std430 and std140 layouts");
@@ -7421,16 +7421,16 @@ ast_process_struct_or_iface_block_members(exec_list *instructions,
                                       "is not a power of 2");
                   } else {
                      fields[i].offset = glsl_align(offset, member_align);
-                     next_offset = glsl_align(fields[i].offset + size, align);
+                     next_offset = fields[i].offset + size;
                   }
                }
             } else {
                fields[i].offset = glsl_align(offset, expl_align);
-               next_offset = glsl_align(fields[i].offset + size, align);
+               next_offset = fields[i].offset + size;
             }
          } else if (!qual->flags.q.explicit_offset) {
             if (align != 0 && size != 0)
-               next_offset = glsl_align(next_offset + size, align);
+               next_offset = glsl_align(next_offset, align) + size;
          }
 
          /* From the ARB_enhanced_layouts spec: