[ 3/3] ddraw: Keep track of interface attached by AddAttachedSurface and detach correct interface when parent is released.
Octavian Voicu
octavian.voicu at gmail.com
Mon Oct 10 01:07:41 CDT 2011
--
Fixes a bug that causes a stencil buffer leak in the game The Longest Journey.
Problem can be seen in the console output log attached to bug #11819.
This patch doesn't fix the original bug, but now game exits cleanly.
The root of the problem is that when releasing a surface, its attached
surfaces are detached using IDirectDraw7_DeleteAttachedSurface, regardless
of what interface was originally attached.
Here are the relevant lines from my own log, before the patch is applied:
trace:ddraw:ddraw_create_surface ddraw 0x1861e8, surface_desc 0x32ecf0, surface 0x32edc8, level 0.
trace:ddraw:ddraw_create_surface (0x1861e8) Requesting surface desc :
trace:ddraw:DDRAW_dump_members - DDSD_CAPS : DDSCAPS_3DDEVICE DDSCAPS_VIDEOMEMORY DDSCAPS_ZBUFFER
trace:ddraw:DDRAW_dump_members - DDSD_HEIGHT : 480
trace:ddraw:DDRAW_dump_members - DDSD_WIDTH : 640
trace:ddraw:DDRAW_dump_members - DDSD_PIXELFORMAT : ( DDPF_ZBUFFER , Z bits : 16)
...
trace:d3d_surface:wined3d_surface_create Created surface 0x1bf6f0.
...
trace:ddraw:ddraw_create_surface Created surface 0x1bf5e0.
trace:ddraw:ddraw_surface1_AddAttachedSurface iface 0x1bc9f8, attachment 0x1bf5f0.
trace:ddraw:ddraw_surface3_AddAttachedSurface iface 0x1bc9f0, attachment 0x1bf5e8.
...
trace:ddraw:ddraw_surface_wined3d_object_destroyed surface 0x1bc9e8.
trace:ddraw:ddraw_surface7_DeleteAttachedSurface iface 0x1bc9e8, flags 0, attachment 0x1bf5e0.
trace:ddraw:ddraw_surface_delete_attached_surface surface 0x1bc9e8, attachment 0x1bf5e0.
trace:ddraw:ddraw_surface7_Release iface 0x1bf5e0 decreasing refcount to 4294967295.
...
trace:d3d:wined3d_device_uninit_3d Releasing depth/stencil buffer 0x1bf6f0.
trace:d3d_surface:wined3d_surface_decref Surface 0x1bf6f0, container (nil) of type 0.
trace:d3d_surface:wined3d_surface_decref 0x1bf6f0 decreasing refcount to 1.
err:d3d:wined3d_device_uninit_3d Something is still holding a reference to depth/stencil buffer 0x1bf6f0.
...
fixme:d3d:wined3d_device_decref Device released with resources still bound, acceptable but unexpected.
fixme:d3d:wined3d_device_decref Leftover resource 0x1bf6f0 with type WINED3DRTYPE_SURFACE (0x1).
---
dlls/ddraw/ddraw_private.h | 1 +
dlls/ddraw/surface.c | 58 +++++++++++++++++++++++++++++++++++-------
dlls/ddraw/tests/refcount.c | 4 +-
3 files changed, 51 insertions(+), 12 deletions(-)
diff --git a/dlls/ddraw/ddraw_private.h b/dlls/ddraw/ddraw_private.h
index e75acc0..da838e5 100644
--- a/dlls/ddraw/ddraw_private.h
+++ b/dlls/ddraw/ddraw_private.h
@@ -165,6 +165,7 @@ struct IDirectDrawSurfaceImpl
/* This implementation handles attaching surfaces to other surfaces */
IDirectDrawSurfaceImpl *next_attached;
IDirectDrawSurfaceImpl *first_attached;
+ void *attached_iface;
/* Complex surfaces are organized in a tree, although the tree is degenerated to a list in most cases.
* In mipmap and primary surfaces each level has only one attachment, which is the next surface level.
diff --git a/dlls/ddraw/surface.c b/dlls/ddraw/surface.c
index 6140ee0..ef81ccd 100644
--- a/dlls/ddraw/surface.c
+++ b/dlls/ddraw/surface.c
@@ -1449,6 +1449,7 @@ static HRESULT WINAPI ddraw_surface7_AddAttachedSurface(IDirectDrawSurface7 *ifa
return hr;
}
ddraw_surface7_AddRef(attachment);
+ attachment_impl->attached_iface = attachment;
return hr;
}
@@ -1468,6 +1469,7 @@ static HRESULT WINAPI ddraw_surface4_AddAttachedSurface(IDirectDrawSurface4 *ifa
}
ddraw_surface4_AddRef(attachment);
ddraw_surface7_Release(&attachment_impl->IDirectDrawSurface7_iface);
+ attachment_impl->attached_iface = attachment;
return hr;
}
static HRESULT WINAPI ddraw_surface3_AddAttachedSurface(IDirectDrawSurface3 *iface, IDirectDrawSurface3 *attachment)
@@ -1512,6 +1514,7 @@ static HRESULT WINAPI ddraw_surface3_AddAttachedSurface(IDirectDrawSurface3 *ifa
return hr;
}
ddraw_surface3_AddRef(attachment);
+ attachment_impl->attached_iface = attachment;
return hr;
}
@@ -1531,6 +1534,7 @@ static HRESULT WINAPI ddraw_surface2_AddAttachedSurface(IDirectDrawSurface2 *ifa
}
ddraw_surface2_AddRef(attachment);
ddraw_surface3_Release(&attachment_impl->IDirectDrawSurface3_iface);
+ attachment_impl->attached_iface = attachment;
return hr;
}
@@ -1550,6 +1554,7 @@ static HRESULT WINAPI ddraw_surface1_AddAttachedSurface(IDirectDrawSurface *ifac
}
ddraw_surface1_AddRef(attachment);
ddraw_surface3_Release(&attachment_impl->IDirectDrawSurface3_iface);
+ attachment_impl->attached_iface = attachment;
return hr;
}
@@ -1624,6 +1629,10 @@ static HRESULT WINAPI ddraw_surface7_DeleteAttachedSurface(IDirectDrawSurface7 *
TRACE("iface %p, flags %#x, attachment %p.\n", iface, flags, attachment);
+ if (attachment_impl->attached_iface != &attachment_impl->IDirectDrawSurface7_iface)
+ WARN("attach_iface %p mismatch for surface %p\n", attachment_impl->attached_iface, attachment_impl);
+ attachment_impl->attached_iface = NULL;
+
hr = ddraw_surface_delete_attached_surface(This, attachment_impl);
if (FAILED(hr))
{
@@ -1642,6 +1651,10 @@ static HRESULT WINAPI ddraw_surface4_DeleteAttachedSurface(IDirectDrawSurface4 *
TRACE("iface %p, flags %#x, attachment %p.\n", iface, flags, attachment);
+ if (attachment_impl->attached_iface != &attachment_impl->IDirectDrawSurface4_iface)
+ WARN("attach_iface %p mismatch for surface %p\n", attachment_impl->attached_iface, attachment_impl);
+ attachment_impl->attached_iface = NULL;
+
hr = ddraw_surface_delete_attached_surface(This, attachment_impl);
if (FAILED(hr))
{
@@ -1659,6 +1672,10 @@ static HRESULT WINAPI ddraw_surface3_DeleteAttachedSurface(IDirectDrawSurface3 *
HRESULT hr;
TRACE("iface %p, flags %#x, attachment %p.\n", iface, flags, attachment);
+ if (attachment_impl->attached_iface != &attachment_impl->IDirectDrawSurface3_iface)
+ WARN("attach_iface %p mismatch for surface %p\n", attachment_impl->attached_iface, attachment_impl);
+ attachment_impl->attached_iface = NULL;
+
hr = ddraw_surface_delete_attached_surface(This, attachment_impl);
if (FAILED(hr))
{
@@ -1676,6 +1693,10 @@ static HRESULT WINAPI ddraw_surface2_DeleteAttachedSurface(IDirectDrawSurface2 *
HRESULT hr;
TRACE("iface %p, flags %#x, attachment %p.\n", iface, flags, attachment);
+ if (attachment_impl->attached_iface != &attachment_impl->IDirectDrawSurface2_iface)
+ WARN("attach_iface %p mismatch for surface %p\n", attachment_impl->attached_iface, attachment_impl);
+ attachment_impl->attached_iface = NULL;
+
hr = ddraw_surface_delete_attached_surface(This, attachment_impl);
if (FAILED(hr))
{
@@ -1693,6 +1714,10 @@ static HRESULT WINAPI ddraw_surface1_DeleteAttachedSurface(IDirectDrawSurface *i
HRESULT hr;
TRACE("iface %p, flags %#x, attachment %p.\n", iface, flags, attachment);
+ if (attachment_impl->attached_iface != &attachment_impl->IDirectDrawSurface_iface)
+ WARN("attach_iface %p mismatch for surface %p\n", attachment_impl->attached_iface, attachment_impl);
+ attachment_impl->attached_iface = NULL;
+
hr = ddraw_surface_delete_attached_surface(This, attachment_impl);
if (FAILED(hr))
{
@@ -5091,6 +5116,27 @@ IDirectDrawSurfaceImpl *unsafe_impl_from_IDirect3DTexture(IDirect3DTexture *ifac
return CONTAINING_RECORD(iface, IDirectDrawSurfaceImpl, IDirect3DTexture_iface);
}
+static HRESULT delete_attached_surface_helper(IDirectDrawSurfaceImpl *root, IDirectDrawSurfaceImpl *detach)
+{
+ if (detach->attached_iface == &detach->IDirectDrawSurface4_iface)
+ return IDirectDrawSurface4_DeleteAttachedSurface(&root->IDirectDrawSurface4_iface, 0,
+ &detach->IDirectDrawSurface4_iface);
+ else if (detach->attached_iface == &detach->IDirectDrawSurface3_iface)
+ return IDirectDrawSurface3_DeleteAttachedSurface(&root->IDirectDrawSurface3_iface, 0,
+ &detach->IDirectDrawSurface3_iface);
+ else if (detach->attached_iface == &detach->IDirectDrawSurface2_iface)
+ return IDirectDrawSurface2_DeleteAttachedSurface(&root->IDirectDrawSurface2_iface, 0,
+ &detach->IDirectDrawSurface2_iface);
+ else if (detach->attached_iface == &detach->IDirectDrawSurface_iface)
+ return IDirectDrawSurface_DeleteAttachedSurface(&root->IDirectDrawSurface_iface, 0,
+ &detach->IDirectDrawSurface_iface);
+ else if (detach->attached_iface != &detach->IDirectDrawSurface7_iface)
+ FIXME("attached_iface %p is invalid for surface %p\n", detach->attached_iface, detach);
+
+ return IDirectDrawSurface7_DeleteAttachedSurface(&root->IDirectDrawSurface7_iface, 0,
+ &detach->IDirectDrawSurface7_iface);
+}
+
static void STDMETHODCALLTYPE ddraw_surface_wined3d_object_destroyed(void *parent)
{
IDirectDrawSurfaceImpl *surface = parent;
@@ -5100,9 +5146,6 @@ static void STDMETHODCALLTYPE ddraw_surface_wined3d_object_destroyed(void *paren
/* Check for attached surfaces and detach them. */
if (surface->first_attached != surface)
{
- IDirectDrawSurface7 *root = &surface->first_attached->IDirectDrawSurface7_iface;
- IDirectDrawSurface7 *detach = &surface->IDirectDrawSurface7_iface;
-
/* Well, this shouldn't happen: The surface being attached is
* referenced in AddAttachedSurface(), so it shouldn't be released
* until DeleteAttachedSurface() is called, because the refcount is
@@ -5111,18 +5154,13 @@ static void STDMETHODCALLTYPE ddraw_surface_wined3d_object_destroyed(void *paren
WARN("Surface is still attached to surface %p.\n", surface->first_attached);
/* The refcount will drop to -1 here */
- if (FAILED(IDirectDrawSurface7_DeleteAttachedSurface(root, 0, detach)))
+ if (FAILED(delete_attached_surface_helper(surface->first_attached, surface)))
ERR("DeleteAttachedSurface failed.\n");
}
while (surface->next_attached)
- {
- IDirectDrawSurface7 *root = &surface->IDirectDrawSurface7_iface;
- IDirectDrawSurface7 *detach = &surface->next_attached->IDirectDrawSurface7_iface;
-
- if (FAILED(IDirectDrawSurface7_DeleteAttachedSurface(root, 0, detach)))
+ if (FAILED(delete_attached_surface_helper(surface, surface->next_attached)))
ERR("DeleteAttachedSurface failed.\n");
- }
/* Having a texture handle set implies that the device still exists. */
if (surface->Handle)
diff --git a/dlls/ddraw/tests/refcount.c b/dlls/ddraw/tests/refcount.c
index 736ce8d..392054e 100644
--- a/dlls/ddraw/tests/refcount.c
+++ b/dlls/ddraw/tests/refcount.c
@@ -270,9 +270,9 @@ static void test_ddraw_objects(void)
ref = IDirectDrawSurface_Release(surface1);
ok(!ref, "Got refcount %d, expected 0\n", ref);
ref = getRefcount( (IUnknown *) stencil1);
- todo_wine ok(ref == 1, "Got refcount %d, expected 1\n", ref);
+ ok(ref == 1, "Got refcount %d, expected 1\n", ref);
ref = IDirectDrawSurface_Release(stencil1);
- todo_wine ok(!ref, "Got refcount %d, expected 0\n", ref);
+ ok(!ref, "Got refcount %d, expected 0\n", ref);
}
else
IDirectDrawSurface_Release(surface1);
--
1.7.4.1
More information about the wine-patches
mailing list