On 04/30/2012 12:54 PM, Marek Olšák wrote:
On Mon, Apr 30, 2012 at 7:53 PM, Brian Paul<bri...@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
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to