[PATCH v3 0/1] MR111: vkd3d-shader/hlsl: Improve handling of "technique" tokens.
This change only implements case-sensitivity of technique keywords and related checks when corresponding profile is used. My immediate plan is to add some synthetic type for technique variables, for example as {object, void}, then add named techniques as variables. This is useful because these names are participating in global scope, and should not collide with normal variables. After that "pass_list" will be split, at least in two variants because of how much d3d9 syntax differs. Some trivial changes will be need later to have some top level fx compilation helper that considers only fx objects and calls ps/vs/gs compiler to create inner shader blobs on same original source. -- v3: vkd3d-shader/hlsl: Improve handling of "technique" tokens. https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/111
From: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.l | 3 ++- libs/vkd3d-shader/hlsl.y | 45 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/libs/vkd3d-shader/hlsl.l b/libs/vkd3d-shader/hlsl.l index e9ae3ccf3..7b7abb416 100644 --- a/libs/vkd3d-shader/hlsl.l +++ b/libs/vkd3d-shader/hlsl.l @@ -125,8 +125,9 @@ string {return KW_STRING; } struct {return KW_STRUCT; } switch {return KW_SWITCH; } tbuffer {return KW_TBUFFER; } -technique {return KW_TECHNIQUE; } +(?i:technique) {return KW_TECHNIQUE; } technique10 {return KW_TECHNIQUE10; } +technique11 {return KW_TECHNIQUE11; } texture {return KW_TEXTURE; } texture1D {return KW_TEXTURE1D; } Texture1D {return KW_TEXTURE1D; } diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index a47246de2..69ff457ce 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -4657,6 +4657,7 @@ static void validate_texture_format_type(struct hlsl_ctx *ctx, struct hlsl_type %token KW_TBUFFER %token KW_TECHNIQUE %token KW_TECHNIQUE10 +%token KW_TECHNIQUE11 %token KW_TEXTURE %token KW_TEXTURE1D %token KW_TEXTURE1DARRAY @@ -4777,6 +4778,7 @@ static void validate_texture_format_type(struct hlsl_ctx *ctx, struct hlsl_type %type <name> any_identifier %type <name> var_identifier +%type <name> technique_name %type <parameter> parameter @@ -4816,8 +4818,51 @@ hlsl_prog: destroy_block($2); } | hlsl_prog preproc_directive + | hlsl_prog technique | hlsl_prog ';' +technique_name: + %empty + { + $$ = NULL; + } + | any_identifier + +pass_list: + %empty + +technique9: + KW_TECHNIQUE technique_name '{' pass_list '}' + { + if (ctx->profile->type == VKD3D_SHADER_TYPE_EFFECT && ctx->profile->major_version == 2) + { + hlsl_fixme(ctx, &@$, "Unsupported keyword \'technique\'."); + } + } + +technique10: + KW_TECHNIQUE10 technique_name '{' pass_list '}' + { + if (ctx->profile->type == VKD3D_SHADER_TYPE_EFFECT && ctx->profile->major_version >= 4) + { + hlsl_fixme(ctx, &@$, "Unsupported keyword \'technique10\'."); + } + } + +technique11: + KW_TECHNIQUE11 technique_name '{' pass_list '}' + { + if (ctx->profile->type == VKD3D_SHADER_TYPE_EFFECT && ctx->profile->major_version >= 4) + { + hlsl_fixme(ctx, &@$, "Unsupported keyword \'technique11\'."); + } + } + +technique: + technique9 + | technique10 + | technique11 + buffer_declaration: buffer_type any_identifier colon_attribute { -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/111
Is this waiting for tests or for something else? Regarding tests, I don't see right now how this could fit in current shader runner, we might be able to compile test, but anything else requires effects handling in the runner. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/111#note_46983
Is this waiting for tests or for something else? Regarding tests, I don't see right now how this could fit in current shader runner, we might be able to compile test, but anything else requires effects handling in the runner.
From my point of view, the main thing it needs is sign-off by Zeb and Giovanni. I think tests would be helpful though, and making sure things (and non-effect profiles in particular) at least compile seems like a good start.
Support for actual effect handling in the shader runner would be a fair bit more involved, but perhaps not terribly hard either? I imagine that initially we'd just need a way to verify the state contained in a compiled effect. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/111#note_47109
On Thu Sep 28 19:11:57 2023 +0000, Henri Verbeet wrote:
Is this waiting for tests or for something else? Regarding tests, I don't see right now how this could fit in current shader runner, we might be able to compile test, but anything else requires effects handling in the runner. From my point of view, the main thing it needs is sign-off by Zeb and Giovanni. I think tests would be helpful though, and making sure things (and non-effect profiles in particular) at least compile seems like a good start. Support for actual effect handling in the shader runner would be a fair bit more involved, but perhaps not terribly hard either? I imagine that initially we'd just need a way to verify the state contained in a compiled effect. What we can do is some extension to our tests that will check state objects for initial values, this will only cover statically initialized ones at first. I don't think it should necessarily render anything. Unfortunately this is tied to d3d API too much, and we'll have to provide some sort of dxvk-like layer. I was thinking we might start using d3dcompiler for d3d10/d3d9 effects tests, once the compiler is capable enough. That would validate at least something, and would be stable enough because of the imported vkd3d.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/111#note_47127
What we can do is some extension to our tests that will check state objects for initial values, this will only cover statically initialized ones at first. I don't think it should necessarily render anything.
Right.
Unfortunately this is tied to d3d API too much, and we'll have to provide some sort of dxvk-like layer.
Are there parts in particular that you're concerned about there? Fundamentally, there doesn't seem much conceptual difference between e.g. declaring some sampler state using "[sampler]" in a .shader_test, or doing the same using an effect. The D3DX effect interfaces do quite a bit more than just parsing effects, of course, but I don't think vkd3d necessarily needs to replicate all of that. If vkd3d could accurately parse effects, something like D3DX in Wine could build its state management on top of that. (For example, d3dx9_create_object() in Wine's d3dx9_36 creates shaders while parsing the effect, but it doesn't need to work that way; it could also create those shaders based on the shader bytecode contained in an effect description it got from vkd3d.)
I was thinking we might start using d3dcompiler for d3d10/d3d9 effects tests, once the compiler is capable enough. That would validate at least something, and would be stable enough because of the imported vkd3d.
Are you talking about Wine tests here? Or in vkd3d? I imagine we could prototype some effect parsing code inside the shader runner before exposing a public API for it. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/111#note_47356
Are there parts in particular that you're concerned about there? Fundamentally, there doesn't seem much conceptual difference between e.g. declaring some sampler state using "\[sampler\]" in a .shader_test, or doing the same using an effect.
What about other states? Or differences between fx_2 and fx_4/5? To render something, what would naturally be d3d9 or d3d10+ states should be mapped to whatever system shader_runner is using.
Are you talking about Wine tests here? Or in vkd3d? I imagine we could prototype some effect parsing code inside the shader runner before exposing a public API for it.
I'm talking about Wine tests. We have parsing code already, moving it somewhere else simply delays fixing a problem at hand. Also that will call for some uniform interface for d3dx9 and d3d10+ effects, and I don't see why we would want such extra work. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/111#note_47364
Are there parts in particular that you're concerned about there? Fundamentally, there doesn't seem much conceptual difference between e.g. declaring some sampler state using "\[sampler\]" in a .shader_test, or doing the same using an effect.
What about other states? Or differences between fx_2 and fx_4/5? To render something, what would naturally be d3d9 or d3d10+ states should be mapped to whatever system shader_runner is using.
Sure. I think that's generally straightforward though. E.g. the shader runner currently uses D3D12_TEXTURE_ADDRESS_MODE_CLAMP for what would be D3DTADDRESS_CLAMP in d3d9. The d3d9 runner already does this particular mapping in the other direction.
Are you talking about Wine tests here? Or in vkd3d? I imagine we could prototype some effect parsing code inside the shader runner before exposing a public API for it.
I'm talking about Wine tests. We have parsing code already, moving it somewhere else simply delays fixing a problem at hand. Also that will call for some uniform interface for d3dx9 and d3d10+ effects, and I don't see why we would want such extra work.
I think effects parsing is generally useful outside of Wine too, but beyond that, I imagine it's going to be much harder to get effects compilation code merged without the tests to go along with it, than it's going to be to add effects parsing code to the shader runner or vkd3d. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/111#note_47367
Is this waiting for tests or for something else? Regarding tests, I don't see right now how this could fit in current shader runner, we might be able to compile test, but anything else requires effects handling in the runner.
Yes, I'd like more tests at least. We are probably going to eventually want effects parsing in vkd3d (right?), but that's not really necessary for me right now, especially since we're not currently implementing effects, only parsing them. Tests that these shaders even compile is good enough for me. I.e. I'd like to see 114 committed first. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/111#note_47634
This merge request was closed by Nikolay Sivov. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/111
participants (4)
-
Henri Verbeet (＠hverbeet) -
Nikolay Sivov
-
Nikolay Sivov (＠nsivov)
-
Zebediah Figura (＠zfigura)