[Mesa-dev] State tracker pipe_surface woes...

[Stéphane Marchesin]

2011/9/2 Stéphane Marchesin <stephane.marchesin at gmail.com>:
> 2011/9/2 Jose Fonseca <jfonseca at vmware.com>:
>> ----- Original Message -----
>>> Hi,
>>>
>>> While debugging some code I ran across the following situation:
>>>
>>> - pipe_context c1 is created
>>> - pipe_surface s1 is created
>>> - strb-> surface is set to s1 (s1's refcount goes up)
>>> - pipe_context c1 is destroyed
>>> - strb is destroyed
>>> - strb->surface is destroyed (so s1's refcount is now 0 and we want
>>> to
>>> destroy it)
>>>
>>> At that point s1 references c1 which is not available any more, so
>>> when we try to call ctx->surface_destroy to destroy s1 we crash.
>>>
>>> We discussed this a bit on IRC, and we agreed that the proper
>>> solution, since surfaces outlive their context, is to make surfaces
>>> screen-bound instead. I'm going to  implement that unless someone
>>> objects.
>>>
>>> As a side note, the same issue will happen with sampler_views, so
>>> it'll get a similar fix.
>>
>> Sampler views and surfaces were previously objects bound to screen, and we changed that because of poor multithreading semantics.  Per-context sampler views / render targets actually matches the 3D APIs semantics better, so I don't think that reverting is the solution.
>>
>> It looks to me that the issue here is that pipe_context should not be destroyed before the surfaces. strb->surface should only be used by one context, and should be destroyed before that context is destroyed.
>>
>> IIUC, strb matches GL renderbuffer semantics and can be shared by multiple context. If so, strb is treating pipe_surfaces as a entity shareable by contexts when really shouldn't.
>>
>> The solution is:
>> - strb can only have pipe_resources, plus the key for the surface (face, level, etc)
>> - the pipe_surfaces that are derived should be stored/cached in the GLcontext.
>> - when the GLcontext / pipe_context is being destroy, the pipe surfaces can be destroyed before
>>
>
> I don't understand some of it. From what I see, it should be enough,
> whenever strb binds a surface, to add a pointer to this strb  to a
> list of strb's to the pipe_context. By doing that, we would be able to
> unbind the surfaces from the strb before we destroy the context.
> However, pipe_context structures can't reference gl structures, so how
> would you solve that?
>
> Also, what difference does it make if strb's only have pipe_resources?
> And why do I need a key?
>
> This all is definitely more complex than it should be.
>

And, yeah, I have a much simpler solution in mind, which I might just
end up doing for my own use:
- remove the ctx arg to surface_destroy()
- add a destroy function pointer to pipe_surface
- call that instead of going through the ctx when we destroy the surface

Stéphane