-
Notifications
You must be signed in to change notification settings - Fork 198
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Validate shader interface compatibility #1923
Validate shader interface compatibility #1923
Conversation
We already handle this chunk type internally. Signed-off-by: Philip Rebohle <[email protected]>
Signed-off-by: Philip Rebohle <[email protected]>
These specific SVs are included in shader signatures. Signed-off-by: Philip Rebohle <[email protected]>
Used only for GS exporting PrimtiveID. Signed-off-by: Philip Rebohle <[email protected]>
With a RTX 4080 on 550.54.14 I've given it a 10-15 minutes test in: Have not found any issues |
a8fb5b5
to
1ae5d19
Compare
switch (shader_stages_lut[i].stage) | ||
{ | ||
case VK_SHADER_STAGE_VERTEX_BIT: | ||
if ((ret = vkd3d_shader_parse_input_signature(&dxbc, &input_signature)) < 0) | ||
if ((ret = vkd3d_shader_parse_input_signature(&dxbc, &vs_input_signature)) < 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't this reuse io_input_signature?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to keep the VS input signature around to deal with the input layout, not just for validation purposes.
{ | ||
vkd3d_test_set_context("Test %u", i); | ||
|
||
expected = so_tests[i].should_compile ? S_OK : E_INVALIDARG; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where do we validate stream-out signature compat?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We kind of don't really, I could add that too (as well as more tests), but the intention of this particular set of tests was to see how SO and especially the rasterized stream index interacts with interface compatibility (FS is removed if rasterization is disabled, which we already did, and otherwise it's kind of broken).
Signed-off-by: Philip Rebohle <[email protected]>
Signed-off-by: Philip Rebohle <[email protected]>
Signed-off-by: Philip Rebohle <[email protected]>
1ae5d19
to
38625c1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Only skimmed over the tests though.
At PSO compile time, the D3D12 runtime validates the input layout against the VS input signature, as well as the interface between each connected shader stage. This can fix some cases of invalid Vulkan usage as well if games rely on runtime validation to catch these mismatches.
Also fixes some small vkd3d-shader issues I've encountered in the process.
Draft because this needs more real-world testing to ensure we don't break existing games somehow.