[DDRAW] Only detach surfaces when releasing a non-texture surface
attached to a complex root.
Elie Morisse
lachienne at wanadoo.fr
Sun Aug 27 10:23:53 CDT 2006
Refcounting fix, Wine devs tell me if I'm wrong as from now on I get something like this w/ The Nomad Soul :
fixme:d3d:IWineD3DDeviceImpl_Release (0x1248028) Device released with resources still bound, acceptable but unexpected
fixme:d3d:dumpResources Leftover resource 0x217f988 with type 1,WINED3DRTYPE_SURFACE
fixme:d3d:dumpResources Leftover resource 0x1abca8 with type 1,WINED3DRTYPE_SURFACE
Other games I tried aren't affected though, so this one may just be buggy..
Good part is this prevents memory faults in The Nomad Soul at exit and when switching resolution.
-------------- next part --------------
>From 85cd13c76490d720b45162ce97923bb9b774d573 Mon Sep 17 00:00:00 2001
From: Elie Morisse <lachienne at wanadoo.fr>
Date: Sat, 26 Aug 2006 12:10:01 +0400
Subject: [PATCH] ddraw: Only detach surfaces when releasing a non-texture surface attached to
a complex root.
This prevents a NULL deference in IDirectDrawSurfaceImpl_Release and memory faults in various
situations.
---
dlls/ddraw/surface.c | 215 +++++++++++++++++++++++++-------------------------
1 files changed, 107 insertions(+), 108 deletions(-)
diff --git a/dlls/ddraw/surface.c b/dlls/ddraw/surface.c
index 587a3e9..74fdd35 100644
--- a/dlls/ddraw/surface.c
+++ b/dlls/ddraw/surface.c
@@ -193,38 +193,6 @@ static void IDirectDrawSurfaceImpl_Destr
WARN("(%p): Destroying surface with refount %ld\n", This, This->ref);
}
- /* Check for attached surfaces and detach them */
- if(This->first_attached != This)
- {
- /* Well, this shouldn't happen: The surface being attached is addref()ed
- * in AddAttachedSurface, so it shouldn't be released until DeleteAttachedSurface
- * is called, because the refcount is held. It looks like the app released()
- * it often enough to force this
- */
- IDirectDrawSurface7 *root = ICOM_INTERFACE(This->first_attached, IDirectDrawSurface7);
- IDirectDrawSurface7 *detach = ICOM_INTERFACE(This, IDirectDrawSurface7);
-
- FIXME("(%p) Freeing a surface that is attached to surface %p\n", This, This->first_attached);
-
- /* The refcount will drop to -1 here */
- if(IDirectDrawSurface7_DeleteAttachedSurface(root, 0, detach) != DD_OK)
- {
- ERR("(%p) DeleteAttachedSurface failed!\n", This);
- }
- }
-
- while(This->next_attached != NULL)
- {
- IDirectDrawSurface7 *root = ICOM_INTERFACE(This, IDirectDrawSurface7);
- IDirectDrawSurface7 *detach = ICOM_INTERFACE(This->next_attached, IDirectDrawSurface7);
-
- if(IDirectDrawSurface7_DeleteAttachedSurface(root, 0, detach) != DD_OK)
- {
- ERR("(%p) DeleteAttachedSurface failed!\n", This);
- assert(0);
- }
- }
-
/* Now destroy the surface. Wait: It could have been released if we are a texture */
if(This->WineD3DSurface)
IWineD3DSurface_Release(This->WineD3DSurface);
@@ -288,105 +256,136 @@ IDirectDrawSurfaceImpl_Release(IDirectDr
if (ref == 0)
{
+ /* Check for attached surfaces and detach them */
+ if(This->first_attached != This)
+ {
+ /* Well, this shouldn't happen: The surface being attached is addref()ed
+ * in AddAttachedSurface, so it shouldn't be released until DeleteAttachedSurface
+ * is called, because the refcount is held. It looks like the app released()
+ * it often enough to force this
+ */
+ IDirectDrawSurface7 *root = ICOM_INTERFACE(This->first_attached, IDirectDrawSurface7);
+ IDirectDrawSurface7 *detach = ICOM_INTERFACE(This, IDirectDrawSurface7);
+
+ FIXME("(%p) Freeing a surface that is attached to surface %p\n", This, This->first_attached);
+
+ /* The refcount will drop to -1 here */
+ if(IDirectDrawSurface7_DeleteAttachedSurface(root, 0, detach) != DD_OK)
+ {
+ ERR("(%p) DeleteAttachedSurface failed!\n", This);
+ }
+ }
- IDirectDrawSurfaceImpl *surf;
- IDirectDrawImpl *ddraw;
- IUnknown *ifaceToRelease = This->ifaceToRelease;
+ while(This->next_attached != NULL)
+ {
+ IDirectDrawSurface7 *root = ICOM_INTERFACE(This, IDirectDrawSurface7);
+ IDirectDrawSurface7 *detach = ICOM_INTERFACE(This->next_attached, IDirectDrawSurface7);
+
+ if(IDirectDrawSurface7_DeleteAttachedSurface(root, 0, detach) != DD_OK)
+ {
+ ERR("(%p) DeleteAttachedSurface failed!\n", This);
+ }
+ }
/* Destroy all complex attached surfaces
* Therefore, start with the first surface,
* except for textures. Not entirely sure what has
* to happen exactly in this case
*/
- if( (This->first_complex != This) && !(This->surface_desc.ddsCaps.dwCaps & DDSCAPS_TEXTURE))
+ if(This->first_complex == This)
{
- FIXME("(%p) Destroying a surface which is a attached to a complex root %p\n", This, This->first_complex);
- }
- ddraw = This->ddraw;
+ IDirectDrawSurfaceImpl *surf;
+ IDirectDrawImpl *ddraw;
+ IUnknown *ifaceToRelease = This->ifaceToRelease;
+
+ ddraw = This->ddraw;
+
+ /* If it's a texture, destroy the WineD3DTexture.
+ * WineD3D will destroy the IParent interfaces
+ * of the sublevels, which destroys the WineD3DSurfaces.
+ * Set the surfaces to NULL to avoid destroying them again later
+ */
+ if(This->wineD3DTexture)
+ {
+ IWineD3DTexture_Release(This->wineD3DTexture);
+ }
+ /* If it's the RenderTarget, destroy the d3ddevice */
+ else if( (ddraw->d3d_initialized) && (This == ddraw->d3d_target))
+ {
+ TRACE("(%p) Destroying the render target, uninitializing D3D\n", This);
- /* If it's a texture, destroy the WineD3DTexture.
- * WineD3D will destroy the IParent interfaces
- * of the sublevels, which destroys the WineD3DSurfaces.
- * Set the surfaces to NULL to avoid destroying them again later
- */
- if(This->wineD3DTexture)
- {
- IWineD3DTexture_Release(This->wineD3DTexture);
- }
- /* If it's the RenderTarget, destroy the d3ddevice */
- else if( (ddraw->d3d_initialized) && (This == ddraw->d3d_target))
- {
- TRACE("(%p) Destroying the render target, uninitializing D3D\n", This);
+ /* Unset any index buffer, just to be sure */
+ IWineD3DDevice_SetIndices(ddraw->wineD3DDevice, NULL, 0);
+
+ if(IWineD3DDevice_Uninit3D(ddraw->wineD3DDevice) != D3D_OK)
+ {
+ /* Not good */
+ ERR("(%p) Failed to uninit 3D\n", This);
+ }
+ else
+ {
+ /* Free the d3d window if one was created */
+ if(ddraw->d3d_window != 0)
+ {
+ TRACE(" (%p) Destroying the hidden render window %p\n", This, ddraw->d3d_window);
+ DestroyWindow(ddraw->d3d_window);
+ ddraw->d3d_window = 0;
+ }
+ /* Unset the pointers */
+ }
- /* Unset any index buffer, just to be sure */
- IWineD3DDevice_SetIndices(ddraw->wineD3DDevice, NULL, 0);
+ ddraw->d3d_initialized = FALSE;
+ ddraw->d3d_target = NULL;
- if(IWineD3DDevice_Uninit3D(ddraw->wineD3DDevice) != D3D_OK)
- {
- /* Not good */
- ERR("(%p) Failed to uninit 3D\n", This);
+ /* Write a trace because D3D unloading was the reason for many
+ * crashes during development.
+ */
+ TRACE("(%p) D3D unloaded\n", This);
}
- else
+ else if(This->surface_desc.ddsCaps.dwCaps & (DDSCAPS_PRIMARYSURFACE |
+ DDSCAPS_3DDEVICE |
+ DDSCAPS_TEXTURE ) )
{
- /* Free the d3d window if one was created */
- if(ddraw->d3d_window != 0)
+ /* It's a render target, but no swapchain was created.
+ * The IParent interfaces have to be released manually.
+ * The same applies for textures without an
+ * IWineD3DTexture object attached
+ */
+ surf = This;
+ while(surf)
{
- TRACE(" (%p) Destroying the hidden render window %p\n", This, ddraw->d3d_window);
- DestroyWindow(ddraw->d3d_window);
- ddraw->d3d_window = 0;
+ IParent *Parent;
+
+ IWineD3DSurface_GetParent(surf->WineD3DSurface,
+ (IUnknown **) &Parent);
+ IParent_Release(Parent); /* For the getParent */
+ IParent_Release(Parent); /* To release it */
+ surf = surf->next_complex;
}
- /* Unset the pointers */
}
- ddraw->d3d_initialized = FALSE;
- ddraw->d3d_target = NULL;
+ /* The refcount test shows that the palette is detached when the surface is destroyed */
+ IDirectDrawSurface7_SetPalette(ICOM_INTERFACE(This, IDirectDrawSurface7),
+ NULL);
- /* Write a trace because D3D unloading was the reason for many
- * crashes during development.
- */
- TRACE("(%p) D3D unloaded\n", This);
- }
- else if(This->surface_desc.ddsCaps.dwCaps & (DDSCAPS_PRIMARYSURFACE |
- DDSCAPS_3DDEVICE |
- DDSCAPS_TEXTURE ) )
- {
- /* It's a render target, but no swapchain was created.
- * The IParent interfaces have to be released manually.
- * The same applies for textures without an
- * IWineD3DTexture object attached
- */
- surf = This;
- while(surf)
+ /* Loop through all complex attached surfaces,
+ * and destroy them
+ */
+ while( (surf = This->next_complex) )
{
- IParent *Parent;
-
- IWineD3DSurface_GetParent(surf->WineD3DSurface,
- (IUnknown **) &Parent);
- IParent_Release(Parent); /* For the getParent */
- IParent_Release(Parent); /* To release it */
- surf = surf->next_complex;
+ This->next_complex = surf->next_complex; /* Unchain it from the complex listing */
+ IDirectDrawSurfaceImpl_Destroy(surf); /* Destroy it */
}
- }
- /* The refcount test shows that the palette is detached when the surface is destroyed */
- IDirectDrawSurface7_SetPalette(ICOM_INTERFACE(This, IDirectDrawSurface7),
- NULL);
+ /* Destroy the surface.
+ */
+ IDirectDrawSurfaceImpl_Destroy(This);
- /* Loop through all complex attached surfaces,
- * and destroy them
- */
- while( (surf = This->next_complex) )
- {
- This->next_complex = surf->next_complex; /* Unchain it from the complex listing */
- IDirectDrawSurfaceImpl_Destroy(surf); /* Destroy it */
+ /* Reduce the ddraw refcount */
+ IUnknown_Release(ifaceToRelease);
+ } else {
+ WARN("(%p) Releasing a surface which is a attached to a complex root %p\n", This, This->first_complex);
}
-
- /* Destroy the surface.
- */
- IDirectDrawSurfaceImpl_Destroy(This);
-
- /* Reduce the ddraw refcount */
- IUnknown_Release(ifaceToRelease);
}
return ref;
--
1.4.2
More information about the wine-patches
mailing list