[PATCH 0/4] MR6658: ddraw: Some fixes for SwapTextureHandles().
First of all it fixes a regression introduced by 5a7ff3b45c78784dca0a465d9e21483f72bce6e5 when surface handles were moved to global table but SwapTextureHandles was left behind (https://bugs.winehq.org/show_bug.cgi?id=57300). Then, I am fixing a couple of more issues with this function spotted on the way. I did some initial testing of SwapTextures across devices and that looks messy, device on which the swap is performed influences the outcome in some not immediately obvious way. I omitted the investigation of that because multiple 3d devices in ddraw is already a very exotic use case and I think we are unlikely to meet the usage of SwapTextures across multiple devices. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6658
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ddraw/tests/ddraw1.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dlls/ddraw/tests/ddraw1.c b/dlls/ddraw/tests/ddraw1.c index cfc14600c5c..d70cd8bef2a 100644 --- a/dlls/ddraw/tests/ddraw1.c +++ b/dlls/ddraw/tests/ddraw1.c @@ -15646,13 +15646,13 @@ static void test_multiple_devices(void) surface_desc.dwHeight = 256; hr = IDirectDraw_CreateSurface(ddraw, &surface_desc, &texture_surf, NULL); ok(hr == D3D_OK, "got %#lx.\n", hr); - hr = IDirectDrawSurface_QueryInterface(texture_surf, &IID_IDirect3DTexture2, (void **)&texture); + hr = IDirectDrawSurface_QueryInterface(texture_surf, &IID_IDirect3DTexture, (void **)&texture); ok(hr == D3D_OK, "got %#lx.\n", hr); hr = IDirect3DTexture_GetHandle(texture, device, &texture_handle); ok(hr == D3D_OK, "got %#lx.\n", hr); hr = IDirect3DTexture_GetHandle(texture, device2, &texture_handle2); ok(hr == D3D_OK, "got %#lx.\n", hr); - ok(texture_handle != texture_handle2, "got same handles.\n"); + ok(texture_handle == texture_handle2, "got same handles.\n"); hr = IDirect3DDevice_CreateMatrix(device, &matrix_handle); ok(hr == D3D_OK, "got %#lx.\n", hr); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6658
From: Paul Gofman <pgofman(a)codeweavers.com> Fixes a regression introduced by 5a7ff3b45c78784dca0a465d9e21483f72bce6e5. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=57300 --- dlls/ddraw/ddraw_private.h | 1 + dlls/ddraw/device.c | 5 ++--- dlls/ddraw/main.c | 8 ++++++++ dlls/ddraw/tests/ddraw1.c | 23 ++++++++++++++++++++--- dlls/ddraw/tests/ddraw2.c | 30 +++++++++++++++++++++++++++--- 5 files changed, 58 insertions(+), 9 deletions(-) diff --git a/dlls/ddraw/ddraw_private.h b/dlls/ddraw/ddraw_private.h index 7d08ffea361..1b2046c660c 100644 --- a/dlls/ddraw/ddraw_private.h +++ b/dlls/ddraw/ddraw_private.h @@ -308,6 +308,7 @@ void ddraw_handle_table_destroy(struct ddraw_handle_table *t); DWORD ddraw_allocate_handle(struct ddraw_handle_table *t, void *object, enum ddraw_handle_type type); void *ddraw_free_handle(struct ddraw_handle_table *t, DWORD handle, enum ddraw_handle_type type); void *ddraw_get_object(struct ddraw_handle_table *t, DWORD handle, enum ddraw_handle_type type); +void ddraw_set_object(struct ddraw_handle_table *t, DWORD handle, void *object); struct d3d_device { diff --git a/dlls/ddraw/device.c b/dlls/ddraw/device.c index 65e0715b5e7..126d6bf7ee9 100644 --- a/dlls/ddraw/device.c +++ b/dlls/ddraw/device.c @@ -562,7 +562,6 @@ static HRESULT WINAPI d3d_device1_GetCaps(IDirect3DDevice *iface, static HRESULT WINAPI d3d_device2_SwapTextureHandles(IDirect3DDevice2 *iface, IDirect3DTexture2 *tex1, IDirect3DTexture2 *tex2) { - struct d3d_device *device = impl_from_IDirect3DDevice2(iface); struct ddraw_surface *surf1 = unsafe_impl_from_IDirect3DTexture2(tex1); struct ddraw_surface *surf2 = unsafe_impl_from_IDirect3DTexture2(tex2); DWORD h1, h2; @@ -573,8 +572,8 @@ static HRESULT WINAPI d3d_device2_SwapTextureHandles(IDirect3DDevice2 *iface, h1 = surf1->Handle - 1; h2 = surf2->Handle - 1; - device->handle_table.entries[h1].object = surf2; - device->handle_table.entries[h2].object = surf1; + ddraw_set_object(NULL, h1, surf2); + ddraw_set_object(NULL, h2, surf1); surf2->Handle = h1 + 1; surf1->Handle = h2 + 1; diff --git a/dlls/ddraw/main.c b/dlls/ddraw/main.c index 034a7b6e406..037dfd3a6d8 100644 --- a/dlls/ddraw/main.c +++ b/dlls/ddraw/main.c @@ -233,6 +233,14 @@ void *ddraw_get_object(struct ddraw_handle_table *t, DWORD handle, enum ddraw_ha return entry->object; } +void ddraw_set_object(struct ddraw_handle_table *t, DWORD handle, void *object) +{ + if (!t) + t = &global_handle_table; + + t->entries[handle].object = object; +} + HRESULT WINAPI GetSurfaceFromDC(HDC dc, IDirectDrawSurface4 **surface, HDC *device_dc) { struct ddraw *ddraw; diff --git a/dlls/ddraw/tests/ddraw1.c b/dlls/ddraw/tests/ddraw1.c index d70cd8bef2a..0738ade1812 100644 --- a/dlls/ddraw/tests/ddraw1.c +++ b/dlls/ddraw/tests/ddraw1.c @@ -15587,16 +15587,16 @@ static void test_multiple_devices(void) 0.0f, 0.0f, 0.0f, 4.0f, }; - D3DTEXTUREHANDLE texture_handle, texture_handle2; + D3DTEXTUREHANDLE texture_handle, texture_handle2, texture_handle3; + IDirectDrawSurface *texture_surf, *texture_surf2; D3DMATERIALHANDLE mat_handle, mat_handle2; IDirect3DViewport *viewport, *viewport2; + IDirect3DTexture *texture, *texture2; IDirect3DDevice *device, *device2; - IDirectDrawSurface *texture_surf; D3DMATRIXHANDLE matrix_handle; IDirectDraw *ddraw, *ddraw2; IDirect3DMaterial *material; DDSURFACEDESC surface_desc; - IDirect3DTexture *texture; D3DMATRIX matrix; ULONG refcount; HWND window; @@ -15648,11 +15648,26 @@ static void test_multiple_devices(void) ok(hr == D3D_OK, "got %#lx.\n", hr); hr = IDirectDrawSurface_QueryInterface(texture_surf, &IID_IDirect3DTexture, (void **)&texture); ok(hr == D3D_OK, "got %#lx.\n", hr); + hr = IDirectDraw_CreateSurface(ddraw, &surface_desc, &texture_surf2, NULL); + ok(hr == D3D_OK, "got %#lx.\n", hr); + hr = IDirectDrawSurface_QueryInterface(texture_surf2, &IID_IDirect3DTexture, (void **)&texture2); + ok(hr == D3D_OK, "got %#lx.\n", hr); + hr = IDirect3DTexture_GetHandle(texture, device, &texture_handle); ok(hr == D3D_OK, "got %#lx.\n", hr); hr = IDirect3DTexture_GetHandle(texture, device2, &texture_handle2); ok(hr == D3D_OK, "got %#lx.\n", hr); ok(texture_handle == texture_handle2, "got same handles.\n"); + hr = IDirect3DTexture_GetHandle(texture2, device, &texture_handle2); + ok(hr == D3D_OK, "got %#lx.\n", hr); + hr = IDirect3DDevice_SwapTextureHandles(device, texture, texture2); + ok(hr == D3D_OK, "got %#lx.\n", hr); + hr = IDirect3DTexture_GetHandle(texture, device, &texture_handle3); + ok(hr == D3D_OK, "got %#lx.\n", hr); + ok(texture_handle3 == texture_handle2, "got different handles.\n"); + hr = IDirect3DTexture_GetHandle(texture2, device2, &texture_handle3); + ok(hr == D3D_OK, "got %#lx.\n", hr); + ok(texture_handle3 == texture_handle, "got different handles.\n"); hr = IDirect3DDevice_CreateMatrix(device, &matrix_handle); ok(hr == D3D_OK, "got %#lx.\n", hr); @@ -15664,6 +15679,8 @@ static void test_multiple_devices(void) ok(hr == D3D_OK, "got %#lx.\n", hr); ok(!memcmp(&matrix, &test_matrix, sizeof(matrix)), "matrix does not match.\n"); + IDirect3DTexture_Release(texture2); + IDirectDrawSurface_Release(texture_surf2); IDirect3DTexture_Release(texture); IDirectDrawSurface_Release(texture_surf); IDirect3DMaterial_Release(material); diff --git a/dlls/ddraw/tests/ddraw2.c b/dlls/ddraw/tests/ddraw2.c index 198e40bab55..a9a96d0761a 100644 --- a/dlls/ddraw/tests/ddraw2.c +++ b/dlls/ddraw/tests/ddraw2.c @@ -16504,15 +16504,15 @@ static void run_for_each_device_type(void (*test_func)(const GUID *)) static void test_multiple_devices(void) { - D3DTEXTUREHANDLE texture_handle, texture_handle2; + D3DTEXTUREHANDLE texture_handle, texture_handle2, texture_handle3; + IDirectDrawSurface *surface, *texture_surf, *texture_surf2; IDirect3DDevice2 *device, *device2, *device3; - IDirectDrawSurface *surface, *texture_surf; D3DMATERIALHANDLE mat_handle, mat_handle2; IDirect3DViewport2 *viewport, *viewport2; + IDirect3DTexture2 *texture, *texture2; IDirectDraw2 *ddraw, *ddraw2; IDirect3DMaterial2 *material; DDSURFACEDESC surface_desc; - IDirect3DTexture2 *texture; IDirect3D2 *d3d; ULONG refcount; DWORD value; @@ -16604,6 +16604,11 @@ static void test_multiple_devices(void) ok(hr == D3D_OK, "got %#lx.\n", hr); hr = IDirectDrawSurface_QueryInterface(texture_surf, &IID_IDirect3DTexture2, (void **)&texture); ok(hr == D3D_OK, "got %#lx.\n", hr); + hr = IDirectDraw2_CreateSurface(ddraw, &surface_desc, &texture_surf2, NULL); + ok(hr == D3D_OK, "got %#lx.\n", hr); + hr = IDirectDrawSurface_QueryInterface(texture_surf2, &IID_IDirect3DTexture2, (void **)&texture2); + ok(hr == D3D_OK, "got %#lx.\n", hr); + hr = IDirect3DTexture2_GetHandle(texture, device, &texture_handle); ok(hr == D3D_OK, "got %#lx.\n", hr); hr = IDirect3DTexture2_GetHandle(texture, device2, &texture_handle2); @@ -16612,14 +16617,33 @@ static void test_multiple_devices(void) hr = IDirect3DTexture2_GetHandle(texture, device3, &texture_handle2); ok(hr == D3D_OK, "got %#lx.\n", hr); ok(texture_handle == texture_handle2, "got different handles.\n"); + hr = IDirect3DTexture2_GetHandle(texture2, device, &texture_handle2); + ok(hr == D3D_OK, "got %#lx.\n", hr); + ok(texture_handle != texture_handle2, "got same handles.\n"); + hr = IDirect3DDevice2_SwapTextureHandles(device, texture, texture2); + ok(hr == D3D_OK, "got %#lx.\n", hr); + hr = IDirect3DTexture2_GetHandle(texture, device, &texture_handle3); + ok(hr == D3D_OK, "got %#lx.\n", hr); + ok(texture_handle3 == texture_handle2, "got different handles.\n"); + hr = IDirect3DTexture2_GetHandle(texture2, device2, &texture_handle3); + ok(hr == D3D_OK, "got %#lx.\n", hr); + ok(texture_handle3 == texture_handle, "got different handles.\n"); + hr = IDirect3DDevice2_SetRenderState(device, D3DRENDERSTATE_TEXTUREHANDLE, texture_handle); ok(hr == D3D_OK, "got %#lx.\n", hr); hr = IDirect3DDevice2_SetRenderState(device2, D3DRENDERSTATE_TEXTUREHANDLE, texture_handle); ok(hr == D3D_OK, "got %#lx.\n", hr); hr = IDirect3DDevice2_SetRenderState(device3, D3DRENDERSTATE_TEXTUREHANDLE, texture_handle); ok(hr == D3D_OK, "got %#lx.\n", hr); + hr = IDirect3DDevice2_SwapTextureHandles(device, texture, texture2); + ok(hr == S_OK, "got %#lx.\n", hr); + hr = IDirect3DDevice2_GetRenderState(device, D3DRENDERSTATE_TEXTUREHANDLE, &texture_handle2); + ok(hr == D3D_OK, "got %#lx.\n", hr); + todo_wine ok(texture_handle2 == texture_handle, "got different handles.\n"); + IDirect3DTexture2_Release(texture2); IDirect3DTexture2_Release(texture); + IDirectDrawSurface_Release(texture_surf2); IDirectDrawSurface_Release(texture_surf); IDirect3DMaterial2_Release(material); IDirect3DViewport2_Release(viewport); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6658
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ddraw/device.c | 3 +++ dlls/ddraw/tests/ddraw1.c | 6 ++++++ dlls/ddraw/tests/ddraw2.c | 6 ++++++ 3 files changed, 15 insertions(+) diff --git a/dlls/ddraw/device.c b/dlls/ddraw/device.c index 126d6bf7ee9..c090f400e32 100644 --- a/dlls/ddraw/device.c +++ b/dlls/ddraw/device.c @@ -568,6 +568,9 @@ static HRESULT WINAPI d3d_device2_SwapTextureHandles(IDirect3DDevice2 *iface, TRACE("iface %p, tex1 %p, tex2 %p.\n", iface, tex1, tex2); + if (!surf1->Handle || !surf2->Handle) + return E_INVALIDARG; + wined3d_mutex_lock(); h1 = surf1->Handle - 1; diff --git a/dlls/ddraw/tests/ddraw1.c b/dlls/ddraw/tests/ddraw1.c index 0738ade1812..167ef8a3dc4 100644 --- a/dlls/ddraw/tests/ddraw1.c +++ b/dlls/ddraw/tests/ddraw1.c @@ -15653,11 +15653,17 @@ static void test_multiple_devices(void) hr = IDirectDrawSurface_QueryInterface(texture_surf2, &IID_IDirect3DTexture, (void **)&texture2); ok(hr == D3D_OK, "got %#lx.\n", hr); + hr = IDirect3DDevice_SwapTextureHandles(device, texture, texture2); + ok(hr == E_INVALIDARG, "got %#lx.\n", hr); hr = IDirect3DTexture_GetHandle(texture, device, &texture_handle); ok(hr == D3D_OK, "got %#lx.\n", hr); + hr = IDirect3DDevice_SwapTextureHandles(device, texture, texture2); + ok(hr == E_INVALIDARG, "got %#lx.\n", hr); hr = IDirect3DTexture_GetHandle(texture, device2, &texture_handle2); ok(hr == D3D_OK, "got %#lx.\n", hr); ok(texture_handle == texture_handle2, "got same handles.\n"); + hr = IDirect3DDevice_SwapTextureHandles(device, texture, texture2); + ok(hr == E_INVALIDARG, "got %#lx.\n", hr); hr = IDirect3DTexture_GetHandle(texture2, device, &texture_handle2); ok(hr == D3D_OK, "got %#lx.\n", hr); hr = IDirect3DDevice_SwapTextureHandles(device, texture, texture2); diff --git a/dlls/ddraw/tests/ddraw2.c b/dlls/ddraw/tests/ddraw2.c index a9a96d0761a..aef8e5d5659 100644 --- a/dlls/ddraw/tests/ddraw2.c +++ b/dlls/ddraw/tests/ddraw2.c @@ -16609,14 +16609,20 @@ static void test_multiple_devices(void) hr = IDirectDrawSurface_QueryInterface(texture_surf2, &IID_IDirect3DTexture2, (void **)&texture2); ok(hr == D3D_OK, "got %#lx.\n", hr); + hr = IDirect3DDevice2_SwapTextureHandles(device, texture, texture2); + ok(hr == E_INVALIDARG, "got %#lx.\n", hr); hr = IDirect3DTexture2_GetHandle(texture, device, &texture_handle); ok(hr == D3D_OK, "got %#lx.\n", hr); + hr = IDirect3DDevice2_SwapTextureHandles(device, texture, texture2); + ok(hr == E_INVALIDARG, "got %#lx.\n", hr); hr = IDirect3DTexture2_GetHandle(texture, device2, &texture_handle2); ok(hr == D3D_OK, "got %#lx.\n", hr); ok(texture_handle == texture_handle2, "got different handles.\n"); hr = IDirect3DTexture2_GetHandle(texture, device3, &texture_handle2); ok(hr == D3D_OK, "got %#lx.\n", hr); ok(texture_handle == texture_handle2, "got different handles.\n"); + hr = IDirect3DDevice2_SwapTextureHandles(device, texture, texture2); + ok(hr == E_INVALIDARG, "got %#lx.\n", hr); hr = IDirect3DTexture2_GetHandle(texture2, device, &texture_handle2); ok(hr == D3D_OK, "got %#lx.\n", hr); ok(texture_handle != texture_handle2, "got same handles.\n"); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6658
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ddraw/device.c | 9 ++++++++- dlls/ddraw/tests/ddraw2.c | 2 +- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/dlls/ddraw/device.c b/dlls/ddraw/device.c index c090f400e32..305446ce081 100644 --- a/dlls/ddraw/device.c +++ b/dlls/ddraw/device.c @@ -564,13 +564,17 @@ static HRESULT WINAPI d3d_device2_SwapTextureHandles(IDirect3DDevice2 *iface, { struct ddraw_surface *surf1 = unsafe_impl_from_IDirect3DTexture2(tex1); struct ddraw_surface *surf2 = unsafe_impl_from_IDirect3DTexture2(tex2); - DWORD h1, h2; + DWORD h1, h2, h; + HRESULT hr; TRACE("iface %p, tex1 %p, tex2 %p.\n", iface, tex1, tex2); if (!surf1->Handle || !surf2->Handle) return E_INVALIDARG; + if (FAILED(hr = IDirect3DDevice2_GetRenderState(iface, D3DRENDERSTATE_TEXTUREHANDLE, &h))) + return hr; + wined3d_mutex_lock(); h1 = surf1->Handle - 1; @@ -582,6 +586,9 @@ static HRESULT WINAPI d3d_device2_SwapTextureHandles(IDirect3DDevice2 *iface, wined3d_mutex_unlock(); + if ((h == surf1->Handle || h == surf2->Handle) + && FAILED(hr = IDirect3DDevice2_SetRenderState(iface, D3DRENDERSTATE_TEXTUREHANDLE, h))) + return hr; return D3D_OK; } diff --git a/dlls/ddraw/tests/ddraw2.c b/dlls/ddraw/tests/ddraw2.c index aef8e5d5659..de4778e8d24 100644 --- a/dlls/ddraw/tests/ddraw2.c +++ b/dlls/ddraw/tests/ddraw2.c @@ -16645,7 +16645,7 @@ static void test_multiple_devices(void) ok(hr == S_OK, "got %#lx.\n", hr); hr = IDirect3DDevice2_GetRenderState(device, D3DRENDERSTATE_TEXTUREHANDLE, &texture_handle2); ok(hr == D3D_OK, "got %#lx.\n", hr); - todo_wine ok(texture_handle2 == texture_handle, "got different handles.\n"); + ok(texture_handle2 == texture_handle, "got different handles.\n"); IDirect3DTexture2_Release(texture2); IDirect3DTexture2_Release(texture); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6658
I tested the game with the patch, and couldn't find any issues. Thanks a lot! -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6658#note_84829
Is there an intent to use ddraw_set_object() in other places? Currently it doesn't seem to be doing much. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6658#note_85228
Well, all the access is done through helpers, global table is not exported and local to the tables handling in the corresponding source file. The other way would be to export the global table instead and use it directly, but I thought a helper which keeps that management in one place is clearer. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6658#note_85229
I'd just export the global table, personally. Or, if not, at least delete the redundant first parameter. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6658#note_85230
participants (4)
-
Elizabeth Figura (＠zfigura) -
Fabian Maurer (＠DarkShadow44)
-
Paul Gofman
-
Paul Gofman (＠gofman)