Ken Thomases ken@codeweavers.com writes:
The specific pixel format was never used. It was effectively just a flag indicating if the window had a GL surface. This generalizes it from a flag to a count, to allow for multiple GL surfaces.
I'm not convinced that the concept of multiple surfaces makes sense, except maybe as a driver implementation detail.
On May 6, 2014, at 8:42 AM, Alexandre Julliard wrote:
Ken Thomases ken@codeweavers.com writes:
The specific pixel format was never used. It was effectively just a flag indicating if the window had a GL surface. This generalizes it from a flag to a count, to allow for multiple GL surfaces.
I'm not convinced that the concept of multiple surfaces makes sense, except maybe as a driver implementation detail.
Well, it doesn't make much sense for user32 to know the particular pixel format, either. There's not a single pixel format for each window. WGL and D3D can use separate formats. That's at the center of what I'm doing.
A change to use a boolean flag instead of a pixel format ID would be as complicated for user32 as this patch and would make the driver work more complicated.
It seems to me that this is purely a practical matter. We need user32 to behave differently when there's any GL surface(s) than when there are none. Something has to keep count and it seems simplest to track the count in user32.
Leaving aside the specifics of the implementation (i.e. the WGL_WINE_surface extension), conceptually, either or both of WGL and D3D need to be able to independently tell user32 to make this change in its behavior.
-Ken
On May 6, 2014, at 10:23 PM, Ken Thomases wrote:
Leaving aside the specifics of the implementation (i.e. the WGL_WINE_surface extension), conceptually, either or both of WGL and D3D need to be able to independently tell user32 to make this change in its behavior.
Oh, and D3D's requests can be temporary.
-Ken
Ken Thomases ken@codeweavers.com writes:
Well, it doesn't make much sense for user32 to know the particular pixel format, either. There's not a single pixel format for each window. WGL and D3D can use separate formats. That's at the center of what I'm doing.
A change to use a boolean flag instead of a pixel format ID would be as complicated for user32 as this patch and would make the driver work more complicated.
It's already treated as a flag as you noted, so I don't see why you'd need to change anything.
It seems to me that this is purely a practical matter. We need user32 to behave differently when there's any GL surface(s) than when there are none. Something has to keep count and it seems simplest to track the count in user32.
Surely the driver already knows if there are surfaces, I don't see why you need a count at all, much less in user32.
On May 7, 2014, at 3:17 AM, Alexandre Julliard wrote:
Ken Thomases ken@codeweavers.com writes:
Well, it doesn't make much sense for user32 to know the particular pixel format, either. There's not a single pixel format for each window. WGL and D3D can use separate formats. That's at the center of what I'm doing.
A change to use a boolean flag instead of a pixel format ID would be as complicated for user32 as this patch and would make the driver work more complicated.
It's already treated as a flag as you noted, so I don't see why you'd need to change anything.
The _effect_ is of a flag, but it's called a pixel format. It'd be nice to fix its name to match its interpretation. And, besides the renaming, the only other change that my patch made to user32 was changing a single "=" to "+=". I don't feel like it's adding much in the way of complexity.
It seems to me that this is purely a practical matter. We need user32 to behave differently when there's any GL surface(s) than when there are none. Something has to keep count and it seems simplest to track the count in user32.
Surely the driver already knows if there are surfaces, I don't see why you need a count at all, much less in user32.
It's the difference between code which, when a surface is destroyed, simply decrements that count in user32 vs. one which has to search in two different lists to see if there are any remaining surfaces for that window and, if there aren't, clearing the "flag" in user32. It's not particularly difficult, it's just simpler and cleaner with a count. Putting it in the driver means adding complexity into each driver.
This entire piece of functionality in user32 is for the benefit of the driver. It's all implementation detail. I don't understand why it shouldn't take the form that's most suited to the needs of the driver.
-Ken
Ken Thomases ken@codeweavers.com writes:
It's the difference between code which, when a surface is destroyed, simply decrements that count in user32 vs. one which has to search in two different lists to see if there are any remaining surfaces for that window and, if there aren't, clearing the "flag" in user32. It's not particularly difficult, it's just simpler and cleaner with a count. Putting it in the driver means adding complexity into each driver.
This entire piece of functionality in user32 is for the benefit of the driver. It's all implementation detail. I don't understand why it shouldn't take the form that's most suited to the needs of the driver.
Sure, but IMO if the driver needs this it's doing something wrong. Maybe I just don't understand what you are trying to do, but the notion of an arbitrary number of surfaces per window doesn't make any sense to me.
On May 7, 2014, at 4:57 AM, Alexandre Julliard wrote:
Sure, but IMO if the driver needs this it's doing something wrong. Maybe I just don't understand what you are trying to do, but the notion of an arbitrary number of surfaces per window doesn't make any sense to me.
I'm writing a WGL extension with a function for creating surfaces. There's one implicit for WGL when the app sets a pixel format on a window, but we want wined3d to create separate ones to avoid messing up externally visible state. Wined3d may create a surface for a window, use it, and then tear it down. However, other threads may still have a DC referencing that surface and have a context current on it. They will eventually clear that up when they next call into wined3d, but there's an indefinite period before that happens. Meanwhile, the first thread may create a new surface for the window. Etc.
Each of these surfaces may have different pixel formats. In the X11 driver, a different pixel format implies a different FBConfig and a different visual, so requires a new X window.
Yes, there's a question about what happens if multiple surfaces are actually being used for rendering simultaneously. Only one can be seen at a time, so what's the point, right? In my current patches, the most recent one created is in front and visible. I've got plans to make it so that the one most recently made current for a context will be in front.
I don't know what happens on Windows if WGL and D3D are used simultaneously against the same window, or if two D3D devices or swapchains attempt to simultaneously target the same window. The tests already show that using D3D on a window which has had a WGL pixel format set on it doesn't generate errors. And we know that some games do that. I just don't know if they actually make a context current or try rendering while D3D is in use. The wined3d code certainly assumes that the app may have made a WGL context current before calling into it and goes to the trouble of restoring it before returning.
In any case, if D3D is used temporarily, then after it's done (on all threads) we'd like the window to be returned to a state as though D3D were never used on it. If no WGL pixel format had been set, then it should have no surfaces and user32 and gdi32 should resume normal rendering. If one had been set, then it should have just the WGL surface.
-Ken
Ken Thomases ken@codeweavers.com writes:
On May 7, 2014, at 4:57 AM, Alexandre Julliard wrote:
Sure, but IMO if the driver needs this it's doing something wrong. Maybe I just don't understand what you are trying to do, but the notion of an arbitrary number of surfaces per window doesn't make any sense to me.
I'm writing a WGL extension with a function for creating surfaces. There's one implicit for WGL when the app sets a pixel format on a window, but we want wined3d to create separate ones to avoid messing up externally visible state. Wined3d may create a surface for a window, use it, and then tear it down. However, other threads may still have a DC referencing that surface and have a context current on it. They will eventually clear that up when they next call into wined3d, but there's an indefinite period before that happens. Meanwhile, the first thread may create a new surface for the window. Etc.
Each of these surfaces may have different pixel formats. In the X11 driver, a different pixel format implies a different FBConfig and a different visual, so requires a new X window.
Yes, there's a question about what happens if multiple surfaces are actually being used for rendering simultaneously. Only one can be seen at a time, so what's the point, right? In my current patches, the most recent one created is in front and visible. I've got plans to make it so that the one most recently made current for a context will be in front.
I don't see why D3D would need anything like that. Do you really need to be able to set a new pixel format but still draw with the old one and expect the output to be visible?
It seems to me all you really want is to restore the pixel format after D3D has used it, to avoid messing up WGL. This may require the driver to maintain a separate D3D pixel format, and possibly two surfaces, or maybe one surface recreated as needed once the app uses WGL again, but I don't see why it would justify adding an API to manage an arbitrary number of surfaces. What are the use cases for this?
On May 7, 2014, at 8:10 AM, Alexandre Julliard wrote:
I don't see why D3D would need anything like that.
I'm not sure if you followed the "Fixing conflicts between WineD3D and WGL" thread. That explains it along with some of the bugs that were known at the time. Since that thread was started, we've identified more bugs (not necessarily new ones) that my patches fix.
Do you really need to be able to set a new pixel format but still draw with the old one and expect the output to be visible?
I am not aware of anything which tries to draw simultaneously using WGL and D3D or two D3D devices/swapchains targeting the same window. But there are definitely apps which care that GetPixelFormat() continues to return the WGL value even while a D3D swapchain is set up to target the same window, for example.
It seems to me all you really want is to restore the pixel format after D3D has used it, to avoid messing up WGL.
Wined3d is already restoring the pixel format, but it caused a bunch of regressions. Wined3d needs to "restore" things every time a call into it returns back to the caller, not just when the device and swapchain are torn down. There's no guarantee that an app won't try to do WGL stuff while a swapchain is set up, even if it's not actually rendering to both simultaneously. With the X11 driver, that restore is expensive and causes X windows to be destroyed and recreated, which causes rendering failures.
The proper solution is for D3D to target a different object which maintains separate metadata.
This may require the driver to maintain a separate D3D pixel format, and possibly two surfaces, or maybe one surface recreated as needed once the app uses WGL again, but I don't see why it would justify adding an API to manage an arbitrary number of surfaces.
I already explained. In the multi-threaded case, one thread can still be using an old surface after another thread released it and created a new one.
Beyond that, although I'm not aware of a case where an app uses two D3D devices to target the same window, I'm also not seeing anything in the docs which says it's not allowed. For example, IDirect3DSwapChain9::Present() takes an hDestWindowOverride parameter which causes that particular present to target a window that was previously unrelated to the swapchain. I see nothing in the docs that suggest there's a limitation on which window you can use (other than that it be visible or top-level). No error code meaning "invalid window" or "window format mismatch".
In any case, once you have support for two surfaces, you're already supporting multiple surfaces. The code would not be especially (any?) simpler if it were limited to one WGL surface and one D3D surface per window.
-Ken
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Am 2014-05-07 22:50, schrieb Ken Thomases:
Beyond that, although I'm not aware of a case where an app uses two D3D devices to target the same window, I'm also not seeing anything in the docs which says it's not allowed.
There are applications that use two devices, even if they don't render from both. Sins of a Solar Empire (the relatively new game on Steam) is such a case. I also remember seeing this in ddraw, but I can't think of any specific game right now.
My general impression of such applications is that they fail to release the old device rather than deliberately use two different devices.
Ken Thomases ken@codeweavers.com writes:
I already explained. In the multi-threaded case, one thread can still be using an old surface after another thread released it and created a new one.
It can't be using an old surface unless you introduced the concept of multiple surfaces in the first place... If the API is only, say, wglSetPixelFormatWINE, then the change is considered instantaneous, and threads that try to use the old format get an error, or get ignored.
Obviously, in the driver implementation, it may require you to recreate an X11 window, and possibly keep the old X11 window around to prevent crashes, but that doesn't mean that the concept of multiple active surfaces needs to become part of the exposed API.
Beyond that, although I'm not aware of a case where an app uses two D3D devices to target the same window, I'm also not seeing anything in the docs which says it's not allowed. For example, IDirect3DSwapChain9::Present() takes an hDestWindowOverride parameter which causes that particular present to target a window that was previously unrelated to the swapchain. I see nothing in the docs that suggest there's a limitation on which window you can use (other than that it be visible or top-level). No error code meaning "invalid window" or "window format mismatch".
Maybe more test cases would be in order then. The concept of supporting multiple active surfaces clearly raises a number of questions, and so far it seems the answer is always "we don't know because no app ever does this". So it feels to me like a solution in search of a problem...
On May 8, 2014, at 3:18 AM, Alexandre Julliard wrote:
Ken Thomases ken@codeweavers.com writes:
I already explained. In the multi-threaded case, one thread can still be using an old surface after another thread released it and created a new one.
It can't be using an old surface unless you introduced the concept of multiple surfaces in the first place...
You don't even acknowledge the need for multiple surfaces _serially_? That wined3d might create a surface on a virgin window and then destroy it, and then later create another one?
If the API is only, say, wglSetPixelFormatWINE, then the change is considered instantaneous, and threads that try to use the old format get an error, or get ignored.
Or compete with the new pixel format causing pixel format thrashing, which is the status quo.
Obviously, in the driver implementation, it may require you to recreate an X11 window, and possibly keep the old X11 window around to prevent crashes, but that doesn't mean that the concept of multiple active surfaces needs to become part of the exposed API.
What requires it is all of the bugs caused by the status quo.
The sole client of the new extension is wined3d. D3D and wined3d allow multiple swapchains to simultaneously target the same window. Each may use a different pixel format or swap interval. Likewise, WGL may set a pixel format or swap interval on a window simultaneously being used for D3D. In both WGL and GLX, the pixel format and the swap interval are properties of the drawable. They currently conflict, causing various failures. We need them to be independent so they don't interfere with each other. Therefore, wined3d requires multiple drawables.
It may be theoretical that any app actually uses such multiple swapchains for rendering simultaneously, but as Stefan pointed out it's not theoretical that apps set up such swapchains. In any case, in designing an API for the use of wined3d, I need to support what wined3d supports, even in theory.
The concept of supporting multiple active surfaces clearly raises a number of questions, and so far it seems the answer is always "we don't know because no app ever does this". So it feels to me like a solution in search of a problem...
That's exactly backward. The problems are real and known. My work solves them. You are hypothesizing that my work will cause problems which would only happen if apps did what "no app ever does". If the presence of multiple surfaces causes such a problem, we'll then have an actual real world case and can debug and solve it. Ideally, we'd do that debugging before the bulk of my work is actually committed, and some of our testers have indeed been helping me identify and debug issues with my patches. So far, the existence of multiple surfaces has not been one of them.
-Ken
Ken Thomases ken@codeweavers.com writes:
On May 8, 2014, at 3:18 AM, Alexandre Julliard wrote:
It can't be using an old surface unless you introduced the concept of multiple surfaces in the first place...
You don't even acknowledge the need for multiple surfaces _serially_? That wined3d might create a surface on a virgin window and then destroy it, and then later create another one?
It's probably a question of terminology. To my mind, there is one place in the window where you can render stuff, using a specific pixel format.
If multiple users try to set a different format on the window at the same time, there's a need to arbitrate that. Your solution is to create multiple surfaces, but then this moves the problem to arbitrating which surface is the "real" one, i.e. the one that actually puts something on the screen. Because in the end there's only one visible window surface.
Obviously under X11 we need to create a new drawable because the format is set at creation time, but that doesn't necessarily have to be something that wined3d knows about. Conceptually it's still the same window surface.
The concept of supporting multiple active surfaces clearly raises a number of questions, and so far it seems the answer is always "we don't know because no app ever does this". So it feels to me like a solution in search of a problem...
That's exactly backward. The problems are real and known. My work solves them. You are hypothesizing that my work will cause problems which would only happen if apps did what "no app ever does".
No, that's not what I'm saying. I'm sure your approach is solving problems. My point is that the mechanism you are introducing raises new questions, like what happens when two surfaces are active at the same time. You are saying that this can be left undefined because no app ever does it, which leads me to wonder whether the API shouldn't better reflect what apps actually do.
On May 9, 2014, at 5:34 AM, Alexandre Julliard wrote:
Ken Thomases ken@codeweavers.com writes:
On May 8, 2014, at 3:18 AM, Alexandre Julliard wrote:
It can't be using an old surface unless you introduced the concept of multiple surfaces in the first place...
You don't even acknowledge the need for multiple surfaces _serially_? That wined3d might create a surface on a virgin window and then destroy it, and then later create another one?
It's probably a question of terminology. To my mind, there is one place in the window where you can render stuff, using a specific pixel format.
If multiple users try to set a different format on the window at the same time, there's a need to arbitrate that. Your solution is to create multiple surfaces, but then this moves the problem to arbitrating which surface is the "real" one, i.e. the one that actually puts something on the screen. Because in the end there's only one visible window surface.
Obviously under X11 we need to create a new drawable because the format is set at creation time, but that doesn't necessarily have to be something that wined3d knows about. Conceptually it's still the same window surface.
Actually, I believe it is something that wined3d needs to know about. Commit 4c4552c5a1910a9d5adf8eccff0ac62d89ffe376 introduced a number of regressions. One of them was a performance drop. This drop happens even in the case where nothing else sets a pixel format on the window. The performance problem comes just from wined3d having to check if there _might_ have been contention over the pixel format – if the app might have changed it since the last time wined3d was entered. To avoid that check, each swapchain must know that the OpenGL context it uses is attached to a private drawable that is not shared either with WGL nor with other swapchains. It has to know that once it sets the pixel format on its private drawable, nothing else can change it. (This is the purpose of the hdc_is_private and hdc_has_format flags introduced with commits 27287382 and f3aa4812.)
Also, it is my understanding – and I hope that Henri, Stefan, or Matteo will correct me if I'm wrong – that swapchains are independent sets of backbuffers. An app can have more than one. Each swapchain has a separate pixel format. The app can issue a bunch of drawing commands to each and they should be independent. It can draw a circle in one and a square in the other. It can then "present" either swapchain and only that swapchain's rendering will make it to screen. It will show either a circle or a square but not a circle overlapping a square or the like. It can then, of course, present the other or not or do it in the other order.
The point is, multiple surfaces is a natural fit with D3D's model. The swapchain which was most recently presented is the visible one. Which means that the X11 driver would have to order a drawable front during wglSwapBuffers() (not when the context is made current, which is what I said in a previous email).
I don't know how wined3d currently deals with the case of multiple swapchains targeting the same window. That is, how it keeps the drawing commands and backbuffers of one independent from those of another. Using separate contexts works to some extent, but only until the OpenGL command buffer fills, at which point the drawing from one would be flushed to the backbuffer the contexts share. Wined3d may ignore this or maybe it uses FBOs or something for the backbuffers of secondary swapchains. In either case, multiple surfaces offers an opportunity to improve the status quo.
I also want to say that I already tried to solve the issues entirely within the X11 driver. For example, I tried an approach where the X11 driver puts off realizing a change of the window pixel format until it's absolutely necessary http://bugs.winehq.org/show_bug.cgi?id=35718#c32. It didn't really solve the problem because wined3d needs to restore the thread's current WGL context each time it returns from a call, and that requires that the window pixel format be restored for real.
I think the fundamental nature of the problem is unavoidable so long as wined3d uses WGL/OpenGL, which relies on implicit thread state. When wined3d issues OpenGL commands, that state needs to be wined3d's but when the app issues its commands, it needs to be the app's. There's no getting around "thrashing" the current context because of that, but the pixel format thrashing can be avoided by using separate surfaces. And because this thread state model extends all the way from the driver through WGL to wined3d, there's no avoiding exposing the existence of those surfaces to wined3d.
-Ken
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Am 2014-05-12 07:06, schrieb Ken Thomases:
Also, it is my understanding – and I hope that Henri, Stefan, or Matteo will correct me if I'm wrong – that swapchains are independent sets of backbuffers. An app can have more than one. Each swapchain has a separate pixel format. The app can issue a bunch of drawing commands to each and they should be independent. It can draw a circle in one and a square in the other. It can then "present" either swapchain and only that swapchain's rendering will make it to screen. It will show either a circle or a square but not a circle overlapping a square or the like.
Correct. There was a d3d tutorial that demonstrated this, but rendered to two different windows. I cannot find it (many d3d9 tutorials seem to be gone). This is also what happens when multihead d3d is used.
It can then, of course, present the other or not or do it in the other order.
The "can" part is correct, but we don't know exactly what happens because we don't have tests. I'd expect the second Present to overwrite the output of the first.
Also note that in d3d the pixelformat is set on the backbuffer, not the window. (present_parameters.BackBufferFormat). There's also the display mode, which specifies a separate pixelformat for the screen (again, not the window). There is a function to check compatibility between screen and backbuffer formats. Usually D3DFMT_A8R8G8B8 is used on backbuffers and compatible with D3DFMT_X8R8G8B8. R5G6B5 is compatible with R5G6B5, and A8R8G8B8 is compatible with A8R8G8B8.
I don't know how wined3d currently deals with the case of multiple swapchains targeting the same window. That is, how it keeps the drawing commands and backbuffers of one independent from those of another.
Based on the render target we know where the draw commands go to. For the output see below.
Using separate contexts works to some extent, but only until the OpenGL command buffer fills, at which point the drawing from one would be flushed to the backbuffer the contexts share. Wined3d may ignore this or maybe it uses FBOs or something for the backbuffers of secondary swapchains. In either case, multiple surfaces offers an opportunity to improve the status quo.
We usually use FBOs to render to backbuffers. This can be disabled by the user (HKCU/Software/Wine/Direct3D/AlwaysOffscreen, or by setting OffScreenRendering = backbuffer), or when FBOs are not available. This legacy mode is still used on some older drivers.
If two contexts share a window and we're not rendering to an FBO the draw commands will interfere with each other. Currently we're using this effect for multithreading support (when the application uses one swapchain to render to one window, but renders from different threads.)
On 12 May 2014 07:06, Ken Thomases ken@codeweavers.com wrote:
On May 9, 2014, at 5:34 AM, Alexandre Julliard wrote:
If multiple users try to set a different format on the window at the same time, there's a need to arbitrate that. Your solution is to create multiple surfaces, but then this moves the problem to arbitrating which surface is the "real" one, i.e. the one that actually puts something on the screen. Because in the end there's only one visible window surface.
Obviously under X11 we need to create a new drawable because the format is set at creation time, but that doesn't necessarily have to be something that wined3d knows about. Conceptually it's still the same window surface.
Actually, I believe it is something that wined3d needs to know about. Commit 4c4552c5a1910a9d5adf8eccff0ac62d89ffe376 introduced a number of regressions. One of them was a performance drop. This drop happens even in the case where nothing else sets a pixel format on the window. The performance problem comes just from wined3d having to check if there _might_ have been contention over the pixel format – if the app might have changed it since the last time wined3d was entered. To avoid that check, each swapchain must know that the OpenGL context it uses is attached to a private drawable that is not shared either with WGL nor with other swapchains. It has to know that once it sets the pixel format on its private drawable, nothing else can change it. (This is the purpose of the hdc_is_private and hdc_has_format flags introduced with commits 27287382 and f3aa4812.)
Well, sort of. There can be multiple D3D devices on the same window, and drawing with those should work, although I'm less sure to what extent the output of a such a setup is supposed to be defined. This needs tests. Something similar applies to drawing with WGL and D3D to the same window.
The internal wined3d interface is context_acquire()/context_enter() and context_leave(). For the purpose of this discussion, and ignoring the bit about switching a window / drawable to fullscreen mode, a "context" mostly consists of a DC, WGL context, pixel format, and swap interval. What wined3d needs is for context_enter()/context_leave() to be fairly fast, and invisible to anything executing outside of a context_enter()/context_leave() pair. Whether such a set of state is called a "surface", a "context", or something else is perhaps mostly terminology. (Although I should note that "surface" already has a different meaning within wined3d, so in that regard it's perhaps not ideal.) Similarly, wined3d doesn't really care what the driver's implementation of all this looks like.