[Mesa-dev] [PATCH] mesa: Fix multithreaded buffer object refcounting.

[Brian Paul]

On 10/19/2011 12:00 AM, Mathias Fröhlich wrote:
>
> Hi Brian,
>
> On Tuesday, October 18, 2011 17:05:53 Brian Paul wrote:
>> This was brought up a few weeks ago by someone else.  I had posted the
>> start of a patch to fix it.  I'm attaching an updated version that I
>> think is pretty clean.  But there's a few lines missing and it's
>> totally untested.  Maybe you could finish this up.  Look for 'XXX'
>> comments.
>
> Yes, the code of this approach looks way cleaner.
> Hmm, I am observing that you are deleting the gl_array_object with the drivers
> DeleteArrayOject callback but you only malloc'ed the struct. This should be
> symmetric at least.

Yes, I think so.


> This raises a question for me:
> Should the push/pop just save a bunch or driver independent variables or
> should we use full driver provided array object?
>
> I ended up with the conclusion that we should do that independent of the
> driver because the driver provided array objects - even if currently not
> overloaded by any driver - will probably do additional work. Work with is
> never used by the push/pop code because it will never really bind the object
> to the context. Also doing this with the driver provided array objects will
> have a lot of overhead since we at first reference the null buffer objects for
> every array object from the drivers NewArrayObject that probably calls
> _mesa_initialize_array_object call chain. Then immediately overwrite all of
> them with the current state of the buffer objects.
> This lead me to an approach that uses less library functions and in a first
> step just makes sure that programs dont crash.
 >
> This appears more efficient in the end. But doing this in the way you started,
> by doing this with full blown array objects, could share much more code. Which
> is probably more maintainable.
> Which variant?

As a first cut, I want something that works correctly and is easy to 
understand and maintain.  We can optimize later if needed.  I think 
most seasoned OpenGL programmers know that glPush/PopAttrib isn't 
exactly a high-performance path.


> Also, when we are at a clean solution, should we just copy/rereference back
> all the array objects on pop or should we really rebind all of them using the
> _mesa_* buffer object function calls?
> This would ensure that the drivers BindBuffer logic is called the way it is
> used every time.
> Also this would ensure that, by looking up the arrays by their names, already
> deleted buffer objects, that are already removed from the name hash and
> forcefully unbound from the current state but might still be referenced by the
> attribute stack, are not poped back into the current state.

We definitely need to properly handle the case that a buffer object is 
"deleted" by the user between push/pop.


It sounds like you've got a good understanding of the issues here so 
please go ahead and implement the rest of the patch.

-Brian