[PATCH v3 0/1] MR218: vkd3d-shader: Implement GetDimensions().
-- v3: vkd3d-shader: Implement GetDimensions(). https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/218
From: Nikolay Sivov <nsivov(a)codeweavers.com> --- Makefile.am | 1 + libs/vkd3d-shader/hlsl.c | 15 ++- libs/vkd3d-shader/hlsl.h | 6 +- libs/vkd3d-shader/hlsl.y | 180 ++++++++++++++++++++++++++- libs/vkd3d-shader/hlsl_codegen.c | 5 +- libs/vkd3d-shader/tpf.c | 66 ++++++++++ tests/hlsl-getdimensions.shader_test | 22 ++++ 7 files changed, 285 insertions(+), 10 deletions(-) create mode 100644 tests/hlsl-getdimensions.shader_test diff --git a/Makefile.am b/Makefile.am index 92c6d0792..41fa19268 100644 --- a/Makefile.am +++ b/Makefile.am @@ -92,6 +92,7 @@ vkd3d_shader_tests = \ tests/hlsl-function-overload.shader_test \ tests/hlsl-gather-offset.shader_test \ tests/hlsl-gather.shader_test \ + tests/hlsl-getdimensions.shader_test \ tests/hlsl-initializer-flatten.shader_test \ tests/hlsl-initializer-implicit-array.shader_test \ tests/hlsl-initializer-invalid-arg-count.shader_test \ diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index 152ec6275..2e04ad9c2 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -1357,6 +1357,7 @@ struct hlsl_ir_node *hlsl_new_resource_load(struct hlsl_ctx *ctx, hlsl_src_from_node(&load->ddx, params->ddx); hlsl_src_from_node(&load->ddy, params->ddy); hlsl_src_from_node(&load->cmp, params->cmp); + hlsl_src_from_node(&load->level, params->level); load->sampling_dim = params->sampling_dim; if (load->sampling_dim == HLSL_SAMPLER_DIM_GENERIC) load->sampling_dim = hlsl_deref_get_type(ctx, &load->resource)->sampler_dim; @@ -1652,6 +1653,7 @@ static struct hlsl_ir_node *clone_resource_load(struct hlsl_ctx *ctx, clone_src(map, &dst->sample_index, &src->sample_index); clone_src(map, &dst->cmp, &src->cmp); clone_src(map, &dst->texel_offset, &src->texel_offset); + clone_src(map, &dst->level, &src->level); dst->sampling_dim = src->sampling_dim; return &dst->node; } @@ -2460,8 +2462,11 @@ static void dump_ir_resource_load(struct vkd3d_string_buffer *buffer, const stru dump_deref(buffer, &load->resource); vkd3d_string_buffer_printf(buffer, ", sampler = "); dump_deref(buffer, &load->sampler); - vkd3d_string_buffer_printf(buffer, ", coords = "); - dump_src(buffer, &load->coords); + if (load->coords.node) + { + vkd3d_string_buffer_printf(buffer, ", coords = "); + dump_src(buffer, &load->coords); + } if (load->sample_index.node) { vkd3d_string_buffer_printf(buffer, ", sample index = "); @@ -2492,6 +2497,11 @@ static void dump_ir_resource_load(struct vkd3d_string_buffer *buffer, const stru vkd3d_string_buffer_printf(buffer, ", cmp = "); dump_src(buffer, &load->cmp); } + if (load->level.node) + { + vkd3d_string_buffer_printf(buffer, ", level = "); + dump_src(buffer, &load->level); + } vkd3d_string_buffer_printf(buffer, ")"); } @@ -2732,6 +2742,7 @@ static void free_ir_resource_load(struct hlsl_ir_resource_load *load) hlsl_src_remove(&load->cmp); hlsl_src_remove(&load->texel_offset); hlsl_src_remove(&load->sample_index); + hlsl_src_remove(&load->level); vkd3d_free(load); } diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index 7d02448e0..ecee68f94 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -624,6 +624,8 @@ enum hlsl_resource_load_type HLSL_RESOURCE_GATHER_GREEN, HLSL_RESOURCE_GATHER_BLUE, HLSL_RESOURCE_GATHER_ALPHA, + HLSL_RESOURCE_SAMPLE_INFO, + HLSL_RESOURCE_RES_INFO, }; struct hlsl_ir_resource_load @@ -631,7 +633,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, ddx, ddy, cmp, sample_index, texel_offset; + struct hlsl_src coords, lod, ddx, ddy, cmp, sample_index, texel_offset, level; enum hlsl_sampler_dim sampling_dim; }; @@ -833,7 +835,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, *ddx, *ddy, *cmp, *sample_index, *texel_offset; + struct hlsl_ir_node *coords, *lod, *ddx, *ddy, *cmp, *sample_index, *texel_offset, *level; enum hlsl_sampler_dim sampling_dim; }; diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index cf483d82c..159cc3c88 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -3527,6 +3527,18 @@ static int intrinsic_function_name_compare(const void *a, const void *b) return strcmp(a, func->name); } +static bool add_void_expr(struct hlsl_ctx *ctx, struct list *instrs, const struct vkd3d_shader_location *loc) +{ + struct hlsl_ir_node *operands[HLSL_MAX_OPERANDS] = {0}; + struct hlsl_ir_node *expr; + + if (!(expr = hlsl_new_expr(ctx, HLSL_OP0_VOID, operands, ctx->builtin_types.Void, loc))) + return false; + list_add_tail(instrs, &expr->entry); + + return true; +} + static struct list *add_call(struct hlsl_ctx *ctx, const char *name, struct parse_initializer *args, const struct vkd3d_shader_location *loc) { @@ -3601,12 +3613,8 @@ static struct list *add_call(struct hlsl_ctx *ctx, const char *name, } else { - struct hlsl_ir_node *operands[HLSL_MAX_OPERANDS] = {0}; - struct hlsl_ir_node *expr; - - if (!(expr = hlsl_new_expr(ctx, HLSL_OP0_VOID, operands, ctx->builtin_types.Void, loc))) + if (!add_void_expr(ctx, args->instrs, loc)) goto fail; - list_add_tail(args->instrs, &expr->entry); } } else if ((intrinsic = bsearch(name, intrinsic_functions, ARRAY_SIZE(intrinsic_functions), @@ -4036,6 +4044,166 @@ static bool add_gather_method_call(struct hlsl_ctx *ctx, struct list *instrs, st return true; } +static bool add_assignment_from_component(struct hlsl_ctx *ctx, struct list *instrs, struct hlsl_ir_node *dest, + struct hlsl_ir_node *src, unsigned int component, const struct vkd3d_shader_location *loc) +{ + struct hlsl_ir_node *load; + + if (!dest) + return true; + + if (!(load = add_load_component(ctx, instrs, src, component, loc))) + return false; + + if (!add_assignment(ctx, instrs, dest, ASSIGN_OP_ASSIGN, load)) + return false; + + return true; +} + +static bool add_getdimensions_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; + struct hlsl_ir_node *sample_info, *res_info; + struct hlsl_type *uint_type; + enum func_argument + { + ARG_MIP_LEVEL, + ARG_WIDTH, + ARG_HEIGHT, + ARG_ELEMENT_COUNT, + ARG_LEVEL_COUNT, + ARG_SAMPLE_COUNT, + ARG_MAX_ARGS, + }; + struct hlsl_ir_node *args[ARG_MAX_ARGS] = { 0 }; + static const struct overload + { + enum hlsl_sampler_dim sampler_dim; + unsigned int args_count; + enum func_argument args[ARG_MAX_ARGS]; + } + overloads[] = + { + { HLSL_SAMPLER_DIM_1D, 1, { ARG_WIDTH } }, + { HLSL_SAMPLER_DIM_1D, 3, { ARG_MIP_LEVEL, ARG_WIDTH, ARG_LEVEL_COUNT } }, + { HLSL_SAMPLER_DIM_1DARRAY, 2, { ARG_WIDTH, ARG_ELEMENT_COUNT } }, + { HLSL_SAMPLER_DIM_1DARRAY, 4, { ARG_MIP_LEVEL, ARG_WIDTH, ARG_ELEMENT_COUNT, ARG_LEVEL_COUNT } }, + { HLSL_SAMPLER_DIM_2D, 2, { ARG_WIDTH, ARG_HEIGHT } }, + { HLSL_SAMPLER_DIM_2D, 4, { ARG_MIP_LEVEL, ARG_WIDTH, ARG_HEIGHT, ARG_LEVEL_COUNT } }, + { HLSL_SAMPLER_DIM_2DARRAY, 3, { ARG_WIDTH, ARG_HEIGHT, ARG_ELEMENT_COUNT } }, + { HLSL_SAMPLER_DIM_2DARRAY, 5, { ARG_MIP_LEVEL, ARG_WIDTH, ARG_HEIGHT, ARG_ELEMENT_COUNT, ARG_LEVEL_COUNT } }, + { HLSL_SAMPLER_DIM_3D, 3, { ARG_WIDTH, ARG_HEIGHT, ARG_ELEMENT_COUNT } }, + { HLSL_SAMPLER_DIM_3D, 5, { ARG_MIP_LEVEL, ARG_WIDTH, ARG_HEIGHT, ARG_ELEMENT_COUNT, ARG_LEVEL_COUNT } }, + { HLSL_SAMPLER_DIM_CUBE, 2, { ARG_WIDTH, ARG_HEIGHT } }, + { HLSL_SAMPLER_DIM_CUBE, 4, { ARG_MIP_LEVEL, ARG_WIDTH, ARG_HEIGHT, ARG_LEVEL_COUNT } }, + { HLSL_SAMPLER_DIM_CUBEARRAY, 3, { ARG_WIDTH, ARG_HEIGHT, ARG_ELEMENT_COUNT } }, + { HLSL_SAMPLER_DIM_CUBEARRAY, 5, { ARG_MIP_LEVEL, ARG_WIDTH, ARG_HEIGHT, ARG_ELEMENT_COUNT, ARG_LEVEL_COUNT } }, + { HLSL_SAMPLER_DIM_2DMS, 3, { ARG_WIDTH, ARG_HEIGHT, ARG_SAMPLE_COUNT } }, + { HLSL_SAMPLER_DIM_2DMSARRAY, 4, { ARG_WIDTH, ARG_HEIGHT, ARG_ELEMENT_COUNT, ARG_SAMPLE_COUNT } }, + }; + const struct overload *o = NULL; + bool uint_resinfo; + unsigned int i, j; + + if (object_type->sampler_dim > HLSL_SAMPLER_DIM_LAST_TEXTURE) + { + hlsl_error(ctx, loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE, "GetDimensions() is not defined for this type."); + } + + uint_type = hlsl_get_scalar_type(ctx, HLSL_TYPE_UINT); + uint_resinfo = true; + for (i = 0; i < ARRAY_SIZE(overloads); ++i) + { + const struct overload *iter = &overloads[i]; + + if (iter->sampler_dim == object_type->sampler_dim && iter->args_count == params->args_count) + { + for (j = 0; j < params->args_count; ++j) + { + args[iter->args[j]] = params->args[j]; + + /* Input parameter. */ + if (iter->args[j] == ARG_MIP_LEVEL) + { + if (!(args[ARG_MIP_LEVEL] = add_implicit_conversion(ctx, instrs, args[ARG_MIP_LEVEL], + hlsl_get_scalar_type(ctx, HLSL_TYPE_UINT), loc))) + { + return false; + } + + continue; + } + + uint_resinfo &= hlsl_types_are_equal(params->args[j]->data_type, uint_type); + + if (params->args[j]->data_type->class != HLSL_CLASS_SCALAR) + { + hlsl_error(ctx, loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE, "Expected scalar arguments."); + break; + } + } + o = iter; + break; + } + } + + if (!o) + { + struct vkd3d_string_buffer *string; + + if ((string = hlsl_type_to_string(ctx, object_type))) + { + hlsl_error(ctx, loc, VKD3D_SHADER_ERROR_HLSL_INVALID_SYNTAX, + "Unexpected number of arguments %u for %s.%s().", params->args_count, string->buffer, name); + hlsl_release_string_buffer(ctx, string); + } + } + + memset(&load_params, 0, sizeof(load_params)); + load_params.type = HLSL_RESOURCE_RES_INFO; + load_params.resource = object; + load_params.level = args[ARG_MIP_LEVEL]; + load_params.format = hlsl_get_vector_type(ctx, uint_resinfo ? HLSL_TYPE_UINT : HLSL_TYPE_FLOAT, 4); + + if (!(res_info = hlsl_new_resource_load(ctx, &load_params, loc))) + return false; + list_add_tail(instrs, &res_info->entry); + + if (!add_assignment_from_component(ctx, instrs, args[ARG_WIDTH], res_info, 0, loc)) + return false; + + if (!add_assignment_from_component(ctx, instrs, args[ARG_HEIGHT], res_info, 1, loc)) + return false; + + if (!add_assignment_from_component(ctx, instrs, args[ARG_ELEMENT_COUNT], res_info, + object_type->sampler_dim == HLSL_SAMPLER_DIM_1DARRAY ? 1 : 2, loc)) + { + return false; + } + + if (!add_assignment_from_component(ctx, instrs, args[ARG_LEVEL_COUNT], res_info, 3, loc)) + return false; + + if (args[ARG_SAMPLE_COUNT]) + { + memset(&load_params, 0, sizeof(load_params)); + load_params.type = HLSL_RESOURCE_SAMPLE_INFO; + load_params.resource = object; + load_params.format = args[ARG_SAMPLE_COUNT]->data_type; + if (!(sample_info = hlsl_new_resource_load(ctx, &load_params, loc))) + return false; + list_add_tail(instrs, &sample_info->entry); + + if (!add_assignment(ctx, instrs, args[ARG_SAMPLE_COUNT], ASSIGN_OP_ASSIGN, sample_info)) + return false; + } + + return add_void_expr(ctx, instrs, loc); +} + static bool add_sample_lod_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) { @@ -4191,6 +4359,8 @@ object_methods[] = { "GatherGreen", add_gather_method_call }, { "GatherRed", add_gather_method_call }, + { "GetDimensions", add_getdimensions_method_call }, + { "Load", add_load_method_call }, { "Sample", add_sample_method_call }, diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 2b6c595a1..56de4135f 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -2669,7 +2669,8 @@ static void compute_liveness_recurse(struct hlsl_block *block, unsigned int loop load->sampler.offset.node->last_read = last_read; } - load->coords.node->last_read = last_read; + if (load->coords.node) + load->coords.node->last_read = last_read; if (load->texel_offset.node) load->texel_offset.node->last_read = last_read; if (load->lod.node) @@ -2682,6 +2683,8 @@ static void compute_liveness_recurse(struct hlsl_block *block, unsigned int loop load->sample_index.node->last_read = last_read; if (load->cmp.node) load->cmp.node->last_read = last_read; + if (load->level.node) + load->level.node->last_read = last_read; break; } case HLSL_IR_RESOURCE_STORE: diff --git a/libs/vkd3d-shader/tpf.c b/libs/vkd3d-shader/tpf.c index 60948d649..fa3208f26 100644 --- a/libs/vkd3d-shader/tpf.c +++ b/libs/vkd3d-shader/tpf.c @@ -4163,6 +4163,64 @@ static void write_sm4_sample(struct hlsl_ctx *ctx, struct vkd3d_bytecode_buffer write_sm4_instruction(buffer, &instr); } +static void write_sm4_sampleinfo(struct hlsl_ctx *ctx, struct vkd3d_bytecode_buffer *buffer, + const struct hlsl_ir_resource_load *load) +{ + const struct hlsl_type *resource_type = load->resource.var->data_type; + const struct hlsl_deref *resource = &load->resource; + const struct hlsl_ir_node *dst = &load->node; + struct sm4_instruction instr; + + memset(&instr, 0, sizeof(instr)); + instr.opcode = VKD3D_SM4_OP_SAMPLE_INFO; + if (dst->data_type->base_type == HLSL_TYPE_UINT) + instr.opcode |= VKD3DSI_SAMPLE_INFO_UINT << VKD3D_SM4_INSTRUCTION_FLAGS_SHIFT; + + sm4_dst_from_node(&instr.dsts[0], dst); + instr.dst_count = 1; + + sm4_src_from_deref(ctx, &instr.srcs[0], resource, resource_type, instr.dsts[0].writemask); + instr.src_count = 1; + + write_sm4_instruction(buffer, &instr); +} + +static void write_sm4_resinfo(struct hlsl_ctx *ctx, struct vkd3d_bytecode_buffer *buffer, + const struct hlsl_ir_resource_load *load) +{ + const struct hlsl_type *resource_type = load->resource.var->data_type; + const struct hlsl_deref *resource = &load->resource; + const struct hlsl_ir_node *dst = &load->node; + struct sm4_instruction instr; + struct sm4_register *reg; + + memset(&instr, 0, sizeof(instr)); + instr.opcode = VKD3D_SM4_OP_RESINFO; + if (dst->data_type->base_type == HLSL_TYPE_UINT) + instr.opcode |= VKD3DSI_RESINFO_UINT << VKD3D_SM4_INSTRUCTION_FLAGS_SHIFT; + + sm4_dst_from_node(&instr.dsts[0], dst); + instr.dst_count = 1; + + if (load->level.node) + { + sm4_src_from_node(&instr.srcs[0], load->level.node, VKD3DSP_WRITEMASK_ALL); + } + else + { + instr.srcs[0].swizzle_type = VKD3D_SM4_SWIZZLE_NONE; + reg = &instr.srcs[0].reg; + reg->type = VKD3D_SM4_RT_IMMCONST; + reg->dim = VKD3D_SM4_DIMENSION_SCALAR; + reg->immconst_uint[0] = 0; + } + + sm4_src_from_deref(ctx, &instr.srcs[1], resource, resource_type, instr.dsts[0].writemask); + instr.src_count = 2; + + write_sm4_instruction(buffer, &instr); +} + static bool type_is_float(const struct hlsl_type *type) { return type->base_type == HLSL_TYPE_FLOAT || type->base_type == HLSL_TYPE_HALF; @@ -4989,6 +5047,14 @@ static void write_sm4_resource_load(struct hlsl_ctx *ctx, write_sm4_gather(ctx, buffer, resource_type, &load->node, &load->resource, &load->sampler, coords, HLSL_SWIZZLE(W, W, W, W), texel_offset); break; + + case HLSL_RESOURCE_SAMPLE_INFO: + write_sm4_sampleinfo(ctx, buffer, load); + break; + + case HLSL_RESOURCE_RES_INFO: + write_sm4_resinfo(ctx, buffer, load); + break; } } diff --git a/tests/hlsl-getdimensions.shader_test b/tests/hlsl-getdimensions.shader_test new file mode 100644 index 000000000..c2262c053 --- /dev/null +++ b/tests/hlsl-getdimensions.shader_test @@ -0,0 +1,22 @@ +[require] +shader model >= 4.0 + +[texture 0] +size (2, 3) + +1.0 0.0 1.0 0.0 1.0 0.0 1.0 0.0 1.0 0.0 1.0 0.0 +1.0 0.0 1.0 0.0 1.0 0.0 1.0 0.0 1.0 0.0 1.0 0.0 + +[pixel shader] +Texture2D t0 : register(t0); + +float4 main() : sv_target +{ + uint width, height; + t0.GetDimensions(width, height); + return float4(width, height, 0, 0); +} + +[test] +draw quad +probe all rgba (2.0, 3.0, 0.0, 0.0) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/218
This merge request was approved by Francisco Casas. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/218
This merge request was approved by Zebediah Figura. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/218
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/tpf.c:
write_sm4_instruction(buffer, &instr); }
+static void write_sm4_sampleinfo(struct hlsl_ctx *ctx, struct vkd3d_bytecode_buffer *buffer, + const struct hlsl_ir_resource_load *load) +{ + const struct hlsl_type *resource_type = load->resource.var->data_type; + const struct hlsl_deref *resource = &load->resource; + const struct hlsl_ir_node *dst = &load->node; + struct sm4_instruction instr; + + memset(&instr, 0, sizeof(instr)); + instr.opcode = VKD3D_SM4_OP_SAMPLE_INFO; + if (dst->data_type->base_type == HLSL_TYPE_UINT) + instr.opcode |= VKD3DSI_SAMPLE_INFO_UINT << VKD3D_SM4_INSTRUCTION_FLAGS_SHIFT;
Can we also assert that the base type is either `UINT` or `FLOAT`? Same thing for `resinfo`. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/218#note_34941
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/tpf.c:
+ + sm4_dst_from_node(&instr.dsts[0], dst); + instr.dst_count = 1; + + if (load->level.node) + { + sm4_src_from_node(&instr.srcs[0], load->level.node, VKD3DSP_WRITEMASK_ALL); + } + else + { + instr.srcs[0].swizzle_type = VKD3D_SM4_SWIZZLE_NONE; + reg = &instr.srcs[0].reg; + reg->type = VKD3D_SM4_RT_IMMCONST; + reg->dim = VKD3D_SM4_DIMENSION_SCALAR; + reg->immconst_uint[0] = 0; + } I'd avoid having this kind of "default arguments" in the IR. Let the frontend stick something in `level`, instead of defaulting to zero in the IR.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/218#note_34943
Giovanni Mascellani (@giomasce) commented about tests/hlsl-getdimensions.shader_test:
+[require] +shader model >= 4.0 + +[texture 0] +size (2, 3) + +1.0 0.0 1.0 0.0 1.0 0.0 1.0 0.0 1.0 0.0 1.0 0.0 +1.0 0.0 1.0 0.0 1.0 0.0 1.0 0.0 1.0 0.0 1.0 0.0 + +[pixel shader] +Texture2D t0 : register(t0); + +float4 main() : sv_target +{ + uint width, height; + t0.GetDimensions(width, height); Maybe we can test other overloads too? Including passing some arguments as `float` and others as `uint`, given that this context is a bit less trivial than what we're used to.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/218#note_34942
On Fri Jun 2 04:54:44 2023 +0000, Nikolay Sivov wrote: > Yes, it accepts anything in there. I pushed scalar check for now. I don't think the `uint` detection algorithm is completely correct right now. For instance, `(int, int)` causes the `float` variant to be called, but `(int, uint)` causes the `uint` variant to be called. Quite strange. >From my tests the algorithm seems something like this: * if at least an argument is `float` or `half`, then use the `float` variant; * otherwise, if at least an argument is `uint`, then use the `uint` variant; * otherwise, use the `float` variant. In theory that shouldn't even change that much, given that texture sizes should never be fractional or bigger than the smallest integer not representable with a `float`; so I won't insist too much on this. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/218#note_34944
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/hlsl.y:
+ { + if (!(args[ARG_MIP_LEVEL] = add_implicit_conversion(ctx, instrs, args[ARG_MIP_LEVEL], + hlsl_get_scalar_type(ctx, HLSL_TYPE_UINT), loc))) + { + return false; + } + + continue; + } + + uint_resinfo &= hlsl_types_are_equal(params->args[j]->data_type, uint_type); + + if (params->args[j]->data_type->class != HLSL_CLASS_SCALAR) + { + hlsl_error(ctx, loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE, "Expected scalar arguments."); + break; Maybe you should `return false` here too, so that the compiler doesn't keep trying to compile hopeless code.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/218#note_34945
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/hlsl.y:
+ o = iter; + break; + } + } + + if (!o) + { + struct vkd3d_string_buffer *string; + + if ((string = hlsl_type_to_string(ctx, object_type))) + { + hlsl_error(ctx, loc, VKD3D_SHADER_ERROR_HLSL_INVALID_SYNTAX, + "Unexpected number of arguments %u for %s.%s().", params->args_count, string->buffer, name); + hlsl_release_string_buffer(ctx, string); + } + } `return false` here too.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/218#note_34946
The code is generally fine, but there are a few things to fix. Also, please add the tests in a dedicated commit and then the implementation in another one. Once upon a time we also used to add features in more atomic commits (i.e., one commit to add the frontend for `GetDimensions()`, another to add the backend for `resinfo` and another one for `sampleinfo`), but it seems this tradition was lost. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/218#note_34947
On Wed Jun 7 14:04:49 2023 +0000, Giovanni Mascellani wrote:
Maybe you should `return false` here too, so that the compiler doesn't keep trying to compile hopeless code. No, in general we don't want to abort parsing if we can avoid it. We want to be able to report as many (unrelated) errors as possible.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/218#note_34949
Once upon a time we also used to add features in more atomic commits (i.e., one commit to add the frontend for `GetDimensions()`, another to add the backend for `resinfo` and another one for `sampleinfo`), but it seems this tradition was lost.
Yes, I still prefer this as well... -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/218#note_34950
participants (5)
-
Francisco Casas (＠fcasas) -
Giovanni Mascellani (＠giomasce)
-
Nikolay Sivov
-
Nikolay Sivov (＠nsivov)
-
Zebediah Figura (＠zfigura)