Re : [DDRAW] Only detach surfaces when releasing a non-texture surface attached to a complex root.
Elie Morisse
lachienne at wanadoo.fr
Mon Aug 28 05:26:16 CDT 2006
Le 27.08.2006 19:23:53, Elie Morisse a écrit :
> 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.
>
>
Please use this one instead, it fixes a regression in Moto Racer 2 ( detach surfaces of complex attached surfaces when releasing the root ).
-------------- next part --------------
>From bf8b6db5c863d1359f3008dbc4f729bd8b4551d5 Mon Sep 17 00:00:00 2001
From: Elie Morisse <lachienne at wanadoo.fr>
Date: Mon, 28 Aug 2006 14:17:54 +0400
Subject: [PATCH] ddraw: Only detach surfaces when releasing a surface attached to a complex root.
This prevents various memory faults.
---
dlls/ddraw/surface.c | 224 +++++++++++++++++++++++++++-----------------------
1 files changed, 120 insertions(+), 104 deletions(-)
diff --git a/dlls/ddraw/surface.c b/dlls/ddraw/surface.c
index 587a3e9..2905b97 100644
--- a/dlls/ddraw/surface.c
+++ b/dlls/ddraw/surface.c
@@ -190,9 +190,48 @@ static void IDirectDrawSurfaceImpl_Destr
* because the 2nd surface was addref()ed when the app
* called GetAttachedSurface
*/
- WARN("(%p): Destroying surface with refount %ld\n", This, This->ref);
+ WARN("(%p): Destroying surface with refcount %ld\n", This, This->ref);
}
+ /* Now destroy the surface. Wait: It could have been released if we are a texture */
+ if(This->WineD3DSurface)
+ IWineD3DSurface_Release(This->WineD3DSurface);
+
+ /* Having a texture handle set implies that the device still exists */
+ if(This->Handle)
+ {
+ This->ddraw->d3ddevice->Handles[This->Handle - 1].ptr = NULL;
+ This->ddraw->d3ddevice->Handles[This->Handle - 1].type = DDrawHandle_Unknown;
+ }
+
+ /* Reduce the ddraw surface count */
+ InterlockedDecrement(&This->ddraw->surfaces);
+ if(This->prev)
+ This->prev->next = This->next;
+ else
+ {
+ assert(This->ddraw->surface_list == This);
+ This->ddraw->surface_list = This->next;
+ }
+ if(This->next)
+ This->next->prev = This->prev;
+
+ HeapFree(GetProcessHeap(), 0, This);
+}
+
+/*****************************************************************************
+ * IDirectDrawSurfaceImpl_DetachAllSurfaces
+ *
+ * A helper function for IDirectDrawSurface7::Release
+ *
+ * Get rid of all attached surfaces.
+ *
+ * Params:
+ * This: Surface to free
+ *
+ *****************************************************************************/
+static void IDirectDrawSurfaceImpl_DetachAllSurfaces(IDirectDrawSurfaceImpl *This)
+{
/* Check for attached surfaces and detach them */
if(This->first_attached != This)
{
@@ -221,34 +260,8 @@ static void IDirectDrawSurfaceImpl_Destr
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);
-
- /* Having a texture handle set implies that the device still exists */
- if(This->Handle)
- {
- This->ddraw->d3ddevice->Handles[This->Handle - 1].ptr = NULL;
- This->ddraw->d3ddevice->Handles[This->Handle - 1].type = DDrawHandle_Unknown;
- }
-
- /* Reduce the ddraw surface count */
- InterlockedDecrement(&This->ddraw->surfaces);
- if(This->prev)
- This->prev->next = This->next;
- else
- {
- assert(This->ddraw->surface_list == This);
- This->ddraw->surface_list = This->next;
- }
- if(This->next)
- This->next->prev = This->prev;
-
- HeapFree(GetProcessHeap(), 0, This);
}
/*****************************************************************************
@@ -288,105 +301,108 @@ IDirectDrawSurfaceImpl_Release(IDirectDr
if (ref == 0)
{
-
- IDirectDrawSurfaceImpl *surf;
- IDirectDrawImpl *ddraw;
- IUnknown *ifaceToRelease = This->ifaceToRelease;
+ IDirectDrawSurfaceImpl_DetachAllSurfaces(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);
- /* 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 */
+ }
- if(IWineD3DDevice_Uninit3D(ddraw->wineD3DDevice) != D3D_OK)
- {
- /* Not good */
- ERR("(%p) Failed to uninit 3D\n", This);
+ ddraw->d3d_initialized = FALSE;
+ ddraw->d3d_target = NULL;
+
+ /* 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_DetachAllSurfaces(surf);
+ 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-devel
mailing list