tests: Print d3dcompiler_47.dll version when running tests on Windows.
Merge request reports
Activity
+#ifdef _WIN32 +static void print_dll_version(const char *file_name) +{ + DWORD size, handle; + bool done = false; + + size = GetFileVersionInfoSizeA(file_name, &handle); + if (size) + { + char *data = malloc(size); + if (GetFileVersionInfoA(file_name, handle, size, data)) { + VS_FIXEDFILEINFO *pFixedVersionInfo; + UINT len; + if (VerQueryValueA(data, "\\", (LPVOID *)&pFixedVersionInfo, &len)) { + trace("%s version: %ld.%ld.%ld.%ld\n", + file_name, + pFixedVersionInfo->dwFileVersionMS >> 16, + pFixedVersionInfo->dwFileVersionMS & 0xffff, + pFixedVersionInfo->dwFileVersionLS >> 16, + pFixedVersionInfo->dwFileVersionLS & 0xffff); + done = true; + } + } + free(data); + } + + if (!done) + trace("%s version: unknown\n", file_name); +} +#endif
This mixes styles, and the most consequential part of that function uses a style that's not the usual vkd3d style at all. Was this perhaps copied a bit too quickly?
Perhaps more significantly, does this work with non-cross Win32 builds of vkd3d? Those builds would link to vkd3d-utils instead of d3dcompiler_47, and while printing the d3dcompiler_47 version anyway in that case might not hurt too much, those builds wouldn't link to the "version" library, right?
Perhaps more significantly, does this work with non-cross Win32 builds of vkd3d? Those builds would link to vkd3d-utils instead of d3dcompiler_47, and while printing the d3dcompiler_47 version anyway in that case might not hurt too much, those builds wouldn't link to the "version" library, right?
That currently doesn't work anyway; we gate all that code on _WIN32 already in d3d12_crosstest.h. Accordingly trying to build the tests fails:
whatsit@camazotz:~/git/mingw-vkd3d64$ make tests/d3d12.exe CCLD tests/d3d12.exe /usr/bin/x86_64-w64-mingw32-ld: tests/d3d12.o: in function `create_adapter': /home/whatsit/git/mingw-vkd3d64/../vkd3d/tests/d3d12_crosstest.h:283: undefined reference to `CreateDXGIFactory1' /usr/bin/x86_64-w64-mingw32-ld: tests/d3d12.o: in function `create_device': /home/whatsit/git/mingw-vkd3d64/../vkd3d/tests/d3d12_crosstest.h:311: undefined reference to `D3D12CreateDevice' ...
added 18 commits
-
081063f6...3fbe2726 - 16 commits from branch
wine:master
- 661fe254 - tests: Distinguish between cross and non-cross Win32 builds.
- d169adb8 - tests: Print d3dcompiler_47.dll version when running tests on Windows.
-
081063f6...3fbe2726 - 16 commits from branch
START_TEST(shader_runner) { -#ifdef _WIN32 +#ifdef VKD3D_CROSSTEST run_shader_tests_d3d9(argc, argv); run_shader_tests_d3d11(argc, argv); #else
Overall this is probably an improvement, but I think that ideally we would run the d3d9 and d3d11 tests on any Win32 build, and always run the Vulkan and d3d12 tests. (Not sure if that's currently feasible; maybe it isn't.)
+ if (GetFileVersionInfoA(file_name, handle, size, data)) + { + VS_FIXEDFILEINFO *pFixedVersionInfo; + UINT len; + + if (VerQueryValueA(data, "\\", (LPVOID *)&pFixedVersionInfo, &len)) + { + trace("%s version: %ld.%ld.%ld.%ld\n", file_name, + pFixedVersionInfo->dwFileVersionMS >> 16, pFixedVersionInfo->dwFileVersionMS & 0xffff, + pFixedVersionInfo->dwFileVersionLS >> 16, pFixedVersionInfo->dwFileVersionLS & 0xffff); + done = true; + } + }
Sorry to be a pain about this, but could we please:
- Avoid "pFixedVersionInfo" and just call it "info", "version_info", "file_info", or something along those lines.
- Avoid "LPVOID".
- Use %u to print unsigned values.
START_TEST(shader_runner) { -#ifdef _WIN32 +#ifdef VKD3D_CROSSTEST run_shader_tests_d3d9(argc, argv); run_shader_tests_d3d11(argc, argv); #else
Overall this is probably an improvement, but I think that ideally we would run the d3d9 and d3d11 tests on any Win32 build, and always run the Vulkan and d3d12 tests. (Not sure if that's currently feasible; maybe it isn't.)
Hmm, should we, though? The counterpoint is that we may want to replace vkd3d_test_platform_is_windows() with something keyed on VKD3D_CROSSTEST, and if so we don't want to run e.g. d3d9 tests unless it's the crosstest.
I'm not sure I completely follow the logic. I guess the interesting thing about the d3d9 and d3d11 tests is that it would be much easier to end up with the Windows D3D + vkd3d-shader configuration, while for the d3d12 tests you'd typically either have Windows D3D + d3dcompiler or vkd3d + vkd3d-shader. (Note though that Wine and the possibility of using native DLLs does complicate that picture a little.) That means we may need to do a better job of distinguishing between Windows D3D and wined3d/vkd3d, separately from distinguishing between d3dcompiler and vkd3d-shader, instead of lumping these together, but that might not be a bad thing anyway?
In the last version D3D9 and D3D11 tests are executed on non-cross builds too. I also added some additional logging so that it's easier to follow what's happening (especially, say, if a log trace is shared and it's not clear what was executed in the first place). How do you both see that?
Eventually it would be nice to run Vulkan tests on Windows too, I guess, but I won't tackle that this time.
Unfortunately there is a warning in non-cross builds I don't completely understand how to get rid of:
In file included from ../vkd3d/tests/d3d12_crosstest.h:46, from ../vkd3d/tests/d3d12.c:24: ../vkd3d/include/private/vkd3d_test.h:323:5: warning: no previous prototype for ‘wmain’ [-Wmissing-prototypes] 323 | int wmain(int argc, WCHAR **wargv) | ^~~~~
Any suggestion for that?
In the last version D3D9 and D3D11 tests are executed on non-cross builds too. I also added some additional logging so that it's easier to follow what's happening (especially, say, if a log trace is shared and it's not clear what was executed in the first place). How do you both see that?
Generally I'm in favour, although the nested #ifdef's do look a bit messy.
Unfortunately there is a warning in non-cross builds I don't completely understand how to get rid of:
In file included from ../vkd3d/tests/d3d12_crosstest.h:46, from ../vkd3d/tests/d3d12.c:24: ../vkd3d/include/private/vkd3d_test.h:323:5: warning: no previous prototype for ‘wmain’ [-Wmissing-prototypes] 323 | int wmain(int argc, WCHAR **wargv) | ^~~~~
Any suggestion for that?
We probably need to add -municode, much like 59d918fd did for the demos.
Zebediah Figura replied on the mailing list:
On 1/18/23 10:14, Henri Verbeet (@hverbeet) wrote: >> In the last version D3D9 and D3D11 tests are executed on non-cross builds too. I also added some additional logging so that it's easier to follow what's happening (especially, say, if a log trace is shared and it's not clear what was executed in the first place). How do you both see that? > > Generally I'm in favour, although the nested #ifdef's do look a bit messy. > >> Unfortunately there is a warning in non-cross builds I don't completely understand how to get rid of: >> >> \`\`\` >> In file included from ../vkd3d/tests/d3d12_crosstest.h:46, >> from ../vkd3d/tests/d3d12.c:24: >> ../vkd3d/include/private/vkd3d_test.h:323:5: warning: no previous prototype for ‘wmain’ [-Wmissing-prototypes] >> 323 | int wmain(int argc, WCHAR **wargv) >> | ^~~~~ >> \`\`\` >> >> Any suggestion for that? > > We probably need to add -municode, much like 59d918fd50e6a4fad4052b563bfd248d8ff87013 did for the demos. > Do we actually need a Unicode main()? Could we just do something like the attached diff?
I'm not sure I completely follow the logic. I guess the interesting thing about the d3d9 and d3d11 tests is that it would be much easier to end up with the Windows D3D + vkd3d-shader configuration, while for the d3d12 tests you'd typically either have Windows D3D + d3dcompiler or vkd3d + vkd3d-shader. (Note though that Wine and the possibility of using native DLLs does complicate that picture a little.) That means we may need to do a better job of distinguishing between Windows D3D and wined3d/vkd3d, separately from distinguishing between d3dcompiler and vkd3d-shader, instead of lumping these together, but that might not be a bad thing anyway?
I was thinking of a division between vkd3d+vkd3d-shader and native (d3d*+d3dcompiler), where "crosstest" means native and normal test means vkd3d. I hadn't really considered the idea of mixing and matching the two. It seems interesting enough to try, but the way v4 of this series does it seems inconsistent—if we're going to build the d3d11 runner to use vkd3d's HLSL compiler with native vkd3d, I'd expect the d3d12 runner from the same executalbe to be built the same way. Granted, there's a printed warning that's not what's happening...
Ultimately we are probably going to have to abandon "crosstests" for the shader runner; they're not going to be expressive enough.
Yeah, d3d12 is different because for d3d9 and d3d11 we can't do other then using the native thing, while for d3d12 in principle we can use either the native library or our own implementation. For testing I usually feel rather maximalist, so I would consider it ideal to have a way to run all the possible combinations:
- compile the shaders with vkd3d-shader and execute them with vkd3d (via its D3D12 interface): this is already happening on non-cross builds (though it's true that with my patch the shader runner would falsely pretend that they're being run with d3d12.dll, so I'll fix the message);
- compile the shaders with vkd3d-shader and execute them with d3d12.dll: this is neither happening currently nor with my patch;
- compile the shaders with vkd3d-shader and execute them with d3d11.dll: this is not currently happening, but would with my patch on non-cross builds;
- compile the shaders with vkd3d-shader and execute them with d3d9.dll: this is not currently happening, but would with my patch on non-cross builds;
- compile the shaders with vkd3d-shader and execute them with Vulkan (via the custom shader runner): this is neither happening currently nor with my patch;
- compile the shaders with d3dcompiler_47.dll and execute them with vkd3d: this is neither happening currently nor with my patch;
- compile the shaders with d3dcompiler_47.dll and execute them with d3d12.dll: this is already happening on cross builds;
- compile the shaders with d3dcompiler_47.dll and execute them with d3d11.dll: this is already happening on cross builds;
- compile the shaders with d3dcompiler_47.dll and execute them with d3d9.dll: this is already happening on cross builds;
- compile the shaders with d3dcompiler_47.dll and execute them with Vulkan (this would involve another recompilation step DXBC->SPIR-V with vkd3d-shader): this is neither happening currently nor with my patch.
All of this of course only makes sense for Win32 (cross) builds. On Unix we can only compile with vkd3d-shader and execute with vkd3d or Vulkan (and we already do both). In principle one could also add other rows/columns to the matrix (i.e., use d3dcompiler_N.dll or d3d10.dll), but maybe that's not terribly meaningful. Also, for tests that are not the shader runner only four lines of those make sense, if I am not mistaken (specifically, compiling with either vkd3d-shader or d3dcompiler_47.dll and executing with either vkd3d or d3d12.dll).
As I said, I feel rather maximalist, so in principle I'd implement all the lines above (though I acknowledge that some lines make more sense than others). Possibly with some options to disable some of them at runtime, for testing speed reasons. Also I'd rig things so that the non-cross builds is able to run all the combinations in a single executable, for convenience reasons.
However, I don't think that this is in scope for this specific MR, so I would ask for it to be accepted as it is (modulo perhaps warnings and code style), and other combinations (if considered desirable) can be added later.
All of this of course only makes sense for Win32 (cross) builds. On Unix we can only compile with vkd3d-shader and execute with vkd3d or Vulkan (and we already do both).
Yes, although a future where vkd3d supports additional Direct3D APIs doesn't seem that hard to imagine.
As I said, I feel rather maximalist, so in principle I'd implement all the lines above (though I acknowledge that some lines make more sense than others). Possibly with some options to disable some of them at runtime, for testing speed reasons. Also I'd rig things so that the non-cross builds is able to run all the combinations in a single executable, for convenience reasons.
Right, ultimately we'll probably want some more control at runtime over what specific configurations get run.
added 1 commit
- 05148a3c - tests: Print DLL versions when running tests on Windows.
added 31 commits
-
05148a3c...9c817e5e - 28 commits from branch
wine:master
- 44d9e2d7 - tests: Distinguish between cross and non-cross Win32 builds.
- 24457430 - tests: Run d3d9 and d3d12 tests on non-cross builds too.
- 1717dc05 - tests: Print DLL versions when running tests on Windows.
Toggle commit list-
05148a3c...9c817e5e - 28 commits from branch