[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-stable
mailing list