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

[Chia-I Wu]

2010/9/14 Kristian Høgsberg <krh at bitplanet.net>:
> 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.
The way glColor4f is dispatched depends on whether it is GL or ES:

  GL (with FEATURE_beginend): glColor4f -> neutral_Color4f -> vbo_Color4f
  ES (w/o  FEATURE_beginend): glColor4f -> _es_Color4f -> vbo_Color4f

In the former case, FLUSH_UPDATE_CURRENT should have been set by
vbo_exec_BeginVertices which is called by neutral_Color4f.  In the latter case,
the flag must be set in vbo_Color4f.  Could it be a bug some where in vtxfmt?

One issue I noticed just last week is that the current scheme does not take
into account "ES with FEATURE_beginend".  This happens with --enable-gles2
build.  Since FEATURE_beginend is enabled in such build, vbo_Color4f does not
set FLUSH_UPDATE_CURRENT.  Yet, an ES context does not use neutral_Color4f.
>
> 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
>>>
>>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>



-- 
olv at LunarG.com