[Mesa-dev] vertex array regression

[Brian Paul]

On 12/16/2011 11:28 AM, Mathias Fröhlich wrote:
>
> Brian,
>
> On Thursday, December 15, 2011 22:36:24 you wrote:
>> I found the problem.  It's this chunk in vbo_context.c:
> [...]
>> 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.
>
> Yep, and these map with the old scheme to often used attributes like the
> vertex position.

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?


>
> Thanks to your hints I believe that I found the underlying problem:
>
> If you look at the resulting bitmasks for the enabled vertex program inputs in
> isosurf by commenting out the printf in _mesa_set_varying_vp_inputs you will
> see surprising results with the old numbering. The current checked in
> numbering looks much more plausible.
> It turns out that this wrong varying_vp_inputs mask sets the _NEW_ARRAY bit
> through _mesa_set_varying_vp_inputs. That in turn cares for some (by now in my
> debug session untracked) state updates. This helps in the end for isosurf.
> If the varying_vp_inputs mask looks plausible the _NEW_ARRAY bit is not set
> and isosurf fails.
>
> This also explaines that only draw paths going through vbo_exec_array.c are
> affected, since the imm variants in vbo_exec_{safe,draw} always set the
> _NEW_ARRAY bit on any draw.

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.


>
> So for me this change
>
> diff --git a/src/mesa/vbo/vbo_exec_array.c b/src/mesa/vbo/vbo_exec_array.c
> index a6e41e9..a2851c4 100644
> --- a/src/mesa/vbo/vbo_exec_array.c
> +++ b/src/mesa/vbo/vbo_exec_array.c
> @@ -446,7 +446,7 @@ recalculate_input_bindings(struct gl_context *ctx)
>          * to revalidate vertex arrays. Not marking the state as dirty also
>          * improves performance (quite significantly in some apps).
>          */
> -      if (!ctx->VertexProgram._MaintainTnlProgram)
> +      /* if (!ctx->VertexProgram._MaintainTnlProgram) */
>            ctx->NewState |= _NEW_ARRAY;
>         break;
>
> makes isosurf work reliable.
>
> I will prepare a patch for that.
>
> Greetings
>
> Mathias

-Brian