From: Nikolay Sivov nsivov@codeweavers.com
Signed-off-by: Nikolay Sivov nsivov@codeweavers.com --- Makefile.am | 1 + libs/vkd3d-shader/hlsl.y | 36 ++++++++++++++++++++++++++++++++++ tests/hlsl/refract.shader_test | 19 ++++++++++++++++++ 3 files changed, 56 insertions(+) create mode 100644 tests/hlsl/refract.shader_test
diff --git a/Makefile.am b/Makefile.am index b36358b2..700af5ad 100644 --- a/Makefile.am +++ b/Makefile.am @@ -139,6 +139,7 @@ vkd3d_shader_tests = \ tests/hlsl/object-references.shader_test \ tests/hlsl/pow.shader_test \ tests/hlsl/reflect.shader_test \ + tests/hlsl/refract.shader_test \ tests/hlsl/register-reservations.shader_test \ tests/hlsl/return-implicit-conversion.shader_test \ tests/hlsl/return.shader_test \ diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 161d1ab4..866b0abc 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -3281,6 +3281,41 @@ static bool intrinsic_reflect(struct hlsl_ctx *ctx, return !!add_binary_arithmetic_expr(ctx, params->instrs, HLSL_OP2_ADD, i, neg, loc); }
+static bool intrinsic_refract(struct hlsl_ctx *ctx, + const struct parse_initializer *params, const struct vkd3d_shader_location *loc) +{ + struct hlsl_ir_node *r = params->args[0]; + struct hlsl_ir_function_decl *func; + struct hlsl_type *type; + char *body; + + static const char template[] = + "%s refract(%s r, %s n, float i)\n" + "{\n" + " float d = dot(r, n);\n" + " float t = 1 - i * i * (1 - d * d);\n" + " return t >= 0.0 ? i * r - (i * d + sqrt(t)) * n : 0;\n" + "}"; + + if (r->data_type->class == HLSL_CLASS_MATRIX) + { + hlsl_error(ctx, loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE, "Invalid argument type."); + return false; + } + + type = hlsl_get_vector_type(ctx, HLSL_TYPE_FLOAT, r->data_type->dimx); + + if (!(body = hlsl_sprintf_alloc(ctx, template, type->name, type->name, type->name))) + return false; + + func = hlsl_compile_internal_function(ctx, "refract", body); + vkd3d_free(body); + if (!func) + return false; + + return add_user_call(ctx, func, params, loc); +} + static bool intrinsic_round(struct hlsl_ctx *ctx, const struct parse_initializer *params, const struct vkd3d_shader_location *loc) { @@ -3644,6 +3679,7 @@ intrinsic_functions[] = {"normalize", 1, true, intrinsic_normalize}, {"pow", 2, true, intrinsic_pow}, {"reflect", 2, true, intrinsic_reflect}, + {"refract", 3, true, intrinsic_refract}, {"round", 1, true, intrinsic_round}, {"rsqrt", 1, true, intrinsic_rsqrt}, {"saturate", 1, true, intrinsic_saturate}, diff --git a/tests/hlsl/refract.shader_test b/tests/hlsl/refract.shader_test new file mode 100644 index 00000000..5fee7a36 --- /dev/null +++ b/tests/hlsl/refract.shader_test @@ -0,0 +1,19 @@ +[pixel shader] +uniform float4 r; +uniform float4 n; +uniform float i; + +float4 main() : sv_target +{ + return refract(r, n, i); +} + +[test] +uniform 0 float4 0.5 -0.1 0.2 0.3 +uniform 4 float4 0.6 0.4 -0.3 1.0 +uniform 8 float4 0.2 0.3 0.4 0.5 +draw quad +probe all rgba (-0.550931, -0.453954, 0.3654653, -1.0248856) 32 +uniform 8 float4 100.0 100.0 100.0 100.0 +draw quad +probe all rgba (0.0, 0.0, 0.0, 0.0)
What if the dimensions of the first and second arguments don't match?
On Thu Sep 7 16:54:55 2023 +0000, Zebediah Figura wrote:
What if the dimensions of the first and second arguments don't match?
It relies on cast failing. On Windows it works as long as conversion works. Same for last argument, that does not have to be strictly scalar.
On Thu Sep 7 16:54:55 2023 +0000, Nikolay Sivov wrote:
It relies on cast failing. On Windows it works as long as conversion works. Same for last argument, that does not have to be strictly scalar.
Converting to the type of the first argument is unusual for intrinsics, and needs tests to show it's the right behaviour. Most intrinsics take the smallest "intersection" of their argument types.
Note also that add_user_call() uses add_cast() which won't actually fail if an implicit conversion can't be performed. We may want to change that anyway; I don't think we have any code that performs an invalid cast, but we do have code that fails to print a warning on e.g. vector truncation.
Francisco Casas (@fcasas) commented about tests/hlsl/refract.shader_test:
+[pixel shader] +uniform float4 r; +uniform float4 n; +uniform float i;
+float4 main() : sv_target +{
- return refract(r, n, i);
+}
+[test] +uniform 0 float4 0.5 -0.1 0.2 0.3 +uniform 4 float4 0.6 0.4 -0.3 1.0 +uniform 8 float4 0.2 0.3 0.4 0.5
We could use a regular `float`. Same for the next `uniform 8` line.
Francisco Casas (@fcasas) commented about libs/vkd3d-shader/hlsl.y:
- char *body;
- static const char template[] =
"%s refract(%s r, %s n, float i)\n""{\n"" float d = dot(r, n);\n"" float t = 1 - i * i * (1 - d * d);\n"" return t >= 0.0 ? i * r - (i * d + sqrt(t)) * n : 0;\n""}";- if (r->data_type->class == HLSL_CLASS_MATRIX)
- {
hlsl_error(ctx, loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE, "Invalid argument type.");return false;- }
If we check that `r` is not a matrix (like native does), I think we should also do it for `n`.
I tried several input values and I don't see differences between the results, so I guess the implementation is correct.
However, we still have to fix some problems regarding type checking, for instance, I tried this: ```hlsl float4 main() : sv_target { float4 r = {-1.0, 0.0, -0.4, -0.6}; float3 n = {0.6, 0.4, -0.3}; float i = -1.2;
float4 res = refract(r, n, i);
return res; } ``` and our compiler throws ``` vkd3d:129954:fixme:fold_cast Cast from float3 to float4. vkd3d:129954:fixme:fold_cast Cast from float3 to float4. vkd3d:129954:fixme:fold_cast Cast from float3 to float4. vkd3d:129954:fixme:fold_cast Cast from float3 to float4. vkd3d-compiler: /home/fcasas/Music/vkd3d/libs/vkd3d-shader/tpf.c:4586: write_sm4_cast: Assertion `src_type->dimx == dst_type->dimx' failed. Aborted (core dumped) ```
This happens because add_user_call() is not performing the implicit_compatible_data_types() type checks that add_call() does through find_function_call().
I am not sure what is the best solution for this, maybe also adding these checks into hlsl_compile_internal_function() ?
On Thu Sep 7 19:38:23 2023 +0000, Zebediah Figura wrote:
Converting to the type of the first argument is unusual for intrinsics, and needs tests to show it's the right behaviour. Most intrinsics take the smallest "intersection" of their argument types. Note also that add_user_call() uses add_cast() which won't actually fail if an implicit conversion can't be performed. We may want to change that anyway; I don't think we have any code that performs an invalid cast, but we do have code that fails to print a warning on e.g. vector truncation.
Oh, I didn't read this thread before, but I found https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/333#note_44607 which I think counts as "code that performs an invalid cast".
Oh, I didn't read this thread before, but I found https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/333#note_44607 which I think counts as "code that performs an invalid cast".
Right, I meant we don't have any such code currently in Wine. This patch would introduce some.
This still has review that needs to be addressed.
-
Francisco Casas (＠fcasas) -
Nikolay Sivov
-
Nikolay Sivov (＠nsivov)
-
Zebediah Figura (＠zfigura)