[PATCH 0/3] MR339: vkd3d-shader/hlsl: Produce 2D resource declarations and loads for tex1D().
Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/339
From: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.y | 39 +++++++++++++++++++++++- tests/hlsl/combined-samplers.shader_test | 6 ++-- 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 67fd9f6f..444fe311 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -3457,7 +3457,44 @@ static bool intrinsic_tex(struct hlsl_ctx *ctx, const struct parse_initializer * if (!(coords = add_implicit_conversion(ctx, params->instrs, params->args[1], hlsl_get_vector_type(ctx, HLSL_TYPE_FLOAT, hlsl_sampler_dim_count(dim)), loc))) - coords = params->args[1]; + { + return false; + } + + /* tex1D() functions never produce 1D resource declarations. For newer profiles half offset + is used for the second coordinate, while older ones appear to replicate first coordinate.*/ + if (dim == HLSL_SAMPLER_DIM_1D) + { + struct hlsl_constant_value half_value; + struct hlsl_ir_load *load; + struct hlsl_ir_node *half; + struct hlsl_ir_var *var; + unsigned int idx = 0; + + if (!(var = hlsl_new_synthetic_var(ctx, "coords", hlsl_get_vector_type(ctx, HLSL_TYPE_FLOAT, 2), loc))) + return false; + + initialize_var_components(ctx, params->instrs, var, &idx, coords); + if (shader_profile_version_ge(ctx, 4, 0)) + { + half_value.u[0].f = 0.5f; + if (!(half = hlsl_new_constant(ctx, hlsl_get_scalar_type(ctx, HLSL_TYPE_FLOAT), &half_value, loc))) + return false; + hlsl_block_add_instr(params->instrs, half); + + initialize_var_components(ctx, params->instrs, var, &idx, half); + } + else + initialize_var_components(ctx, params->instrs, var, &idx, coords); + + if (!(load = hlsl_new_var_load(ctx, var, loc))) + return false; + hlsl_block_add_instr(params->instrs, &load->node); + + coords = &load->node; + + dim = HLSL_SAMPLER_DIM_2D; + } load_params.coords = coords; load_params.resource = params->args[0]; diff --git a/tests/hlsl/combined-samplers.shader_test b/tests/hlsl/combined-samplers.shader_test index 57644c26..465c11cb 100644 --- a/tests/hlsl/combined-samplers.shader_test +++ b/tests/hlsl/combined-samplers.shader_test @@ -121,7 +121,7 @@ float4 main() : sv_target } -[pixel shader todo] +[pixel shader] sampler sam[2]; float4 main() : sv_target @@ -130,8 +130,8 @@ float4 main() : sv_target } [test] -todo draw quad -todo probe all rgba (1, 1, 1, 11) +draw quad +probe all rgba (1, 1, 1, 11) [require] -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/339
From: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- libs/vkd3d-shader/d3dbc.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/libs/vkd3d-shader/d3dbc.c b/libs/vkd3d-shader/d3dbc.c index 2b02d51f..d5104ae9 100644 --- a/libs/vkd3d-shader/d3dbc.c +++ b/libs/vkd3d-shader/d3dbc.c @@ -1982,10 +1982,6 @@ static void write_sm1_sampler_dcl(struct hlsl_ctx *ctx, struct vkd3d_bytecode_bu switch (sampler_dim) { - case HLSL_SAMPLER_DIM_1D: - res_type = VKD3D_SM1_RESOURCE_TEXTURE_1D; - break; - case HLSL_SAMPLER_DIM_2D: res_type = VKD3D_SM1_RESOURCE_TEXTURE_2D; break; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/339
From: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- libs/vkd3d-shader/d3d_asm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/vkd3d-shader/d3d_asm.c b/libs/vkd3d-shader/d3d_asm.c index d72402eb..f0c386f1 100644 --- a/libs/vkd3d-shader/d3d_asm.c +++ b/libs/vkd3d-shader/d3d_asm.c @@ -714,7 +714,7 @@ static void shader_dump_decl_usage(struct vkd3d_d3d_asm_compiler *compiler, break; case VKD3D_DECL_USAGE_TEXCOORD: - shader_addline(buffer, "texture%u", semantic->usage_idx); + shader_addline(buffer, "texcoord%u", semantic->usage_idx); break; case VKD3D_DECL_USAGE_TANGENT: -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/339
This merge request was approved by Zebediah Figura. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/339
I'm accepting this because I think this needs to get fixed before the release, but as I said in 219, I don't think we really need to care about the difference between profile version. It's possible to get sm1 to put other junk in the y coordinate; I think we can just use the sm4 behaviour always. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/339#note_45037
On Tue Sep 12 18:26:00 2023 +0000, Zebediah Figura wrote:
I'm accepting this because I think this needs to get fixed before the release, but as I said in 219, I don't think we really need to care about the difference between profile version. It's possible to get sm1 to put other junk in the y coordinate; I think we can just use the sm4 behaviour always. Compiling for ps_2_0 produces:
``` mov r0.xy, c0.y texld ..., r0, ... ``` For ps_3_0 it does: ```texld .., c0.y, ...``` I'm not sure what it means. Is it possible c0.y here means c0.yyyy? My point is, I'm not sure it's exactly junk that it's putting there. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/339#note_45046
On Tue Sep 12 18:26:00 2023 +0000, Nikolay Sivov wrote:
Compiling for ps_2_0 produces: ``` mov r0.xy, c0.y texld ..., r0, ... ``` For ps_3_0 it does: ```texld .., c0.y, ...``` I'm not sure what it means. Is it possible c0.y here means c0.yyyy? My point is, I'm not sure it's exactly junk that it's putting there. For example this shader will use c0.y:
``` uniform float4 f; sampler1D t; float4 main(float4 tex : texcoord0) : color { return tex1D(t, f.x); } ```
I'm not sure what it means. Is it possible c0.y here means c0.yyyy?
Yes, a single component implies replicate swizzle. Similarly omitted swizzle means .xyzw. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/339#note_45050
I'm accepting this because I think this needs to get fixed before the release,
Could you elaborate on that? I'm probably missing some context, and the commits in this MR unfortunately don't provide it. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/339#note_45107
On Tue Sep 12 21:20:53 2023 +0000, Henri Verbeet wrote:
I'm accepting this because I think this needs to get fixed before the release, Could you elaborate on that? I'm probably missing some context, and the commits in this MR unfortunately don't provide it. Current code is simply using 1D types for tex1D(), which is my fault. In reality it should never use 1D sampler type, but 2D type instead. That applies for sm4+ profiles as well.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/339#note_45111
Could you elaborate on that? I'm probably missing some context, and the commits in this MR unfortunately don't provide it.
Yes, sorry for the lack of context. To be clear, as of c5d680d141 we're outputting incorrect code for uses of tex1D(), and unfortunately despite two reviewers nobody noticed that. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/339#note_45116
Francisco Casas (@fcasas) commented about libs/vkd3d-shader/hlsl.y:
+ if (dim == HLSL_SAMPLER_DIM_1D) + { + struct hlsl_constant_value half_value; + struct hlsl_ir_load *load; + struct hlsl_ir_node *half; + struct hlsl_ir_var *var; + unsigned int idx = 0; + + if (!(var = hlsl_new_synthetic_var(ctx, "coords", hlsl_get_vector_type(ctx, HLSL_TYPE_FLOAT, 2), loc))) + return false; + + initialize_var_components(ctx, params->instrs, var, &idx, coords); + if (shader_profile_version_ge(ctx, 4, 0)) + { + half_value.u[0].f = 0.5f; + if (!(half = hlsl_new_constant(ctx, hlsl_get_scalar_type(ctx, HLSL_TYPE_FLOAT), &half_value, loc))) I think you can use hlsl_new_float_constant() here.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/339#note_45135
participants (5)
-
Francisco Casas (＠fcasas) -
Henri Verbeet (＠hverbeet)
-
Nikolay Sivov
-
Nikolay Sivov (＠nsivov)
-
Zebediah Figura (＠zfigura)