[Mesa-dev] [PATCH] u_vbuf_mgr: restore buffer offsets

[Marek Olšák]

On Thu, Jul 21, 2011 at 9:48 AM, Chia-I Wu <olvaffe at gmail.com> wrote:
> On Wed, Jul 20, 2011 at 11:32 PM, Marek Olšák <maraeo at gmail.com> wrote:
>> Well spotted, thanks. Some comments below.
>>
>> On Wed, Jul 20, 2011 at 3:25 PM, Chia-I Wu <olvaffe at gmail.com> wrote:
>>> From: Chia-I Wu <olv at lunarg.com>
>>>
>>> u_vbuf_upload_buffers modifies the buffer offsets.  If they are not
>>> restored, and any of the vertex formats is not supported natively, the
>>> next u_vbuf_mgr_draw_begin call will translate the vertex buffers with
>>> incorrect buffer offsets.
>>> ---
>>>  src/gallium/auxiliary/util/u_vbuf_mgr.c |   14 ++++++++++++++
>>>  1 files changed, 14 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/src/gallium/auxiliary/util/u_vbuf_mgr.c b/src/gallium/auxiliary/util/u_vbuf_mgr.c
>>> index 374fc33..47e7027 100644
>>> --- a/src/gallium/auxiliary/util/u_vbuf_mgr.c
>>> +++ b/src/gallium/auxiliary/util/u_vbuf_mgr.c
>>> @@ -79,6 +79,8 @@ struct u_vbuf_mgr_priv {
>>>    void *saved_ve, *fallback_ve;
>>>    boolean ve_binding_lock;
>>>
>>> +   unsigned saved_buffer_offset[PIPE_MAX_ATTRIBS];
>>> +
>>>    boolean any_user_vbs;
>>>    boolean incompatible_vb_layout;
>>>  };
>>> @@ -488,6 +490,7 @@ void u_vbuf_mgr_set_vertex_buffers(struct u_vbuf_mgr *mgrb,
>>>
>>>       pipe_resource_reference(&mgr->b.vertex_buffer[i].buffer, vb->buffer);
>>>       pipe_resource_reference(&mgr->b.real_vertex_buffer[i], NULL);
>>> +      mgr->saved_buffer_offset[i] = vb->buffer_offset;
>>>
>>>       if (!vb->buffer) {
>>>          continue;
>>> @@ -647,6 +650,17 @@ u_vbuf_mgr_draw_begin(struct u_vbuf_mgr *mgrb,
>>>  void u_vbuf_mgr_draw_end(struct u_vbuf_mgr *mgrb)
>>>  {
>>>    struct u_vbuf_mgr_priv *mgr = (struct u_vbuf_mgr_priv*)mgrb;
>>> +   unsigned i;
>>> +
>>> +   /* buffer offsets were modified in u_vbuf_upload_buffers */
>>> +   if (mgr->any_user_vbs) {
>>
>> That conditional should probably be:
>>
>> if (mgr->any_user_vbs || mgr->fallback_ve)
>>
>> Because the translate fallback changes the buffer offsets too.
> Translate fallback adds a new buffer at mgr->translate_vb_slot.  It
> does not seem to change the offsets of the original buffers?

Ah, that's right. Sorry about the noise.

>>> +      for (i = 0; i < mgr->ve->count; i++) {
>>> +         unsigned index = mgr->ve->ve[i].vertex_buffer_index;
>>> +         struct pipe_vertex_buffer *vb = &mgr->b.vertex_buffer[index];
>>> +
>>> +         vb->buffer_offset = mgr->saved_buffer_offset[index];
>>> +      }
>>
>> Why not loop over vertex buffers instead of vertex elements?
> I followed the way u_vbuf_upload_buffers loops the buffers.  I will
> update this chunk to loop over vertex buffers.

OK, you don't have to resend the patch after that, feel free to commit directly.

Marek