[PATCH v2 0/2] MR161: vkd3d-shader/hlsl: Implement D3DCOLORtoUBYTE4().
-- v2: vkd3d-shader/hlsl: Implement D3DCOLORtoUBYTE4() function. vkd3d-shader/hlsl: Partially implement trunc(). https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/161
From: Nikolay Sivov <nsivov(a)codeweavers.com> --- Makefile.am | 1 + libs/vkd3d-shader/hlsl.c | 1 + libs/vkd3d-shader/hlsl.h | 1 + libs/vkd3d-shader/hlsl.y | 12 ++++++++++ libs/vkd3d-shader/hlsl_sm4.c | 5 ++++ tests/hlsl-trunc.shader_test | 44 ++++++++++++++++++++++++++++++++++++ 6 files changed, 64 insertions(+) create mode 100644 tests/hlsl-trunc.shader_test diff --git a/Makefile.am b/Makefile.am index bff65a85..9f28bf83 100644 --- a/Makefile.am +++ b/Makefile.am @@ -120,6 +120,7 @@ vkd3d_shader_tests = \ tests/hlsl-struct-semantics.shader_test \ tests/hlsl-ternary.shader_test \ tests/hlsl-transpose.shader_test \ + tests/hlsl-trunc.shader_test \ tests/hlsl-type-names.shader_test \ tests/hlsl-vector-indexing.shader_test \ tests/hlsl-vector-indexing-uniform.shader_test \ diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index 869638a6..60cf8e19 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -2286,6 +2286,7 @@ const char *debug_hlsl_expr_op(enum hlsl_ir_expr_op op) [HLSL_OP1_SIN] = "sin", [HLSL_OP1_SIN_REDUCED] = "sin_reduced", [HLSL_OP1_SQRT] = "sqrt", + [HLSL_OP1_TRUNC] = "trunc", [HLSL_OP2_ADD] = "+", [HLSL_OP2_BIT_AND] = "&", diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index b38fba1c..db716217 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -490,6 +490,7 @@ enum hlsl_ir_expr_op HLSL_OP1_SIN, HLSL_OP1_SIN_REDUCED, /* Reduced range [-pi, pi] */ HLSL_OP1_SQRT, + HLSL_OP1_TRUNC, HLSL_OP2_ADD, HLSL_OP2_BIT_AND, diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 0ddae6ee..c3c3d7de 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -3277,6 +3277,17 @@ static bool intrinsic_transpose(struct hlsl_ctx *ctx, return true; } +static bool intrinsic_trunc(struct hlsl_ctx *ctx, + const struct parse_initializer *params, const struct vkd3d_shader_location *loc) +{ + struct hlsl_ir_node *arg; + + if (!(arg = intrinsic_float_convert_arg(ctx, params, params->args[0], loc))) + return false; + + return !!add_unary_arithmetic_expr(ctx, params->instrs, HLSL_OP1_TRUNC, arg, loc); +} + static const struct intrinsic_function { const char *name; @@ -3323,6 +3334,7 @@ intrinsic_functions[] = {"tex2D", -1, false, intrinsic_tex2D}, {"tex3D", -1, false, intrinsic_tex3D}, {"transpose", 1, true, intrinsic_transpose}, + {"trunc", 1, true, intrinsic_trunc}, }; static int intrinsic_function_name_compare(const void *a, const void *b) diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c index 8be848c5..e925b55d 100644 --- a/libs/vkd3d-shader/hlsl_sm4.c +++ b/libs/vkd3d-shader/hlsl_sm4.c @@ -1817,6 +1817,11 @@ static void write_sm4_expr(struct hlsl_ctx *ctx, write_sm4_unary_op(buffer, VKD3D_SM4_OP_SQRT, &expr->node, arg1, 0); break; + case HLSL_OP1_TRUNC: + assert(type_is_float(dst_type)); + write_sm4_unary_op(buffer, VKD3D_SM4_OP_ROUND_Z, &expr->node, arg1, 0); + break; + case HLSL_OP2_ADD: switch (dst_type->base_type) { diff --git a/tests/hlsl-trunc.shader_test b/tests/hlsl-trunc.shader_test new file mode 100644 index 00000000..419608fb --- /dev/null +++ b/tests/hlsl-trunc.shader_test @@ -0,0 +1,44 @@ +[pixel shader] +float4 main(uniform float4 u) : sv_target +{ + return trunc(u); +} + +[test] +uniform 0 float4 -0.5 6.5 7.5 3.4 +draw quad +probe all rgba (0.0, 6.0, 7.0, 3.0) +uniform 0 float4 -1.5 6.5 7.5 3.4 +draw quad +probe all rgba (-1.0, 6.0, 7.0, 3.0) + +[pixel shader] +float4 main(uniform float4 u) : sv_target +{ + float a = trunc(u.r); + int2 b = trunc(u.gb); + float4 res = float4(b, a, u.a); + return trunc(res); +} + +[test] +uniform 0 float4 -0.5 6.5 7.5 3.4 +draw quad +probe all rgba (6.0, 7.0, 0.0, 3.0) + +[require] +shader model >= 4.0 + +[pixel shader] +float4 main(uniform int4 u) : sv_target +{ + float a = trunc(u.r); + int2 b = trunc(u.gb); + float4 res = float4(b, a, u.a); + return trunc(res); +} + +[test] +uniform 0 int4 -1 6 7 3 +draw quad +probe all rgba (6.0, 7.0, -1.0, 3.0) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/161
From: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- Makefile.am | 1 + libs/vkd3d-shader/hlsl.y | 57 +++++++++++++++++++++++ tests/hlsl-d3dcolor-to-ubyte4.shader_test | 24 ++++++++++ 3 files changed, 82 insertions(+) create mode 100644 tests/hlsl-d3dcolor-to-ubyte4.shader_test diff --git a/Makefile.am b/Makefile.am index 9f28bf83..abc85a49 100644 --- a/Makefile.am +++ b/Makefile.am @@ -75,6 +75,7 @@ vkd3d_shader_tests = \ tests/hlsl-clamp.shader_test \ tests/hlsl-comma.shader_test \ tests/hlsl-cross.shader_test \ + tests/hlsl-d3dcolor-to-ubyte4.shader_test \ tests/hlsl-dot.shader_test \ tests/hlsl-duplicate-modifiers.shader_test \ tests/hlsl-for.shader_test \ diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index c3c3d7de..39f07aac 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -3288,6 +3288,62 @@ static bool intrinsic_trunc(struct hlsl_ctx *ctx, return !!add_unary_arithmetic_expr(ctx, params->instrs, HLSL_OP1_TRUNC, arg, loc); } +static bool intrinsic_d3dcolor_to_ubyte4(struct hlsl_ctx *ctx, + const struct parse_initializer *params, const struct vkd3d_shader_location *loc) +{ + struct hlsl_ir_node *arg = params->args[0], *ret; + struct hlsl_type *arg_type = arg->data_type; + struct hlsl_ir_swizzle *swizzle; + struct hlsl_ir_constant *c; + + if (arg_type->class != HLSL_CLASS_SCALAR && !(arg_type->class == HLSL_CLASS_VECTOR && arg_type->dimx == 4)) + { + struct vkd3d_string_buffer *string; + + if ((string = hlsl_type_to_string(ctx, arg_type))) + { + hlsl_error(ctx, loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE, "Wrong argument type '%s'.", string->buffer); + hlsl_release_string_buffer(ctx, string); + } + + return false; + } + + if (!(arg = intrinsic_float_convert_arg(ctx, params, arg, loc))) + return false; + + if (arg_type->class == HLSL_CLASS_SCALAR) + { + if (!(c = hlsl_new_float_constant(ctx, 255.001953f, loc))) + return false; + list_add_tail(params->instrs, &c->node.entry); + } + else + { + if (!(c = hlsl_new_constant(ctx, hlsl_get_vector_type(ctx, HLSL_TYPE_FLOAT, 4), loc))) + return false; + c->value[0].f = 255.001953f; + c->value[1].f = 255.001953f; + c->value[2].f = 255.001953f; + c->value[3].f = 255.001953f; + list_add_tail(params->instrs, &c->node.entry); + + if (!(swizzle = hlsl_new_swizzle(ctx, HLSL_SWIZZLE(Z, Y, X, W), 4, arg, loc))) + return false; + list_add_tail(params->instrs, &swizzle->node.entry); + + arg = &swizzle->node; + } + + if (!(ret = add_binary_arithmetic_expr(ctx, params->instrs, HLSL_OP2_MUL, arg, &c->node, loc))) + return false; + + if (ctx->profile->major_version >= 4) + return !!add_unary_arithmetic_expr(ctx, params->instrs, HLSL_OP1_TRUNC, ret, loc); + + return true; +} + static const struct intrinsic_function { const char *name; @@ -3299,6 +3355,7 @@ static const struct intrinsic_function intrinsic_functions[] = { /* Note: these entries should be kept in alphabetical order. */ + {"D3DCOLORtoUBYTE4", 1, true, intrinsic_d3dcolor_to_ubyte4}, {"abs", 1, true, intrinsic_abs}, {"all", 1, true, intrinsic_all}, {"asuint", -1, true, intrinsic_asuint}, diff --git a/tests/hlsl-d3dcolor-to-ubyte4.shader_test b/tests/hlsl-d3dcolor-to-ubyte4.shader_test new file mode 100644 index 00000000..e31ef61c --- /dev/null +++ b/tests/hlsl-d3dcolor-to-ubyte4.shader_test @@ -0,0 +1,24 @@ +[require] +shader model >= 4.0 + +[pixel shader] +float4 main(uniform float4 u) : sv_target +{ + return D3DCOLORtoUBYTE4(u); +} + +[test] +uniform 0 float4 -0.5 6.5 7.5 3.4 +draw quad +probe all rgba (1912.0, 1657.0, -127.0, 867.0) 1 + +[pixel shader] +float4 main(uniform float4 u) : sv_target +{ + return D3DCOLORtoUBYTE4(u.x); +} + +[test] +uniform 0 float4 -0.5 6.5 7.5 3.4 +draw quad +probe all rgba (-127.0, -127.0, -127.0, -127.0) 1 -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/161
On Mon Apr 17 09:36:59 2023 +0000, Giovanni Mascellani wrote:
It seems that calling with a scalar is fine too. Fixed now, thanks.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/161#note_30252
On Mon Apr 17 09:36:59 2023 +0000, Giovanni Mascellani wrote:
I wonder what's the reason for such a complicated test. It seems that the tests for `floor()` already had something like that, but I don't completely understand what's the value. Yeah, I didn't notice this for floor(), but I don't understand what this is proving?
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/161#note_30253
Zebediah Figura (@zfigura) commented about tests/hlsl-d3dcolor-to-ubyte4.shader_test:
+[require] +shader model >= 4.0 + It seems that this function works fine with sm1?
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/161#note_30254
On Mon Apr 17 19:26:45 2023 +0000, Zebediah Figura wrote:
Yeah, I didn't notice this for floor(), but I don't understand what this is proving? I think I took it from round() tests. If you mean what does calling floor/trunc/round on a result from floor/trunc/round, I don't know. Other calls were probably for scalar and shorter vectors.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/161#note_30255
On Mon Apr 17 19:27:24 2023 +0000, Zebediah Figura wrote:
It seems that this function works fine with sm1? It does, but it doesn't truncate result on sm1.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/161#note_30256
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/hlsl.y:
if (!(arg = intrinsic_float_convert_arg(ctx, params, arg, loc))) return false;
- if (!(c = hlsl_new_constant(ctx, hlsl_get_vector_type(ctx, HLSL_TYPE_FLOAT, 4), loc))) - return false; - c->value[0].f = 255.001953f; - c->value[1].f = 255.001953f; - c->value[2].f = 255.001953f; - c->value[3].f = 255.001953f; - list_add_tail(params->instrs, &c->node.entry); + if (arg_type->class == HLSL_CLASS_SCALAR) + { + if (!(c = hlsl_new_float_constant(ctx, 255.001953f, loc))) + return false; + list_add_tail(params->instrs, &c->node.entry);
Notice that you don't need to create two different constants in the two `if` branches. If you always create a scalar, `add_binary_arithmetic_expr()` will swizzle it for you. It's not wrong as it is now, though. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/161#note_30338
On Mon Apr 17 19:31:33 2023 +0000, Nikolay Sivov wrote:
I think I took it from round() tests. If you mean what does calling floor/trunc/round on a result from floor/trunc/round, I don't know. Other calls were probably for scalar and shorter vectors. I don't think I'd miss these tests if they were dropped, but I don't think they're going to harm us either.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/161#note_30339
This merge request was approved by Giovanni Mascellani. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/161
```diff + if (!(c = hlsl_new_constant(ctx, hlsl_get_vector_type(ctx, HLSL_TYPE_FLOAT, 4), loc))) + return false; + c->value[0].f = 255.001953f; + c->value[1].f = 255.001953f; + c->value[2].f = 255.001953f; + c->value[3].f = 255.001953f; ```
"255.001953f" doesn't seem quite as readable/obvious as e.g. "255.0f + (0.5f / 256.0f)". -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/161#note_30348
participants (5)
-
Giovanni Mascellani (＠giomasce) -
Henri Verbeet (＠hverbeet)
-
Nikolay Sivov
-
Nikolay Sivov (＠nsivov)
-
Zebediah Figura (＠zfigura)