[PATCH v2 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. -- v2: ddraw: Update state if d3d_device2_SwapTextureHandles() results in texture change. ddraw: Validate handles in d3d_device2_SwapTextureHandles(). ddraw: Use global handle table in d3d_device2_SwapTextureHandles(). ddraw/tests: Fix texture interface IID in test_multiple_devices() for ddraw1. 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 | 10 +++++++++- dlls/ddraw/tests/ddraw1.c | 23 ++++++++++++++++++++--- dlls/ddraw/tests/ddraw2.c | 30 +++++++++++++++++++++++++++--- 5 files changed, 59 insertions(+), 10 deletions(-) diff --git a/dlls/ddraw/ddraw_private.h b/dlls/ddraw/ddraw_private.h index 7d08ffea361..36c63ee1160 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); +extern struct ddraw_handle_table global_handle_table; struct d3d_device { diff --git a/dlls/ddraw/device.c b/dlls/ddraw/device.c index 65e0715b5e7..3fbbb8da722 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; + global_handle_table.entries[h1].object = surf2; + global_handle_table.entries[h2].object = surf1; surf2->Handle = h1 + 1; surf1->Handle = h2 + 1; diff --git a/dlls/ddraw/main.c b/dlls/ddraw/main.c index 034a7b6e406..499ea8146e6 100644 --- a/dlls/ddraw/main.c +++ b/dlls/ddraw/main.c @@ -37,7 +37,7 @@ static HINSTANCE instance; /* value of ForceRefreshRate */ DWORD force_refresh_rate = 0; -static struct ddraw_handle_table global_handle_table; +struct ddraw_handle_table global_handle_table; /* Structure for converting DirectDrawEnumerateA to DirectDrawEnumerateExA */ struct callback_info @@ -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 3fbbb8da722..c50f3ffad52 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 c50f3ffad52..399857adf9d 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
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=149051 Your paranoid android. === w10pro64 (64 bit report) === ddraw: ddraw1.c:8134: Test failed: Failed to set foreground window. ddraw1.c:8136: Test failed: Got unexpected hr 0. ddraw1.c:8138: Test failed: Got unexpected hr 0. ddraw1.c:8140: Test failed: Got unexpected hr 0. ddraw1.c:8142: Test failed: Got unexpected hr 0. ddraw1.c:8152: Test failed: Got unexpected hr 0. ddraw1.c:8154: Test failed: Got unexpected hr 0. ddraw1.c:8156: Test failed: Got unexpected hr 0. ddraw1.c:8160: Test failed: Failed to set foreground window. ddraw1.c:8162: Test failed: Got unexpected hr 0. ddraw1.c:8164: Test failed: Got unexpected hr 0. ddraw1.c:8170: Test failed: Got unexpected hr 0. ddraw1.c:8218: Test failed: Failed to set foreground window. ddraw1.c:8230: Test failed: Failed to set foreground window.
v2: - do not introduce handle setting helper. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6658#note_85234
participants (3)
-
Marvin -
Paul Gofman -
Paul Gofman (＠gofman)