[Mesa-dev] vertex array regression

[Brian Paul]

On 12/19/2011 01:12 PM, Mathias Fröhlich wrote:
>
> Brian,
>
> On Monday, December 19, 2011 17:01:07 you wrote:
>> I still don't get what the purpose of the special mapping of the last
>> four/five elements is all about.  Can you explain that?
>
> As far as I can see the last ones should not map to anything.
> The map is used only in fixed function mode. And there these arrays should not
> show up as inputs to any fixed function shader. I think that the purpose of the
> previous mapping was just to fill in anything. The changes I did serve exactly
> this purpose.
>
> But I agree that here should not be any mapping of these attributes in this
> mode. Attached is a small change that is probably a more clean solution to
> this mapping problem.
>
> The change passes a quick r600g piglit test.
>
>> Yeah, I see how the incorrect varying_inputs values was causing
>> _NEW_ARRAY to get set, thus hiding the problem.  The solution looks
>> correct, but I'm a little worried about performance, as is mentioned
>> in the comment.
>>
>> Marek, made that change back in March.  Maybe he can take a look at
>> this too.
> I cannot comment on this, since I do not see a performance regression on my
> use cases.
> Marek?

I dug a bit deeper into this.  When the isosurface corruption is 
visible it's basically because the glVertex and glNormal values are 
transposed.

With glBegin/End we're sending interleaved (normal3f, vertex3f) 
values.  But with glDrawArrays we're sending interleaved (vertex3f, 
normal3f) values.  If we don't signal _NEW_ARRAY when switching from 
one method to the other, the state tracker is skipping array 
validation and we're drawing with normals and positions transposed.

The glBegin/End code is building vertex arrays but never directly sets 
_NEW_ARRAY as a side-effect.

I've implemented a solution to this that simply sets _NEW_ARRAY 
whenever we transition between glBegin/End and glDrawArrays drawing.

This fixes the isosurf bug and removes all occurances of NewState |= 
_NEW_ARRAY in the recalculate_input_bindings() function to avoid a 
performance regression.

I think this fixes the regression while preserving Marek's performance 
fix.

I'll post a patch.

-Brian