spirv: Make VertexIndex and VertexId both non-zero-based

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 <jason@jlekstrand.net>

diff --git a/src/compiler/spirv/vtn_variables.c b/src/compiler/spirv/vtn_variables.c
index 8dab86abd74d131f623038b7bfeb17679a0de2f9..571a14cf4cf6a06285c5c3abaa318964665cf3ef 100644 (file)
--- 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: