[Mesa-dev] [PATCH 1/3] vbo: move vbo_draw_method into vbo_context.h

[Brian Paul]

On 05/07/2012 09:27 AM, Marek Olšák wrote:
> On Mon, May 7, 2012 at 7:11 AM, Mathias Fröhlich
> <Mathias.Froehlich at gmx.net>  wrote:
>>
>> Hi Marek,
>>
>> On Saturday, May 05, 2012 22:12:51 Marek Olšák wrote:
>>> could somebody please take a look at this series? It changes a couple
>>> of things in gl_context and the vbo module.
>>>
>>> The other two patches are:
>>> [PATCH 2/3] mesa: move gl_client_array*[] from vbo_draw_func into gl_context
>>> [PATCH 3/3] mesa: add gl_context::NewDriverState and use it for vertex
>>> arrays
>>
>> I am probably the wrong one to give a architectural ack for the series since I
>> am only a part time worker in some corners.
>> But I can tell what I had in mind in this area when I did the last changes.
>> You are partly heading in the same direction with moving the client array as
>> state into mesa.
>>
>> My plan was:
>> Do draws in terms of a gl_array_object that is active for draw. That would
>> play the role of your bare client array pointer currently. For the immediate
>> mode save and exec context code paths we would use a hidden array object that
>> is used on a drivers call code path. For the OpenGL api's draw arrays we would
>> use the contexts gl_array_object for draws.
>> In my plan the client_array should be located in the gl_array_object.
>> Therefore the NewArray bitfield also moved to gl_array_object recently. You can
>> then there build up a self contained set of methods on the gl_array_object to
>> update its state and track the dirty bits in the NewArray bitfield also in
>> combination with the Enabled bitfield where we can play a lot of tricks with
>> the bitfields to reduce a lot of loops iterating all 33 arrays with loops only
>> working on the changed arrays.
>>
>> For the drivers side I think that we can get rid of the client_array
>> completely for the gallium side. To do that, I wanted to derive a
>> st_array_object and a classic_array_object from gl_array_object. The
>> client_array in its current form could then move into the classic_array_object
>> where we can still play intelligent games with the bitfields to cut down the
>> length 33 loops int the fast draw path. But the st_array_object would only
>> have space for pipe_vertex_{buffer,elements} in addition to the fields from the
>> base gl_array_object class. These elements could then be incrementally updated
>> using the NewArray and Enabled bitfields and applied to the gallium state when
>> needed.
>
> As far as the gallium side is concerned, I don't think the array
> object can fully encapsulate vertex buffers and vertex elements. Those
> two also depend on the vertex shader, because vertex buffer bindings
> currently map 1:1 to vertex elements unless the vertex attribs are
> interleaved (though that can be changed), and vertex elements always
> map 1:1 to vertex shader inputs. Whether it's worth to break the
> dependency between those states is questionable. I think that binding
> 32 vertex buffers and letting vertex elements decide which buffers to
> use would be a disaster for our CPU performance. Who knows. All in
> all, I've got no idea how to take advantage of array objects in
> st/mesa, because it doesn't seem to map to gallium well. Maybe you
> guys know better.

I mentioned the idea before and Mathias is interested in it as well.

The basic idea is:

1. OpenGL's vertex array objects (gl_array_object) basically 
encapsulates the state of a bunch of glVertexPointer, 
glTexCoordPointer, etc. arrays.

2. In gallium we have a set of pipe_vertex_element that basically 
encodes the same state.

3. Hopefully we could avoid re-deriving the later state from the 
former if we do the encapsulation that Mathias describes.

But as you said, the derived array state that we pass to draw calls 
also depends on the currently bound vertex program and its inputs.

It seems to me though, that a given gl_array_object state would almost 
always be used with a vertex shader (or set of vertex shaders) that 
use precisely the enabled arrays.  So there could be a quick check if 
the set of enabled arrays matched the set of vertex shader inputs and 
we could quickly re-use the pipe_vertex_element state.



>> Given that I think that I would prefer to have the client_array (not the
>> pointer to the first element) itself in the gl_array_object. And the role of
>> your gl_client_array** pointer would be replaced by a gl_array_object pointer
>> with the same three instances (drawelements api path, vbo_exec, vbo_save)
>> around that your patch uses for the *gl_client_array[].
>
> I'd prefer to leave this change to another patch series and keep this
> series simple.

I agree.


>> The benefit would be that you could probably encapsulate the the array state
>> tracking completely into this subobject, track changes using nifty bitfields
>> using ffs based way smaller loops and only track changes to the gl_array_object
>> draw pointer at the context toplevel.
>>
>> So, for my interrest, what are your further plans beyond this change?
>
> Concerning core Mesa, I'd like to use gl_context::DriverFlags for more
> states than just vertex arrays. I'll probably do blend state next.

Sounds good.

> I don't have any other plans for core Mesa at the moment.
>
> Marek

-Brian