[Mesa-dev] vertex array regression

[Brian Paul]

2011/12/15 Mathias Fröhlich <Mathias.Froehlich at web.de>:
>
> Brian,
>
> On Thursday, December 15, 2011 17:08:57 Brian Paul wrote:
>> There's a regression in vertex array drawing with this commit:
>>
>> commit ed42c2580717527b2005580940fc766d95bb6b0b
>> Author: Mathias Fröhlich <Mathias.Froehlich at web.de>
>> Date:   Mon Oct 31 16:23:40 2011 +0100
>>
>>      vbo: Use The VERT_{ATTRIB,BIT} defines.
>>
>>      Signed-off-by: Mathias Froehlich <Mathias.Froehlich at web.de>
>>      Reviewed-by: Brian Paul <brianp at vmware.com>
>>      Reviewed-by: Eric Anholt <eric at anholt.net>
>>
>>
>> To see the problem, run mesa/demos/src/demos/isosurf, choose
>> "glDrawArrays" or "glDrawElements" from the pop-up menu (right mouse
>> button).  I see the problem (random/missing vertices or failed
>> assertion) with all gallium drivers.  The swrast/i965 drivers seem
>> uneffected.
>>
>> I'll try to debug it further, but maybe you could double-check your work.
>
> I will look into that. Probably not today but latest at the weekend.

I found the problem.  It's this chunk in vbo_context.c:

@@ -182,14 +177,15 @@ GLboolean _vbo_CreateContext( struct gl_context *ctx )
       GLuint i;

       /* When no vertex program, pull in the material attributes in
-       * the 16..32 generic range.
+       * the generic range.
        */
-      for (i = 0; i < 16; i++)
+      for (i = 0; i < VERT_ATTRIB_FF_MAX; i++)
         vbo->map_vp_none[i] = i;
-      for (i = 0; i < 12; i++)
-        vbo->map_vp_none[16+i] = VBO_ATTRIB_MAT_FRONT_AMBIENT + i;
-      for (i = 0; i < 4; i++)
-        vbo->map_vp_none[28+i] = i;
+      for (i = 0; i < NR_MAT_ATTRIBS; i++)
+        vbo->map_vp_none[VERT_ATTRIB_GENERIC(i)]
+            = VBO_ATTRIB_MAT_FRONT_AMBIENT + i;
+      for (i = NR_MAT_ATTRIBS; i < VERT_ATTRIB_GENERIC_MAX; i++)
+        vbo->map_vp_none[VERT_ATTRIB_GENERIC(i)] = i;

       for (i = 0; i < Elements(vbo->map_vp_arb); i++)
         vbo->map_vp_arb[i] = i;

Or more precisely:

-      for (i = 0; i < 4; i++)
-        vbo->map_vp_none[28+i] = i;
+      for (i = NR_MAT_ATTRIBS; i < VERT_ATTRIB_GENERIC_MAX; i++)
+        vbo->map_vp_none[VERT_ATTRIB_GENERIC(i)] = i;

This change resulted in a different mapping in the map_vp_none[] array.

The original code produced:
vbo->map_vp_none = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14,
15, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 0, 1, 2, 3}

The new code produces:
vbo->map_vp_none = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14,
15, 16, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 12, 13, 14,
15}

The last four/five elements are different.  Though, I don't really
know what their purpose is.  It seems to me that they could/should
just be identity entries, ex: map_vp_none[i]=i as is the case for the
map_vp_arb[] array.

And I found that simply removing those lines fixes the problem:

@@ -184,8 +184,6 @@ GLboolean _vbo_CreateContext( struct gl_context *ctx )
       for (i = 0; i < NR_MAT_ATTRIBS; i++)
         vbo->map_vp_none[VERT_ATTRIB_GENERIC(i)]
             = VBO_ATTRIB_MAT_FRONT_AMBIENT + i;
-      for (i = NR_MAT_ATTRIBS; i < VERT_ATTRIB_GENERIC_MAX; i++)
-        vbo->map_vp_none[VERT_ATTRIB_GENERIC(i)] = i;

       for (i = 0; i < Elements(vbo->map_vp_arb); i++)
         vbo->map_vp_arb[i] = i;

For fixed function, the point is to simply place the per-vertex
material attributes in the generic attribute arrays.  There are 12
such material attributes.  So there's four slots left over.

-Brian