Re: WineD3D refcounting fun part II :)
H. Verbeet wrote: I also suspect there might be a couple of circular references in the form of device-stateblock-object-device, but I haven't had time to really look into that yet. I *think* wined3d shouldn't keep references to objects in the device's Set* methods, but I remember that when I removed them in IWineD3DDeviceImpl_SetVertexShader Oliver put them back again in a later patch for some reason. It would be good to write out on paper or otherwise the hierarchy of which lifetimes a given object controls so that you can be sure of this. There should be no loops. I am not at all familiar with the wined3d code, but from what I've seen discussed here it would seem that wined3d shouldn't keep references to any objects. Instead the other objects should release the corresponding wined3d object when they are destroyed. -- Rob Shearman
Re: WineD3D refcounting fun part II :)
On 24/02/06, Robert Shearman [EMAIL PROTECTED] wrote: I am not at all familiar with the wined3d code, but from what I've seen discussed here it would seem that wined3d shouldn't keep references to any objects. Instead the other objects should release the corresponding wined3d object when they are destroyed. I don't think it does, generally. d3d8/9 objects usually contain a wined3d object that they wrap, and when the reference count of the d3d8/9 object reaches 0 it releases its wined3d object. That works fine as long as the is no relation between the wined3d objects. The moment a wined3d object keeps a reference to another wined3d object there is a problem though, because the d3d8/9 object that wraps that object can be released while there are wined3d objects that still keep a reference to that wined3d object. What I mentioned to Roderick on IRC was not really doing refcounts on wined3d objects, but on the wrapper objects instead. Essentially calling AddRef on a wined3d surface would just increase the reference count on the d3d8/9 surface. When the d3d8/9 object's refcount reaches 0 it just calls the wined3d object's cleanup function directly instead of the release. From Roderick's post I don't get the impression that that particular issue is the problem here though.
Re: WineD3D refcounting fun part II :)
Hi, If you run the test on d3d9 or using oliver's full d3d8 patch, you'll notice that not all memory is returned when quitting the program. (In 3dmark2001 this is a big issue as because of this we are out of memory after running a few demos) The problem appears that wined3d surfaces contain a reference to the wined3d device. So when you run a game the d3d8 and wined3d device refcounts are out of sync (the wined3d refcount is a lot higher because of all the refcounts from the surfaces). Are you sure that the Direct3DDevice9 should be released? Windows addrefs the DirectDraw interface when a surface is created, I'd be surprised if Microsoft changed that behavior in d3d8 or d3d9. From your description I'd say that there are still Direct3D9 surfaces hanging, which shouldn't happen. It could be that some surfaces(or textures) are in use by the WineD3DDevice, for example they are set as textures with IWineD3DDevice::SetTexture. Settexure addrefs the texture interface, which makes sense and is done by Windows too. This means that the textures must be unset before the surfaces and the device can be destroyed, if an app doesn't do so, I'd consider it buggy. I'll look at the multitexture demo, and I'll get an IRC client running. I've never used IRC before(shame on me), but I think it makes discussing this easier. Stefan pgpHd4aa2Q9u4.pgp Description: PGP signature
Re: WineD3D refcounting fun part II :)
Hi,
From the multitexture demo:
void loadTexture( void )
{
D3DXCreateTextureFromFile( g_pd3dDevice, test.bmp, g_pTexture_0 );
D3DXCreateTextureFromFile( g_pd3dDevice, checker.bmp, g_pTexture_1 );
g_pd3dDevice-SetSamplerState(0, D3DSAMP_MINFILTER, D3DTEXF_LINEAR);
g_pd3dDevice-SetSamplerState(0, D3DSAMP_MAGFILTER, D3DTEXF_LINEAR);
}
Two textures are created
//-
// Name: shutDown()
// Desc:
//-
void shutDown( void )
{
if( g_pTexture_0 != NULL )
g_pTexture_0-Release();
if( g_pVertexBuffer != NULL )
g_pVertexBuffer-Release();
if( g_pd3dDevice != NULL )
g_pd3dDevice-Release();
if( g_pD3D != NULL )
g_pD3D-Release();
}
One texture is released
void render( void )
{
snip
g_pd3dDevice-SetTexture( 0, g_pTexture_0 );
g_pd3dDevice-SetTexture( 1, g_pTexture_1 );
The texture stage is set - Texure AddRef. The texture isn't unset.
The missing release of the 2nd texture is definitly an application bug. What
remains open is if not unsetting the texture stages is a bug too. Can someone
check on Windows if (a) creating a surface or texture addrefs the
Direct3DDevice, and if (b) assigning a texture to a texture stage addrefs the
Texture? If (a) isn't true, we should unset all texture stages when the
WineD3DDevice is released, if (b) is false we shouldn't do that too. If (a)
and (b) are true, the application is buggy too. I'm sorry that I can't test,
because I've no D3D9-Equipped windows machine available ATM.
pgpe28MHFguYZ.pgp
Description: PGP signature
Re: WineD3D refcounting fun part II :)
On 24/02/06, Stefan Dösinger [EMAIL PROTECTED] wrote: Are you sure that the Direct3DDevice9 should be released? Windows addrefs the DirectDraw interface when a surface is created, I'd be surprised if Microsoft changed that behavior in d3d8 or d3d9. From your description I'd say that there are still Direct3D9 surfaces hanging, which shouldn't happen. I assume Roderick is talking about the final Release on the d3d device, ie, the one you got from CreateDevice. The surfaces are holding a couple of references to the device, and those should get released when the surface is destroyed, yes. But the application is also holding a single reference to de device. Looking at the trace from the multitexture demo, it appears GetContainer sometimes adds an extra reference to the device. I've submitted a patch for this to wine-patches.
Re: WineD3D refcounting fun part II :)
On 24/02/06, Stefan Dösinger [EMAIL PROTECTED] wrote: remains open is if not unsetting the texture stages is a bug too. Can someone check on Windows if (a) creating a surface or texture addrefs the Direct3DDevice, and if (b) assigning a texture to a texture stage addrefs the Texture? If (a) isn't true, we should unset all texture stages when the WineD3DDevice is released, if (b) is false we shouldn't do that too. If (a) and (b) are true, the application is buggy too. I'm sorry that I can't test, because I've no D3D9-Equipped windows machine available ATM. CreateTexture adds a reference to the device. SetTexture doesn't add a reference to the texture.
WineD3D refcounting fun part II :)
Hi, During the d3d8 - wined3d transition I noticed one huge refcounting issue which affects both d3d8 and d3d9. The issue is illustrated by running http://www.codesampler.com/source/dx9_multitexture.zip (the same demo is available for dx8) and then modify wined3d's globalChangeGlRam in wined3d_main.c to let it output how it adjusts the memory. If you run the test on d3d9 or using oliver's full d3d8 patch, you'll notice that not all memory is returned when quitting the program. (In 3dmark2001 this is a big issue as because of this we are out of memory after running a few demos) The problem appears that wined3d surfaces contain a reference to the wined3d device. So when you run a game the d3d8 and wined3d device refcounts are out of sync (the wined3d refcount is a lot higher because of all the refcounts from the surfaces). At the moment you quit a game the d3d8 device refcount nicely becomes zero and the object tries to kill itself. The release function also calls the wined3d device release function but the wined3d device object won't kill itself because its refcount doesn't become zero due to all the surfaces which still contain a reference to it. (the surfaces which useally are front-, backbuffers and more are normally released in the wined3d device release call when the refcount becomes zero) One way to 'fix' the problem is to let d3d8/d3d9 handle the reference counts (remove the refcounts from wined3d; and let d3d8/d3d9 call some cleanup function to remove the garbage in case its refcount becomes zero) Does anyone have a better solution? Roderick
Re: WineD3D refcounting fun part II :)
O right. From the conversation on IRC I got the impression the issue was the application calling the device's Release before calling the surface's Release. Similar to the Surface / Texture issue. On windows that should work, on wine it will probably fail at the moment. Releasing the device after all its children are deleted should work though, and it might simply be a matter of the object (be it a surface or something else) not releasing its references to the device properly upon deletion. I also suspect there might be a couple of circular references in the form of device-stateblock-object-device, but I haven't had time to really look into that yet. I *think* wined3d shouldn't keep references to objects in the device's Set* methods, but I remember that when I removed them in IWineD3DDeviceImpl_SetVertexShader Oliver put them back again in a later patch for some reason.
Re: Refcounting fun
On 13/02/06, Stefan Dösinger [EMAIL PROTECTED] wrote: I'd suggest that you send a patch taking whatever way you like, and ask AJ for his opinion. If nobody else has a real opinion on the subject, I'll probably just implement your idea, and see what problems I'll run into.
Re: Refcounting fun
I'm not sure that connection is much more complex than the way it currently is. Right now the d3d7/8/9 wrapper objects are responsible for cleaning up their wined3d object, while with that change the wined3d object would become responsible for deleting the d3d7/8/9 objects. Err, the WineD3D objects clean themselves, it's the duty of the d3d7/8/9 objects the release them if they are not needed any more. If the WineD3D refcount falls to 0 then(the general case), the WineD3D object will clean itself. If the object is in use somewhere internally in WineD3D, it's destroyed as soon WineD3D releases that refcount. I should have said releasing, of course :) It probably would. However, it's not all that different from the other option. The main difference seems to be that you don't separate the CleanUp and Release code, but use the parent in combination with a reference count that's guaranteed to be one as some sort of flag to decide whether to do the entire release code or just the cleanup part. I personally think that we should stay withhin the possibilities that COM gives us, instead of adding cross-library destroy callbacks. Why? Read the current ddraw surface creation code and the surface release code, and follow the callbacks between the different ddraw / surface types. That, and in order the use the container in that way you either have to store it somewhere in d3d7/8/9 or retrieve it on every call to Release. I'm not sure if that reduces the complexity all that much. Regardless of whether separating the Release and CleanUp code would make things a lot more complex, I do think it's somewhat cleaner. Also, note that most (all?) wined3d objects already have a CleanUp function. Well, at last it's up to Alexandre to decide which way to take. I also think that seperated release / cleanup code makes it easy to do sloopy and incorrect refcounting. The ddraw code is full of bad surface reference counting, which is cascaded by a bad reference counting in ddraw, which works for most games, but breaks e.g. Empire Earth. Well, I agree it's not a great construction. I'm just not sure if using the container in that way makes things much better. It would mean having to retrieve the surface container in each call to IDirect3D9Surface_Release. You can't easily store it in d3d9 since when the wined3d surface's container changes the d3d9 container would have to change as well, but you can't call much of anything in d3d9 without adding another callback. Also, you're now duplicating the relation between a surface and it's container in d3d9, while I don't think d3d9 should know that much about the specific implementation in wined3d. That doesn't mean I couldn't live with going with that solution, just that I'm not sure what the advantage would be.
Re: Refcounting fun
Hi, Well, I agree it's not a great construction. I'm just not sure if using the container in that way makes things much better. It would mean having to retrieve the surface container in each call to IDirect3D9Surface_Release. Do you think it might be a performance problem? You can't easily store it in d3d9 since when the wined3d surface's container changes the d3d9 container would have to change as well, but you can't call much of anything in d3d9 without adding another callback. Can containers change? I thought that the container is set at creation time and can't change? Also, you're now duplicating the relation between a surface and it's container in d3d9, while I don't think d3d9 should know that much about the specific implementation in wined3d. That doesn't mean I couldn't live with going with that solution, just that I'm not sure what the advantage would be. I think that we can't avoid keeping certain management things in d3d7/8/9 because of the differences between the versions. I've had a look at your proposal again, and I could handle it in ddraw. The only problem is that I have to be able to release(and destroy) the wined3dSurface without having the ddraw surface destroyed. This happens when I switch the surface implementation, or when I lose the primary surface due to a screen resolution change(Restoring the surface re-creates the WineD3DSurface in this case). The best solution for this would be to allow the cleanup CB to be NULL. My opinion is to use the container in the d3d9 addref and release function, as it uses only methods that COM and WineD3D already have, and both d3d9 surfaces and wined3d surfaces are valid COM objects(they have their own refcount) and we avoid callbacks. Furthermore the d3d9 and wined3d objects are clearly seperated. What's the opinion of the other directx people? Lionel? Oliver? pgpgRHGkRafal.pgp Description: PGP signature
Re: Refcounting fun
On 13/02/06, Stefan Dösinger [EMAIL PROTECTED] wrote: Hi, Well, I agree it's not a great construction. I'm just not sure if using the container in that way makes things much better. It would mean having to retrieve the surface container in each call to IDirect3D9Surface_Release. Do you think it might be a performance problem? I'm not sure, probably not that much, but it depends on whether the AddRef / Release functions are called a lot. If we use GetContainer, it would make a call to QueryInterface. If the surface hasn't got a container it makes a call to GetDevice and returns the wined3d device. We'd have to account for that as well. Or we could write a function to just retrieve the pointer. We would of course also have to get the d3d9 parent of the container. You can't easily store it in d3d9 since when the wined3d surface's container changes the d3d9 container would have to change as well, but you can't call much of anything in d3d9 without adding another callback. Can containers change? I thought that the container is set at creation time and can't change? I'm not sure if it happens or not (and if it is actually required when it happens :)), but the function to change it is there. Of course if we can be sure it never happens that would mean we could just store the pointer to the container in d3d9 during creation time and set it to NULL when releasing the container.
Re: Refcounting fun
I'm not sure, probably not that much, but it depends on whether the AddRef / Release functions are called a lot. If we use GetContainer, it would make a call to QueryInterface. If the surface hasn't got a container it makes a call to GetDevice and returns the wined3d device. We'd have to account for that as well. Or we could write a function to just retrieve the pointer. We would of course also have to get the d3d9 parent of the container. I think the returned device pointer is not a problem - just check the returned pointer against your device instead of NULL. As for GetParent / QueryInterface / GetContainer calls, it's necessary to release any unwanted increased refcount. If however, a GetContainer call for an D3D9 texture causes an AddRef or Release of that texture, the approach is impossible. I didn't find any evidence of that yet. I'm not sure if it happens or not (and if it is actually required when it happens :)), but the function to change it is there. Of course if we can be sure it never happens that would mean we could just store the pointer to the container in d3d9 during creation time and set it to NULL when releasing the container. That would make everything much easier :) I'd suggest that you send a patch taking whatever way you like, and ask AJ for his opinion. pgp7V9mp9jXzu.pgp Description: PGP signature
Refcounting fun
I've run into a somewhat annoying problem with d3d9/wined3d recently. It appears that on d3d9 on Windows a surface either forwards its AddRef / Release calls to its container if it has one, or just shares the refcount directly. Either way, it comes down to the same thing. Calling AddRef on the surface increases the texture's refcount and calling AddRef on the texture increases the surface's refcount. Same thing for Release. As a consequence, on Windows, it's perfectly legal to do something like this: /* Create a texture */ IDirect3DDevice9_CreateTexture() /* Get a surface. Adds a reference to the surface/texture combination */ IDirect3DTexture9_GetSurfaceLevel() /* Release the texture. Note that the texture isn't freed yet, because we still hold a reference to the surface */ IDirect3DTexture9_Release() Do some stuff with the surface, perhaps call GetContainer, etc /* Release the surface. The texture and all of it's surfaces will now be released. */ IDirect3DSurface9_Release() In wine, the call to IDirect3DTexture9_Release() frees the texture. (It also calls Release on it's surfaces, but since the surface had an extra reference from the call to GetSurfaceLevel the surface is still ok). Calling any function on the surface that needs access to the container will now have undefines results, most likely a crash. Naively, I tried to fix this by having the surface keep a reference to it's container. That keeps the code above from crashing, but only because it creates a circular reference between the surface and the container. Oops. Fixing the problem for wined3d is probably not even that hard. Just separate the Release code from the Cleanup code, then have WineD3DSurface forward its AddRef / Release calls to the container, if it has one. The container can then call the Cleanup code for its surfaces when it gets deleted. However, there is another problem, related to the way d3d9 (and ddraw/d3d7/d3d8, when their respective rewrites are finished) wraps wined3d. After fixing the problem for wined3d as above, the relation between IDirect3DSurface9 and IDirect3DTexture9 in wine would look roughly like this: |---| |---| | IDirect3DSurface9 | | IDirect3DTexture9 | |---| |---| ^ ^ | | || || | WineD3DSurface |-| WineD3DTexture | || || WineD3DSurface and WineD3DTexture share reference counts, IDirect3DSurface9 keeps a reference to WineD3DSurface, and IDirect3DTexture keeps a reference to WineD3DTexture. WineD3DSurface and WineD3DTexture don't keep references to their parents (and can't, without creating circular references). So releasing the texture before the surface would still not work, since the d3d9 texture still has only 1 reference, the one it got when it was created. That means that in the call to IDirect3DTexture9_Release the d3d9 texture gets freed, while the wined3d texture doesn't. When we do something with the d3d9 surface that needs the container, the wined3d surface correctly returns its container, but when d3d9 then tries to get the parent of that container it still dies. One way to solve this would be to have wined3d handle the reference counting for d3d7/8/9 as well, so that their AddRef/Release calls just call AddRef/Release on the wined3d object they wrap. That would also mean we need to pass a pointer to the d3d7/8/9 object's cleanup function. (Releasing a d3d9 surface to 0 should cause a d3d9 texture to be freed). Note that for creating a texture, currently a similair construction is used, in that IWineD3DDeviceImpl_CreateTexture gets passed a pointer to a d3d9 surface create function, in order to create the texture's surfaces. Obviously doing all that would be quite a change in the way d3d is setup in wine, and would also have some implications for the d3d7 and d3d8 rewrites. I'm wondering if this solution would be acceptable, and whether perhaps there are other, better solutions. Anyone care to comment?
Re: Refcounting fun
One way to solve this would be to have wined3d handle the reference counting for d3d7/8/9 as well, so that their AddRef/Release calls just call AddRef/Release on the wined3d object they wrap. That would also mean we need to pass a pointer to the d3d7/8/9 object's cleanup function. (Releasing a d3d9 surface to 0 should cause a d3d9 texture to be freed). Note that for creating a texture, currently a similair construction is used, in that IWineD3DDeviceImpl_CreateTexture gets passed a pointer to a d3d9 surface create function, in order to create the texture's surfaces. I have had some reference counting problems with ddraw too, but they were more simple(Mainly, that needed WineD3D objects had no ddraw counterpart, or that WineD3D destroys the ddraw parents, although they were still needed). DDraw doesn't use containers, instead it has surface attachments and complex surfaces, with some reference counting difficulties. My solution was to handle that things in ddraw. Could it be that d3d9 surfaces and textures are the same objects on windows? What happens if you QueryInterface the surface for the texture GUID? Obviously doing all that would be quite a change in the way d3d is setup in wine, and would also have some implications for the d3d7 and d3d8 rewrites. I'm wondering if this solution would be acceptable, and whether perhaps there are other, better solutions. Anyone care to comment? It might be interesting if there are more such refcount connections. For textures, my parent/child connections are: DDraw surface 1 - WineD3DTexture IParent - WineD3DSurface 1 DDraw surface 2 - WineD3DSurface 2 etc Directdraw textures are the same objects as directdraw surfaces, so they share the refcount. First, I create a ddraw surface, and create a WineD3DTexture for it. The surface is the child for texture. WineD3D calls my surface creation callback, which creates a WineD3DSurface for the ddraw surface on the first call, and a Parent object for the WineD3Dsurface. The IParent objects only purpose is to release it's child when it's destroyed. For other levels, I create a new ddraw surface, which is the parent of the new WineD3D surface, and attach it to the first ddraw surface. The attachment list is managed in ddraw. When the app releases the compound, it (hopefully) calls the release method of the first surface. This releases the WineD3D texture, which causes a release to the IParent object and the further ddraw surfaces. The parent and the ddraw surfaces release the Wined3d surfaces. (That code works for 1 level textures, I haven't yet tried an app which uses multiple levels, or I silently broke that functionality) My suggestion is to keep d3dX specific refcounting in ddraw / d3d8 / d3d9, and use the WineD3D refcounts for wined3d internal things. A quick guessis to seperate the D3D9 release code from the cleanup code, instead of doing this in WineD3D. What happens if a texture has 2 surface levels? A GetSurfaceLevel for the first level AddRefs both the texture and the surface. A GetSurfaceLevel for the secound addrefs the texture and the 2nd surface, but what happens to the first one? A Release() of the first surface Releases() the texture, what happens to the secound surface? Stefan
Re: Refcounting fun
On 12/02/06, Stefan Dösinger [EMAIL PROTECTED] wrote:
Could it be that d3d9 surfaces and textures are the same objects on windows?
What happens if you QueryInterface the surface for the texture GUID?
It returns 0x80004002 (E_NOINTERFACE). Same thing for the reverse.
When the app releases the compound, it (hopefully) calls the release method of
the first surface.
At least for d3d9, it's not guaranteed which surface / texture gets
released last. Could be the texture, could be the first surface, could
be any of the other surfaces.
My suggestion is to keep d3dX specific refcounting in ddraw / d3d8 / d3d9, and
use the WineD3D refcounts for wined3d internal things. A quick guessis to
seperate the D3D9 release code from the cleanup code, instead of doing this
in WineD3D.
With the solution mentioned in my previous post, d3d7/8/9 objects
would essentially not be real COM objects on their own, they would
just wrap the relevant wined3d AddRef/Release functions, and not have
refcounts of their own. Wined3d would be responsible for it's own
refcounting, and cleaning up the d3d7/8/9 object as part of it's own
cleanup. Separating the d3d9 cleanup and release code would obviously
be part of that. Note that if we separate the cleanup and release code
we can't just call the cleanup code from wined3d, because we don't
know the type of the parent, all we have is an IUnknown pointer. Which
is why we would have to pass a pointer to that function to wined3d
during object creation.
What happens if a texture has 2 surface levels? A GetSurfaceLevel for the
first level AddRefs both the texture and the surface. A GetSurfaceLevel for
the secound addrefs the texture and the 2nd surface, but what happens to the
first one? A Release() of the first surface Releases() the texture, what
happens to the secound surface?
The reference count appears to be shared between the texture and all
of its surfaces. This is the output of a small test program I wrote:
texture_refs.c:67:Calling CreateTexture
texture_refs.c:69:texture @ 001DB220
texture_refs.c:71:texture refs: 1
texture_refs.c:73:Calling GetSurfaceLevel (0)
texture_refs.c:75:surface 0 @ 001DB420
texture_refs.c:78:texture refs: 2
texture_refs.c:79:surface 0 refs: 2
texture_refs.c:82:QueryInterface for surface on texture returned
0x80004002, ptr
texture_refs.c:84:QueryInterface for texture on surface 0 returned
0x80004002, ptr
texture_refs.c:86:Calling GetSurfaceLevel (1)
texture_refs.c:88:surface 1 @ 001DB540
texture_refs.c:92:texture refs: 3
texture_refs.c:93:surface 0 refs: 3
texture_refs.c:94:surface 1 refs: 3
texture_refs.c:96:Calling Release on texture
texture_refs.c:101:texture refs: 2
texture_refs.c:102:surface 0 refs: 2
texture_refs.c:103:surface 1 refs: 2
texture_refs.c:105:Calling Release on surface 1
texture_refs.c:110:texture refs: 1
texture_refs.c:111:surface 0 refs: 1
texture_refs.c:112:surface 1 refs: 1
texture_refs.c:114:Calling AddRef on surface 0
texture_refs.c:119:texture refs: 2
texture_refs.c:120:surface 0 refs: 2
texture_refs.c:121:surface 1 refs: 2
The refcounts in that program are retrieved like this:
static int get_refcount(IUnknown *object)
{
IUnknown_AddRef(object);
return IUnknown_Release(object);
}
Re: Refcounting fun
With the solution mentioned in my previous post, d3d7/8/9 objects would essentially not be real COM objects on their own, they would just wrap the relevant wined3d AddRef/Release functions, and not have refcounts of their own. Wined3d would be responsible for it's own refcounting, and cleaning up the d3d7/8/9 object as part of it's own cleanup. Separating the d3d9 cleanup and release code would obviously be part of that. Note that if we separate the cleanup and release code we can't just call the cleanup code from wined3d, because we don't know the type of the parent, all we have is an IUnknown pointer. Which is why we would have to pass a pointer to that function to wined3d during object creation. I don't think that this will work for ddraw. The ddraw object connections are a little different from D3D9 / WineD3D. For example, in ddraw, the app creates a DirectDraw interface. This interface doesn't have any surfaces on it's own, the primary surfaces are created by the app. When a Direct3DDevice is created, the surfaces already exist. In D3D9, creating the Direct3DDevice automatically creates the surfaces. There's no Texture object in ddraw too. I don't think that a direct DDraw - WineD3D refcount mapping can work. What happens if a texture has 2 surface levels? A GetSurfaceLevel for the first level AddRefs both the texture and the surface. A GetSurfaceLevel for the secound addrefs the texture and the 2nd surface, but what happens to the first one? A Release() of the first surface Releases() the texture, what happens to the secound surface? The reference count appears to be shared between the texture and all of its surfaces. This is the output of a small test program I wrote: texture_refs.c:67:Calling CreateTexture texture_refs.c:69:texture @ 001DB220 texture_refs.c:71:texture refs: 1 texture_refs.c:73:Calling GetSurfaceLevel (0) texture_refs.c:75:surface 0 @ 001DB420 texture_refs.c:78:texture refs: 2 texture_refs.c:79:surface 0 refs: 2 texture_refs.c:82:QueryInterface for surface on texture returned 0x80004002, ptr texture_refs.c:84:QueryInterface for texture on surface 0 returned 0x80004002, ptr texture_refs.c:86:Calling GetSurfaceLevel (1) texture_refs.c:88:surface 1 @ 001DB540 texture_refs.c:92:texture refs: 3 texture_refs.c:93:surface 0 refs: 3 texture_refs.c:94:surface 1 refs: 3 texture_refs.c:96:Calling Release on texture texture_refs.c:101:texture refs: 2 texture_refs.c:102:surface 0 refs: 2 texture_refs.c:103:surface 1 refs: 2 texture_refs.c:105:Calling Release on surface 1 texture_refs.c:110:texture refs: 1 texture_refs.c:111:surface 0 refs: 1 texture_refs.c:112:surface 1 refs: 1 texture_refs.c:114:Calling AddRef on surface 0 texture_refs.c:119:texture refs: 2 texture_refs.c:120:surface 0 refs: 2 texture_refs.c:121:surface 1 refs: 2 Seems to be really one refcount. I'd suggest to create a addref_override and release_ovveride member for Direct3D9 Surfaces (and Direct3D8), and set them when a texture is created, and unset them when the texture is destroyed and before the surfaces are released the last time(so destroying the surface works normally with IDirect3DSurface9::Release). As a sidenote, such overrides bring some problems. The current ddraw implementation is full of them. The callbacks and the existance of 3 different IDirectDraw(Main, user, hal) implementations and 5 different IDirectDrawSurface(Main, user, hal, dib, zbuffer) implementations made it quite difficult to understand the code. I don't think that I've fully understood it by now. I'll have a look about the consequences of a WineD3DTexture - WineD3DSurface refcount connection for ddraw. I think it's not hard to cope with that case, but I can't promise right now. Can you check if there are more refcount connections? I think that I should line out the changes to WineD3D I am planning: I'm going to add some methods, like IWineD3DSurface::Blt, IWineD3DSurface::BltFast, ..., this doesn't make any difference for D3D9. I split up the WineD3DDevice initalisation code into the CreateDevice part, which doesn't initialize OpenGL, and a IWineD3DDevice::Init3D to create a swapchain and the render targets. As a counterpart, there's a IWineD3DDevice::Uninit3D, which destroys the swapchain, but not the rendertargets, and the IWineD3DDevice::Release method, which frees the rest. The necessary changes to D3D9 are a call to IWineD3DDevice::Init3D during creation, and IWineD3DDevice::Uninit3D when the d3d9 device is destroyed. The d3d9 device has to release it's rendertarget surfaces on it's own too. For 2D operation without OpenGL, I've created a secound surface implementation, IWineX11Surface(well, bad name). It replaces a few methods(Blt, Lock, Unlock, Flip, ...), but uses the same management code. CreateSurface takes another parameter which determines the surface type. To avoid a too complex code, there's no way to change a X11 surface into
Re: Refcounting fun
Hi, I've thought over this: If you want to connect the IWineD3DTexture and IWineD3DSurface refcounts only, then there's no problem for me. If you want to connect the Texture's and Surface's Parents refcount, then you have do the same for the rendertargets and the swapchain(I guess that Windows does that too), and I can use that connection to manage complex ddraw surfaces. Stefan pgpwAKmwOyez0.pgp Description: PGP signature
Re: Refcounting fun
On 12/02/06, Stefan Dösinger [EMAIL PROTECTED] wrote:
Hi,
I've thought over this: If you want to connect the IWineD3DTexture and
IWineD3DSurface refcounts only, then there's no problem for me. If you want
to connect the Texture's and Surface's Parents refcount,
Are you talking about IDirect3DSurface9 = IDirect3DTexture9 or do
you mean IDirect3DSurface9 = IWineD3DSurface?
then you have do the
same for the rendertargets and the swapchain(I guess that Windows does that
too), and I can use that connection to manage complex ddraw surfaces.
I haven't verified it, but the MSDN documentation for GetContainer
appears to imply that it does.
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/directx9_c/IDirect3DSurface9__GetContainer.asp
Unfortunately my knowledge of ddraw and d3d7 is somewhat limited, so
it's a bit hard for me to oversee all the implications those changes
would have for ddraw / d3d7. The choice for Surface and Texture as an
example is somewhat arbitrary I guess, but probably the one for which
it matters the most. (As in, is most likely to break stuff if it isn't
implemented correctly). In principle it would change for any object
that has a container or uses similar construction. Perhaps it would
even make sense to forward AddRef and Release calls for all d3d7/8/9
objects that directly wrap a wined3d object.
For reference, the code for Release CleanUp would look something
like this for the different objects:
/* WineD3D Surface */
ULONG WINAPI IWineD3DSurfaceImpl_Release(IWineD3DSurface *iface) {
IWineD3DSurfaceImpl *This = (IWineD3DSurfaceImpl *)iface;
if (This-container) {
return IWineD3DBase_Release(This-container);
} else {
/* Handle our own refcounting */
}
}
void WINAPI IWineD3DSurfaceImpl_CleanUp(IWineD3DSurface *iface) {
IWineD3DSurfaceImpl *This = (IWineD3DSurfaceImpl *)iface;
/* Do some cleaning for ourselves */
This-parentCleanUp(This-parent);
}
/* WineD3D Texture */
ULONG WINAPI IWineD3DTextureImpl_Release(IWineD3DTexture *iface) {
IWineD3DTextureImpl *This = (IWineD3DTextureImpl *)iface;
ULONG ref = InterlockedDecrement(This-resource.ref);
if (!ref) {
IWineD3DTexture_CleanUp(iface);
}
return ref;
}
void WINAPI IWineD3DTextureImpl_CleanUp(IWineD3DTexture *iface) {
IWineD3DTextureImpl *This = (IWineD3DTextureImpl *)iface;
int i;
for (i = 0; i This-baseTexture.levels; ++i) {
IWineD3DSurface_Cleanup(This-surfaces[i]);
}
/* Do some cleaning for ourselves */
This-parentCleanUp(This-parent);
}
/* IDirect3DSurface9 */
ULONG WINAPI IDirect3DSurface9Impl_Release(IDirect3DSurface9 *iface) {
IDirect3DSurface9Impl *This = (IDirect3DSurface9Impl *)iface;
return IWineD3DSurface_Release(This-wineD3DSurface);
}
void WINAPI IDirect3DSurface9Impl_CleanUp(IDirect3DSurface9 *iface) {
IDirect3DSurface9Impl *This = (IDirect3DSurface9Impl *)iface;
/* Cleanup Some stuff */
}
/* IDirect3DTexture9 */
ULONG WINAPI IDirect3DTexture9Impl_Release(IDirect3DTexture9 *iface) {
IDirect3DTexture9Impl *This = (IDirect3DTexture9Impl *)iface;
return IWineD3DTexture_Release(This-wineD3DTexture);
}
void WINAPI IDirect3DTexture9Impl_CleanUp(IDirect3DTexture9 *iface) {
IDirect3DTexture9Impl *This = (IDirect3DTexture9Impl *)iface;
/* Cleanup Some stuff */
}
AddRef would follow a similar pattern, although somewhat simpler since
it doesn't have to handle cleaning up the object.
Re: Refcounting fun
Hi,
Are you talking about IDirect3DSurface9 = IDirect3DTexture9 or do
you mean IDirect3DSurface9 = IWineD3DSurface?
I meant IWineD3DSurface = IWineD3DTexture and Parent(IWineD3DSurface) =
Parent(IWineD3DTexture). The first case has no consequences for ddraw, if the
secound one is forced by WineD3D this would cause a connection between
IDirect3DSurface9 = IDirect3DTexture9 and between {Direct Draw texture root
surface} = {Other ddraw texture surfaces}
Unfortunately my knowledge of ddraw and
d3d7 is somewhat limited, so it's a bit hard for me to oversee all the
implications those changes would have for ddraw / d3d7. The choice for
Surface and Texture as an example is somewhat arbitrary I guess, but
probably the one for which it matters the most. (As in, is most likely to
break stuff if it isn't implemented correctly). In principle it would
change for any object that has a container or uses similar construction.
Perhaps it would even make sense to forward AddRef and Release calls for
all d3d7/8/9 objects that directly wrap a wined3d object.
For reference, the code for Release CleanUp would look something
like this for the different objects:
I could deal with such a Release / cleanup difference in ddraw, but I don't
like the idea. I think it creates a too complex connection between WineD3D
and the d3d libs. My d3d9 knowledge is limited too, but what if you do
something like this: (sort of pseude-code only)
IDirect3DSurface9::AddRef
{
if(This-container /* Or get the container from WineD3D */)
{
return IUnknown_AddRef(This-container);
}
else
{
ULONG ref = This-ref++;
etc
}
}
IDirect3DSurface9::Releae
{
if(This-container /* Or get the container from WineD3D */)
{
return IUnknown_Release(This-container);
}
else
{
ULONG ref = This-ref--;
if(ref == 0)
{
/*Destroy code*/
IWineD3DSurface_Release(This-wineD3DSurface);
}
return ref;
}
}
IDirect3DTexure9::AddRef
{
/* normal addref, nothing special */
}
IDirect3DTexure9::Release
{
ULONG ref = This-ref--;
if(ref == 0)
{
for(all surfaces in this container)
{
surfaceImpl-container = NULL;
/* Or place this call in WineD3DTexture::Releaase*/
IWineD3DTexture_Release(This-wineD3DTexture);
/* IWineD3DTexture_Release will call the release method
* of the IDirect3D9Surface(because it releases the parent),
* run the surface's release code(not the container, because
* it's set to NULL and destroy the surface, because it's own
* refcount is 1 from creation, and we never changed it.
* The D3D9 surface release code releases the wineD3DSurface,
* and everthing is cleaned.
*/
HeapFree(GetProcessHeap, 0, This);
}
}
}
The important part is to set the surface's container when they are created,
before the first AddRef() or Release() is called, and to set the container to
NULL when the container is destroyed and before the surfaces are released.
You can eighter track the containers in d3d9, or use wineD3D to get the
container.
Could this work?
Stefan
pgpgDc14pzzRd.pgp
Description: PGP signature
Re: Refcounting fun
Hi
IDirect3DTexure9::Release
{
ULONG ref = This-ref--;
if(ref == 0)
{
for(all surfaces in this container)
{
surfaceImpl-container = NULL;
/* Or place this call in WineD3DTexture::Releaase*/
IWineD3DTexture_Release(This-wineD3DTexture);
/* IWineD3DTexture_Release will call the release method
* of the IDirect3D9Surface(because it releases the parent),
* run the surface's release code(not the container, because
* it's set to NULL and destroy the surface, because it's own
* refcount is 1 from creation, and we never changed it.
* The D3D9 surface release code releases the wineD3DSurface,
* and everthing is cleaned.
*/
HeapFree(GetProcessHeap, 0, This);
}
}
}
Oops, little problem here: That should be
IDirect3DTexure9::Release
{
ULONG ref = This-ref--;
if(ref == 0)
{
for(all surfaces in this container)
{
surfaceImpl-container = NULL;
/* Don't free the objects in this loop of course ;) */
}
/* Or place this call in WineD3DTexture::Releaase*/
IWineD3DTexture_Release(This-wineD3DTexture);
/* IWineD3DTexture_Release will call the release method
* of the IDirect3D9Surface(because it releases the parent),
* run the surface's release code(not the container, because
* it's set to NULL and destroy the surface, because it's own
* refcount is 1 from creation, and we never changed it.
* The D3D9 surface release code releases the wineD3DSurface,
* and everthing is cleaned.
*/
HeapFree(GetProcessHeap, 0, This);
}
}
pgpHYJNFmRtkY.pgp
Description: PGP signature
Re: Refcounting fun
On 12/02/06, Stefan Dösinger [EMAIL PROTECTED] wrote:
I meant IWineD3DSurface = IWineD3DTexture and Parent(IWineD3DSurface) =
Parent(IWineD3DTexture). The first case has no consequences for ddraw, if the
secound one is forced by WineD3D this would cause a connection between
IDirect3DSurface9 = IDirect3DTexture9 and between {Direct Draw texture root
surface} = {Other ddraw texture surfaces}
Connecting the parents' refcounts wasn't really the idea, at least not
directly. I think it's probably a good idea to have as little logic in
the wrappers as possible. However, they are of course indirectly
connected through wined3d; The connection between IDirect3DSurface9
and IDirect3DTexture9 is the reason for doing it in the first place.
For reference, the code for Release CleanUp would look something
like this for the different objects:
I could deal with such a Release / cleanup difference in ddraw, but I don't
like the idea. I think it creates a too complex connection between WineD3D
and the d3d libs.
I'm not sure that connection is much more complex than the way it
currently is. Right now the d3d7/8/9 wrapper objects are responsible
for cleaning up their wined3d object, while with that change the
wined3d object would become responsible for deleting the d3d7/8/9
objects.
The important part is to set the surface's container when they are created,
before the first AddRef() or Release() is called, and to set the container to
NULL when the container is destroyed and before the surfaces are released.
You can eighter track the containers in d3d9, or use wineD3D to get the
container.
Could this work?
It probably would. However, it's not all that different from the other
option. The main difference seems to be that you don't separate the
CleanUp and Release code, but use the parent in combination with a
reference count that's guaranteed to be one as some sort of flag to
decide whether to do the entire release code or just the cleanup part.
That, and in order the use the container in that way you either have
to store it somewhere in d3d7/8/9 or retrieve it on every call to
Release. I'm not sure if that reduces the complexity all that much.
Regardless of whether separating the Release and CleanUp code would
make things a lot more complex, I do think it's somewhat cleaner.
Also, note that most (all?) wined3d objects already have a CleanUp
function.
Re: Refcounting fun
Connecting the parents' refcounts wasn't really the idea, at least not directly. I think it's probably a good idea to have as little logic in the wrappers as possible. However, they are of course indirectly connected through wined3d; The connection between IDirect3DSurface9 and IDirect3DTexture9 is the reason for doing it in the first place. Right, I can't imagine how a IWineD3DTexture = IWineD3DSurface connection can help you, except if you use that connection to build the Direct3D9Texture = Direct3D9Surface connection. I'm not sure that connection is much more complex than the way it currently is. Right now the d3d7/8/9 wrapper objects are responsible for cleaning up their wined3d object, while with that change the wined3d object would become responsible for deleting the d3d7/8/9 objects. Err, the WineD3D objects clean themselves, it's the duty of the d3d7/8/9 objects the release them if they are not needed any more. If the WineD3D refcount falls to 0 then(the general case), the WineD3D object will clean itself. If the object is in use somewhere internally in WineD3D, it's destroyed as soon WineD3D releases that refcount. The exception is when WineD3D asked d3d7/8/9 to create objects via callbacks(e.g. the callback during texture creation), then wineD3D releases the parents of the objects it created. The parents take care for cleaning up, and they release their WineD3D child. It probably would. However, it's not all that different from the other option. The main difference seems to be that you don't separate the CleanUp and Release code, but use the parent in combination with a reference count that's guaranteed to be one as some sort of flag to decide whether to do the entire release code or just the cleanup part. I personally think that we should stay withhin the possibilities that COM gives us, instead of adding cross-library destroy callbacks. Why? Read the current ddraw surface creation code and the surface release code, and follow the callbacks between the different ddraw / surface types. That, and in order the use the container in that way you either have to store it somewhere in d3d7/8/9 or retrieve it on every call to Release. I'm not sure if that reduces the complexity all that much. Regardless of whether separating the Release and CleanUp code would make things a lot more complex, I do think it's somewhat cleaner. Also, note that most (all?) wined3d objects already have a CleanUp function. Well, at last it's up to Alexandre to decide which way to take. I also think that seperated release / cleanup code makes it easy to do sloopy and incorrect refcounting. The ddraw code is full of bad surface reference counting, which is cascaded by a bad reference counting in ddraw, which works for most games, but breaks e.g. Empire Earth. pgp6kYgx6pwvm.pgp Description: PGP signature
