On 18/07/07, Stefan Dösinger stefan@codeweavers.com wrote:
This code does not really have a home. For one part the device is in charge of managing the gl fbos, on the other hand this is gl surface specific code and should go into IWineD3DSurface::Release.
No, the surface release code shouldn't know about render targets, FBOs, etc. (It's not really surface specific either, other than that the pointers we're cleaning up happen to be surfaces.) The device gets notified when resources of a certain type are destroyed here, so it can do extra cleanup on it's internal structures, so this is the correct place to do this kind of stuff.
Am Donnerstag, 19. Juli 2007 00:13 schrieb H. Verbeet:
On 18/07/07, Stefan Dösinger stefan@codeweavers.com wrote:
This code does not really have a home. For one part the device is in charge of managing the gl fbos, on the other hand this is gl surface specific code and should go into IWineD3DSurface::Release.
No, the surface release code shouldn't know about render targets, FBOs, etc. (It's not really surface specific either, other than that the pointers we're cleaning up happen to be surfaces.) The device gets notified when resources of a certain type are destroyed here, so it can do extra cleanup on it's internal structures, so this is the correct place to do this kind of stuff.
What I meant was that selecting the gl codepath or the no gl codepath. This is preferably done by calling a COM method on the right vtable(d3d surface / non d3d surface). My first patch(never sent here) checked the vtable address to find out if the surface is d3d or gdi, this was ugly. Now this patch checks a device internal status flag, which is better because the device can decide on which path to take based on it's own properties, rather than depending on the surface type.
On 19/07/07, Stefan Dösinger stefandoesinger@gmx.at wrote:
What I meant was that selecting the gl codepath or the no gl codepath. This is preferably done by calling a COM method on the right vtable(d3d surface / non d3d surface). My first patch(never sent here) checked the vtable address to find out if the surface is d3d or gdi, this was ugly. Now this patch checks a device internal status flag, which is better because the device can decide on which path to take based on it's own properties, rather than depending on the surface type.
I think the flag is fine. Executing that part of the code or not doesn't really depend on the surface type. GDI surfaces would never be attached to an FBO anyway, it's the GL_LIMITS(buffers) that would cause problems here. Making sure the limit is 0 if GL isn't available would probably work as well.
Am Donnerstag, 19. Juli 2007 09:10 schrieb H. Verbeet:
I think the flag is fine. Executing that part of the code or not doesn't really depend on the surface type. GDI surfaces would never be attached to an FBO anyway, it's the GL_LIMITS(buffers) that would cause problems here. Making sure the limit is 0 if GL isn't available would probably work as well.
Exactly, the GL_LIMITS is the problem. The gl info structure is stored in the adapter, and the adapter is found via a pointer in the device. If there is no gl there is no d3d adapter, ergo the pointer is NULL.
Making sure that GL_LIMITS does not crash but just returns 0 sounds tempting, but I am afraid that this provokes more nasty issues. We should just watch out not to call it without gl, and crashing if we do is a nice way to show us that something is wrong.
-
H. Verbeet -
Stefan Dösinger