[Mesa-dev] [PATCH 3/9] st/mesa: make user index buffers optional

[Brian Paul]

On 04/30/2012 12:54 PM, Marek Olšák wrote:
> On Mon, Apr 30, 2012 at 7:53 PM, Brian Paul<brianp at vmware.com>  wrote:
>> On 04/26/2012 12:54 PM, Jose Fonseca wrote:
>>>
>>> ----- Original Message -----
>>>>
>>>> ---
>>>>   src/mesa/state_tracker/st_context.c    |    4 +++-
>>>>   src/mesa/state_tracker/st_context.h    |    1 +
>>>>   src/mesa/state_tracker/st_draw.c       |    5 +++++
>>>>   src/mesa/state_tracker/st_extensions.c |    4 ++++
>>>>   4 files changed, 13 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/src/mesa/state_tracker/st_context.c
>>>> b/src/mesa/state_tracker/st_context.c
>>>> index 84aae81..164cc45 100644
>>>> --- a/src/mesa/state_tracker/st_context.c
>>>> +++ b/src/mesa/state_tracker/st_context.c
>>>> @@ -155,7 +155,9 @@ st_create_context_priv( struct gl_context *ctx,
>>>> struct pipe_context *pipe )
>>>>      st->dirty.mesa = ~0;
>>>>      st->dirty.st = ~0;
>>>>
>>>> -   st->uploader = u_upload_create(st->pipe, 65536, 4,
>>>> PIPE_BIND_VERTEX_BUFFER);
>>>> +   st->uploader = u_upload_create(st->pipe, 128 * 1024, 4,
>>>> +                                  PIPE_BIND_VERTEX_BUFFER |
>>>> +                                  PIPE_BIND_INDEX_BUFFER);
>>>
>>>
>>> Marek,
>>>
>>> Instead of lumping this into the same hardware buffer, I think it would be
>>> better to use two separate uploaders so that the driver can effectively do
>>> optimization based on PIPE_BIND_VERTEX_BUFFER or PIPE_BIND_INDEX_BUFFER.  A
>>> quick look on current drivers showed that they do look at these bind flags.
>>>
>>> Otherwise I don't see anything wrong with this series. It seems a nice
>>> cleanup/speedup.
>>>
>>> Brian's OOTO till Monday, so allow more time for him to comment.
>>>
>>> Also, once you updated the series, please provide it in a clonable git
>>> branch for testing.
>>
>>
>> I'm testing the gallium-userbuf branch now.  With piglit's draw-batch test
>> there's a failing assertion in svga_resource_buffer_upload.c at line 557
>> (the buffer in question is in a mapped state).  I disabled the assertion but
>> then the test fails (it passes on master).  I'll try to dig a big deeper.
>
> Ooops, sorry about that. There's a missing call to u_upload_unmap in
> setup_index_buffer (st_draw.c). I'll commit a fix in a moment.

OK, that helps.  Thanks.

But now I'm seeing a crash w/ google earth.  At line 70 of 
svga_swtnl_draw.c (below) there's a null vertex buffer pointer 
(svga->curr.vb[0].buffer=null).

64	
65	   /*
66	    * Map vertex buffers
67	    */
68	   for (i = 0; i < svga->curr.num_vertex_buffers; i++) {
69	      map = pipe_buffer_map(&svga->pipe,
70	                            svga->curr.vb[i].buffer,
71	                            PIPE_TRANSFER_READ,
72				    &vb_transfer[i]);
73	

It looks like when pipe->set_vertex_buffers() is called, some of the 
buffers in [0..count-1] may be null.  I don't think that ever happened 
before.  That is, buffers[0..num_buffers-1] would always be non-null.

The set_vertex_buffers() function is called from u_vbuf.c.  I took a 
quick look but I'm not sure what's all going on there.

I hacked the svga code to avoid/skip the null buffers so I guess I can 
fix things there.  But maybe you can clarify the story with the 
buffers passed to pipe->set_vertex_buffers().

-Brian