The first two patches here address an issue I encountered when using Wine's d3dx9 with DXVK in the game Command and Conquer 3. The game calls `D3DXLoadSurfaceFromMemory` on a multisampled render target. This ends up going through the `lock_surface()` helper in d3dx9, which eventually calls `UpdateSurface()` in `unlock_surface()` to copy the staging surface back to the multisampled render target. This succeeds on wined3d even though it shouldn't, and fails on DXVK like it should.
The second two patches address an issue reported in Empires: Dawn of the Modern World, where the game expects a device reset to reset the scene state.
-- v2: wined3d: Clear scene state on device state reset. d3d8/tests: Add a test for device reset after beginning a scene. d3d9/tests: Add a test for device reset after beginning a scene. ddraw/tests: Add tests for preserving d3d scene state during primary surface creation. d3d9: Return failure if a multisampled surface is passed to IDirect3DDevice9::UpdateSurface(). d3d9/tests: Add tests for IDirect3DDevice9::UpdateSurface() with a multisampled surface.
From: Connor McAdams cmcadams@codeweavers.com
Signed-off-by: Connor McAdams cmcadams@codeweavers.com --- dlls/d3d9/tests/visual.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/dlls/d3d9/tests/visual.c b/dlls/d3d9/tests/visual.c index 724d9ef1e39..806ab8f2802 100644 --- a/dlls/d3d9/tests/visual.c +++ b/dlls/d3d9/tests/visual.c @@ -4546,6 +4546,13 @@ static void test_multisample_stretch_rect(void) hr = IDirect3DDevice9_SetRenderTarget(device, 0, rt); ok(hr == D3D_OK, "Got unexpected hr %#lx.\n", hr);
+ /* UpdateSurface does not support multisampled surfaces. */ + hr = IDirect3DDevice9_UpdateSurface(device, rt, NULL, ms_rt, NULL); + todo_wine ok(hr == D3DERR_INVALIDCALL, "Got unexpected hr %#lx.\n", hr); + + hr = IDirect3DDevice9_UpdateSurface(device, ms_rt, NULL, rt, NULL); + todo_wine ok(hr == D3DERR_INVALIDCALL, "Got unexpected hr %#lx.\n", hr); + for (i = 0; i < ARRAY_SIZE(filters); ++i) { hr = IDirect3DDevice9_Clear(device, 0, NULL, D3DCLEAR_TARGET, 0xffffffff, 0.0f, 0);
From: Connor McAdams cmcadams@codeweavers.com
Signed-off-by: Connor McAdams cmcadams@codeweavers.com --- dlls/d3d9/device.c | 7 +++++++ dlls/d3d9/tests/visual.c | 4 ++-- 2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/dlls/d3d9/device.c b/dlls/d3d9/device.c index 408dace48ad..e5b5463dbba 100644 --- a/dlls/d3d9/device.c +++ b/dlls/d3d9/device.c @@ -1800,6 +1800,13 @@ static HRESULT WINAPI d3d9_device_UpdateSurface(IDirect3DDevice9Ex *iface, return D3DERR_INVALIDCALL; }
+ if (src_desc.multisample_type != WINED3D_MULTISAMPLE_NONE || dst_desc.multisample_type != WINED3D_MULTISAMPLE_NONE) + { + wined3d_mutex_unlock(); + WARN("Cannot use UpdateSurface with multisampled surfaces.\n"); + return D3DERR_INVALIDCALL; + } + if (src_rect) wined3d_box_set(&src_box, src_rect->left, src_rect->top, src_rect->right, src_rect->bottom, 0, 1); else diff --git a/dlls/d3d9/tests/visual.c b/dlls/d3d9/tests/visual.c index 806ab8f2802..3f3de411143 100644 --- a/dlls/d3d9/tests/visual.c +++ b/dlls/d3d9/tests/visual.c @@ -4548,10 +4548,10 @@ static void test_multisample_stretch_rect(void)
/* UpdateSurface does not support multisampled surfaces. */ hr = IDirect3DDevice9_UpdateSurface(device, rt, NULL, ms_rt, NULL); - todo_wine ok(hr == D3DERR_INVALIDCALL, "Got unexpected hr %#lx.\n", hr); + ok(hr == D3DERR_INVALIDCALL, "Got unexpected hr %#lx.\n", hr);
hr = IDirect3DDevice9_UpdateSurface(device, ms_rt, NULL, rt, NULL); - todo_wine ok(hr == D3DERR_INVALIDCALL, "Got unexpected hr %#lx.\n", hr); + ok(hr == D3DERR_INVALIDCALL, "Got unexpected hr %#lx.\n", hr);
for (i = 0; i < ARRAY_SIZE(filters); ++i) {
From: Connor McAdams cmcadams@codeweavers.com
Signed-off-by: Connor McAdams cmcadams@codeweavers.com --- dlls/ddraw/tests/ddraw2.c | 4 ++++ dlls/ddraw/tests/ddraw4.c | 4 ++++ dlls/ddraw/tests/ddraw7.c | 4 ++++ 3 files changed, 12 insertions(+)
diff --git a/dlls/ddraw/tests/ddraw2.c b/dlls/ddraw/tests/ddraw2.c index 48a49815c58..198e40bab55 100644 --- a/dlls/ddraw/tests/ddraw2.c +++ b/dlls/ddraw/tests/ddraw2.c @@ -16670,6 +16670,8 @@ static void test_d3d_state_reset(void) ok(hr == DD_OK, "got %#lx.\n", hr); hr = IDirect3DDevice2_SetRenderState(device, D3DRENDERSTATE_ZENABLE, TRUE); ok(hr == DD_OK, "got %#lx.\n", hr); + hr = IDirect3DDevice2_BeginScene(device); + ok(hr == DD_OK, "got %#lx.\n", hr);
memset(¶m, 0, sizeof(param)); hr = IDirectDraw2_EnumDisplayModes(ddraw, 0, NULL, ¶m, find_different_mode_callback); @@ -16718,6 +16720,8 @@ static void test_d3d_state_reset(void) hr = IDirect3DDevice2_GetRenderState(device, D3DRENDERSTATE_ZENABLE, &state); ok(hr == DD_OK, "got %#lx.\n", hr); ok(state == TRUE, "got %#lx.\n", state); + hr = IDirect3DDevice2_BeginScene(device); + ok(hr == D3DERR_SCENE_IN_SCENE, "Unexpected hr %#lx.\n", hr);
hr = IDirectDraw2_SetCooperativeLevel(ddraw, NULL, DDSCL_NORMAL); ok(hr == DD_OK, "got %#lx.\n", hr); diff --git a/dlls/ddraw/tests/ddraw4.c b/dlls/ddraw/tests/ddraw4.c index 8abd9cd44fd..7f7c563303f 100644 --- a/dlls/ddraw/tests/ddraw4.c +++ b/dlls/ddraw/tests/ddraw4.c @@ -19774,6 +19774,8 @@ static void test_d3d_state_reset(void) ok(hr == DD_OK, "got %#lx.\n", hr); hr = IDirect3DDevice3_SetRenderState(device, D3DRENDERSTATE_ZENABLE, TRUE); ok(hr == DD_OK, "got %#lx.\n", hr); + hr = IDirect3DDevice3_BeginScene(device); + ok(hr == DD_OK, "got %#lx.\n", hr);
memset(¶m, 0, sizeof(param)); hr = IDirectDraw4_EnumDisplayModes(ddraw, 0, NULL, ¶m, find_different_mode_callback); @@ -19822,6 +19824,8 @@ static void test_d3d_state_reset(void) hr = IDirect3DDevice3_GetRenderState(device, D3DRENDERSTATE_ZENABLE, &state); ok(hr == DD_OK, "got %#lx.\n", hr); ok(state == TRUE, "got %#lx.\n", state); + hr = IDirect3DDevice3_BeginScene(device); + ok(hr == D3DERR_SCENE_IN_SCENE, "Unexpected hr %#lx.\n", hr);
hr = IDirectDraw4_SetCooperativeLevel(ddraw, NULL, DDSCL_NORMAL); ok(hr == DD_OK, "got %#lx.\n", hr); diff --git a/dlls/ddraw/tests/ddraw7.c b/dlls/ddraw/tests/ddraw7.c index 3e03d3e4dd6..76c52be0258 100644 --- a/dlls/ddraw/tests/ddraw7.c +++ b/dlls/ddraw/tests/ddraw7.c @@ -20229,6 +20229,8 @@ static void test_d3d_state_reset(void) ok(hr == DD_OK, "got %#lx.\n", hr); hr = IDirect3DDevice7_SetRenderState(device, D3DRENDERSTATE_ZENABLE, TRUE); ok(hr == DD_OK, "got %#lx.\n", hr); + hr = IDirect3DDevice7_BeginScene(device); + ok(hr == DD_OK, "got %#lx.\n", hr);
hr = IDirect3DDevice7_GetViewport(device, &vp1); ok(hr == DD_OK, "got %#lx.\n", hr); @@ -20269,6 +20271,8 @@ static void test_d3d_state_reset(void) hr = IDirect3DDevice7_GetRenderState(device, D3DRENDERSTATE_ZENABLE, &state); ok(hr == DD_OK, "got %#lx.\n", hr); ok(state == TRUE, "got %#lx.\n", state); + hr = IDirect3DDevice7_BeginScene(device); + ok(hr == D3DERR_SCENE_IN_SCENE, "Unexpected hr %#lx.\n", hr);
hr = IDirectDraw7_SetCooperativeLevel(ddraw, NULL, DDSCL_NORMAL); ok(hr == DD_OK, "got %#lx.\n", hr);
From: Connor McAdams cmcadams@codeweavers.com
Signed-off-by: Connor McAdams cmcadams@codeweavers.com --- dlls/d3d9/tests/device.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/dlls/d3d9/tests/device.c b/dlls/d3d9/tests/device.c index 53dc9ee55ed..0cd290367b5 100644 --- a/dlls/d3d9/tests/device.c +++ b/dlls/d3d9/tests/device.c @@ -2718,6 +2718,19 @@ static void test_scene(void) hr = IDirect3DDevice9_EndScene(device); ok(hr == D3DERR_INVALIDCALL, "Got hr %#lx.\n", hr);
+ /* Calling Reset clears scene state. */ + hr = IDirect3DDevice9_BeginScene(device); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + reset_device(device, NULL); + hr = IDirect3DDevice9_EndScene(device); + todo_wine ok(hr == D3DERR_INVALIDCALL, "Got hr %#lx.\n", hr); + + hr = IDirect3DDevice9_BeginScene(device); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IDirect3DDevice9_EndScene(device); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + /* Create some surfaces to test stretchrect between the scenes */ hr = IDirect3DDevice9_CreateOffscreenPlainSurface(device, 128, 128, D3DFMT_A8R8G8B8, D3DPOOL_DEFAULT, &surface1, NULL);
From: Connor McAdams cmcadams@codeweavers.com
Signed-off-by: Connor McAdams cmcadams@codeweavers.com --- dlls/d3d8/tests/device.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/dlls/d3d8/tests/device.c b/dlls/d3d8/tests/device.c index 8261a3139c5..d88fa2abc19 100644 --- a/dlls/d3d8/tests/device.c +++ b/dlls/d3d8/tests/device.c @@ -2174,6 +2174,19 @@ static void test_scene(void) hr = IDirect3DDevice8_EndScene(device); ok(hr == D3DERR_INVALIDCALL, "Got hr %#lx.\n", hr);
+ /* Calling Reset clears scene state. */ + hr = IDirect3DDevice8_BeginScene(device); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + reset_device(device, NULL); + hr = IDirect3DDevice8_EndScene(device); + todo_wine ok(hr == D3DERR_INVALIDCALL, "Got hr %#lx.\n", hr); + + hr = IDirect3DDevice8_BeginScene(device); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IDirect3DDevice8_EndScene(device); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + /* StretchRect does not exit in Direct3D8, so no equivalent to the d3d9 stretchrect tests */
refcount = IDirect3DDevice8_Release(device);
From: Connor McAdams cmcadams@codeweavers.com
Signed-off-by: Connor McAdams cmcadams@codeweavers.com --- dlls/d3d8/tests/device.c | 2 +- dlls/d3d9/device.c | 1 + dlls/d3d9/tests/device.c | 2 +- dlls/wined3d/device.c | 2 ++ 4 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/dlls/d3d8/tests/device.c b/dlls/d3d8/tests/device.c index d88fa2abc19..10c06b17ba7 100644 --- a/dlls/d3d8/tests/device.c +++ b/dlls/d3d8/tests/device.c @@ -2180,7 +2180,7 @@ static void test_scene(void)
reset_device(device, NULL); hr = IDirect3DDevice8_EndScene(device); - todo_wine ok(hr == D3DERR_INVALIDCALL, "Got hr %#lx.\n", hr); + ok(hr == D3DERR_INVALIDCALL, "Got hr %#lx.\n", hr);
hr = IDirect3DDevice8_BeginScene(device); ok(hr == S_OK, "Got hr %#lx.\n", hr); diff --git a/dlls/d3d9/device.c b/dlls/d3d9/device.c index e5b5463dbba..b6a3e1f58ba 100644 --- a/dlls/d3d9/device.c +++ b/dlls/d3d9/device.c @@ -1185,6 +1185,7 @@ static HRESULT d3d9_device_reset(struct d3d9_device *device, surface->parent_device = &device->IDirect3DDevice9Ex_iface; }
+ device->in_scene = FALSE; device->device_state = D3D9_DEVICE_STATE_OK;
if (extended) diff --git a/dlls/d3d9/tests/device.c b/dlls/d3d9/tests/device.c index 0cd290367b5..62f308e0734 100644 --- a/dlls/d3d9/tests/device.c +++ b/dlls/d3d9/tests/device.c @@ -2724,7 +2724,7 @@ static void test_scene(void)
reset_device(device, NULL); hr = IDirect3DDevice9_EndScene(device); - todo_wine ok(hr == D3DERR_INVALIDCALL, "Got hr %#lx.\n", hr); + ok(hr == D3DERR_INVALIDCALL, "Got hr %#lx.\n", hr);
hr = IDirect3DDevice9_BeginScene(device); ok(hr == S_OK, "Got hr %#lx.\n", hr); diff --git a/dlls/wined3d/device.c b/dlls/wined3d/device.c index acc70256462..1d9f96e897b 100644 --- a/dlls/wined3d/device.c +++ b/dlls/wined3d/device.c @@ -5227,6 +5227,8 @@ HRESULT CDECL wined3d_device_reset(struct wined3d_device *device, if (reset_state) { TRACE("Resetting state.\n"); + if (device->inScene) + wined3d_device_end_scene(device); wined3d_device_context_emit_reset_state(&device->cs->c, false); state_cleanup(state);
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=147108
Your paranoid android.
=== debian11 (32 bit fr report) ===
Report validation errors: d3d9:device crashed (c0000028)
=== debian11 (32 bit he:IL report) ===
Report validation errors: d3d8:device crashed (c0000028) d3d9:device crashed (c0000028)
=== debian11 (32 bit hi:IN report) ===
Report validation errors: d3d8:device crashed (c0000028) d3d9:device crashed (c0000028)
=== debian11 (32 bit ja:JP report) ===
Report validation errors: d3d8:device crashed (c0000028) d3d9:device crashed (c0000028)
=== debian11 (32 bit zh:CN report) ===
Report validation errors: d3d8:device crashed (c0000028) d3d9:device crashed (c0000028)
=== debian11b (64 bit WoW report) ===
comctl32: button.c:801: Test failed: [0] expected uItemState 16, got 80 button.c:801: Test failed: [0] expected uItemState 16, got 80 button.c:810: Test failed: [0] expected uItemState 0, got 64 button.c:810: Test failed: [0] expected uItemState 0, got 64 button.c:819: Test failed: [0] expected uItemState 0, got 64 button.c:819: Test failed: [0] expected uItemState 0, got 64 button.c:834: Test failed: expected state 0, got 0200 button.c:838: Test failed: [0] expected uItemState 1, got 65 button.c:838: Test failed: [0] expected uItemState 1, got 65 button.c:847: Test failed: expected state 0x0004, got 0204 button.c:854: Test failed: [0] expected uItemState 0, got 64 button.c:854: Test failed: [0] expected uItemState 0, got 64 button.c:863: Test failed: expected state 0, got 0200 button.c:898: Test failed: [0] expected uItemState 0, got 64 button.c:898: Test failed: [0] expected uItemState 0, got 64 button.c:898: Test failed: [0] expected uItemState 0, got 64 button.c:898: Test failed: [0] expected uItemState 0, got 64
I've updated the MR with tests for scene state behavior after device reset for ddraw/d3d8/d3d9.
The only thing I'm unsure of here is where exactly to put the `wined3d_device_end_scene()` call in `wined3d_device_reset()`. Since it seems like it's effectively a noop right now the positioning in code currently doesn't matter, but if for some reason we decided to do something on calls to begin/end scene in the future this might be an issue.
On Fri Jul 12 16:27:29 2024 +0000, Connor McAdams wrote:
I solved the problems I was having with the ddraw tests, I'll try to get the patches for that done on Monday and update this MR with them.
I believe the new tests address what you've asked for here, but I'll let you mark this as resolved to be sure.
Elizabeth Figura (@zfigura) commented about dlls/wined3d/device.c:
if (reset_state) { TRACE("Resetting state.\n");
if (device->inScene)wined3d_device_end_scene(device);
Unfortunately this won't be hit in the d3d9ex case, and a quick test shows that d3d9ex should end the scene here. So we probably do need to do wined3d_device_end_scene() from the client libraries after all :-/
-
Connor McAdams -
Connor McAdams (＠cmcadams)
-
Elizabeth Figura (＠zfigura)
-
Marvin