[PATCH v3 0/2] MR184: Draft: vkd3d-shader/hlsl: Add support for SampleGrad() method
~~This one's marked as a draft, as there seems to be a blocker with the method parameters.~~ ~~The first commit totally works, _if_ the ddx/ddy parameters are literals - they do _not_ work when passing a variable of any kind. The test comes from tests/d3d12.c, so I'm mostly just trying to migrate that to the HLSL test suite, but it currently hits an assert before we get to the resource load (which does eventually work) and I'm not sure what's causing it:~~ ``` vkd3d-compiler: libs/vkd3d-shader/tpf.c:3190: sm4_register_from_node: Assertion `instr->reg.allocated' failed. ``` ~~Seems like it's surprised when we try to load from the constant buffer maybe?~~ Fixed! -- v3: tests: Add a basic compilation test for SampleGrad() method. https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/184
From: Ethan Lee <flibitijibibo(a)gmail.com> Signed-off-by: Ethan Lee <flibitijibibo(a)gmail.com> --- libs/vkd3d-shader/hlsl.c | 17 ++++++++ libs/vkd3d-shader/hlsl.h | 5 ++- libs/vkd3d-shader/hlsl.y | 71 ++++++++++++++++++++++++++++++++ libs/vkd3d-shader/hlsl_codegen.c | 4 ++ libs/vkd3d-shader/tpf.c | 13 +++++- 5 files changed, 107 insertions(+), 3 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index f0b6a5ef..7f110781 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -1320,6 +1320,8 @@ struct hlsl_ir_resource_load *hlsl_new_resource_load(struct hlsl_ctx *ctx, hlsl_src_from_node(&load->coords, params->coords); hlsl_src_from_node(&load->texel_offset, params->texel_offset); hlsl_src_from_node(&load->lod, params->lod); + hlsl_src_from_node(&load->ddx, params->ddx); + hlsl_src_from_node(&load->ddy, params->ddy); return load; } @@ -1598,6 +1600,8 @@ static struct hlsl_ir_node *clone_resource_load(struct hlsl_ctx *ctx, } clone_src(map, &dst->coords, &src->coords); clone_src(map, &dst->lod, &src->lod); + clone_src(map, &dst->ddx, &src->ddx); + clone_src(map, &dst->ddy, &src->ddy); clone_src(map, &dst->texel_offset, &src->texel_offset); return &dst->node; } @@ -2384,6 +2388,7 @@ static void dump_ir_resource_load(struct vkd3d_string_buffer *buffer, const stru [HLSL_RESOURCE_SAMPLE] = "sample", [HLSL_RESOURCE_SAMPLE_LOD] = "sample_lod", [HLSL_RESOURCE_SAMPLE_LOD_BIAS] = "sample_biased", + [HLSL_RESOURCE_SAMPLE_GRAD] = "sample_grad", [HLSL_RESOURCE_GATHER_RED] = "gather_red", [HLSL_RESOURCE_GATHER_GREEN] = "gather_green", [HLSL_RESOURCE_GATHER_BLUE] = "gather_blue", @@ -2407,6 +2412,16 @@ static void dump_ir_resource_load(struct vkd3d_string_buffer *buffer, const stru vkd3d_string_buffer_printf(buffer, ", lod = "); dump_src(buffer, &load->lod); } + if (load->ddx.node) + { + vkd3d_string_buffer_printf(buffer, ", ddx = "); + dump_src(buffer, &load->ddx); + } + if (load->ddy.node) + { + vkd3d_string_buffer_printf(buffer, ", ddy = "); + dump_src(buffer, &load->ddy); + } vkd3d_string_buffer_printf(buffer, ")"); } @@ -2642,6 +2657,8 @@ static void free_ir_resource_load(struct hlsl_ir_resource_load *load) hlsl_cleanup_deref(&load->resource); hlsl_src_remove(&load->coords); hlsl_src_remove(&load->lod); + hlsl_src_remove(&load->ddx); + hlsl_src_remove(&load->ddy); hlsl_src_remove(&load->texel_offset); vkd3d_free(load); } diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index 2ff18536..01897b55 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -589,6 +589,7 @@ enum hlsl_resource_load_type HLSL_RESOURCE_SAMPLE, HLSL_RESOURCE_SAMPLE_LOD, HLSL_RESOURCE_SAMPLE_LOD_BIAS, + HLSL_RESOURCE_SAMPLE_GRAD, HLSL_RESOURCE_GATHER_RED, HLSL_RESOURCE_GATHER_GREEN, HLSL_RESOURCE_GATHER_BLUE, @@ -600,7 +601,7 @@ struct hlsl_ir_resource_load struct hlsl_ir_node node; enum hlsl_resource_load_type load_type; struct hlsl_deref resource, sampler; - struct hlsl_src coords, lod, texel_offset; + struct hlsl_src coords, lod, ddx, ddy, texel_offset; }; struct hlsl_ir_resource_store @@ -798,7 +799,7 @@ struct hlsl_resource_load_params struct hlsl_type *format; enum hlsl_resource_load_type type; struct hlsl_ir_node *resource, *sampler; - struct hlsl_ir_node *coords, *lod, *texel_offset; + struct hlsl_ir_node *coords, *lod, *ddx, *ddy, *texel_offset; }; static inline struct hlsl_ir_call *hlsl_ir_call(const struct hlsl_ir_node *node) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 10deda75..69fd3fb0 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -3883,6 +3883,71 @@ static bool add_sample_lod_method_call(struct hlsl_ctx *ctx, struct list *instrs return true; } +static bool add_sample_grad_method_call(struct hlsl_ctx *ctx, struct list *instrs, struct hlsl_ir_node *object, + const char *name, const struct parse_initializer *params, const struct vkd3d_shader_location *loc) +{ + const struct hlsl_type *object_type = object->data_type; + struct hlsl_resource_load_params load_params = { 0 }; + const unsigned int sampler_dim = hlsl_sampler_dim_count(object_type->sampler_dim); + const unsigned int offset_dim = hlsl_offset_dim_count(object_type->sampler_dim); + const struct hlsl_type *sampler_type; + struct hlsl_ir_resource_load *load; + + load_params.type = HLSL_RESOURCE_SAMPLE_GRAD; + + if (params->args_count < 4 || params->args_count > 5 + !!offset_dim) + { + hlsl_error(ctx, loc, VKD3D_SHADER_ERROR_HLSL_WRONG_PARAMETER_COUNT, + "Wrong number of arguments to method '%s': expected from 4 to %u, but got %u.", + name, 5 + !!offset_dim, params->args_count); + return false; + } + + sampler_type = params->args[0]->data_type; + if (sampler_type->class != HLSL_CLASS_OBJECT || sampler_type->base_type != HLSL_TYPE_SAMPLER + || sampler_type->sampler_dim != HLSL_SAMPLER_DIM_GENERIC) + { + struct vkd3d_string_buffer *string; + + if ((string = hlsl_type_to_string(ctx, sampler_type))) + hlsl_error(ctx, loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE, + "Wrong type for argument 0 of %s(): expected 'sampler', but got '%s'.", name, string->buffer); + hlsl_release_string_buffer(ctx, string); + return false; + } + + if (!(load_params.coords = add_implicit_conversion(ctx, instrs, params->args[1], + hlsl_get_vector_type(ctx, HLSL_TYPE_FLOAT, sampler_dim), loc))) + load_params.coords = params->args[1]; + + if (!(load_params.ddx = add_implicit_conversion(ctx, instrs, params->args[2], + hlsl_get_vector_type(ctx, HLSL_TYPE_FLOAT, sampler_dim), loc))) + load_params.ddx = params->args[2]; + + if (!(load_params.ddy = add_implicit_conversion(ctx, instrs, params->args[3], + hlsl_get_vector_type(ctx, HLSL_TYPE_FLOAT, sampler_dim), loc))) + load_params.ddy = params->args[3]; + + if (offset_dim && params->args_count > 4) + { + if (!(load_params.texel_offset = add_implicit_conversion(ctx, instrs, params->args[4], + hlsl_get_vector_type(ctx, HLSL_TYPE_INT, offset_dim), loc))) + return false; + } + + if (params->args_count > 4 + !!offset_dim) + hlsl_fixme(ctx, loc, "Tiled resource status argument."); + + load_params.format = object_type->e.resource_format; + load_params.resource = object; + load_params.sampler = params->args[0]; + + if (!(load = hlsl_new_resource_load(ctx, &load_params, loc))) + return false; + list_add_tail(instrs, &load->node.entry); + return true; +} + static bool add_method_call(struct hlsl_ctx *ctx, struct list *instrs, struct hlsl_ir_node *object, const char *name, const struct parse_initializer *params, const struct vkd3d_shader_location *loc) { @@ -3933,6 +3998,12 @@ static bool add_method_call(struct hlsl_ctx *ctx, struct list *instrs, struct hl { return add_sample_lod_method_call(ctx, instrs, object, name, params, loc); } + else if (!strcmp(name, "SampleGrad") + && object_type->sampler_dim != HLSL_SAMPLER_DIM_2DMS + && object_type->sampler_dim != HLSL_SAMPLER_DIM_2DMSARRAY) + { + return add_sample_grad_method_call(ctx, instrs, object, name, params, loc); + } else { struct vkd3d_string_buffer *string; diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 42f8ab3b..db64b4f1 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -2496,6 +2496,10 @@ static void compute_liveness_recurse(struct hlsl_block *block, unsigned int loop load->texel_offset.node->last_read = instr->index; if (load->lod.node) load->lod.node->last_read = instr->index; + if (load->ddx.node) + load->ddx.node->last_read = instr->index; + if (load->ddy.node) + load->ddy.node->last_read = instr->index; break; } case HLSL_IR_RESOURCE_STORE: diff --git a/libs/vkd3d-shader/tpf.c b/libs/vkd3d-shader/tpf.c index d104c078..7dd676d3 100644 --- a/libs/vkd3d-shader/tpf.c +++ b/libs/vkd3d-shader/tpf.c @@ -3036,7 +3036,7 @@ struct sm4_instruction struct sm4_register reg; enum vkd3d_sm4_swizzle_type swizzle_type; unsigned int swizzle; - } srcs[4]; + } srcs[5]; unsigned int src_count; uint32_t idx[3]; @@ -3721,6 +3721,10 @@ static void write_sm4_sample(struct hlsl_ctx *ctx, struct vkd3d_bytecode_buffer instr.opcode = VKD3D_SM4_OP_SAMPLE_B; break; + case HLSL_RESOURCE_SAMPLE_GRAD: + instr.opcode = VKD3D_SM4_OP_SAMPLE_GRAD; + break; + default: vkd3d_unreachable(); } @@ -3748,6 +3752,12 @@ static void write_sm4_sample(struct hlsl_ctx *ctx, struct vkd3d_bytecode_buffer sm4_src_from_node(&instr.srcs[3], load->lod.node, VKD3DSP_WRITEMASK_ALL); ++instr.src_count; } + else if (load->load_type == HLSL_RESOURCE_SAMPLE_GRAD) + { + sm4_src_from_node(&instr.srcs[3], load->ddx.node, VKD3DSP_WRITEMASK_ALL); + sm4_src_from_node(&instr.srcs[4], load->ddy.node, VKD3DSP_WRITEMASK_ALL); + instr.src_count += 2; + } write_sm4_instruction(buffer, &instr); } @@ -4501,6 +4511,7 @@ static void write_sm4_resource_load(struct hlsl_ctx *ctx, case HLSL_RESOURCE_SAMPLE: case HLSL_RESOURCE_SAMPLE_LOD_BIAS: + case HLSL_RESOURCE_SAMPLE_GRAD: if (!load->sampler.var) { hlsl_fixme(ctx, &load->node.loc, "SM4 combined sample expression."); -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/184
From: Ethan Lee <flibitijibibo(a)gmail.com> Signed-off-by: Ethan Lee <flibitijibibo(a)gmail.com> --- tests/sampler.shader_test | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/sampler.shader_test b/tests/sampler.shader_test index 2cb70a0b..7e6fb820 100644 --- a/tests/sampler.shader_test +++ b/tests/sampler.shader_test @@ -45,3 +45,16 @@ float4 main() : sv_target [test] draw quad probe all rgba (0.25, 0, 0.25, 0) + +[pixel shader] +SamplerState s; +Texture2D t; + +float4 main() : sv_target +{ + return t.SampleGrad(s, float2(0.5, 0.5), float2(0, 0), float2(0, 0)); +} + +[test] +draw quad +probe all rgba (0.25, 0, 0.25, 0) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/184
Latest push updates the test to fall in line with the others in sampler.shader_test - like SampleBias it mostly just checks to make sure that it properly recognizes the function signature, and otherwise produces a result similar to plain Sample(). Porting over the full d3d12 test ended up being a pretty beefy task since it does a per-pixel test with mips, so I fell back to what the HLSL tests were already doing and will let the d3d12 test do the hard work :sweat_smile: -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/184#note_31403
Francisco Casas (@fcasas) commented about libs/vkd3d-shader/hlsl.y:
{ return add_sample_lod_method_call(ctx, instrs, object, name, params, loc); } + else if (!strcmp(name, "SampleGrad") + && object_type->sampler_dim != HLSL_SAMPLER_DIM_2DMS + && object_type->sampler_dim != HLSL_SAMPLER_DIM_2DMSARRAY)
SampleGrad is also not supported for TextureCube, TextureCubeArray. It is probably better to use positive logic here and numerate all the allowed values for sampler_dim. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/184#note_31452
On Thu Apr 27 21:54:59 2023 +0000, Francisco Casas wrote:
SampleGrad is also not supported for TextureCube, TextureCubeArray. It is probably better to use positive logic here and numerate all the allowed values for sampler_dim. I could be misreading the spec but it should be supported for Cube and CubeArray as long as Offset isn't provided - this appears to be the case for all Sample types, so it looks like there needs to be an additional patch for all Sample functions to check `offsetProvided && type != CUBE && type != CUBEARRAY`.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/184#note_31459
I could be misreading the spec but it should be supported for Cube and CubeArray as long as Offset isn't provided
Ah, you are right. My bad.
this appears to be the case for all Sample types, so it looks like there needs to be an additional patch for all Sample functions to check `offsetProvided && type != CUBE && type != CUBEARRAY`.
This already done in the check for the number of arguments: ```c if (params->args_count < 3 || params->args_count > 4 + !!offset_dim) ``` -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/184#note_31460
This merge request was approved by Francisco Casas. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/184
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/hlsl.y:
+ "Wrong type for argument 0 of %s(): expected 'sampler', but got '%s'.", name, string->buffer); + hlsl_release_string_buffer(ctx, string); + return false; + } + + if (!(load_params.coords = add_implicit_conversion(ctx, instrs, params->args[1], + hlsl_get_vector_type(ctx, HLSL_TYPE_FLOAT, sampler_dim), loc))) + load_params.coords = params->args[1]; + + if (!(load_params.ddx = add_implicit_conversion(ctx, instrs, params->args[2], + hlsl_get_vector_type(ctx, HLSL_TYPE_FLOAT, sampler_dim), loc))) + load_params.ddx = params->args[2]; + + if (!(load_params.ddy = add_implicit_conversion(ctx, instrs, params->args[3], + hlsl_get_vector_type(ctx, HLSL_TYPE_FLOAT, sampler_dim), loc))) + load_params.ddy = params->args[3]; That doesn't look correct. According to the docs (which, incredibly, seem to make sense in this particular case) derivatives require a different number of components than `sampler_dim`: https://learn.microsoft.com/en-us/windows/win32/direct3dhlsl/dx-graphics-hls...
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/184#note_31498
Giovanni Mascellani (@giomasce) commented about tests/sampler.shader_test:
[test] draw quad probe all rgba (0.25, 0, 0.25, 0) + +[pixel shader] +SamplerState s; +Texture2D t; + +float4 main() : sv_target +{ + return t.SampleGrad(s, float2(0.5, 0.5), float2(0, 0), float2(0, 0)); Given this is a relatively complex operation, it wouldn't be bad to test some less trivial input values and check that our implementation is reasonably similar to native.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/184#note_31499
On Fri Apr 28 13:14:46 2023 +0000, Giovanni Mascellani wrote:
That doesn't look correct. According to the docs (which, incredibly, seem to make sense in this particular case) derivatives require a different number of components than `sampler_dim`: https://learn.microsoft.com/en-us/windows/win32/direct3dhlsl/dx-graphics-hls... It looks like it's the same dimensions as `coords`, right?
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/184#note_31501
On Fri Apr 28 13:44:48 2023 +0000, Ethan Lee wrote:
It looks like it's the same dimensions as `coords`, right? Doh, nope, read the table wrong, will fix this now...
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/184#note_31502
participants (4)
-
Ethan Lee -
Ethan Lee (＠flibitijibibo)
-
Francisco Casas (＠fcasas)
-
Giovanni Mascellani (＠giomasce)