[PATCH 0/2] MR475: vkd3d-shader/hlsl: Add radians() and degrees() functions.
From: Akihiro Sagawa <sagawa.aki(a)gmail.com> --- Makefile.am | 1 + libs/vkd3d-shader/hlsl.y | 16 ++++++++++++++++ tests/hlsl/angle-unit.shader_test | 12 ++++++++++++ 3 files changed, 29 insertions(+) create mode 100644 tests/hlsl/angle-unit.shader_test diff --git a/Makefile.am b/Makefile.am index 002746829..de7398af6 100644 --- a/Makefile.am +++ b/Makefile.am @@ -46,6 +46,7 @@ vkd3d_cross_tests = \ vkd3d_shader_tests = \ tests/hlsl/abs.shader_test \ tests/hlsl/all.shader_test \ + tests/hlsl/angle-unit.shader_test \ tests/hlsl/any.shader_test \ tests/hlsl/arithmetic-float-uniform.shader_test \ tests/hlsl/arithmetic-float.shader_test \ diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index e8f84fe64..1b3b37d90 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -3341,6 +3341,21 @@ static bool intrinsic_pow(struct hlsl_ctx *ctx, return !!add_pow_expr(ctx, params->instrs, params->args[0], params->args[1], loc); } +static bool intrinsic_radians(struct hlsl_ctx *ctx, + const struct parse_initializer *params, const struct vkd3d_shader_location *loc) +{ + struct hlsl_ir_node *arg, *rad; + + if (!(arg = intrinsic_float_convert_arg(ctx, params, params->args[0], loc))) + return false; + + /* 1 degree = pi/180 rad = 0.0174532925f rad */ + if (!(rad = hlsl_new_float_constant(ctx, 0.0174532925f, loc))) + return false; + + return !!add_binary_arithmetic_expr(ctx, params->instrs, HLSL_OP2_MUL, arg, rad, loc); +} + static bool intrinsic_reflect(struct hlsl_ctx *ctx, const struct parse_initializer *params, const struct vkd3d_shader_location *loc) { @@ -3869,6 +3884,7 @@ intrinsic_functions[] = {"mul", 2, true, intrinsic_mul}, {"normalize", 1, true, intrinsic_normalize}, {"pow", 2, true, intrinsic_pow}, + {"radians", 1, true, intrinsic_radians}, {"reflect", 2, true, intrinsic_reflect}, {"round", 1, true, intrinsic_round}, {"rsqrt", 1, true, intrinsic_rsqrt}, diff --git a/tests/hlsl/angle-unit.shader_test b/tests/hlsl/angle-unit.shader_test new file mode 100644 index 000000000..89bd14958 --- /dev/null +++ b/tests/hlsl/angle-unit.shader_test @@ -0,0 +1,12 @@ +[pixel shader] +uniform float4 a; + +float4 main() : sv_target +{ + return radians(a); +} + +[test] +uniform 0 float4 0.0 30.0 150.0 180.0 +todo(sm>=6) draw quad +probe all rgba (0.0, 0.52359877, 2.61799387, 3.14159265) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/475
From: Akihiro Sagawa <sagawa.aki(a)gmail.com> --- libs/vkd3d-shader/hlsl.y | 16 ++++++++++++++++ tests/hlsl/angle-unit.shader_test | 14 ++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 1b3b37d90..87de4d0b6 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -2878,6 +2878,21 @@ static bool intrinsic_ddy_coarse(struct hlsl_ctx *ctx, return !!add_unary_arithmetic_expr(ctx, params->instrs, HLSL_OP1_DSY_COARSE, arg, loc); } +static bool intrinsic_degrees(struct hlsl_ctx *ctx, + const struct parse_initializer *params, const struct vkd3d_shader_location *loc) +{ + struct hlsl_ir_node *arg, *deg; + + if (!(arg = intrinsic_float_convert_arg(ctx, params, params->args[0], loc))) + return false; + + /* 1 rad = 180/pi degree = 57.2957795 degree */ + if (!(deg = hlsl_new_float_constant(ctx, 57.2957795, loc))) + return false; + + return !!add_binary_arithmetic_expr(ctx, params->instrs, HLSL_OP2_MUL, arg, deg, loc); +} + static bool intrinsic_ddy_fine(struct hlsl_ctx *ctx, const struct parse_initializer *params, const struct vkd3d_shader_location *loc) { @@ -3864,6 +3879,7 @@ intrinsic_functions[] = {"ddy", 1, true, intrinsic_ddy}, {"ddy_coarse", 1, true, intrinsic_ddy_coarse}, {"ddy_fine", 1, true, intrinsic_ddy_fine}, + {"degrees", 1, true, intrinsic_degrees}, {"distance", 2, true, intrinsic_distance}, {"dot", 2, true, intrinsic_dot}, {"exp", 1, true, intrinsic_exp}, diff --git a/tests/hlsl/angle-unit.shader_test b/tests/hlsl/angle-unit.shader_test index 89bd14958..af4b01e7d 100644 --- a/tests/hlsl/angle-unit.shader_test +++ b/tests/hlsl/angle-unit.shader_test @@ -10,3 +10,17 @@ float4 main() : sv_target uniform 0 float4 0.0 30.0 150.0 180.0 todo(sm>=6) draw quad probe all rgba (0.0, 0.52359877, 2.61799387, 3.14159265) + + +[pixel shader] +uniform float4 a; + +float4 main() : sv_target +{ + return degrees(a); +} + +[test] +uniform 0 float4 0.0 0.78539816 1.57079632 2.35619449 +todo(sm>=6) draw quad +probe all rgba (0.0, 45.0, 90.0, 135.0) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/475
Francisco Casas (@fcasas) commented about libs/vkd3d-shader/hlsl.y:
return !!add_pow_expr(ctx, params->instrs, params->args[0], params->args[1], loc); }
+static bool intrinsic_radians(struct hlsl_ctx *ctx, + const struct parse_initializer *params, const struct vkd3d_shader_location *loc) +{ + struct hlsl_ir_node *arg, *rad; + + if (!(arg = intrinsic_float_convert_arg(ctx, params, params->args[0], loc))) + return false; + + /* 1 degree = pi/180 rad = 0.0174532925f rad */ + if (!(rad = hlsl_new_float_constant(ctx, 0.0174532925f, loc)))
When creating a node using a `hlsl_new_*()` function you also have to add it to the instruction block using `hlsl_block_add_instr()`, as you can see in other uses of `hlsl_new_float_constant()` in the same file. More complex functions such as `intrinsic_float_convert_arg()` and those that start with `add_` do it themselves, so they don't require to call `hlsl_block_add_instr()` afterwards. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/475#note_52443
Francisco Casas (@fcasas) commented about libs/vkd3d-shader/hlsl.y:
return !!add_unary_arithmetic_expr(ctx, params->instrs, HLSL_OP1_DSY_COARSE, arg, loc); }
+static bool intrinsic_degrees(struct hlsl_ctx *ctx, + const struct parse_initializer *params, const struct vkd3d_shader_location *loc) +{ + struct hlsl_ir_node *arg, *deg; + + if (!(arg = intrinsic_float_convert_arg(ctx, params, params->args[0], loc))) + return false; + + /* 1 rad = 180/pi degree = 57.2957795 degree */ + if (!(deg = hlsl_new_float_constant(ctx, 57.2957795, loc)))
Same for this new float constant. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/475#note_52444
Francisco Casas (@fcasas) commented about tests/hlsl/angle-unit.shader_test:
uniform 0 float4 0.0 30.0 150.0 180.0 todo(sm>=6) draw quad probe all rgba (0.0, 0.52359877, 2.61799387, 3.14159265) + + +[pixel shader] +uniform float4 a; + +float4 main() : sv_target +{ + return degrees(a); +} + +[test] +uniform 0 float4 0.0 0.78539816 1.57079632 2.35619449 +todo(sm>=6) draw quad Both tests work thanks to the work being done to support DXIL, so these `todo(sm>=6)` qualifiers can be removed in both commits.
To check this by yourself you need to configure the shader runner to load libdxcompiler.so from Microsoft, as explained in the README, but for now I think that removing the `todo(sm>=6)` tags is enough. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/475#note_52445
I am ready to approve once my comments have been addressed. Besides those, the implementation seems correct to me. Thanks! -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/475#note_52446
On Wed Nov 15 22:50:11 2023 +0000, Francisco Casas wrote:
When creating a node using a `hlsl_new_*()` function you also have to add it to the instruction block using `hlsl_block_add_instr()`, as you can see in other uses of `hlsl_new_float_constant()` in the same file. More complex functions such as `intrinsic_float_convert_arg()` and those that start with `add_` do it themselves, so they don't require to call `hlsl_block_add_instr()` afterwards. I wonder if it was copied from log/log10. I see I missed add_insrt() in these two. We probably could have an add_float_constant() helper.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/475#note_52447
On Wed Nov 15 23:09:51 2023 +0000, Nikolay Sivov wrote:
I wonder if it was copied from log/log10. I see I missed add_insrt() in these two. We probably could have an add_float_constant() helper. Oh, good catch! log/log10 also need this call to `hlsl_block_add_instr()`. I think it is just serendipity that compilation works without these nodes getting inserted in the instruction block.
You can improve this MR with a commit adding those missing calls for the log/log10 functions. I don't think that an `add_float_constant()` helper is necessary though. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/475#note_52448
On Wed Nov 15 23:15:01 2023 +0000, Francisco Casas wrote:
Oh, good catch! log/log10 also need this call to `hlsl_block_add_instr()`. I think it is just serendipity that compilation works without these nodes getting inserted in the instruction block. You can improve this MR with a commit adding those missing calls for the log/log10 functions. I don't think that an `add_float_constant()` helper is necessary though. Oh sorry @nsivov, I thought that I was replying to @sgwaki. The similarity in the user avatars got me confused.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/475#note_52449
On Wed Nov 15 23:17:54 2023 +0000, Francisco Casas wrote:
Oh sorry @nsivov, I thought that I was replying to @sgwaki. The similarity in the user avatars got me confused. I submitted a fix for log() !476.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/475#note_52453
On Thu Nov 16 10:53:23 2023 +0000, Nikolay Sivov wrote:
I submitted a fix for log() !476. Thanks for comments. Yes, they're surely based on log/log10 functions. I'll add `hlsl_block_add_instr()` where needed.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/475#note_52503
On Thu Nov 16 10:54:01 2023 +0000, Francisco Casas wrote:
Same for this new float constant. I'll do, too.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/475#note_52504
On Thu Nov 16 11:05:32 2023 +0000, Francisco Casas wrote:
Both tests work thanks to the work being done to support DXIL, so these `todo(sm>=6)` qualifiers can be removed in both commits. To check this by yourself you need to configure the shader runner to load libdxcompiler.so from Microsoft, as explained in the README, but for now I think that removing the `todo(sm>=6)` tags is enough. I wasn't sure what caused the error, so your comment is helpful. I'll submit updated version, soon.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/475#note_52505
participants (4)
-
Akihiro Sagawa -
Akihiro Sagawa (＠sgwaki)
-
Francisco Casas (＠fcasas)
-
Nikolay Sivov (＠nsivov)