On Fri, Apr 18, 2014 at 3:52 PM, Ian Romanick <i...@freedesktop.org> wrote: > On 03/31/2014 02:00 PM, Anuj Phogat wrote: >> In OpenGL 3.1 attribute 0 becomes non-magic, just like in >> OpenGL ES 2.0. Earlier versions of OpenGL used attribute 0 >> exclusively for vertex position. >> >> Fixes 4 Khronos OpenGL CTS failures: >> glGetVertexAttrib >> depth24_basic >> depth24_precision >> rgb8_rgba8_rgb > > The functions that are changed in this patch aren't used very often, so > it doesn't surprise me that they were wrong. > > Are we correctly handling attribute 0 in, say, VertexAttribPointer? No special handling exists for attrib zero in VertexAttribPointer(). There is a comment above _mesa_VertexAttribPointer() function stating: /** * Set a generic vertex attribute array. * Note that these arrays DO NOT alias the conventional GL vertex arrays * (position, normal, color, fog, texcoord, etc). */
This comment was originally added in a commit by Brian: http://lists.freedesktop.org/archives/mesa-commit/2009-May/009236.html Page 27 of OpenGL 3.0 spec describes the VertexAttrib*() functions: "Setting generic vertex attribute zero specifies a vertex; the four vertex coordinates are taken from the values of attribute zero. A Vertex2, Vertex3, or Vertex4 command is completely equivalent to the corresponding VertexAttrib* command with an index of zero. Setting any other generic vertex attribute updates the current values of the attribute. There are no current values for vertex attribute zero. There is no aliasing among generic attributes and conventional attributes. In other words, an application can set all MAX_VERTEX_ATTRIBS generic attributes and all conventional attributes without fear of one particular attribute overwriting the value of another attribute." No special handling for attribute zero is mentioned in description of VertexAttribPointer(). > >> Cc: <mesa-sta...@lists.freedesktop.org> >> Signed-off-by: Anuj Phogat <anuj.pho...@gmail.com> >> --- >> src/mesa/vbo/vbo_attrib_tmp.h | 50 >> ++++++++++++++++++++++++------------------- >> 1 file changed, 28 insertions(+), 22 deletions(-) >> >> diff --git a/src/mesa/vbo/vbo_attrib_tmp.h b/src/mesa/vbo/vbo_attrib_tmp.h >> index a2ea6d0..2865431 100644 >> --- a/src/mesa/vbo/vbo_attrib_tmp.h >> +++ b/src/mesa/vbo/vbo_attrib_tmp.h >> @@ -494,12 +494,18 @@ TAG(MultiTexCoord4fv)(GLenum target, const GLfloat * v) >> } >> >> >> +/* In OpenGL 3.1 attribute 0 becomes non-magic, just like in OpenGL ES 2.0. >> */ >> +static inline bool >> +attr_zero_specifies_vertex(struct gl_context *ctx) { > > I think I'd replace 'specifies' with 'aliases'. > yes, that sounds better. Will make the change. >> + return (ctx->API != API_OPENGLES2 >> + && (ctx->API != API_OPENGL_CORE || ctx->Version < 31)); > > This is also easier to understand if written as a positive condition > instead of a negative condition. > > return ctx->API == API_OPENGLES > || (ctx->API == API_OPENGL_COMPAT && ctx->Version < 31); > ok. will make the change. > I'm not 100% we need the ctx->Version check since Mesa doesn't support > compatibility extensions / profiles. > I lifted the condition as it is from get_current_attrib() function in varray.c. A comment in the function clarifies the usage of ctx->Version: /* In OpenGL 3.1 attribute 0 becomes non-magic, just like in OpenGL ES * 2.0. Note that we cannot just check for API_OPENGL_CORE here because * that will erroneously allow this usage in a 3.0 forward-compatible * context too. */ To avoid confusion, I'll create a utility function _mesa_is_attr_zero_aliases_vertex() in varray.h incuding the above comment. It can then be utilized at different places in the driver. >> +} >> >> static void GLAPIENTRY >> TAG(VertexAttrib1fARB)(GLuint index, GLfloat x) >> { >> GET_CURRENT_CONTEXT(ctx); >> - if (index == 0) >> + if (index == 0 && attr_zero_specifies_vertex(ctx)) >> ATTR1F(0, x); >> else if (index < MAX_VERTEX_GENERIC_ATTRIBS) >> ATTR1F(VBO_ATTRIB_GENERIC0 + index, x); >> @@ -511,7 +517,7 @@ static void GLAPIENTRY >> TAG(VertexAttrib1fvARB)(GLuint index, const GLfloat * v) >> { >> GET_CURRENT_CONTEXT(ctx); >> - if (index == 0) >> + if (index == 0 && attr_zero_specifies_vertex(ctx)) >> ATTR1FV(0, v); >> else if (index < MAX_VERTEX_GENERIC_ATTRIBS) >> ATTR1FV(VBO_ATTRIB_GENERIC0 + index, v); >> @@ -523,7 +529,7 @@ static void GLAPIENTRY >> TAG(VertexAttrib2fARB)(GLuint index, GLfloat x, GLfloat y) >> { >> GET_CURRENT_CONTEXT(ctx); >> - if (index == 0) >> + if (index == 0 && attr_zero_specifies_vertex(ctx)) >> ATTR2F(0, x, y); >> else if (index < MAX_VERTEX_GENERIC_ATTRIBS) >> ATTR2F(VBO_ATTRIB_GENERIC0 + index, x, y); >> @@ -535,7 +541,7 @@ static void GLAPIENTRY >> TAG(VertexAttrib2fvARB)(GLuint index, const GLfloat * v) >> { >> GET_CURRENT_CONTEXT(ctx); >> - if (index == 0) >> + if (index == 0 && attr_zero_specifies_vertex(ctx)) >> ATTR2FV(0, v); >> else if (index < MAX_VERTEX_GENERIC_ATTRIBS) >> ATTR2FV(VBO_ATTRIB_GENERIC0 + index, v); >> @@ -547,7 +553,7 @@ static void GLAPIENTRY >> TAG(VertexAttrib3fARB)(GLuint index, GLfloat x, GLfloat y, GLfloat z) >> { >> GET_CURRENT_CONTEXT(ctx); >> - if (index == 0) >> + if (index == 0 && attr_zero_specifies_vertex(ctx)) >> ATTR3F(0, x, y, z); >> else if (index < MAX_VERTEX_GENERIC_ATTRIBS) >> ATTR3F(VBO_ATTRIB_GENERIC0 + index, x, y, z); >> @@ -559,7 +565,7 @@ static void GLAPIENTRY >> TAG(VertexAttrib3fvARB)(GLuint index, const GLfloat * v) >> { >> GET_CURRENT_CONTEXT(ctx); >> - if (index == 0) >> + if (index == 0 && attr_zero_specifies_vertex(ctx)) >> ATTR3FV(0, v); >> else if (index < MAX_VERTEX_GENERIC_ATTRIBS) >> ATTR3FV(VBO_ATTRIB_GENERIC0 + index, v); >> @@ -571,7 +577,7 @@ static void GLAPIENTRY >> TAG(VertexAttrib4fARB)(GLuint index, GLfloat x, GLfloat y, GLfloat z, >> GLfloat w) >> { >> GET_CURRENT_CONTEXT(ctx); >> - if (index == 0) >> + if (index == 0 && attr_zero_specifies_vertex(ctx)) >> ATTR4F(0, x, y, z, w); >> else if (index < MAX_VERTEX_GENERIC_ATTRIBS) >> ATTR4F(VBO_ATTRIB_GENERIC0 + index, x, y, z, w); >> @@ -583,7 +589,7 @@ static void GLAPIENTRY >> TAG(VertexAttrib4fvARB)(GLuint index, const GLfloat * v) >> { >> GET_CURRENT_CONTEXT(ctx); >> - if (index == 0) >> + if (index == 0 && attr_zero_specifies_vertex(ctx)) >> ATTR4FV(0, v); >> else if (index < MAX_VERTEX_GENERIC_ATTRIBS) >> ATTR4FV(VBO_ATTRIB_GENERIC0 + index, v); >> @@ -600,7 +606,7 @@ static void GLAPIENTRY >> TAG(VertexAttribI1i)(GLuint index, GLint x) >> { >> GET_CURRENT_CONTEXT(ctx); >> - if (index == 0) >> + if (index == 0 && attr_zero_specifies_vertex(ctx)) >> ATTR1I(0, x); >> else if (index < MAX_VERTEX_GENERIC_ATTRIBS) >> ATTR1I(VBO_ATTRIB_GENERIC0 + index, x); >> @@ -612,7 +618,7 @@ static void GLAPIENTRY >> TAG(VertexAttribI2i)(GLuint index, GLint x, GLint y) >> { >> GET_CURRENT_CONTEXT(ctx); >> - if (index == 0) >> + if (index == 0 && attr_zero_specifies_vertex(ctx)) >> ATTR2I(0, x, y); >> else if (index < MAX_VERTEX_GENERIC_ATTRIBS) >> ATTR2I(VBO_ATTRIB_GENERIC0 + index, x, y); >> @@ -624,7 +630,7 @@ static void GLAPIENTRY >> TAG(VertexAttribI3i)(GLuint index, GLint x, GLint y, GLint z) >> { >> GET_CURRENT_CONTEXT(ctx); >> - if (index == 0) >> + if (index == 0 && attr_zero_specifies_vertex(ctx)) >> ATTR3I(0, x, y, z); >> else if (index < MAX_VERTEX_GENERIC_ATTRIBS) >> ATTR3I(VBO_ATTRIB_GENERIC0 + index, x, y, z); >> @@ -636,7 +642,7 @@ static void GLAPIENTRY >> TAG(VertexAttribI4i)(GLuint index, GLint x, GLint y, GLint z, GLint w) >> { >> GET_CURRENT_CONTEXT(ctx); >> - if (index == 0) >> + if (index == 0 && attr_zero_specifies_vertex(ctx)) >> ATTR4I(0, x, y, z, w); >> else if (index < MAX_VERTEX_GENERIC_ATTRIBS) >> ATTR4I(VBO_ATTRIB_GENERIC0 + index, x, y, z, w); >> @@ -648,7 +654,7 @@ static void GLAPIENTRY >> TAG(VertexAttribI2iv)(GLuint index, const GLint *v) >> { >> GET_CURRENT_CONTEXT(ctx); >> - if (index == 0) >> + if (index == 0 && attr_zero_specifies_vertex(ctx)) >> ATTR2IV(0, v); >> else if (index < MAX_VERTEX_GENERIC_ATTRIBS) >> ATTR2IV(VBO_ATTRIB_GENERIC0 + index, v); >> @@ -660,7 +666,7 @@ static void GLAPIENTRY >> TAG(VertexAttribI3iv)(GLuint index, const GLint *v) >> { >> GET_CURRENT_CONTEXT(ctx); >> - if (index == 0) >> + if (index == 0 && attr_zero_specifies_vertex(ctx)) >> ATTR3IV(0, v); >> else if (index < MAX_VERTEX_GENERIC_ATTRIBS) >> ATTR3IV(VBO_ATTRIB_GENERIC0 + index, v); >> @@ -672,7 +678,7 @@ static void GLAPIENTRY >> TAG(VertexAttribI4iv)(GLuint index, const GLint *v) >> { >> GET_CURRENT_CONTEXT(ctx); >> - if (index == 0) >> + if (index == 0 && attr_zero_specifies_vertex(ctx)) >> ATTR4IV(0, v); >> else if (index < MAX_VERTEX_GENERIC_ATTRIBS) >> ATTR4IV(VBO_ATTRIB_GENERIC0 + index, v); >> @@ -689,7 +695,7 @@ static void GLAPIENTRY >> TAG(VertexAttribI1ui)(GLuint index, GLuint x) >> { >> GET_CURRENT_CONTEXT(ctx); >> - if (index == 0) >> + if (index == 0 && attr_zero_specifies_vertex(ctx)) >> ATTR1UI(0, x); >> else if (index < MAX_VERTEX_GENERIC_ATTRIBS) >> ATTR1UI(VBO_ATTRIB_GENERIC0 + index, x); >> @@ -701,7 +707,7 @@ static void GLAPIENTRY >> TAG(VertexAttribI2ui)(GLuint index, GLuint x, GLuint y) >> { >> GET_CURRENT_CONTEXT(ctx); >> - if (index == 0) >> + if (index == 0 && attr_zero_specifies_vertex(ctx)) >> ATTR2UI(0, x, y); >> else if (index < MAX_VERTEX_GENERIC_ATTRIBS) >> ATTR2UI(VBO_ATTRIB_GENERIC0 + index, x, y); >> @@ -713,7 +719,7 @@ static void GLAPIENTRY >> TAG(VertexAttribI3ui)(GLuint index, GLuint x, GLuint y, GLuint z) >> { >> GET_CURRENT_CONTEXT(ctx); >> - if (index == 0) >> + if (index == 0 && attr_zero_specifies_vertex(ctx)) >> ATTR3UI(0, x, y, z); >> else if (index < MAX_VERTEX_GENERIC_ATTRIBS) >> ATTR3UI(VBO_ATTRIB_GENERIC0 + index, x, y, z); >> @@ -725,7 +731,7 @@ static void GLAPIENTRY >> TAG(VertexAttribI4ui)(GLuint index, GLuint x, GLuint y, GLuint z, GLuint w) >> { >> GET_CURRENT_CONTEXT(ctx); >> - if (index == 0) >> + if (index == 0 && attr_zero_specifies_vertex(ctx)) >> ATTR4UI(0, x, y, z, w); >> else if (index < MAX_VERTEX_GENERIC_ATTRIBS) >> ATTR4UI(VBO_ATTRIB_GENERIC0 + index, x, y, z, w); >> @@ -737,7 +743,7 @@ static void GLAPIENTRY >> TAG(VertexAttribI2uiv)(GLuint index, const GLuint *v) >> { >> GET_CURRENT_CONTEXT(ctx); >> - if (index == 0) >> + if (index == 0 && attr_zero_specifies_vertex(ctx)) >> ATTR2UIV(0, v); >> else if (index < MAX_VERTEX_GENERIC_ATTRIBS) >> ATTR2UIV(VBO_ATTRIB_GENERIC0 + index, v); >> @@ -749,7 +755,7 @@ static void GLAPIENTRY >> TAG(VertexAttribI3uiv)(GLuint index, const GLuint *v) >> { >> GET_CURRENT_CONTEXT(ctx); >> - if (index == 0) >> + if (index == 0 && attr_zero_specifies_vertex(ctx)) >> ATTR3UIV(0, v); >> else if (index < MAX_VERTEX_GENERIC_ATTRIBS) >> ATTR3UIV(VBO_ATTRIB_GENERIC0 + index, v); >> @@ -761,7 +767,7 @@ static void GLAPIENTRY >> TAG(VertexAttribI4uiv)(GLuint index, const GLuint *v) >> { >> GET_CURRENT_CONTEXT(ctx); >> - if (index == 0) >> + if (index == 0 && attr_zero_specifies_vertex(ctx)) >> ATTR4UIV(0, v); >> else if (index < MAX_VERTEX_GENERIC_ATTRIBS) >> ATTR4UIV(VBO_ATTRIB_GENERIC0 + index, v); >> > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
