>From bf8b6db5c863d1359f3008dbc4f729bd8b4551d5 Mon Sep 17 00:00:00 2001 From: Elie Morisse 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