This is the only pending patch from this series, would appreciate a review. As everything involving the VertexIndex/Id, it is somewhat tricky. I'm CCing Kenneth too as he was involved on the OpenGL mess with gl_VertexId and similar.
FWIW, I already executed the Intel CI, and there are no regressions. On 09/08/18 15:43, Alejandro Piñeiro wrote: > From: Neil Roberts <nrobe...@igalia.com> > > 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. I can’t find > any mention of what VertexId is supposed to be. > > GL_ARB_spirv removes VertexIndex and defines VertexId to be the same > as gl_VertexId (which is alo 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. > --- > src/compiler/spirv/vtn_variables.c | 15 ++++++++------- > 1 file changed, 8 insertions(+), 7 deletions(-) > > diff --git a/src/compiler/spirv/vtn_variables.c > b/src/compiler/spirv/vtn_variables.c > index 8dab86abd74..b92bda59828 100644 > --- a/src/compiler/spirv/vtn_variables.c > +++ b/src/compiler/spirv/vtn_variables.c > @@ -1011,15 +1011,16 @@ 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: > + /* For whatever reason, both of these are defined in the SPIR-V spec. > + * The Vulkan spec defines VertexIndex to be non-zero-based and doesn’t > + * mention VertexId. The ARB_gl_spirv spec defines VertexId to be the > + * same as gl_VertexID, which is non-zero-based, and removes > + * VertexIndex. Assuming there is no use for VertexId in Vulkan yet, we > + * can just make them both be the same. > */ > - *location = SYSTEM_VALUE_VERTEX_ID_ZERO_BASE; > + *location = SYSTEM_VALUE_VERTEX_ID; > set_mode_system_value(b, mode); > break; > case SpvBuiltInInstanceIndex: _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
