From d9bd3b1cb874c44e4250c28605031ddf1c520cd1 Mon Sep 17 00:00:00 2001 From: Niklas Haas Date: Sun, 16 Dec 2018 05:40:14 +0100 Subject: [PATCH] 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 Reviewed-by: Ilia Mirkin --- src/compiler/glsl/ast_to_hir.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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: -- 2.30.2