From be6f472b2387f508e55ffbe758da8cbea75717d6 Mon Sep 17 00:00:00 2001 From: Neil Roberts Date: Thu, 19 Apr 2018 17:17:41 +0200 Subject: [PATCH] spirv: Make VertexIndex and VertexId both non-zero-based MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit GLSL has gl_VertexID which is supposed to be non-zero-based. SPIR-V has both VertexIndex and VertexId builtins whose meanings are defined by the APIs. Vulkan defines VertexIndex as being non-zero-based. In Vulkan VertexId and InstanceId have no meaning and are pretty much just reserved for OpenGL at this point. GL_ARB_spirv removes VertexIndex and defines VertexId to be the same as gl_VertexId (which is also non-zero-based). Previously in Mesa it was treating VertexIndex as non-zero-based and VertexId as zero-based, so it was breaking for GL. This behaviour was apparently based on Khronos bug 14255. However that bug doesn’t seem to have made a final decision for VertexId. Assuming there really is no other definition for VertexId for Vulkan it seems better to just make them both have the same value. v2: update comment and commit descriptions, based on Jason Ekstrand explanation of the meaning/rationale behind all those builtins (Jason) Reviewed-by: Jason Ekstrand --- src/compiler/spirv/vtn_variables.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/compiler/spirv/vtn_variables.c b/src/compiler/spirv/vtn_variables.c index 8dab86a..571a14c 100644 --- a/src/compiler/spirv/vtn_variables.c +++ b/src/compiler/spirv/vtn_variables.c @@ -1011,15 +1011,15 @@ vtn_get_builtin_location(struct vtn_builder *b, case SpvBuiltInCullDistance: *location = VARYING_SLOT_CULL_DIST0; break; - case SpvBuiltInVertexIndex: - *location = SYSTEM_VALUE_VERTEX_ID; - set_mode_system_value(b, mode); - break; case SpvBuiltInVertexId: - /* Vulkan defines VertexID to be zero-based and reserves the new - * builtin keyword VertexIndex to indicate the non-zero-based value. + case SpvBuiltInVertexIndex: + /* The Vulkan spec defines VertexIndex to be non-zero-based and doesn't + * allow VertexId. The ARB_gl_spirv spec defines VertexId to be the + * same as gl_VertexID, which is non-zero-based, and removes + * VertexIndex. Since they're both defined to be non-zero-based, we use + * SYSTEM_VALUE_VERTEX_ID for both. */ - *location = SYSTEM_VALUE_VERTEX_ID_ZERO_BASE; + *location = SYSTEM_VALUE_VERTEX_ID; set_mode_system_value(b, mode); break; case SpvBuiltInInstanceIndex: -- 2.7.4