[Mesa-dev] gallium st incorrect drawable <-> context bindings.

[Thomas Hellstrom]

On 05/24/2011 12:13 PM, Jakob Bornecrantz wrote:
> On Tue, May 24, 2011 at 11:53 AM, Thomas Hellstrom
> <thellstrom at vmware.com>  wrote:
>    
>> Hi,
>>
>> I'm not sure when this use of drawable<->  context bindings started, but the
>> below commit uses an invalid assumption to fix a problem the root cause of
>> which is also an invalid assumption.
>>
>> It's incorrect to associate a drawable with a context.
>> A drawable may have multiple contexts bound to it, and attempts to associate
>> a drawable with a single context will fail miserably in a multithreading
>> environment.
>>
>> This means that the fix below should be reverted, and
>> drawable->driContextPriv should never have been used in the first place. It
>> should nave no users whatsoever except old dri1 code using it as a possible
>> swapbuffer context, but even that usage is undesired.
>>
>> Furthermore, the st interface that uses a specific context to notify about
>> drawable invalidation is also incorrect. There should be no context
>> associated with that.
>>      
> You are right, we need a list of contexts, and notify all of the contexts.
>
> Cheers Jakob.
>
>    

I don't think that's the correct solution either. If the state-trackers 
notion of the drawable is just marked invalid, the first state tracker 
context using the drawable will go ahead and update it. Subsequent 
contexts will see the updated drawable, but might want to adjust their 
viewports. (Looks like st/mesa is doing something similar already).

I think the dri1 state tracker handled this correctly.

/Thomas









>> /Thomas
>>
>>
>>
>> Author: Jakob Bornecrantz<wallbraker at gmail.com>    2010-12-01 05:04:25
>> Committer: Marek Olšák<maraeo at gmail.com>    2011-02-20 16:31:48
>> Parent: 9e872a5865c66ed0a518dd1c6c54e72f3afa71f1 (i965: Fix VB packet reuse
>> when offset for the new buffer isn't stride aligned.)
>> Child:  3377faffcdc7227bd27381894c87c7600547744f (i965: Zero the offset into
>> the vbo when uploading non-interleaved)
>> Branches: master, remotes/origin/arb_robustness,
>> remotes/origin/arb_sampler_objects, remotes/origin/master,
>> remotes/origin/nvc0, remotes/origin/pipe-video,
>> remotes/origin/remove-driver-date
>> Follows: snb-magic
>> Precedes:
>>
>>     st/dri: Track drawable context bindings
>>
>>     Needs to track this ourself since because we get into a race condition
>> with
>>     the dri_util.c code on make current when rendering to the front buffer.
>>
>>     This is what happens:
>>     Old context is rendering to the front buffer.
>>
>>     App calls MakeCurrent with a new context. dri_util.c sets
>>     drawable->driContextPriv to the new context and then calls the driver
>> make
>>     current. st/dri make current flushes the old context, which calls back
>> into
>>     st/dri via the flush frontbuffer hook. st/dri calls dri loader flush
>>     frontbuffer, which calls invalidate buffer on the drawable into st/dri.
>>
>>     This is where things gets wrong. st/dri grabs the context from the dri
>>     drawable (which now points to the new context) and calls invalidate
>>     framebuffer to the new context which has not yet set the new drawable as
>> its
>>     framebuffers since we have not called make current yet, it asserts.
>>
>>