Re: [6/8] WineD3D: Return a different success value if D3D is not available
Aside from the fact that you can't know WineDirect3DCreate() failed because of a lack of OpenGL without looking at its implementation, I liked the create flags approach better.
Am Thursday 06 August 2009 09:35:12 schrieb Henri Verbeet:
Aside from the fact that you can't know WineDirect3DCreate() failed because of a lack of OpenGL without looking at its implementation, I liked the create flags approach better. Well, the WINED3DOK_NO3D is intended to say "you got your d3d object, but it will only do 2D drawing for you", and then its up to the client to accept this or not(Although I just see I forgot to release the returned object in this case). I don't see the problem.
I dislike create flags as much as dxVersion checks, and we should handle as much as possible without them. I feel strongly against using a create flag in patch 4. I just think passing in a flag to call X and call Y's behavior will miraculously change is ugly, although sometimes we can't avoid it. The only places where I think we really need a create flag: * Fixed buffer declarations * D3D7 VB conversion / no conversion * device vs surface palette * and the COM priv data addref or no addref, although maybe there's a way to add a release() in ddraw * the surface pitch alignment That's quite a lot of them already
2009/8/6 Stefan Dösinger <stefan(a)codeweavers.com>:
Am Thursday 06 August 2009 09:35:12 schrieb Henri Verbeet:
Aside from the fact that you can't know WineDirect3DCreate() failed because of a lack of OpenGL without looking at its implementation, I liked the create flags approach better. Well, the WINED3DOK_NO3D is intended to say "you got your d3d object, but it will only do 2D drawing for you", and then its up to the client to accept this or not(Although I just see I forgot to release the returned object in this case). I don't see the problem.
The problem is that wined3d without 3D capabilities really doesn't make a whole lot of sense for anything except ddraw with the gdi renderer. Arguably that code path shouldn't even depend on wined3d. A wined3d object without 3D capabilities is the exception, and should be handled as such.
I dislike create flags as much as dxVersion checks, and we should handle as much as possible without them. I feel strongly against using a create flag in patch 4. I just think passing in a flag to call X and call Y's behavior will miraculously change is ugly, although sometimes we can't avoid it.
If done properly, miracles have nothing to do with it... The problem with patch 4 is that you really want the initial value to change, but instead you change it afterwards, and hope you caught all the cases. Fortunately the tests should help a bit there, but I don't think it's a good principle if you can easily avoid it. Another advantage of the flags is that you have a reasonably centralised overview of differences between the different wined3d client libraries.
Am Thursday 06 August 2009 11:54:58 schrieb Henri Verbeet:
If done properly, miracles have nothing to do with it... The problem with patch 4 is that you really want the initial value to change, but instead you change it afterwards, and hope you caught all the cases. Fortunately the tests should help a bit there, but I don't think it's a good principle if you can easily avoid it. The approach doesn't scale though. Luckily we have very few different default values(that are known currently). We'd probably want some other system, like providing a template stateblock, but that seems like an overkill as well.
Another advantage of the flags is that you have a reasonably centralised overview of differences between the different wined3d client libraries. Not really, we have a number of cases in ddraw already where the default value is overwritten(e.g. depth test at device creation), and there are many other differences beyond defaults.
2009/8/6 Stefan Dösinger <stefan(a)codeweavers.com>:
Am Thursday 06 August 2009 11:54:58 schrieb Henri Verbeet:
Another advantage of the flags is that you have a reasonably centralised overview of differences between the different wined3d client libraries. Not really, we have a number of cases in ddraw already where the default value is overwritten(e.g. depth test at device creation), and there are many other differences beyond defaults.
I'm not sure we should take ddraw as an example of how we prefer to do things.
Am Thursday 06 August 2009 12:26:20 schrieb Henri Verbeet:
Not really, we have a number of cases in ddraw already where the default value is overwritten(e.g. depth test at device creation), and there are many other differences beyond defaults.
I'm not sure we should take ddraw as an example of how we prefer to do things. Maybe, maybe not, but that doesn't change the fact that d3d7 has most of the differences(with just d3d8+d3d9 all we'd have to change is the pointsizemin default value), and it doesn't make behavior flags scale better. (Of course adding a full sized system like a default stateblock for each problem doesn't scale either)
That's why I still think we should use behavior flags as a last resort, and use other problem specific ways where possible and reasonable. (an example for what's not reasonable: Clone the entire private data code in ddraw to avoid the AddRef - I clearly prefer a behavior flag for that)
2009/8/6 Stefan Dösinger <stefan(a)codeweavers.com>:
That's why I still think we should use behavior flags as a last resort, and use other problem specific ways where possible and reasonable. (an example for what's not reasonable: Clone the entire private data code in ddraw to avoid the AddRef - I clearly prefer a behavior flag for that)
Actually, if you wanted to avoid create flags, handling that AddRef in ddraw would be less ugly than what patch 4 does. (And no, you don't need to clone resource.c for that, just return the flags from resource_get_private_data() and its callers, and immediately call Release() in the appropriate case in ddraw.)
Am Thursday 06 August 2009 14:08:20 schrieb Henri Verbeet:
2009/8/6 Stefan Dösinger <stefan(a)codeweavers.com>:
That's why I still think we should use behavior flags as a last resort, and use other problem specific ways where possible and reasonable. (an example for what's not reasonable: Clone the entire private data code in ddraw to avoid the AddRef - I clearly prefer a behavior flag for that)
Actually, if you wanted to avoid create flags, handling that AddRef in ddraw would be less ugly than what patch 4 does. (And no, you don't need to clone resource.c for that, just return the flags from resource_get_private_data() and its callers, and immediately call Release() in the appropriate case in ddraw.) True, that sounds like an idea.
I still don't see the problem with the SetRenderState(or any other state set call) though. For the app CreateDevice and Reset are atomic calls. We have full control over the wined3d device. If wined3d's setRenderState suddenly gets any new side effects a SetRenderState in CreateDevice is a pretty small concern. WineD3D will not start recording a stateblock after creation(in which case SetRenderState DOES have side effects¹). I don't see in which situation it would matter that the WineD3DDevice initial state is different from the state we return it to the app. All the app bothers about is the D3D8Device's initial state(or reset state), and for that the outcome is the same. Thus I am more worried about the number of create flags getting out of control than a SetRenderState doing something unintended. ¹: This is why I need a create flag for SetRenderTarget: If a stateblock is being recorded, I can neither update the viewport from d3d8/d3d9, nor attempt to restore the old viewport in ddraw - the change would go into the recorded stateblock, not the initial one.
participants (2)
-
Henri Verbeet -
Stefan Dösinger