Am Donnerstag, 22. Mai 2008 16:17:26 schrieb Jens Albretsen:
Changelog: d3d1-7 surfaces was not ref counting ddraw so if there was surface and ddraw was released, wine would kill everything. Now the Return to Mysterious Island works and does a clean exit afterwards.
I think the patch is wrong. There's a test that shows that this is not happening. If it doen't break any existing tests, please add a test to demonstrate that the change is correct
On Thursday 22 May 2008 16:55:31 Stefan Dösinger wrote:
Am Donnerstag, 22. Mai 2008 16:17:26 schrieb Jens Albretsen:
Changelog: d3d1-7 surfaces was not ref counting ddraw so if there was surface and ddraw was released, wine would kill everything. Now the Return to Mysterious Island works and does a clean exit afterwards.
I think the patch is wrong.
Well, as remember this the normal procedure of d3d, textures, surfaces and streams is to hold on to ddraw/d3d until everything is destroyed or the process is terminated.
Anyway I thought that the idea about fixing regressions is to fix them for 1.0?
There's a test that shows that this is not happening.
Ehh, where? I see no test doing the following 1. DirectDrawCreate 2. CreateSurface 3. Release DirectDrawObject from 1 4. Use the surface 5. Release surface to release ddraw
If it doen't break any existing tests, please add a test to demonstrate that the change is correct
I will.
Am Donnerstag, 22. Mai 2008 19:17:34 schrieb Jens Albretsen:
Well, as remember this the normal procedure of d3d, textures, surfaces and streams is to hold on to ddraw/d3d until everything is destroyed or the process is terminated.
DirectDraw 7 does this, but DirectDraw (1) doesn't
Anyway I thought that the idea about fixing regressions is to fix them for 1.0?
Yes, but we can't fix them in an incorrect way. What happens to fix one app could break other apps
Ehh, where? I see no test doing the following
- DirectDrawCreate
- CreateSurface
- Release DirectDrawObject from 1
- Use the surface
- Release surface to release ddraw
I think I once wrote a test that does this: DirectDrawCreate CreateSurface DirectDraw_AddRef ref = DirectDraw_Release ok(ref == 1)
of course even if CreateSurface doesn't addref the publically visible refcount, it could still hold some internal reference to prevent the ddraw object from beeing destroyed. If that's the case, we need an additional test showing that the ddraw object survives even though it has ref 0
On Thursday 22 May 2008 20:08:57 you wrote:
Am Donnerstag, 22. Mai 2008 19:17:34 schrieb Jens Albretsen:
Well, as remember this the normal procedure of d3d, textures, surfaces and streams is to hold on to ddraw/d3d until everything is destroyed or the process is terminated.
DirectDraw 7 does this, but DirectDraw (1) doesn't
Anyway I thought that the idea about fixing regressions is to fix them for 1.0?
Yes, but we can't fix them in an incorrect way. What happens to fix one app could break other apps
Ehh, where? I see no test doing the following
- DirectDrawCreate
ref=1, myref=1
- CreateSurface
ref=1. myref=2
- Release DirectDrawObject from step 1
ref=0(release ddraw), myref=1
- Use the surface
crash, myref=1
- Release surface to release ddraw
dead, myref=0(release ddraw) You're sure this is not the correct ddraw behavior, I have no way of testing this at home, only got windows at work.
I think I once wrote a test that does this: DirectDrawCreate
ref=1, myref=1
CreateSurface
ref=1, myref=2
DirectDraw_AddRef
ref=2, myref=3
ref = DirectDraw_Release
ref=1, myref=2
ok(ref == 1)
So this does'nt really get directdraw object released
of course even if CreateSurface doesn't addref the publically visible refcount, it could still hold some internal reference to prevent the ddraw object from beeing destroyed. If that's the case, we need an additional test showing that the ddraw object survives even though it has ref 0
So what you're saying I should not use the public visible refcount and add a internal AddRef/Release so surfaces can keep ddraw alive this way.
<< snip >>
of course even if CreateSurface doesn't addref the publically visible refcount, it could still hold some internal reference to prevent the ddraw object from beeing destroyed. If that's the case, we need an additional test showing that the ddraw object survives even though it has ref 0
So what you're saying I should not use the public visible refcount and add a internal AddRef/Release so surfaces can keep ddraw alive this way.
Stefan can I use the numIfaces variable instead, it seems like an internal refcount for the directdraw interfaces and since directdrawsurface is a interface it would make sense or should I make yet another ref counter.
/Jens
-
Jens Albretsen -
Stefan Dösinger -
Stefan Dösinger