[Mesa-dev] [PATCH] vbo: Correctly handle attribute offsets in dlist draw.

[Erik Faye-Lund]

On Fri, Aug 19, 2016 at 10:38 AM, Mathias Fröhlich
<Mathias.Froehlich at gmx.net> wrote:
> On Friday, 19 August 2016 09:20:41 CEST Erik Faye-Lund wrote:
>> On Thu, Aug 18, 2016 at 7:50 PM,  <Mathias.Froehlich at gmx.net> wrote:
>> > From: Mathias Fröhlich <mathias.froehlich at web.de>
>> >
>> > Hi,
>> >
>> > I found the below while fixing a similar problem lately in
>> > the immediate mode glBegin/glEnd code path.
>> >
>> > Please review
>> > Thanks
>> >
>> > Mathias
>> >
>> >
>> >
>> > When executing a display list draw with a shader program
>> > using the generic0 attribute the offset computation
>> > may get out of sync. To fix precompute the offsets
>> > on the full attribute list and store the offsets in
>> > the display list node.
>> >
>> > Signed-off-by: Mathias Fröhlich <Mathias.Froehlich at web.de>
>> > ---
>> >  src/mesa/vbo/vbo_save.h      |  1 +
>> >  src/mesa/vbo/vbo_save_api.c  |  6 ++++++
>> >  src/mesa/vbo/vbo_save_draw.c | 35 +++++++++++++++++------------------
>> >  3 files changed, 24 insertions(+), 18 deletions(-)
>> >
>> > diff --git a/src/mesa/vbo/vbo_save.h b/src/mesa/vbo/vbo_save.h
>> > index 2843b3c..a61973f 100644
>> > --- a/src/mesa/vbo/vbo_save.h
>> > +++ b/src/mesa/vbo/vbo_save.h
>> > @@ -64,6 +64,7 @@ struct vbo_save_vertex_list {
>> >     GLbitfield64 enabled; /**< mask of enabled vbo arrays. */
>> >     GLubyte attrsz[VBO_ATTRIB_MAX];
>> >     GLenum attrtype[VBO_ATTRIB_MAX];
>> > +   GLushort offsets[VBO_ATTRIB_MAX];
>> >     GLuint vertex_size;  /**< size in GLfloats */
>> >
>> >     /* Copy of the final vertex from node->vertex_store->bufferobj.
>> > diff --git a/src/mesa/vbo/vbo_save_api.c b/src/mesa/vbo/vbo_save_api.c
>> > index f648ccc..4473fef 100644
>> > --- a/src/mesa/vbo/vbo_save_api.c
>> > +++ b/src/mesa/vbo/vbo_save_api.c
>> > @@ -436,6 +436,12 @@ _save_compile_vertex_list(struct gl_context *ctx)
>> >     node->vertex_size = save->vertex_size;
>> >     node->buffer_offset =
>> >        (save->buffer - save->vertex_store->buffer) * sizeof(GLfloat);
>> > +   GLushort offset = 0;
>> > +   int i;
>> > +   for (i = 0; i < VBO_ATTRIB_MAX; ++i) {
>> > +      node->offsets[i] = offset;
>> > +      offset += node->attrsz[i] * sizeof(GLfloat);
>> > +   }
>>
>> I'm not sure what the policy regarding C99 is ATM, but this seems to
>> introduce declaration-after-statement for the first time in this
>> source.
>
> Someone requested lately in a review that I should declare variables where I
> use them the first time. To my following question, if this is now a treewide
> policy, I got no answer. IIRC the file in question was some mesa core file.
> I can easily change my personal coding back to 'adapt to the surronding code
> style' what I usually do for patches in any other project I contribute to and
> what I did on mesa before I got the request to start moving to somewhere else.
> Any official policy?

AFAIK, there used to be some parts of mesa that had to be compiled
with an older MSVC version without support for such C99-isms in
C-sources. But I *think* that requirement has been lifted still.

Personally, I'd just stay with the local style in the source, but I'm
no authority on this. But whatever, if nobody else reacts to this,
just keep it as-is, it's probably fine. And if it's not, it can be
fixed up after-the-fact.