[PATCH 0/3] MR2705: d3dcompiler: D3DAssemble is a private export.
This series allows linking of our d3dx9.dll against d3dcompiler.lib from the Windows SDK. It presumably also fixes the d3dcompiler tests, although I didn't bother building them with Visual Studio so far. And yes, I did check, that stringification macro works on MSVC. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2705
From: Stefan Dösinger <stefan(a)codeweavers.com> This allows linking our d3dx9.dll to Microsoft's d3dcompiler.lib implib. --- dlls/d3dx9_36/shader.c | 33 +++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/dlls/d3dx9_36/shader.c b/dlls/d3dx9_36/shader.c index 1aa75d64dc5..52d8e9bd26b 100644 --- a/dlls/d3dx9_36/shader.c +++ b/dlls/d3dx9_36/shader.c @@ -26,11 +26,6 @@ WINE_DEFAULT_DEBUG_CHANNEL(d3dx); -/* This function is not declared in the SDK headers yet. */ -HRESULT WINAPI D3DAssemble(const void *data, SIZE_T datasize, const char *filename, - const D3D_SHADER_MACRO *defines, ID3DInclude *include, UINT flags, - ID3DBlob **shader, ID3DBlob **error_messages); - static inline BOOL is_valid_bytecode(DWORD token) { return (token & 0xfffe0000) == 0xfffe0000; @@ -193,19 +188,41 @@ HRESULT WINAPI D3DXFindShaderComment(const DWORD *byte_code, DWORD fourcc, const return S_FALSE; } +static BOOL WINAPI load_d3dassemble_once(INIT_ONCE *once, void *param, void **context) +{ + /* FIXME: This hardcodes the fact that dlls/d3dcompiler_47/Makefile.in calls its + * import lib "d3dcompiler.lib" and that no other version of this DLL took over. + * GetModuleHandleExA(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS, D3DCompile) would + * be nice, but "D3DCompile" will point to the import stub, not d3dcompiler_xy.dll */ + HMODULE mod = GetModuleHandleA("d3dcompiler_47"); + void **assemble = param; + + if (!mod) + ERR("d3dcompiler_47.dll not found - which d3dcompiler are we linked against?\n"); + + *assemble = (void *)GetProcAddress(mod, "D3DAssemble"); + return TRUE; +} + HRESULT WINAPI D3DXAssembleShader(const char *data, UINT data_len, const D3DXMACRO *defines, ID3DXInclude *include, DWORD flags, ID3DXBuffer **shader, ID3DXBuffer **error_messages) { + static HRESULT (WINAPI *pD3DAssemble)(const void *data, SIZE_T datasize, const char *filename, + const D3D_SHADER_MACRO * defines, ID3DInclude * include, UINT flags, + ID3DBlob * *shader, ID3DBlob * *error_messages); + static INIT_ONCE init_once = INIT_ONCE_STATIC_INIT; HRESULT hr; TRACE("data %p, data_len %u, defines %p, include %p, flags %#lx, shader %p, error_messages %p.\n", data, data_len, defines, include, flags, shader, error_messages); + InitOnceExecuteOnce(&init_once, load_d3dassemble_once, &pD3DAssemble, NULL); + /* Forward to d3dcompiler: the parameter types aren't really different, the actual data types are equivalent */ - hr = D3DAssemble(data, data_len, NULL, (D3D_SHADER_MACRO *)defines, - (ID3DInclude *)include, flags, (ID3DBlob **)shader, - (ID3DBlob **)error_messages); + hr = pD3DAssemble(data, data_len, NULL, (D3D_SHADER_MACRO *)defines, + (ID3DInclude *)include, flags, (ID3DBlob **)shader, + (ID3DBlob **)error_messages); if(hr == E_FAIL) hr = D3DXERR_INVALIDDATA; return hr; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2705
From: Stefan Dösinger <stefan(a)codeweavers.com> --- dlls/d3dcompiler_43/tests/asm.c | 23 +++++++++++++++-------- dlls/d3dcompiler_43/tests/hlsl_d3d9.c | 11 +++++++++-- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/dlls/d3dcompiler_43/tests/asm.c b/dlls/d3dcompiler_43/tests/asm.c index 6c22eae66c8..9fe84b05e69 100644 --- a/dlls/d3dcompiler_43/tests/asm.c +++ b/dlls/d3dcompiler_43/tests/asm.c @@ -27,7 +27,7 @@ perhaps with a different name? */ #define D3DXERR_INVALIDDATA 0x88760b59 -HRESULT WINAPI D3DAssemble(const void *data, SIZE_T datasize, const char *filename, +static HRESULT (* WINAPI pD3DAssemble)(const void *data, SIZE_T datasize, const char *filename, const D3D_SHADER_MACRO *defines, ID3DInclude *include, UINT flags, ID3DBlob **shader, ID3DBlob **error_messages); @@ -57,7 +57,7 @@ static void exec_tests(const char *name, struct shader_test tests[], unsigned in for(i = 0; i < count; i++) { /* D3DAssemble sets messages to 0 if there aren't error messages */ messages = NULL; - hr = D3DAssemble(tests[i].text, strlen(tests[i].text), NULL, NULL, + hr = pD3DAssemble(tests[i].text, strlen(tests[i].text), NULL, NULL, NULL, D3DCOMPILE_SKIP_VALIDATION, &shader, &messages); ok(hr == S_OK, "Test %s, shader %u: D3DAssemble failed with error %#lx - %ld.\n", name, i, hr, hr & 0xffff); if(messages) { @@ -1431,7 +1431,7 @@ static void failure_test(void) { { shader = NULL; messages = NULL; - hr = D3DAssemble(tests[i], strlen(tests[i]), NULL, NULL, NULL, D3DCOMPILE_SKIP_VALIDATION, &shader, &messages); + hr = pD3DAssemble(tests[i], strlen(tests[i]), NULL, NULL, NULL, D3DCOMPILE_SKIP_VALIDATION, &shader, &messages); ok(hr == D3DXERR_INVALIDDATA, "Test %u: Got unexpected hr %#lx.\n", i, hr); if (messages) { @@ -1556,7 +1556,7 @@ static void assembleshader_test(void) { /* defines test */ shader = NULL; messages = NULL; - hr = D3DAssemble(test1, strlen(test1), NULL, defines, NULL, D3DCOMPILE_SKIP_VALIDATION, &shader, &messages); + hr = pD3DAssemble(test1, strlen(test1), NULL, defines, NULL, D3DCOMPILE_SKIP_VALIDATION, &shader, &messages); ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); if (messages) { @@ -1567,14 +1567,14 @@ static void assembleshader_test(void) { /* NULL messages test */ shader = NULL; - hr = D3DAssemble(test1, strlen(test1), NULL, defines, NULL, D3DCOMPILE_SKIP_VALIDATION, &shader, NULL); + hr = pD3DAssemble(test1, strlen(test1), NULL, defines, NULL, D3DCOMPILE_SKIP_VALIDATION, &shader, NULL); ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); if (shader) ID3D10Blob_Release(shader); /* NULL shader test */ messages = NULL; - hr = D3DAssemble(test1, strlen(test1), NULL, defines, NULL, D3DCOMPILE_SKIP_VALIDATION, NULL, &messages); + hr = pD3DAssemble(test1, strlen(test1), NULL, defines, NULL, D3DCOMPILE_SKIP_VALIDATION, NULL, &messages); ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); if (messages) { @@ -1586,7 +1586,7 @@ static void assembleshader_test(void) { shader = NULL; messages = NULL; include.ID3DInclude_iface.lpVtbl = &D3DInclude_Vtbl; - hr = D3DAssemble(testshader, strlen(testshader), NULL, NULL, + hr = pD3DAssemble(testshader, strlen(testshader), NULL, NULL, &include.ID3DInclude_iface, D3DCOMPILE_SKIP_VALIDATION, &shader, &messages); ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); if (messages) @@ -1599,7 +1599,7 @@ static void assembleshader_test(void) { /* NULL shader tests */ shader = NULL; messages = NULL; - hr = D3DAssemble(NULL, 0, NULL, NULL, NULL, D3DCOMPILE_SKIP_VALIDATION, &shader, &messages); + hr = pD3DAssemble(NULL, 0, NULL, NULL, NULL, D3DCOMPILE_SKIP_VALIDATION, &shader, &messages); ok(hr == D3DXERR_INVALIDDATA, "Got unexpected hr %#lx.\n", hr); if (messages) { @@ -1762,6 +1762,13 @@ static void test_disassemble_shader(void) START_TEST(asm) { +#define XSTR(a) STR(a) +#define STR(a) #a + HMODULE d3dcompiler = GetModuleHandleA("d3dcompiler_" XSTR(D3D_COMPILER_VERSION)); + pD3DAssemble = (void *)GetProcAddress(d3dcompiler, "D3DAssemble"); +#undef STR +#undef XSTR + preproc_test(); ps_1_1_test(); vs_1_1_test(); diff --git a/dlls/d3dcompiler_43/tests/hlsl_d3d9.c b/dlls/d3dcompiler_43/tests/hlsl_d3d9.c index 4fef301a721..506475008ba 100644 --- a/dlls/d3dcompiler_43/tests/hlsl_d3d9.c +++ b/dlls/d3dcompiler_43/tests/hlsl_d3d9.c @@ -26,7 +26,7 @@ #define D3DXERR_INVALIDDATA 0x88760b59 -HRESULT WINAPI D3DAssemble(const void *data, SIZE_T datasize, const char *filename, +static HRESULT (WINAPI *pD3DAssemble)(const void *data, SIZE_T datasize, const char *filename, const D3D_SHADER_MACRO *defines, ID3DInclude *include, UINT flags, ID3DBlob **shader, ID3DBlob **error_messages); @@ -1499,7 +1499,7 @@ static HRESULT call_D3DAssemble(const char *source_name, ID3DInclude *include, I "#include \"include\\include3.h\"\n" "mov oC0, c0"; - return D3DAssemble(ps_code, sizeof(ps_code), source_name, NULL, include, 0, blob, errors); + return pD3DAssemble(ps_code, sizeof(ps_code), source_name, NULL, include, 0, blob, errors); } static HRESULT call_D3DCompile(const char *source_name, ID3DInclude *include, ID3D10Blob **blob, ID3D10Blob **errors) @@ -1770,6 +1770,13 @@ START_TEST(hlsl_d3d9) } pD3DXGetShaderConstantTable = (void *)GetProcAddress(mod, "D3DXGetShaderConstantTable"); +#define XSTR(a) STR(a) +#define STR(a) #a + mod = GetModuleHandleA("d3dcompiler_" XSTR(D3D_COMPILER_VERSION)); + pD3DAssemble = (void *)GetProcAddress(mod, "D3DAssemble"); +#undef STR +#undef XSTR + test_swizzle(); test_math(); test_conditionals(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2705
From: Stefan Dösinger <stefan(a)codeweavers.com> --- Changes to files other than d3dcompiler_47.spec are academic because the import libraries they generate don't exist in the Windows SDK. --- dlls/d3dcompiler_41/d3dcompiler_41.spec | 2 +- dlls/d3dcompiler_42/d3dcompiler_42.spec | 2 +- dlls/d3dcompiler_43/d3dcompiler_43.spec | 2 +- dlls/d3dcompiler_46/d3dcompiler_46.spec | 2 +- dlls/d3dcompiler_47/d3dcompiler_47.spec | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/dlls/d3dcompiler_41/d3dcompiler_41.spec b/dlls/d3dcompiler_41/d3dcompiler_41.spec index e5127bc4ab9..604c7748cf3 100644 --- a/dlls/d3dcompiler_41/d3dcompiler_41.spec +++ b/dlls/d3dcompiler_41/d3dcompiler_41.spec @@ -1,4 +1,4 @@ -@ stdcall D3DAssemble(ptr long str ptr ptr long ptr ptr) +@ stdcall -private D3DAssemble(ptr long str ptr ptr long ptr ptr) @ stub DebugSetMute @ stdcall D3DCompile(ptr long str ptr ptr str str long long ptr ptr) @ stub D3DDisassemble10Effect(ptr long ptr) diff --git a/dlls/d3dcompiler_42/d3dcompiler_42.spec b/dlls/d3dcompiler_42/d3dcompiler_42.spec index e5127bc4ab9..604c7748cf3 100644 --- a/dlls/d3dcompiler_42/d3dcompiler_42.spec +++ b/dlls/d3dcompiler_42/d3dcompiler_42.spec @@ -1,4 +1,4 @@ -@ stdcall D3DAssemble(ptr long str ptr ptr long ptr ptr) +@ stdcall -private D3DAssemble(ptr long str ptr ptr long ptr ptr) @ stub DebugSetMute @ stdcall D3DCompile(ptr long str ptr ptr str str long long ptr ptr) @ stub D3DDisassemble10Effect(ptr long ptr) diff --git a/dlls/d3dcompiler_43/d3dcompiler_43.spec b/dlls/d3dcompiler_43/d3dcompiler_43.spec index ab6cfa04aff..7e2d0411b4e 100644 --- a/dlls/d3dcompiler_43/d3dcompiler_43.spec +++ b/dlls/d3dcompiler_43/d3dcompiler_43.spec @@ -1,4 +1,4 @@ -@ stdcall D3DAssemble(ptr long str ptr ptr long ptr ptr) +@ stdcall -private D3DAssemble(ptr long str ptr ptr long ptr ptr) @ stub DebugSetMute @ stdcall D3DCompile(ptr long str ptr ptr str str long long ptr ptr) @ stub D3DCompressShaders diff --git a/dlls/d3dcompiler_46/d3dcompiler_46.spec b/dlls/d3dcompiler_46/d3dcompiler_46.spec index b069779db06..d6742566502 100644 --- a/dlls/d3dcompiler_46/d3dcompiler_46.spec +++ b/dlls/d3dcompiler_46/d3dcompiler_46.spec @@ -1,4 +1,4 @@ -@ stdcall D3DAssemble(ptr long str ptr ptr long ptr ptr) +@ stdcall -private D3DAssemble(ptr long str ptr ptr long ptr ptr) @ stdcall D3DCompile(ptr long str ptr ptr str str long long ptr ptr) @ stdcall D3DCompile2(ptr long str ptr ptr str str long long long ptr long ptr ptr) @ stdcall D3DCompileFromFile(wstr ptr ptr str str long long ptr ptr) diff --git a/dlls/d3dcompiler_47/d3dcompiler_47.spec b/dlls/d3dcompiler_47/d3dcompiler_47.spec index 38ae8960dc5..ca132979565 100644 --- a/dlls/d3dcompiler_47/d3dcompiler_47.spec +++ b/dlls/d3dcompiler_47/d3dcompiler_47.spec @@ -1,4 +1,4 @@ -@ stdcall D3DAssemble(ptr long str ptr ptr long ptr ptr) +@ stdcall -private D3DAssemble(ptr long str ptr ptr long ptr ptr) @ stdcall D3DCompile(ptr long str ptr ptr str str long long ptr ptr) @ stdcall D3DCompile2(ptr long str ptr ptr str str long long long ptr long ptr ptr) @ stdcall D3DCompileFromFile(wstr ptr ptr str str long long ptr ptr) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2705
Why do we care about using Windows import libraries? They seem to cause a lot of problems for us... -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2705#note_31116
On Tue Apr 25 20:07:20 2023 +0000, Zebediah Figura wrote:
Why do we care about using Windows import libraries? They seem to cause a lot of problems for us... It's not the hill I plan to die on, no.
Though I am under the impression that generally we care about compatibility with the Windows SDK - e.g. same include dependencies in headers, not defining private symbols in public headers, etc. Presumably for this reason we don't have D3DAssemble in our d3dcompiler.h either. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2705#note_31130
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/shader.c:
return S_FALSE; }
+static BOOL WINAPI load_d3dassemble_once(INIT_ONCE *once, void *param, void **context) +{ + /* FIXME: This hardcodes the fact that dlls/d3dcompiler_47/Makefile.in calls its + * import lib "d3dcompiler.lib" and that no other version of this DLL took over. + * GetModuleHandleExA(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS, D3DCompile) would + * be nice, but "D3DCompile" will point to the import stub, not d3dcompiler_xy.dll */
Have you checked that's actually the case? Just curious. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2705#note_31185
Matteo Bruni (@Mystral) commented about dlls/d3dcompiler_43/tests/asm.c:
perhaps with a different name? */ #define D3DXERR_INVALIDDATA 0x88760b59
-HRESULT WINAPI D3DAssemble(const void *data, SIZE_T datasize, const char *filename, +static HRESULT (* WINAPI pD3DAssemble)(const void *data, SIZE_T datasize, const char *filename,
I guess that works but we usually write this as (WINAPI *pD3DAssemble), like in the hlsl_d3d9.c hunk below. No big deal of course, but for consistency's sake we might as well go for the "usual" form here too. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2705#note_31186
It wouldn't be a review from me without some bikeshedding... Could you please use some more accurate names for the commits? Something that explains what the commit does ideally, like "d3dx9: Call D3DAssemble() dynamically." or "d3dcompiler: Make D3DAssemble() a private export." -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2705#note_31187
On Tue Apr 25 20:07:20 2023 +0000, Stefan Dösinger wrote:
It's not the hill I plan to die on, no. Though I am under the impression that generally we care about compatibility with the Windows SDK - e.g. same include dependencies in headers, not defining private symbols in public headers, etc. Presumably for this reason we don't have D3DAssemble in our d3dcompiler.h either. The changes seem innocuous enough to me...
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2705#note_31190
On Wed Apr 26 08:38:54 2023 +0000, Matteo Bruni wrote:
Have you checked that's actually the case? Just curious. Sadly yeah, something like this
``` ret = GetModuleHandleExA(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS, (void *)D3DCompile, &mod2); ret = GetModuleHandleExA(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS, (void *)load_d3dassemble_once, &mod3); ERR("mod2 %p, mod3 %p, d3dcompile %p, this function %p, d3dassemble %p, d3dcompiler.dll %p\n", mod2, mod3, D3DCompile, load_d3dassemble_once, *assemble, mod); ``` Gives `26f8:err:d3dx:load_d3dassemble_once mod2 5C510000, mod3 5C510000, d3dcompile 5C56D190, this function 5C5F4220, d3dassemble 5BE64D30, d3dcompiler.dll 5BD00000` So getting the pointer of a implib imported function gives the IAT stub inside our own DLL. If Raymond Chen is correct that would not be the case if we compiled this thing as C++ though ( https://devblogs.microsoft.com/oldnewthing/20060725-00/?p=30383 ). I'm not going to test this :-) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2705#note_31200
On Wed Apr 26 08:38:55 2023 +0000, Matteo Bruni wrote:
I guess that works but we usually write this as (WINAPI *pD3DAssemble), like in the hlsl_d3d9.c hunk below. No big deal of course, but for consistency's sake we might as well go for the "usual" form here too. And the wrong version also causes a compiler warning in msvc (https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compi...), which I didn't spot because I haven't bothered building d3dcompiler tests with msvc yet (and probably never will).
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2705#note_31204
participants (3)
-
Matteo Bruni (＠Mystral) -
Stefan Dösinger
-
Zebediah Figura (＠zfigura)