Fixing conflicts between WineD3D and WGL

[Ken Thomases]

On Jan 29, 2014, at 5:10 AM, Henri Verbeet wrote:

> On 29 January 2014 01:47, Ken Thomases <ken at codeweavers.com> wrote:
>> The most important things to achieve are a) to not "consume" the only opportunity the app has to set the window's pixel format, and b) to restore the app's current GL context (and implicitly its DC) when returning control to it.  I don't think it's necessary to avoid WineD3D ever changing WGL's notion of the current context.  Certainly, while a WineD3D GL context is current, GL functions like glClear() will operate on WineD3D's context.  If the app thinks its context is current and calls GL functions, its context better really be current.  So, WineD3D really needs to restore the app's context before returning control (and it generally does, I think).  If it's going to do that, then wglGetCurrentContext() will take care of itself.  Same for wglGetCurrentDC().
>> 
> There may be performance reasons though. The wglMakeCurrent() required
> to restore the previous GL context is pretty expensive, so
> applications that have a GL context current while calling into wined3d
> take a considerable performance hit. I think I've only ever seen 1 or
> 2 applications that were affected that though, so perhaps we don't
> care enough.

My point is that the current GL context is exposed through WGL but it also affects the behavior of GL functions (of course).  So, while we might be able to hide the change of the current GL context from wglGetCurrentContext(), that doesn't do us any good because we need GL functions to target the right context.  There's just no getting around switching the current GL context to WineD3D's when WineD3D needs to call GL functions and then back to the app's when control is returned to it.  (Except Chris's point of doing all GL on a different thread.)

Do you know what makes wglMakeCurrent() expensive?  For X11, we'll still have to use glXMakeCurrent(), which sets the context drawable, even for my proposed wglMakeContextCurrentWINE().  Maybe it's the setting of the drawable that's expensive.  For the Mac, we can just make the context current without setting the drawable again.  That might be less expensive, although maybe there's a lot of thread state inside the GL engine that has to be swapped in and out.  (I would hope that that would all be stored in the context and switching contexts would be just a matter of swapping a pointer somewhere.)


>> Extension WGL_WINE_make_current:
>> 
>> BOOL wglMakeContextCurrentWINE(HGLRC hglrc);
>> 
>> The standard WGL function wglMakeCurrent() combines two operations.  It sets the a GL context's drawable to the provided DC and it makes the GL context current for the thread.  This new function, wglMakeContextCurrentWINE(), would only do the latter.  It would make a GL context current for the thread without changing which DC it's associated with.  Both the X11 and Mac drivers already maintain the necessary state to do that.  They don't need to have the DC passed in again.  (If a GL context is associated with separate draw and read DCs using wglMakeContextCurrentARB(), then this function will leave it associated with both DCs.)
>> 
>> This allows WineD3D to faithfully restore the app's previously-current WGL context.  This addresses bug 28869.
>> 
> Maybe. Note that some of the quirk detection code renders to this
> context. I think all rendering there goes to FBOs, so in that regard
> it's probably safe, but it does feel a bit fragile. For regular
> wined3d rendering we do of course need a drawable, although this may
> work out in practice.

I'm not sure I understand.  "quirk detection code renders to this context" – to which context do you refer?

The point of this extension is just for restoring the app's GL context.  It would not necessarily be used to make a WineD3D context current, it would only be used to restore the app's WGL context to be current.  It doesn't take a DC so that this works even for the case where the original DC was destroyed/released.

(It could be used to make a WineD3D context current a second time, if you don't want to change the draw DC.  But to associate the draw DC with the context the first time, you'd still use wglMakeCurrent().)


> I think WGL_WINE_surface would work from wined3d's point of view, at
> the very least interface wise. There are probably some details to work
> out about how it should interact with gdi32 rendering and things like
> the window moves, minimization, etc. I don't have as good of an
> overview of how it would work out for winex11. I'm not so sure about
> WGL_WINE_make_current, but it may be enough in practice.

OK, thanks.  I guess my next step will be to put together a patch to implement this all.  I may do it quick-and-dirty at first just to prove the concept.

I _think_ that window moves, etc. should be handled roughly as they are already.  For a surface created from a window, things should work almost exactly as they currently do.  The main difference would be where the pixel format is stored.  With a window DC – how things currently work – it's conceptually stored as a property of the window.  With a surface DC, it would be stored as a property of the surface.

The other thing needing an update would be sync_gl_drawable().  That's for window moves and resizes.  Currently, it looks up the single drawable for a given HWND.  With surfaces, it would have to iterate over the surfaces for that window, as well.

-Ken