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

[Brian Paul]

On 08/19/2016 02:52 AM, Erik Faye-Lund wrote:
> 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.

Now that we require MSVC 2013, declarations after code are fine.  We 
should feel free to put declarations where they make the most sense.

-Brian