[Mesa-dev] [Mesa-stable] [PATCH] mesa: Use location VERT_ATTRIB_GENERIC0 for vertex attribute 0

Anuj Phogat anuj.phogat at gmail.com
Mon Apr 21 16:54:36 PDT 2014


On Fri, Apr 18, 2014 at 3:52 PM, Ian Romanick <idr at 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-stable at lists.freedesktop.org>
>> Signed-off-by: Anuj Phogat <anuj.phogat at 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);
>>
>


More information about the mesa-dev mailing list