DDraw: use proper handles
Stefan Dösinger
stefandoesinger at gmx.at
Fri Jun 16 16:44:33 CDT 2006
In ddraw a few functions work with handles to interfaces. Those handles are
basically DWORDs, and up to now those where casted to the implementation
object pointer and vice versa. As I understand it this is not 64 bit safe,
and means blindly dereferencing a DWORD passed by the app.
This patch keeps a handle to pointer array in the Direct3DDevice, which allows
checking for the handle type and bounds and doesn't need pointer to integer
casts.
This patch only changes texture and material handles. Matrix handles and
stateblock handles will follow later to avoid making the patch too big.
ChangeLog:
Stefan Dösinger: Implement a proper handle management for DDraw
-------------- next part --------------
From nobody Mon Sep 17 00:00:00 2001
From: Stefan D?singer <stefan at codeweavers.com>
Date: Fri Jun 16 23:33:34 2006 +0200
Subject: [PATCH] DDraw: Proper handle management
---
dlls/ddraw/ddraw_private.h | 21 ++++++
dlls/ddraw/device.c | 147 ++++++++++++++++++++++++++++++++++++++++----
dlls/ddraw/direct3d.c | 3 +
dlls/ddraw/material.c | 34 +++++++---
dlls/ddraw/surface.c | 7 ++
dlls/ddraw/texture.c | 22 ++++---
dlls/ddraw/viewport.c | 57 ++++++++++++++---
7 files changed, 247 insertions(+), 44 deletions(-)
e2281c9e96b03cd82dd6c4ade5ba9f6beb5f43ce
diff --git a/dlls/ddraw/ddraw_private.h b/dlls/ddraw/ddraw_private.h
index 66ce166..11071f5 100644
--- a/dlls/ddraw/ddraw_private.h
+++ b/dlls/ddraw/ddraw_private.h
@@ -232,6 +232,8 @@ struct IDirectDrawSurfaceImpl
/* For the ddraw surface list */
IDirectDrawSurfaceImpl *next;
IDirectDrawSurfaceImpl *prev;
+
+ DWORD Handle;
};
/* VTable declaration. It's located in surface.c / surface_thunks.c */
@@ -264,6 +266,19 @@ const IParentVtbl IParent_Vtbl;
/*****************************************************************************
* IDirect3DDevice implementation
*****************************************************************************/
+typedef enum
+{
+ DDrawHandle_Unknown = 0,
+ DDrawHandle_Texture = 1,
+ DDrawHandle_Material = 2
+} DDrawHandleTypes;
+
+struct HandleEntry
+{
+ void *ptr;
+ DDrawHandleTypes type;
+};
+
struct IDirect3DDeviceImpl
{
/* IUnknown */
@@ -296,6 +311,10 @@ struct IDirect3DDeviceImpl
LPBYTE vertex_buffer;
DWORD vertex_size;
DWORD buffer_size;
+
+ /* Handle management */
+ struct HandleEntry *Handles;
+ DWORD numHandles;
};
/* Vtables in various versions */
@@ -309,6 +328,7 @@ const GUID IID_D3DDEVICE_WineD3D;
/* Helper functions */
HRESULT IDirect3DImpl_GetCaps(IWineD3D *WineD3D, D3DDEVICEDESC *Desc123, D3DDEVICEDESC7 *Desc7);
+DWORD IDirect3DDeviceImpl_CreateHandle(IDirect3DDeviceImpl *This);
/* Structures */
struct EnumTextureFormatsCBS
@@ -443,6 +463,7 @@ struct IDirect3DMaterialImpl
IDirect3DDeviceImpl *active_device;
D3DMATERIAL mat;
+ DWORD Handle;
void (*activate)(IDirect3DMaterialImpl* this);
};
diff --git a/dlls/ddraw/device.c b/dlls/ddraw/device.c
index f0eb86c..8c454f5 100644
--- a/dlls/ddraw/device.c
+++ b/dlls/ddraw/device.c
@@ -286,6 +286,8 @@ IDirect3DDeviceImpl_7_Release(IDirect3DD
if (ref == 0)
{
IParent *IndexBufferParent;
+ DWORD i;
+
/* Free the index buffer */
IWineD3DDevice_SetIndices(This->wineD3DDevice,
NULL,
@@ -316,6 +318,38 @@ IDirect3DDeviceImpl_7_Release(IDirect3DD
ERR(" (%p) The wineD3D device %p was destroyed unexpectadely. Prepare for trouble\n", This, This->wineD3DDevice);
}
+ /* The texture handles should be unset by now, but there might be some bits
+ * missing in our reference counting(needs test). Do a sanity check
+ */
+ for(i = 0; i < This->numHandles; i++)
+ {
+ if(This->Handles[i].ptr)
+ {
+ switch(This->Handles[i].type)
+ {
+ case DDrawHandle_Texture:
+ {
+ IDirectDrawSurfaceImpl *surf = (IDirectDrawSurfaceImpl *) This->Handles[i].ptr;
+ FIXME("Texture Handle %ld not unset properly\n", i + 1);
+ surf->Handle = 0;
+ }
+ break;
+
+ case DDrawHandle_Material:
+ {
+ IDirect3DMaterialImpl *mat = (IDirect3DMaterialImpl *) This->Handles[i].ptr;
+ FIXME("Material handle %ld not unset properly\n", i + 1);
+ mat->Handle = 0;
+ }
+
+ default:
+ FIXME("Unknown handle %ld not unset properly\n", i + 1);
+ }
+ }
+ }
+
+ HeapFree(GetProcessHeap(), 0, This->Handles);
+
/* Release the render target and the WineD3D render target
* (See IDirect3D7::CreateDevice for more comments on this)
*/
@@ -492,22 +526,17 @@ IDirect3DDeviceImpl_2_SwapTextureHandles
IDirect3DTexture2 *Tex2)
{
ICOM_THIS_FROM(IDirect3DDeviceImpl, IDirect3DDevice2, iface);
- IWineD3DTexture *tmp;
+ DWORD swap;
IDirectDrawSurfaceImpl *surf1 = ICOM_OBJECT(IDirectDrawSurfaceImpl, IDirect3DTexture2, Tex1);
IDirectDrawSurfaceImpl *surf2 = ICOM_OBJECT(IDirectDrawSurfaceImpl, IDirect3DTexture2, Tex2);
- FIXME("(%p)->(%p,%p)\n", This, surf1, surf2);
+ TRACE("(%p)->(%p,%p)\n", This, surf1, surf2);
+ This->Handles[surf1->Handle - 1].ptr = surf2;
+ This->Handles[surf2->Handle - 1].ptr = surf1;
- /* The texture handle is simply the interface address of the WineD3DTexture
- * interface. Swap the interface pointers.
- */
- tmp = surf1->wineD3DTexture;
- surf1->wineD3DTexture = surf2->wineD3DTexture;
- surf2->wineD3DTexture = tmp;
-
- /* What about the parent? Does it have to change too? This could cause a
- * problem when Releasing the surfaces, Therefore the fixme
- */
+ swap = surf2->Handle;
+ surf2->Handle = surf1->Handle;
+ surf1->Handle = swap;
return D3D_OK;
}
@@ -2029,9 +2058,32 @@ IDirect3DDeviceImpl_7_SetRenderState(IDi
switch(RenderStateType)
{
case D3DRENDERSTATE_TEXTUREHANDLE:
- return IWineD3DDevice_SetTexture(This->wineD3DDevice,
- 0,
- (IWineD3DBaseTexture *) Value);
+ {
+ if(Value == 0)
+ {
+ return IWineD3DDevice_SetTexture(This->wineD3DDevice,
+ 0,
+ NULL);
+ }
+
+ if(Value > This->numHandles)
+ {
+ FIXME("Specified handle %ld out of range\n", Value);
+ return DDERR_INVALIDPARAMS;
+ }
+ if(This->Handles[Value - 1].type != DDrawHandle_Texture)
+ {
+ FIXME("Handle %ld isn't a texture handle\n", Value);
+ return DDERR_INVALIDPARAMS;
+ }
+ else
+ {
+ IDirectDrawSurfaceImpl *surf = (IDirectDrawSurfaceImpl *) This->Handles[Value - 1].ptr;
+ return IWineD3DDevice_SetTexture(This->wineD3DDevice,
+ 0,
+ (IWineD3DBaseTexture *) surf->wineD3DTexture);
+ }
+ }
case D3DRENDERSTATE_TEXTUREMAG:
{
@@ -4572,3 +4624,68 @@ const IDirect3DDeviceVtbl IDirect3DDevic
Thunk_IDirect3DDeviceImpl_1_BeginScene,
Thunk_IDirect3DDeviceImpl_1_GetDirect3D
};
+
+/*****************************************************************************
+ * IDirect3DDeviceImpl_CreateHandle
+ *
+ * Not called from the VTable
+ *
+ * Some older interface versions operate with handles, which are basically
+ * DWORDs which identify an interface, for example
+ * IDirect3DDevice::SetRenderState with DIRECT3DRENDERSTATE_TEXTUREHANDLE
+ *
+ * Those handle could be just casts to the interface pointers or vice versa,
+ * but that is not 64 bit safe and would mean blindly derefering a DWORD
+ * passed by the app. Instead there is a dynamic array in the device which
+ * keeps a DWORD to pointer information and a type for the handle.
+ *
+ * Basically this array only grows, when a handle is freed its pointer is
+ * just set to NULL. There will be much more reads from the array than
+ * insertion operations, so a dynamic array is fine.
+ *
+ * Params:
+ * This: D3DDevice implementation for which this handle should be created
+ *
+ * Returns:
+ * A free handle on success
+ * 0 on failure
+ *
+ *****************************************************************************/
+DWORD
+IDirect3DDeviceImpl_CreateHandle(IDirect3DDeviceImpl *This)
+{
+ DWORD i;
+ struct HandleEntry *oldHandles = This->Handles;
+
+ TRACE("(%p)\n", This);
+
+ for(i = 0; i < This->numHandles; i++)
+ {
+ if(This->Handles[i].ptr == NULL &&
+ This->Handles[i].type == DDrawHandle_Unknown)
+ {
+ TRACE("Reusing freed handle %ld\n", i + 1);
+ return i + 1;
+ }
+ }
+
+ TRACE("Growing the handle array\n");
+
+ This->numHandles++;
+ This->Handles = HeapAlloc(GetProcessHeap(), 0, sizeof(struct HandleEntry) * This->numHandles);
+ if(!This->Handles)
+ {
+ ERR("Out of memory\n");
+ This->Handles = oldHandles;
+ This->numHandles--;
+ return 0;
+ }
+ if(oldHandles)
+ {
+ memcpy(This->Handles, oldHandles, (This->numHandles - 1) * sizeof(struct HandleEntry));
+ HeapFree(GetProcessHeap(), 0, oldHandles);
+ }
+
+ TRACE("Returning %ld\n", This->numHandles);
+ return This->numHandles;
+}
diff --git a/dlls/ddraw/direct3d.c b/dlls/ddraw/direct3d.c
index 4691ebf..23efef9 100644
--- a/dlls/ddraw/direct3d.c
+++ b/dlls/ddraw/direct3d.c
@@ -729,6 +729,9 @@ IDirect3DImpl_7_CreateDevice(IDirect3D7
object->material = 0;
object->target = target;
+ object->Handles = NULL;
+ object->numHandles = 0;
+
/* This is for convenience */
object->wineD3DDevice = This->wineD3DDevice;
diff --git a/dlls/ddraw/material.c b/dlls/ddraw/material.c
index 36e617d..c2acce1 100644
--- a/dlls/ddraw/material.c
+++ b/dlls/ddraw/material.c
@@ -147,9 +147,16 @@ IDirect3DMaterialImpl_Release(IDirect3DM
TRACE("(%p)->() decrementing from %lu.\n", This, ref + 1);
- if (!ref) {
+ if (!ref)
+ {
+ if(This->Handle)
+ {
+ This->ddraw->d3ddevice->Handles[This->Handle - 1].ptr = NULL;
+ This->ddraw->d3ddevice->Handles[This->Handle - 1].type = DDrawHandle_Unknown;
+ }
+
HeapFree(GetProcessHeap(), 0, This);
- return 0;
+ return 0;
}
return ref;
}
@@ -301,15 +308,24 @@ IDirect3DMaterialImpl_GetHandle(IDirect3
D3DMATERIALHANDLE *lpHandle)
{
ICOM_THIS_FROM(IDirect3DMaterialImpl, IDirect3DMaterial3, iface);
- TRACE("(%p/%p)->(%p,%p)\n", This, iface, lpDirect3DDevice3, lpHandle);
-
- This->active_device = ICOM_OBJECT(IDirect3DDeviceImpl, IDirect3DDevice3, lpDirect3DDevice3);
- *lpHandle = (DWORD) This; /* Warning: this is not 64 bit clean.
- Maybe also we need to store this material somewhere in the device ? */
+ IDirect3DDeviceImpl *device = ICOM_OBJECT(IDirect3DDeviceImpl, IDirect3DDevice3, lpDirect3DDevice3);
+ TRACE("(%p/%p)->(%p,%p)\n", This, iface, device, lpHandle);
+ if(!This->Handle)
+ {
+ This->Handle = IDirect3DDeviceImpl_CreateHandle(device);
+ if(!This->Handle)
+ {
+ ERR("Error creating a handle\n");
+ return DDERR_INVALIDPARAMS; /* Unchecked */
+ }
+ device->Handles[This->Handle - 1].ptr = This;
+ device->Handles[This->Handle - 1].type = DDrawHandle_Material;
+ }
+ *lpHandle = This->Handle;
TRACE(" returning handle %08lx.\n", *lpHandle);
-
- return DD_OK;
+
+ return D3D_OK;
}
static HRESULT WINAPI
diff --git a/dlls/ddraw/surface.c b/dlls/ddraw/surface.c
index 212b9b3..7de5acd 100644
--- a/dlls/ddraw/surface.c
+++ b/dlls/ddraw/surface.c
@@ -229,6 +229,13 @@ static void IDirectDrawSurfaceImpl_Destr
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)
diff --git a/dlls/ddraw/texture.c b/dlls/ddraw/texture.c
index 07b5edb..c3a2717 100644
--- a/dlls/ddraw/texture.c
+++ b/dlls/ddraw/texture.c
@@ -45,6 +45,7 @@
#include "wine/debug.h"
WINE_DEFAULT_DEBUG_CHANNEL(d3d7);
+WINE_DECLARE_DEBUG_CHANNEL(ddraw_thunk);
/*****************************************************************************
* IUnknown interfaces. They are thunks to IDirectDrawSurface7
@@ -218,14 +219,16 @@ IDirect3DTextureImpl_GetHandle(IDirect3D
TRACE("(%p)->(%p,%p)\n", This, d3d, lpHandle);
- /* The handle is the WineD3DTexture interface. SetRenderState depends on this */
- if(This->ddraw->wineD3DDevice)
- *lpHandle = (D3DTEXTUREHANDLE) This->wineD3DTexture;
- else
+ if(!This->Handle)
{
- /* This is to fool applications which create a texture without a D3DDevice */
- *lpHandle = (D3DTEXTUREHANDLE) This;
+ This->Handle = IDirect3DDeviceImpl_CreateHandle(d3d);
+ if(This->Handle)
+ {
+ d3d->Handles[This->Handle - 1].ptr = This;
+ d3d->Handles[This->Handle - 1].type = DDrawHandle_Texture;
+ }
}
+ *lpHandle = This->Handle;
TRACE(" returning handle %08lx.\n", *lpHandle);
@@ -238,10 +241,11 @@ Thunk_IDirect3DTextureImpl_1_GetHandle(I
LPD3DTEXTUREHANDLE lpHandle)
{
ICOM_THIS_FROM(IDirectDrawSurfaceImpl, IDirect3DTexture, iface);
- TRACE("(%p)->(%p,%p) thunking to IDirect3DTexture2 interface.\n", This, lpDirect3DDevice, lpHandle);
+ IDirect3DDeviceImpl *d3d = ICOM_OBJECT(IDirect3DDeviceImpl, IDirect3DDevice, lpDirect3DDevice);
+ TRACE_(ddraw_thunk)("(%p)->(%p,%p) thunking to IDirect3DTexture2 interface.\n", This, d3d, lpHandle);
- return IDirect3DTexture2_GetHandle(COM_INTERFACE_CAST(IDirectDrawSurfaceImpl, IDirect3DTexture, IDirect3DTexture2, iface),
- COM_INTERFACE_CAST(IDirect3DDeviceImpl, IDirect3DDevice, IDirect3DDevice2, lpDirect3DDevice),
+ return IDirect3DTexture2_GetHandle(ICOM_INTERFACE(This, IDirect3DTexture2),
+ ICOM_INTERFACE(d3d, IDirect3DDevice2),
lpHandle);
}
diff --git a/dlls/ddraw/viewport.c b/dlls/ddraw/viewport.c
index f1ac9e3..84c17be 100644
--- a/dlls/ddraw/viewport.c
+++ b/dlls/ddraw/viewport.c
@@ -384,14 +384,33 @@ IDirect3DViewportImpl_SetBackground(IDir
D3DMATERIALHANDLE hMat)
{
ICOM_THIS_FROM(IDirect3DViewportImpl, IDirect3DViewport3, iface);
- TRACE("(%p)->(%08lx)\n", This, (DWORD) hMat);
-
- This->background = (IDirect3DMaterialImpl *) hMat;
- TRACE(" setting background color : %f %f %f %f\n",
- This->background->mat.u.diffuse.u1.r,
- This->background->mat.u.diffuse.u2.g,
- This->background->mat.u.diffuse.u3.b,
- This->background->mat.u.diffuse.u4.a);
+ TRACE("(%p)->(%ld)\n", This, (DWORD) hMat);
+
+ if(hMat && hMat > This->ddraw->d3ddevice->numHandles)
+ {
+ WARN("Specified Handle %ld out of range\n", hMat);
+ return DDERR_INVALIDPARAMS;
+ }
+ else if(hMat && This->ddraw->d3ddevice->Handles[hMat - 1].type != DDrawHandle_Material)
+ {
+ WARN("Handle %ld is not a material handle\n", hMat);
+ return DDERR_INVALIDPARAMS;
+ }
+
+ if(hMat)
+ {
+ This->background = (IDirect3DMaterialImpl *) This->ddraw->d3ddevice->Handles[hMat - 1].ptr;
+ TRACE(" setting background color : %f %f %f %f\n",
+ This->background->mat.u.diffuse.u1.r,
+ This->background->mat.u.diffuse.u2.g,
+ This->background->mat.u.diffuse.u3.b,
+ This->background->mat.u.diffuse.u4.a);
+ }
+ else
+ {
+ This->background = NULL;
+ TRACE("Setting background to NULL\n");
+ }
return D3D_OK;
}
@@ -406,8 +425,7 @@ IDirect3DViewportImpl_SetBackground(IDir
* lpValid: is set to FALSE if no background is set, TRUE if one is set
*
* Returns:
- * D3D_OK, because it's a stub
- * (DDERR_INVALIDPARAMS if Mat or Valid is NULL)
+ * D3D_OK
*
*****************************************************************************/
static HRESULT WINAPI
@@ -416,7 +434,24 @@ IDirect3DViewportImpl_GetBackground(IDir
BOOL *lpValid)
{
ICOM_THIS_FROM(IDirect3DViewportImpl, IDirect3DViewport3, iface);
- FIXME("(%p)->(%p,%p): stub!\n", This, lphMat, lpValid);
+ TRACE("(%p)->(%p,%p)\n", This, lphMat, lpValid);
+
+ if(lpValid)
+ {
+ *lpValid = This->background != NULL;
+ }
+ if(lphMat)
+ {
+ if(This->background)
+ {
+ *lphMat = This->background->Handle;
+ }
+ else
+ {
+ *lphMat = 0;
+ }
+ }
+
return D3D_OK;
}
--
1.2.4
More information about the wine-patches
mailing list