[Mesa-dev] [PATCH] vbo: Set FLUSH_UPDATE_CURRENT when setting vertex attibutes
krh at bitplanet.net
Tue Sep 14 07:22:56 PDT 2010
2010/9/14 Chia-I Wu <olvaffe at gmail.com>:
> 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
>>> // 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
>> 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.
And that's exactly the problem I have. Setting FLUSH_UPDATE_CURRENT
in the ATTR macro makes sure that the FLUSH_CURRENT in
vbo_exec_DrawArrays (and other array draw funcs) ends up calling
vbo_exec_copy_to_current(), which then pushes the values into
ctx->Current before the draw call. I don't see a problem with this
approach; in a begin/end, this flag is already set, so there's no
overhead, outside begin/end (ES2) it's required for correct behaviour.
More information about the mesa-dev