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.
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/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/d3d9/device.c | 6 ++++++ dlls/d3d9/tests/device.c | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/dlls/d3d9/device.c b/dlls/d3d9/device.c index e5b5463dbba..3af87dcba39 100644 --- a/dlls/d3d9/device.c +++ b/dlls/d3d9/device.c @@ -1078,6 +1078,12 @@ static HRESULT d3d9_device_reset(struct d3d9_device *device,
wined3d_mutex_lock();
+ if (device->in_scene) + { + wined3d_device_end_scene(device->wined3d_device); + device->in_scene = FALSE; + } + wined3d_streaming_buffer_cleanup(&device->vertex_buffer); wined3d_streaming_buffer_cleanup(&device->index_buffer);
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);
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=146965
Your paranoid android.
=== w8 (32 bit report) ===
d3d9: device.c:5244: Test failed: Failed to reset device, hr 0x88760868.
=== debian11 (build log) ===
WineRunWineTest.pl:error: The task timed out
=== 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:859: Test failed: BM_SETSTATE/FALSE on a button: the msg sequence is not complete: expected 0000 - actual 0084 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
Scene resetting probably should apply to d3d8 and ddraw as well; would you mind testing that and then moving the wined3d_device_end_scene() call to wined3d_device_reset() if so?
On Thu Jul 11 12:46:25 2024 +0000, Elizabeth Figura wrote:
Scene resetting probably should apply to d3d8 and ddraw as well; would you mind testing that and then moving the wined3d_device_end_scene() call to wined3d_device_reset() if so?
Sure! Writing tests for that in d3d8 seems relatively easy, but ddraw is proving a bit more difficult. There doesn't seem to be an explicit `Reset` method for D3D7 and below, the closest I can find is in `dlls/ddraw/tests/ddraw4.c:test_d3d_state_reset(void)`. I'm trying to mix in the `BeginScene()` and `EndScene()` calls there and not having much luck.
Either way, if we're adding tests for those, it's probably worth splitting off from this MR. Would that make sense to you as well? Just having the `UpdateSurface()` with multisampled surfaces in this MR, and then opening up a separate MR for the scene state on device reset patches?
On Thu Jul 11 12:46:25 2024 +0000, Connor McAdams wrote:
Sure! Writing tests for that in d3d8 seems relatively easy, but ddraw is proving a bit more difficult. There doesn't seem to be an explicit `Reset` method for D3D7 and below, the closest I can find is in `dlls/ddraw/tests/ddraw4.c:test_d3d_state_reset(void)`. I'm trying to mix in the `BeginScene()` and `EndScene()` calls there and not having much luck. Either way, if we're adding tests for those, it's probably worth splitting off from this MR. Would that make sense to you as well? Just having the `UpdateSurface()` with multisampled surfaces in this MR, and then opening up a separate MR for the scene state on device reset patches?
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.
-
Connor McAdams -
Connor McAdams (＠cmcadams)
-
Elizabeth Figura (＠zfigura)
-
Marvin