[PATCH 0/4] MR152: vkd3d-shader/hlsl: Fixes for sm1 semantics.
From: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl_sm1.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/libs/vkd3d-shader/hlsl_sm1.c b/libs/vkd3d-shader/hlsl_sm1.c index be32c8db..2cfdb1dc 100644 --- a/libs/vkd3d-shader/hlsl_sm1.c +++ b/libs/vkd3d-shader/hlsl_sm1.c @@ -302,7 +302,10 @@ static void sm1_sort_externs(struct hlsl_ctx *ctx) struct hlsl_ir_var *var, *next; LIST_FOR_EACH_ENTRY_SAFE(var, next, &ctx->extern_vars, struct hlsl_ir_var, extern_entry) - sm1_sort_extern(&sorted, var); + { + if (var->is_uniform) + sm1_sort_extern(&sorted, var); + } list_move_tail(&ctx->extern_vars, &sorted); } -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/152
From: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl_sm1.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libs/vkd3d-shader/hlsl_sm1.c b/libs/vkd3d-shader/hlsl_sm1.c index 2cfdb1dc..bd0d5b68 100644 --- a/libs/vkd3d-shader/hlsl_sm1.c +++ b/libs/vkd3d-shader/hlsl_sm1.c @@ -37,6 +37,9 @@ bool hlsl_sm1_register_from_semantic(struct hlsl_ctx *ctx, const struct hlsl_sem } register_table[] = { + {"color", false, VKD3D_SHADER_TYPE_PIXEL, 1, D3DSPR_INPUT}, + {"texcoord", false, VKD3D_SHADER_TYPE_PIXEL, 1, D3DSPR_TEXTURE}, + {"color", true, VKD3D_SHADER_TYPE_PIXEL, 2, D3DSPR_COLOROUT}, {"depth", true, VKD3D_SHADER_TYPE_PIXEL, 2, D3DSPR_DEPTHOUT}, {"sv_depth", true, VKD3D_SHADER_TYPE_PIXEL, 2, D3DSPR_DEPTHOUT}, -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/152
From: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl_sm1.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/vkd3d-shader/hlsl_sm1.c b/libs/vkd3d-shader/hlsl_sm1.c index bd0d5b68..4b4452fa 100644 --- a/libs/vkd3d-shader/hlsl_sm1.c +++ b/libs/vkd3d-shader/hlsl_sm1.c @@ -645,7 +645,7 @@ static void write_sm1_semantic_dcls(struct hlsl_ctx *ctx, struct vkd3d_bytecode_ bool write_in = false, write_out = false; struct hlsl_ir_var *var; - if (ctx->profile->type == VKD3D_SHADER_TYPE_PIXEL) + if (ctx->profile->type == VKD3D_SHADER_TYPE_PIXEL && ctx->profile->major_version >= 2) write_in = true; else if (ctx->profile->type == VKD3D_SHADER_TYPE_VERTEX && ctx->profile->major_version == 3) write_in = write_out = true; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/152
From: Zebediah Figura <zfigura(a)codeweavers.com> We were previously (accidentally) rejecting them because they didn't have a usage. --- libs/vkd3d-shader/hlsl_codegen.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 5d7e426f..7f79a0fd 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -2837,7 +2837,8 @@ static void allocate_semantic_register(struct hlsl_ctx *ctx, struct hlsl_ir_var D3DDECLUSAGE usage; uint32_t usage_idx; - if (!hlsl_sm1_usage_from_semantic(&var->semantic, &usage, &usage_idx)) + builtin = hlsl_sm1_register_from_semantic(ctx, &var->semantic, output, &type, ®); + if (!builtin && !hlsl_sm1_usage_from_semantic(&var->semantic, &usage, &usage_idx)) { hlsl_error(ctx, &var->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_SEMANTIC, "Invalid semantic '%s'.", var->semantic.name); @@ -2846,8 +2847,6 @@ static void allocate_semantic_register(struct hlsl_ctx *ctx, struct hlsl_ir_var if ((!output && !var->last_read) || (output && !var->first_write)) return; - - builtin = hlsl_sm1_register_from_semantic(ctx, &var->semantic, output, &type, ®); } else { -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/152
Approving this because nothing screams of being wrong and I'm not uncomfortable with anything in this MR, but my knowledge of the low level bits of SM1 is pretty weak, so I can't really give an independent confirmation that we want these changes. But I have a general trust that Zeb knows what she's doing! :-) -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/152#note_30063
This merge request was approved by Giovanni Mascellani. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/152
Approving this because nothing screams of being wrong and I'm not uncomfortable with anything in this MR, but my knowledge of the low level bits of SM1 is pretty weak, so I can't really give an independent confirmation that we want these changes. But I have a general trust that Zeb knows what she's doing! :-)
Sanity-checking the code changes is always appreciated even if you have to trust that the functional change is correct :-) 1/4 and 4/4 were both things I came across while trying to get sm1 tests to work, so in a sense we already have tests for those, we just can't quite run them yet. 2/4 and 3/4 are things related to ps_1_*, which I think we originally dismissed as not worth implementing, on the basis that native doesn't support emitting them and implicitly promotes them to ps_2_*. But native d3dx9_31 does support emitting ps_1_*, and perhaps more importantly, we want to be able to test that our sm1 -> glsl/spirv translator can correctly handle those versions. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/152#note_30115
This merge request was approved by Matteo Bruni. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/152
On Fri Apr 14 19:23:49 2023 +0000, Zebediah Figura wrote:
Approving this because nothing screams of being wrong and I'm not uncomfortable with anything in this MR, but my knowledge of the low level bits of SM1 is pretty weak, so I can't really give an independent confirmation that we want these changes. But I have a general trust that Zeb knows what she's doing! :-) Sanity-checking the code changes is always appreciated even if you have to trust that the functional change is correct :-) 1/4 and 4/4 were both things I came across while trying to get sm1 tests to work, so in a sense we already have tests for those, we just can't quite run them yet. 2/4 and 3/4 are things related to ps_1_*, which I think we originally dismissed as not worth implementing, on the basis that native doesn't support emitting them and implicitly promotes them to ps_2_*. But native d3dx9_31 does support emitting ps_1_*, and perhaps more importantly, we want to be able to test that our sm1 -> glsl/spirv translator can correctly handle those versions. Basically yes, except it's not quite that it's not worth implementing ps_1_* but that a full implementation requires a TON of very specific compiler stuff that's going to get in the way of other compiler improvements.
This kind of small and localized fixes are certainly okay. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/152#note_30123
2/4 and 3/4 are things related to ps_1\_*, which I think we originally dismissed as not worth implementing, on the basis that native doesn't support emitting them and implicitly promotes them to ps_2\_*.
I've certainly heard that view before, but I don't quite agree. That particular fact may certainly make it a bit harder to justify spending much time on these older shader models in a more corporate environment, but I'll happily take the patches if someone writes them. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/152#note_30351
This merge request was approved by Henri Verbeet. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/152
participants (5)
-
Giovanni Mascellani (＠giomasce) -
Henri Verbeet (＠hverbeet)
-
Matteo Bruni (＠Mystral)
-
Zebediah Figura
-
Zebediah Figura (＠zfigura)