DXVK fails to create a device for the desktop window which causes the test to crash.
Instead of crashing, it should just skip the remaining desktop window tests if device creation fails.
From: Robin Kertels robin.kertels@gmail.com
Signed-off-by: Robin Kertels robin.kertels@gmail.com --- dlls/d3d9/tests/visual.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/dlls/d3d9/tests/visual.c b/dlls/d3d9/tests/visual.c index 2d54920ddba..a1f8c59c50f 100644 --- a/dlls/d3d9/tests/visual.c +++ b/dlls/d3d9/tests/visual.c @@ -26438,16 +26438,23 @@ static void test_desktop_window(void) device = create_device(d3d, GetDesktopWindow(), GetDesktopWindow(), TRUE); ok(!!device, "Failed to create a D3D device.\n");
- hr = IDirect3DDevice9_Clear(device, 0, NULL, D3DCLEAR_TARGET, 0xffff0000, 1.0f, 0); - ok(SUCCEEDED(hr), "Failed to clear, hr %#lx.\n", hr); - color = getPixelColor(device, 1, 1); - ok(color == 0x00ff0000, "Got unexpected color 0x%08x.\n", color); + if (device) + { + hr = IDirect3DDevice9_Clear(device, 0, NULL, D3DCLEAR_TARGET, 0xffff0000, 1.0f, 0); + ok(SUCCEEDED(hr), "Failed to clear, hr %#lx.\n", hr); + color = getPixelColor(device, 1, 1); + ok(color == 0x00ff0000, "Got unexpected color 0x%08x.\n", color);
- hr = IDirect3DDevice9_Present(device, NULL, NULL, NULL, NULL); - ok(SUCCEEDED(hr), "Failed to present, hr %#lx.\n", hr); + hr = IDirect3DDevice9_Present(device, NULL, NULL, NULL, NULL); + ok(SUCCEEDED(hr), "Failed to present, hr %#lx.\n", hr);
- refcount = IDirect3DDevice9_Release(device); - ok(!refcount, "Device has %lu references left.\n", refcount); + refcount = IDirect3DDevice9_Release(device); + ok(!refcount, "Device has %lu references left.\n", refcount); + } + else + { + skip("Failed to create a D3D device for the desktop window, skipping tests.\n"); + }
/* test device with NULL HWND */ device = create_device(d3d, NULL, NULL, TRUE);
What is the end goal here?
I'm trying to reduce the amount of test failures of DXVK.
I guess what I'm asking is, how are you running the tests? Manually? Automatically? Do you intend to have all the tests pass? Are you interested in collaborating on tests?
For instance, if your end goal is to have all the tests pass, I'm not immediately sure what this kind of commit gets you, since there's still a failure to fix. If you're running CI, it's not going to be useful if you have a bunch of failing tests. I don't see any value in this kind of thing in general—a failure is a failure.
On the other hand, if you're not just interested in using the existing tests, but also in collaborating on tests and helping to contribute to the test suite, I'm certainly open to adding e.g. todo_dxvk annotations and specifically accounting for DXVK-specific failures.
Right now I'm running them manually. Maybe we'll set up CI once most tests pass. I am interested in collaborating on tests.
One of the main reasons why I'd like to land this MR is that I'm not entirely sure it's possible to make rendering to the desktop work. It would be useful to have the D3D9 tests be able to run with DXVK on Windows and creating a Vulkan swapchain for the desktop window fails with VK_ERROR_INITIALIZATION_FAILED.
todo_dxvk would work too, I guess. I personally think skipping if device creation fails is cleaner.
So normally I object to this kind of patch in the context of "fixing" Wine tests, since it doesn't really fix anything—a failure is a failure. This is obviously a bit of a different situation though, but I guess the point is that this is something that I'd sign off on only for dxvk's benefit, not because it'd be desirable "anyway".
I'm still curious what the end goal will look like here—this patch makes a lot more sense if we also have "todo_dxvk" annotations in tree everywhere—but if it helps for now I'm not opposed to accepting anyway, at least until we work that out.
This merge request was approved by Zebediah Figura.
This merge request was approved by Jan Sikorski.
-
Jan Sikorski (＠jsikorski) -
Robin Kertels
-
Robin Kertels (＠K0bin)
-
Zebediah Figura (＠zfigura)