[Mesa-dev] [PATCH] vbo: Set FLUSH_UPDATE_CURRENT when setting vertex attibutes

[Kristian Høgsberg]

2010/9/13 keith whitwell <keith.whitwell at gmail.com>:
> Hey Kristian,
>
> The first question is whether this is necessary - from vague memory I
> have an idea that current attributes need not be updated by vertex
> buffer rendering - ie. it's optional/implementation-dependent.
>
> I assume you're concerned with the case where you have something like
>
>   // ctx->Current.Color is xyz
>
>   glDrawArrays();
>
>   // has ctx->Current.Color been updated??
>
> But assuming I'm wrong about that & we really do want to make
> DrawArrays set the current values, the patch looks good...

No, what I'm seeing is that the code in question sets three generic
vertex attributes and then calls glDrawArrays().  The value of the
last attribute is not propagates into the shader.

The problem is that the vertex array code keeps the values in
exec->vtx.vertex, but the implementation of glDrawArrays looks in
ctx->Current (that's what I assume, I didn't track that down).  When
the code hits a case where the size of an attribute is smaller that
what we're trying to set, it recomputes the layout of the
exec->vtx.vertex values and as a side effect copies the
exec->vtx.vertex values to ctx->Current.  Since we start out with
attrsz == 0 for all attributes, each new attribute will trigger this
recomputation and thus effectively flushes all previous values to
ctx->Current.  Which is why all but the last attribute make it to the
shader.

Note that the ATTR macro is defined differently, depending on
FEATURE_beginend - the !FEATURE_beginend case sets the
FLUSH_UPDATE_CURRENT flag too.  I don't know why we wouldn't also set
it in the FEATURE_beginend case, not using begin/end in that case is
still an option.

Kristian


> 2010/9/13 Kristian Høgsberg <krh at bitplanet.net>:
>> Setting constant vertex attributes with glDrawArrays() doesn't work right
>> because the last attribute isn't copied to ctx->Current.  Typically,
>> only the last attribute doesn't get set, since vbo_exec_wrap_upgrade_vertex()
>> ends up getting called when setting a new attribute, and it will copy all
>> previously set attributes to Current.
>> ---
>>  src/mesa/vbo/vbo_exec_api.c |    3 +++
>>  1 files changed, 3 insertions(+), 0 deletions(-)
>>
>> I'm not too familiar with this code, so I'd appreciate if somebody who
>> knows the vbo code better could take a quick look.
>>
>> Kristian
>>
>> diff --git a/src/mesa/vbo/vbo_exec_api.c b/src/mesa/vbo/vbo_exec_api.c
>> index 9df75a8..90c3dd4 100644
>> --- a/src/mesa/vbo/vbo_exec_api.c
>> +++ b/src/mesa/vbo/vbo_exec_api.c
>> @@ -359,6 +359,9 @@ static void vbo_exec_fixup_vertex( GLcontext *ctx,
>>  do {                                                           \
>>    struct vbo_exec_context *exec = &vbo_context(ctx)->exec;    \
>>                                                                \
>> +   /* FLUSH_UPDATE_CURRENT needs to be set manually */         \
>> +   exec->ctx->Driver.NeedFlush |= FLUSH_UPDATE_CURRENT;                \
>> +                                                               \
>>    if (exec->vtx.active_sz[A] != N)                            \
>>       vbo_exec_fixup_vertex(ctx, A, N);                        \
>>                                                                \
>> --
>> 1.7.2.1
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>