[PATCH 0/4] MR114: shader-runner: Add some tests for "technique" tokens.
Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/114
From: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- tests/shader_runner.c | 48 +++++++++++++++++++++++-------------- tests/shader_runner.h | 2 ++ tests/shader_runner_d3d11.c | 12 ++++++++++ tests/shader_runner_d3d12.c | 9 +++++++ 4 files changed, 53 insertions(+), 18 deletions(-) diff --git a/tests/shader_runner.c b/tests/shader_runner.c index 507fbdaf..54736b89 100644 --- a/tests/shader_runner.c +++ b/tests/shader_runner.c @@ -106,30 +106,41 @@ static bool match_string(const char *line, const char *token, const char **const return true; } -static void parse_require_directive(struct shader_runner *runner, const char *line) +static void parse_get_shader_model(const char *line, enum shader_model *model) { - if (match_string(line, "shader model >=", &line)) + static const char *const model_strings[] = { - static const char *const model_strings[] = - { - [SHADER_MODEL_2_0] = "2.0", - [SHADER_MODEL_4_0] = "4.0", - [SHADER_MODEL_4_1] = "4.1", - [SHADER_MODEL_5_0] = "5.0", - [SHADER_MODEL_5_1] = "5.1", - }; - unsigned int i; + [SHADER_MODEL_2_0] = "2.0", + [SHADER_MODEL_4_0] = "4.0", + [SHADER_MODEL_4_1] = "4.1", + [SHADER_MODEL_5_0] = "5.0", + [SHADER_MODEL_5_1] = "5.1", + }; + unsigned int i; - for (i = 0; i < ARRAY_SIZE(model_strings); ++i) + for (i = 0; i < ARRAY_SIZE(model_strings); ++i) + { + if (match_string(line, model_strings[i], &line)) { - if (match_string(line, model_strings[i], &line)) - { - runner->minimum_shader_model = i; - return; - } + *model = i; + return; } + } - fatal_error("Unknown shader model '%s'.\n", line); + fatal_error("Unknown shader model '%s'.\n", line); +} + +static void parse_require_directive(struct shader_runner *runner, const char *line) +{ + if (match_string(line, "shader model >=", &line)) + { + parse_get_shader_model(line, &runner->minimum_shader_model); + runner->maximum_shader_model = SHADER_MODEL_COUNT; + } + else if (match_string(line, "shader model ==", &line)) + { + parse_get_shader_model(line, &runner->minimum_shader_model); + runner->maximum_shader_model = runner->minimum_shader_model; } else { @@ -706,6 +717,7 @@ void run_shader_tests(struct shader_runner *runner, int argc, char **argv, const FILE *f; runner->minimum_shader_model = SHADER_MODEL_2_0; + runner->maximum_shader_model = SHADER_MODEL_COUNT; for (i = 1; i < argc; ++i) { diff --git a/tests/shader_runner.h b/tests/shader_runner.h index 82d7fa18..876a43ca 100644 --- a/tests/shader_runner.h +++ b/tests/shader_runner.h @@ -34,6 +34,7 @@ enum shader_model SHADER_MODEL_4_1, SHADER_MODEL_5_0, SHADER_MODEL_5_1, + SHADER_MODEL_COUNT, }; enum texture_data_type @@ -105,6 +106,7 @@ struct shader_runner char *ps_source; char *cs_source; enum shader_model minimum_shader_model; + enum shader_model maximum_shader_model; bool last_render_failed; diff --git a/tests/shader_runner_d3d11.c b/tests/shader_runner_d3d11.c index ba9e3179..aa541556 100644 --- a/tests/shader_runner_d3d11.c +++ b/tests/shader_runner_d3d11.c @@ -322,6 +322,17 @@ static void destroy_test_context(struct d3d11_shader_runner *runner) ok(!ref, "Device has %lu references left.\n", ref); } +static bool d3d11_runner_check_requirements(struct shader_runner *r) +{ + if (r->minimum_shader_model > SHADER_MODEL_4_0) + return false; + + if (r->maximum_shader_model < SHADER_MODEL_4_0) + return false; + + return true; +} + static ID3D11Buffer *create_buffer(ID3D11Device *device, unsigned int bind_flags, unsigned int size, const void *data) { D3D11_SUBRESOURCE_DATA resource_data; @@ -680,6 +691,7 @@ static void d3d11_runner_release_readback(struct shader_runner *r, struct resour static const struct shader_runner_ops d3d11_runner_ops = { + .check_requirements = d3d11_runner_check_requirements, .create_resource = d3d11_runner_create_resource, .destroy_resource = d3d11_runner_destroy_resource, .dispatch = d3d11_runner_dispatch, diff --git a/tests/shader_runner_d3d12.c b/tests/shader_runner_d3d12.c index 3e661151..b932446a 100644 --- a/tests/shader_runner_d3d12.c +++ b/tests/shader_runner_d3d12.c @@ -83,6 +83,14 @@ static ID3D10Blob *compile_shader(const struct d3d12_shader_runner *runner, cons return blob; } +static bool d3d12_runner_check_requirements(struct shader_runner *r) +{ + if (r->maximum_shader_model < SHADER_MODEL_5_0) + return false; + + return true; +} + #define MAX_RESOURCE_DESCRIPTORS (MAX_RESOURCES * 2) static struct resource *d3d12_runner_create_resource(struct shader_runner *r, const struct resource_params *params) @@ -501,6 +509,7 @@ static void d3d12_runner_release_readback(struct shader_runner *r, struct resour static const struct shader_runner_ops d3d12_runner_ops = { + .check_requirements = d3d12_runner_check_requirements, .create_resource = d3d12_runner_create_resource, .destroy_resource = d3d12_runner_destroy_resource, .dispatch = d3d12_runner_dispatch, -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/114
From: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- tests/shader_runner.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/shader_runner.c b/tests/shader_runner.c index 54736b89..383f5aca 100644 --- a/tests/shader_runner.c +++ b/tests/shader_runner.c @@ -676,7 +676,7 @@ static void compile_shader(struct shader_runner *runner, const char *source, siz static const char *const shader_models[] = { - [SHADER_MODEL_2_0] = "4_0", + [SHADER_MODEL_2_0] = "2_0", [SHADER_MODEL_4_0] = "4_0", [SHADER_MODEL_4_1] = "4_1", [SHADER_MODEL_5_0] = "5_0", -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/114
From: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- tests/shader_runner.c | 35 +++++++++++++++++++++++++++++++++++ tests/shader_runner.h | 1 + 2 files changed, 36 insertions(+) diff --git a/tests/shader_runner.c b/tests/shader_runner.c index 383f5aca..4cf72fb6 100644 --- a/tests/shader_runner.c +++ b/tests/shader_runner.c @@ -85,6 +85,8 @@ enum parse_state STATE_SHADER_PIXEL, STATE_SHADER_PIXEL_TODO, STATE_SHADER_VERTEX, + STATE_EFFECT, + STATE_EFFECT_TODO, STATE_TEST, }; @@ -800,6 +802,17 @@ void run_shader_tests(struct shader_runner *runner, int argc, char **argv, const shader_source_size = 0; break; + case STATE_EFFECT: + case STATE_EFFECT_TODO: + todo_if (state == STATE_EFFECT_TODO) + compile_shader(runner, shader_source, shader_source_len, "fx", expect_hr); + free(runner->fx_source); + runner->fx_source = shader_source; + shader_source = NULL; + shader_source_len = 0; + shader_source_size = 0; + break; + case STATE_PREPROC_INVALID: { ID3D10Blob *blob = NULL, *errors = NULL; @@ -995,6 +1008,26 @@ void run_shader_tests(struct shader_runner *runner, int argc, char **argv, const { state = STATE_SHADER_VERTEX; } + else if (!strcmp(line, "[effect]\n")) + { + state = STATE_EFFECT; + expect_hr = S_OK; + } + else if (!strcmp(line, "[effect todo]\n")) + { + state = STATE_EFFECT_TODO; + expect_hr = S_OK; + } + else if (!strcmp(line, "[effect fail]\n")) + { + state = STATE_EFFECT; + expect_hr = E_FAIL; + } + else if (!strcmp(line, "[effect fail todo]\n")) + { + state = STATE_EFFECT_TODO; + expect_hr = E_FAIL; + } else if (!strcmp(line, "[input layout]\n")) { state = STATE_INPUT_LAYOUT; @@ -1025,6 +1058,8 @@ void run_shader_tests(struct shader_runner *runner, int argc, char **argv, const case STATE_SHADER_PIXEL: case STATE_SHADER_PIXEL_TODO: case STATE_SHADER_VERTEX: + case STATE_EFFECT: + case STATE_EFFECT_TODO: { size_t len = strlen(line); diff --git a/tests/shader_runner.h b/tests/shader_runner.h index 876a43ca..2e4ca506 100644 --- a/tests/shader_runner.h +++ b/tests/shader_runner.h @@ -105,6 +105,7 @@ struct shader_runner char *vs_source; char *ps_source; char *cs_source; + char *fx_source; enum shader_model minimum_shader_model; enum shader_model maximum_shader_model; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/114
From: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- Makefile.am | 2 + tests/fx-technique-fx_2.shader_test | 105 ++++++++++++++++++++++++++++ tests/fx-technique-fx_4.shader_test | 83 ++++++++++++++++++++++ 3 files changed, 190 insertions(+) create mode 100644 tests/fx-technique-fx_2.shader_test create mode 100644 tests/fx-technique-fx_4.shader_test diff --git a/Makefile.am b/Makefile.am index 29692432..0b73fee9 100644 --- a/Makefile.am +++ b/Makefile.am @@ -66,6 +66,8 @@ vkd3d_shader_tests = \ tests/floor.shader_test \ tests/frac.shader_test \ tests/function-return.shader_test \ + tests/fx-technique-fx_2.shader_test \ + tests/fx-technique-fx_4.shader_test \ tests/hlsl-array-dimension.shader_test \ tests/hlsl-attributes.shader_test \ tests/hlsl-bool-cast.shader_test \ diff --git a/tests/fx-technique-fx_2.shader_test b/tests/fx-technique-fx_2.shader_test new file mode 100644 index 00000000..acc81e27 --- /dev/null +++ b/tests/fx-technique-fx_2.shader_test @@ -0,0 +1,105 @@ +[require] +shader model == 2.0 + + +[pixel shader fail todo] +float4 main() : SV_TARGET +{ + float4 teChnique = {0, 0, 0, 0}; + return teChnique; +} + +[pixel shader fail] +float4 main() : sv_target +{ + float4 technique10 = {0, 0, 0, 0}; + return technique10; +} + +[pixel shader fail todo] +float4 main() : sv_target +{ + float4 technique11 = {0, 0, 0, 0}; + return technique11; +} + +[pixel shader fail] +typedef float4 technique10; + +float4 main() : sv_target +{ + return float4(0, 0, 0, 0); +} + +[pixel shader fail todo] +typedef float4 Technique; + +float4 main() : sv_target +{ + return float4(0, 0, 0, 0); +} + +[pixel shader] +typedef float4 Technique10; +typedef float4 Technique11; + +float4 main() : sv_target +{ + return float4(0, 0, 0, 0); +} + +[pixel shader] +float4 main() : sv_target +{ + float4 teChnique10 = {0, 0, 0, 0}; + float4 teChnique11 = {0, 0, 0, 0}; + return teChnique10 + teChnique11; +} + +[pixel shader] +float4 main() : sv_target +{ + float4 teChnique11 = {0, 0, 0, 0}; + return teChnique11; +} + +[effect todo] +technique +{ +} + +technique10 +{ +} + +% Effects without techniques are not allowed for fx_2_0 +[effect fail] +float4 f; + +% fx_5_0 keyword fails with fx_2_0 profile +[effect fail] +technique +{ +} + +technique11 +{ +} + +[effect fail] +technique +{ +} + +tEchnique10 +{ +} + +[effect fail] +technique +{ +} + +tEchnique11 +{ +} diff --git a/tests/fx-technique-fx_4.shader_test b/tests/fx-technique-fx_4.shader_test new file mode 100644 index 00000000..299798e0 --- /dev/null +++ b/tests/fx-technique-fx_4.shader_test @@ -0,0 +1,83 @@ +[require] +shader model == 4.0 + + +[pixel shader fail todo] +float4 main() : SV_TARGET +{ + float4 teChnique = {0, 0, 0, 0}; + return teChnique; +} + +[pixel shader] +float4 main() : SV_TARGET +{ + float4 teChnique10 = {0, 0, 0, 0}; + return teChnique10; +} + +[pixel shader] +float4 main() : SV_TARGET +{ + float4 teChnique11 = {0, 0, 0, 0}; + return teChnique11; +} + +[effect todo] +technique +{ +} + +technique10 +{ +} + +% Effects without techniques are not allowed for fx_4_0+ +[effect todo] +float4 f; + +% fx_2_0 keyword is allowed with fx_4_0+ profiles +[effect todo] +technique +{ +} + +technique11 +{ +} + +[effect fail] +technique +{ +} + +tEchnique10 +{ +} + +[effect fail] +technique +{ +} + +tEchnique11 +{ +} + +[effect fail] +float4 technique; + +[effect fail] +float4 technIque; + +[effect fail] +float4 technique10; + +[effect fail] +float4 technique11; + +[effect todo] +float4 technIque10; + +[effect todo] +float4 technIque11; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/114
Zebediah Figura (@zfigura) commented about tests/shader_runner_d3d11.c:
ok(!ref, "Device has %lu references left.\n", ref); }
+static bool d3d11_runner_check_requirements(struct shader_runner *r) +{ + if (r->minimum_shader_model > SHADER_MODEL_4_0) + return false;
That doesn't seem right; d3d11 can handle 4.0 up to 5.0. It can't handle 5.1 I assume, although that's also mildly out of scope of this patch. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/114#note_26150
Zebediah Figura (@zfigura) commented about tests/shader_runner_d3d12.c:
return blob; }
+static bool d3d12_runner_check_requirements(struct shader_runner *r) +{ + if (r->maximum_shader_model < SHADER_MODEL_5_0) + return false;
d3d12 should be able to handle 4.0. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/114#note_26151
Zebediah Figura (@zfigura) commented about tests/shader_runner.c:
return true; }
-static void parse_require_directive(struct shader_runner *runner, const char *line) +static void parse_get_shader_model(const char *line, enum shader_model *model)
Any reason not to just return the model? -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/114#note_26153
Zebediah Figura (@zfigura) commented about tests/shader_runner.c:
FILE *f;
runner->minimum_shader_model = SHADER_MODEL_2_0; + runner->maximum_shader_model = SHADER_MODEL_COUNT;
This seems a bit unclear, or at least oddly named. How about defining SHADER_MODEL_MAX and aliasing that to 5_1? -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/114#note_26152
On Fri Mar 3 23:29:38 2023 +0000, Zebediah Figura wrote:
That doesn't seem right; d3d11 can handle 4.0 up to 5.0. It can't handle 5.1 I assume, although that's also mildly out of scope of this patch. Yes, this does not have to be that confusing. If we are going to compile in advance to check for compilation errors, it does not matter what runner supports. So maybe we should run compilation first, report compilation errors, and then run d3d9/d3d11/d3d12/vulkan tests passes. This way check_requirements() could be adjusted to reflect actual support - 5.0 in d3d11 for example. For compilation pass we don't need to check requirements at all.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/114#note_26158
On Sat Mar 4 08:05:55 2023 +0000, Nikolay Sivov wrote:
Yes, this does not have to be that confusing. If we are going to compile in advance to check for compilation errors, it does not matter what runner supports. So maybe we should run compilation first, report compilation errors, and then run d3d9/d3d11/d3d12/vulkan tests passes. This way check_requirements() could be adjusted to reflect actual support - 5.0 in d3d11 for example. For compilation pass we don't need to check requirements at all. Another approach could be adding optional explicit profile to header like [effect fx_2_0]. Requirement setting with [require] could then be used only for draw tests. There is one more thing to consider, right now if check_requirements() fails the rest of the test source is ignored, that's not ideal and we could read until next [require] to re-check.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/114#note_26159
Another approach could be adding optional explicit profile to header like \[effect fx_2_0\]. Requirement setting with \[require\] could then be used only for draw tests.
I don't think I understand. Why would we want to use different profiles between the compile test and draw tests?
There is one more thing to consider, right now if check_requirements() fails the rest of the test source is ignored, that's not ideal and we could read until next \[require\] to re-check.
Probably. The original intent was basically that there would be one [require] section at the top which applied to the whole file, but that's not quite how we've been using it... -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/114#note_26231
Another approach could be adding optional explicit profile to header like \[effect fx_2_0\]. Requirement setting with \[require\] could then be used only for draw tests.
I don't think I understand. Why would we want to use different profiles between the compile test and draw tests?
Because they are not related. Drawing tests use specific runtime system, each of these support a number of compilation targets. To test that something compiles without drawing it we don't need to know what is supported by particular runner. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/114#note_26261
On Mon Mar 6 22:11:33 2023 +0000, Nikolay Sivov wrote:
Another approach could be adding optional explicit profile to header like \[effect fx_2_0\]. Requirement setting with \[require\] could then be used only for draw tests.
I don't think I understand. Why would we want to use different profiles between the compile test and draw tests? Because they are not related. Drawing tests use specific runtime system, each of these support a number of compilation targets. To test that something compiles without drawing it we don't need to know what is supported by particular runner. I still don't understand this assertion. The minimum shader model is a property of the HLSL. We want to compile-test it with that version, and then also draw-test it with any backend that can support it. The backend might need to promote it to a higher version, but there should be no reason that the person writing the tests would care.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/114#note_26271
As far as I remember, we'd still use `tests` as subject prefix even when working on the shader runner specifically. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/114#note_26524
On Mon Mar 6 22:41:36 2023 +0000, Zebediah Figura wrote:
I still don't understand this assertion. The minimum shader model is a property of the HLSL. We want to compile-test it with that version, and then also draw-test it with any backend that can support it. The backend might need to promote it to a higher version, but there should be no reason that the person writing the tests would care. Reading this again, I think there's some talking past each other going on here?
My vision is: if we have a shader [effect or not] that is capable of being compiled with 2.0, we'd like to (a) compile-test it with 2.0, (b) test it with every backend that is capable of handling the shader. For some backends, that may mean implicitly promoting the version to 4.0. In light of that I don't see why we'd need to specify the version separately in [require] and in the shader header. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/114#note_26551
participants (4)
-
Giovanni Mascellani (＠giomasce) -
Nikolay Sivov
-
Nikolay Sivov (＠nsivov)
-
Zebediah Figura (＠zfigura)