[PATCH 0/3] MR6925: d3dcompiler: Enable double-as-float option for older modules.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=55448 Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6925
From: Nikolay Sivov <nsivov(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=55448 Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/d3dcompiler_43/compiler.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/dlls/d3dcompiler_43/compiler.c b/dlls/d3dcompiler_43/compiler.c index 6fffcfb82ab..18efcb85001 100644 --- a/dlls/d3dcompiler_43/compiler.c +++ b/dlls/d3dcompiler_43/compiler.c @@ -550,12 +550,14 @@ HRESULT WINAPI D3DCompile2(const void *data, SIZE_T data_size, const char *filen option->value = VKD3D_SHADER_COMPILE_OPTION_PACK_MATRIX_COLUMN_MAJOR; } + option = &options[compile_info.option_count++]; + option->name = VKD3D_SHADER_COMPILE_OPTION_BACKWARD_COMPATIBILITY; + option->value = 0; +#if D3D_COMPILER_VERSION <= 35 + option->value |= VKD3D_SHADER_COMPILE_OPTION_DOUBLE_AS_FLOAT_ALIAS; +#endif if (flags & D3DCOMPILE_ENABLE_BACKWARDS_COMPATIBILITY) - { - option = &options[compile_info.option_count++]; - option->name = VKD3D_SHADER_COMPILE_OPTION_BACKWARD_COMPATIBILITY; - option->value = VKD3D_SHADER_COMPILE_OPTION_BACKCOMPAT_MAP_SEMANTIC_NAMES; - } + option->value |= VKD3D_SHADER_COMPILE_OPTION_BACKCOMPAT_MAP_SEMANTIC_NAMES; if (effect_flags & D3DCOMPILE_EFFECT_CHILD_EFFECT) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6925
From: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/d3dcompiler_43/tests/asm.c | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/dlls/d3dcompiler_43/tests/asm.c b/dlls/d3dcompiler_43/tests/asm.c index 55d84213e1e..001ec979c23 100644 --- a/dlls/d3dcompiler_43/tests/asm.c +++ b/dlls/d3dcompiler_43/tests/asm.c @@ -1740,7 +1740,9 @@ static const DWORD vs_2_0[] = static void test_disassemble_shader(void) { - ID3DBlob *blob; + static const char fx_4_0[] = "technique10 {}"; + static const char fx_5_0[] = "technique11 {}"; + ID3DBlob *blob, *blob2; HRESULT hr; hr = D3DDisassemble(vs_2_0, 0, 0, NULL, &blob); @@ -1753,6 +1755,34 @@ static void test_disassemble_shader(void) hr = D3DDisassemble(vs_2_0, sizeof(vs_2_0), 0, NULL, &blob); ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); ID3D10Blob_Release(blob); + + /* Effects profiles. */ + hr = D3DCompile(fx_4_0, strlen(fx_4_0), NULL, NULL, NULL, "", "fx_4_0", 0, 0, &blob, NULL); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + hr = D3DDisassemble(ID3D10Blob_GetBufferPointer(blob), ID3D10Blob_GetBufferSize(blob), 0, NULL, &blob2); + todo_wine + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + if (SUCCEEDED(hr)) + ID3D10Blob_Release(blob2); + ID3D10Blob_Release(blob); + + hr = D3DCompile(fx_4_0, strlen(fx_4_0), NULL, NULL, NULL, "", "fx_4_1", 0, 0, &blob, NULL); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + hr = D3DDisassemble(ID3D10Blob_GetBufferPointer(blob), ID3D10Blob_GetBufferSize(blob), 0, NULL, &blob2); + todo_wine + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + if (SUCCEEDED(hr)) + ID3D10Blob_Release(blob2); + ID3D10Blob_Release(blob); + + hr = D3DCompile(fx_5_0, strlen(fx_5_0), NULL, NULL, NULL, "", "fx_5_0", 0, 0, &blob, NULL); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + hr = D3DDisassemble(ID3D10Blob_GetBufferPointer(blob), ID3D10Blob_GetBufferSize(blob), 0, NULL, &blob2); + todo_wine + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + if (SUCCEEDED(hr)) + ID3D10Blob_Release(blob2); + ID3D10Blob_Release(blob); } START_TEST(asm) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6925
From: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/d3dcompiler_43/compiler.c | 36 +++++++++++++++++++++++++++------ dlls/d3dcompiler_43/tests/asm.c | 12 +++-------- 2 files changed, 33 insertions(+), 15 deletions(-) diff --git a/dlls/d3dcompiler_43/compiler.c b/dlls/d3dcompiler_43/compiler.c index 18efcb85001..e68ee6b6cec 100644 --- a/dlls/d3dcompiler_43/compiler.c +++ b/dlls/d3dcompiler_43/compiler.c @@ -672,9 +672,12 @@ HRESULT WINAPI D3DDisassemble(const void *data, SIZE_T size, UINT flags, const c { struct vkd3d_shader_compile_info compile_info; enum vkd3d_shader_source_type source_type; - struct vkd3d_shader_code asm_code; + struct vkd3d_shader_code source, asm_code; + struct vkd3d_shader_dxbc_desc desc; const char *ptr = data; + unsigned int i; char *messages; + uint32_t token; HRESULT hr; int ret; @@ -692,15 +695,36 @@ HRESULT WINAPI D3DDisassemble(const void *data, SIZE_T size, UINT flags, const c return E_INVALIDARG; #endif - if (size >= 4 && read_u32(&ptr) == TAG_DXBC) + source.code = data; + source.size = size; + + source_type = VKD3D_SHADER_SOURCE_D3D_BYTECODE; + if (vkd3d_shader_parse_dxbc(&source, 0, &desc, NULL) >= 0) + { source_type = VKD3D_SHADER_SOURCE_DXBC_TPF; - else - source_type = VKD3D_SHADER_SOURCE_D3D_BYTECODE; + + for (i = 0; i < desc.section_count; ++i) + { + if (TAG_FX10 == desc.sections[i].tag) + { + source = desc.sections[i].data; + source_type = VKD3D_SHADER_SOURCE_FX; + break; + } + } + + vkd3d_shader_free_dxbc(&desc); + } + else if (size >= 4) + { + token = read_u32(&ptr); + if ((token & 0xffff0000) == 0xfeff0000) + source_type = VKD3D_SHADER_SOURCE_FX; + } compile_info.type = VKD3D_SHADER_STRUCTURE_TYPE_COMPILE_INFO; compile_info.next = NULL; - compile_info.source.code = data; - compile_info.source.size = size; + compile_info.source = source; compile_info.source_type = source_type; compile_info.target_type = VKD3D_SHADER_TARGET_D3D_ASM; compile_info.options = NULL; diff --git a/dlls/d3dcompiler_43/tests/asm.c b/dlls/d3dcompiler_43/tests/asm.c index 001ec979c23..95ea11e17b5 100644 --- a/dlls/d3dcompiler_43/tests/asm.c +++ b/dlls/d3dcompiler_43/tests/asm.c @@ -1760,28 +1760,22 @@ static void test_disassemble_shader(void) hr = D3DCompile(fx_4_0, strlen(fx_4_0), NULL, NULL, NULL, "", "fx_4_0", 0, 0, &blob, NULL); ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); hr = D3DDisassemble(ID3D10Blob_GetBufferPointer(blob), ID3D10Blob_GetBufferSize(blob), 0, NULL, &blob2); - todo_wine ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); - if (SUCCEEDED(hr)) - ID3D10Blob_Release(blob2); + ID3D10Blob_Release(blob2); ID3D10Blob_Release(blob); hr = D3DCompile(fx_4_0, strlen(fx_4_0), NULL, NULL, NULL, "", "fx_4_1", 0, 0, &blob, NULL); ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); hr = D3DDisassemble(ID3D10Blob_GetBufferPointer(blob), ID3D10Blob_GetBufferSize(blob), 0, NULL, &blob2); - todo_wine ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); - if (SUCCEEDED(hr)) - ID3D10Blob_Release(blob2); + ID3D10Blob_Release(blob2); ID3D10Blob_Release(blob); hr = D3DCompile(fx_5_0, strlen(fx_5_0), NULL, NULL, NULL, "", "fx_5_0", 0, 0, &blob, NULL); ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); hr = D3DDisassemble(ID3D10Blob_GetBufferPointer(blob), ID3D10Blob_GetBufferSize(blob), 0, NULL, &blob2); - todo_wine ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); - if (SUCCEEDED(hr)) - ID3D10Blob_Release(blob2); + ID3D10Blob_Release(blob2); ID3D10Blob_Release(blob); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6925
Matteo Bruni (@Mystral) commented about dlls/d3dcompiler_43/compiler.c:
#endif
- if (size >= 4 && read_u32(&ptr) == TAG_DXBC) + source.code = data; + source.size = size; + + source_type = VKD3D_SHADER_SOURCE_D3D_BYTECODE; + if (vkd3d_shader_parse_dxbc(&source, 0, &desc, NULL) >= 0) + { source_type = VKD3D_SHADER_SOURCE_DXBC_TPF; - else - source_type = VKD3D_SHADER_SOURCE_D3D_BYTECODE; + + for (i = 0; i < desc.section_count; ++i) + { + if (TAG_FX10 == desc.sections[i].tag) Very nitpicky, but I think we consistently put the constant on the right side in this kind of comparisons.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6925#note_89507
Matteo Bruni (@Mystral) commented about dlls/d3dcompiler_43/compiler.c:
option->value = VKD3D_SHADER_COMPILE_OPTION_PACK_MATRIX_COLUMN_MAJOR; }
+ option = &options[compile_info.option_count++]; + option->name = VKD3D_SHADER_COMPILE_OPTION_BACKWARD_COMPATIBILITY; + option->value = 0; +#if D3D_COMPILER_VERSION <= 35 + option->value |= VKD3D_SHADER_COMPILE_OPTION_DOUBLE_AS_FLOAT_ALIAS; +#endif
FWIW I see no compilation failure with d3dx9_24, d3dx9_36 or d3dx9_43 (or even with d3dcompiler_43, for that matter). I do see the expected error with d3dcompiler_47. That was with a ps_2_0 target and on 32-bit, in case it matters. I haven't tested it exhaustively yet, maybe it is in fact target-dependent and we want something more specific. Which brings me to: I think we want some tests for this [*]. I hacked something locally to look into it; I guess I should make a proper MR that you can then expand further if needed. I'll see what I can quickly come up with. Depending on the test results, it might be that this patch is generally right e.g. it's just a matter of updating the `#if` condition, in which case I'm okay with the patch in principle. [*] We currently don't have tests for d3dx9_xx with xx != 36 at all. Ideally we'd replicate the tests for all the DLL iterations e.g. with PARENTSRC like we do for d3dcompiler, but that opens a huge can of worms and especially now it doesn't seem to be the time, with code freeze coming very soon. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6925#note_89506
After b2a6af3677c5 we need this to live in `D3DCompile2VKD3D()`. I'll probably get to it at some point but anyone's very welcome to go ahead and open a vkd3d-utils MR for it. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6925#note_97575
This merge request was closed by Matteo Bruni. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6925
On Wed Mar 12 18:49:08 2025 +0000, Matteo Bruni wrote:
After b2a6af3677c5 we need this to live in `D3DCompile2VKD3D()`. I'll probably get to it at some point but anyone's very welcome to go ahead and open a vkd3d-utils MR for it. With D3DCompile2VKD3D() we don't really need those compatibility options at all, or at least we don't need to add more.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6925#note_97578
On Wed Mar 12 18:49:08 2025 +0000, Nikolay Sivov wrote:
With D3DCompile2VKD3D() we don't really need those compatibility options at all, or at least we don't need to add more. Not on the Wine side, no. Don't we need to handle the double-as-float thing in there though?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6925#note_97579
On Wed Mar 12 18:54:18 2025 +0000, Matteo Bruni wrote:
Not on the Wine side, no. Don't we need to handle the double-as-float thing in there though? In where? I mean if we have this single function now that will presumably enable every quirk it needs to enable in vkd3d-shader, using just a version number, we don't really need those non-standard options in vkd3d api. That's what I meant.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6925#note_97580
On Wed Mar 12 18:58:46 2025 +0000, Nikolay Sivov wrote:
In where? I mean if we have this single function now that will presumably enable every quirk it needs to enable in vkd3d-shader, using just a version number, we don't really need those non-standard options in vkd3d api. That's what I meant. I think there's a bit of talking-past going on here; Matteo is pointing out that D3DCompile2VKD3D() doesn't currently have this version-specific behaviour implemented yet, so we need to submit a patch to vkd3d for it.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6925#note_97583
On Wed Mar 12 19:11:22 2025 +0000, Elizabeth Figura wrote:
I think there's a bit of talking-past going on here; Matteo is pointing out that D3DCompile2VKD3D() doesn't currently have this version-specific behaviour implemented yet, so we need to submit a patch to vkd3d for it. Well sure, I'm talking about vkd3d-shader side already, whether we want this option exposed and documented for any vkd3d_shader_compile() user, or make it into something internal. I don't see how any end user of vkd3d-shader, outside of wine, would think of using any of those custom options. Also I'm surprised we ended up having any logic involving windows module version numbering in vkd3d-shader.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6925#note_97584
On Wed Mar 12 19:19:26 2025 +0000, Nikolay Sivov wrote:
Well sure, I'm talking about vkd3d-shader side already, whether we want this option exposed and documented for any vkd3d_shader_compile() user, or make it into something internal. I don't see how any end user of vkd3d-shader, outside of wine, would think of using any of those custom options. Also I'm surprised we ended up having any logic involving windows module version numbering in vkd3d-shader. It's in vkd3d-utils though, not vkd3d-shader.
The `compiler_version` argument to `D3DCompile2VKD3D()` seems pretty reasonable to me, each _xx version effectively represents a separate API. Sometimes they happen to match, but sometimes not, which leads to needing to pass different vkd3d-shader flags depending on the version. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6925#note_97585
Well sure, I'm talking about vkd3d-shader side already, whether we want this option exposed and documented for any vkd3d_shader_compile() user, or make it into something internal.
I don't understand? It already is exposed and documented. We can't remove it.
I don't see how any end user of vkd3d-shader, outside of wine, would think of using any of those custom options.
I can't say I see it as likely either, but giving vkd3d-utils a secret unstable window into vkd3d-shader seems unnecessary for a case like this.
Also I'm surprised we ended up having any logic involving windows module version numbering in vkd3d-shader.
As Matteo says it's not in vkd3d-shader; it's in vkd3d-utils. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6925#note_97586
participants (4)
-
Elizabeth Figura (＠zfigura) -
Matteo Bruni (＠Mystral)
-
Nikolay Sivov
-
Nikolay Sivov (＠nsivov)