Re: winex11.drv: Make wglShareLists() cope with previously selected destination context (try 3)
Hi,
First of all note that glXDestroyContext only destroys a context after no of the other threads is using it (it is in the specs of glXDestroyContext). Second there are some more issues in wglShareLists which shouldn't get fixed in your patch but which I plan to fix at a later stage which you should take into account. Some BAD class of apps calls wglShareLists before making the source context current (so it isn't backed by a GLX context) while it has made the destination one current. In such case we need to perform a swap as right now such programs crash. MSDN clearly mentions that the hglrc2 parameter shouldn't contain any existing display lists, so this situation isn't allowed. This would mean wglShareLists(dest, source) is invalid and second that when perform a legal wglShareLists(source, dest) that dest can't have been current in any thread.
Since wglShareLists is a tricky function which might require very ugly hacks I want to keep it as clean as possible. This means that you should write a small wine test case for the behavior of wglShareLists in the situation affected by your patch.
Roderick
The current wglShareLists() will fail if the destination context has ever been made current. This causes a crash in Homeworld 2, unless pbuffers are disabled. This patch fixes the crash by deleting and recreating the underlying OpenGL context, unless it is still current in some thread.
Improved patch by: Moving the deletion of the old context into the wine_tsx11_lock where it belongs; tracking whether a WGL context is current in some thread so we don't deselect it out from under the app; adding an explanatory comment; a more professional change entry ;)
First of all note that glXDestroyContext only destroys a context after no of the other threads is using it (it is in the specs of glXDestroyContext).
Surely this means we've got to be doubly careful about keeping track of contexts that are current, otherwise we'd end up with WGL contexts backed by GLX contexts that aren't current in the thread they're supposed to be current in, and get hard-to-diagnose bugs.
Second there are some more issues in wglShareLists which shouldn't get fixed in your patch but which I plan to fix at a later stage which you should take into account. Some BAD class of apps calls wglShareLists before making the source context current (so it isn't backed by a GLX context) while it has made the destination one current. In such case we need to perform a swap as right now such programs crash.
OK. There's a patch floating about somewhere that swaps source and destination contexts for a couple of games that need it, I considered something along those lines but thought better of it.
MSDN clearly mentions that the hglrc2 parameter shouldn't contain any existing display lists, so this situation isn't allowed. This would mean wglShareLists(dest, source) is invalid and second that when perform a legal wglShareLists(source, dest) that dest can't have been current in any thread.
I took that statement from MSDN to imply that you can't have created any display lists glGenLists() in the destination context, but not say anything about having previously made it current in some thread. Does it imply that as well? Windows clearly allows it (or at least, allows the app to get away with doing it), or Windows apps wouldn't do it.
Since wglShareLists is a tricky function which might require very ugly hacks I want to keep it as clean as possible. This means that you should write a small wine test case for the behavior of wglShareLists in the situation affected by your patch.
Will do, thanks.
jim
Resending to the proper email address, this time..
On Friday 19 September 2008 06:37:03 am Jim Cameron wrote:
MSDN clearly mentions that the hglrc2 parameter shouldn't contain any existing display lists, so this situation isn't allowed. This would mean wglShareLists(dest, source) is invalid and second that when perform a legal wglShareLists(source, dest) that dest can't have been current in any thread.
I took that statement from MSDN to imply that you can't have created any display lists glGenLists() in the destination context, but not say anything about having previously made it current in some thread. Does it imply that as well? Windows clearly allows it (or at least, allows the app to get away with doing it), or Windows apps wouldn't do it.
The referred to "lists" aren't display lists. It's more appropritely (specific sets of) resources.. eg. the list of valid textures, the list of valid shader objects, etc, are what's shared.
Resending to the proper email address, this time..
And I likewise ;) (I was just thinking, "I swear I hit Reply All"...)
The referred to "lists" aren't display lists. It's more appropritely (specific sets of) resources.. eg. the list of valid textures, the list of valid shader objects, etc, are what's shared.
Ah. (Or should I say Arrrr?) I thought it might be something like that. It's not as if MSDN has ever been unclear in the past, or anything...
jim
Resending to the proper email address, this time..
And I likewise ;) (I was just thinking, "I swear I hit Reply All"...)
The referred to "lists" aren't display lists. It's more appropritely (specific sets of) resources.. eg. the list of valid textures, the list of valid shader objects, etc, are what's shared.
Ah. (Or should I say Arrrr?) I thought it might be something like that. It's not as if MSDN has ever been unclear in the past, or anything...
jim
That's the reason we need such tests .. I know it is a pain but bugs are much more complicated to debug in the end. Roderick
-
Chris Robinson -
Jim Cameron -
Roderick Colenbrander