[PATCH] wined3d: Allow wined3d_stream_info_from_declaration() to include inputs with no buffer set.
Include these inputs in the Vulkan pipeline input descriptor. This prevents MoltenVK from rejecting the pipeline when they are consumed by the shader. We should probably also bind a null resource in that case to comply with Vulkan spec.
Signed-off-by: Jan Sikorski jsikorski@codeweavers.com --- dlls/wined3d/context.c | 6 +++--- dlls/wined3d/context_vk.c | 2 +- dlls/wined3d/device.c | 2 +- dlls/wined3d/wined3d_private.h | 3 ++- 4 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/dlls/wined3d/context.c b/dlls/wined3d/context.c index 2debf50de7f..f2fdcddc261 100644 --- a/dlls/wined3d/context.c +++ b/dlls/wined3d/context.c @@ -154,7 +154,7 @@ static BOOL fixed_get_input(BYTE usage, BYTE usage_idx, unsigned int *regnum)
/* Context activation is done by the caller. */ void wined3d_stream_info_from_declaration(struct wined3d_stream_info *stream_info, - const struct wined3d_state *state, const struct wined3d_d3d_info *d3d_info) + const struct wined3d_state *state, const struct wined3d_d3d_info *d3d_info, bool require_buffer) { /* We need to deal with frequency data! */ struct wined3d_vertex_declaration *declaration = state->vertex_declaration; @@ -182,7 +182,7 @@ void wined3d_stream_info_from_declaration(struct wined3d_stream_info *stream_inf TRACE("%p Element %p (%u of %u).\n", declaration->elements, element, i + 1, declaration->element_count);
- if (!stream->buffer) + if (!stream->buffer && require_buffer) continue;
TRACE("offset %u input_slot %u usage_idx %d.\n", element->offset, element->input_slot, element->usage_idx); @@ -269,7 +269,7 @@ void context_update_stream_info(struct wined3d_context *context, const struct wi unsigned int i; WORD map;
- wined3d_stream_info_from_declaration(stream_info, state, d3d_info); + wined3d_stream_info_from_declaration(stream_info, state, d3d_info, true);
stream_info->all_vbo = 1; for (i = 0, map = stream_info->use_map; map; map >>= 1, ++i) diff --git a/dlls/wined3d/context_vk.c b/dlls/wined3d/context_vk.c index 45133eabb69..96b732fb3aa 100644 --- a/dlls/wined3d/context_vk.c +++ b/dlls/wined3d/context_vk.c @@ -2032,7 +2032,7 @@ static bool wined3d_context_vk_update_graphics_pipeline_key(struct wined3d_conte || wined3d_context_is_graphics_state_dirty(&context_vk->c, STATE_STREAMSRC) || wined3d_context_is_graphics_state_dirty(&context_vk->c, STATE_SHADER(WINED3D_SHADER_TYPE_VERTEX))) { - wined3d_stream_info_from_declaration(&stream_info, state, d3d_info); + wined3d_stream_info_from_declaration(&stream_info, state, d3d_info, false); divisor_count = 0; for (i = 0, mask = 0, attribute_count = 0, binding_count = 0; i < ARRAY_SIZE(stream_info.elements); ++i) { diff --git a/dlls/wined3d/device.c b/dlls/wined3d/device.c index 9240bad9d27..4bc683d3c40 100644 --- a/dlls/wined3d/device.c +++ b/dlls/wined3d/device.c @@ -3789,7 +3789,7 @@ HRESULT CDECL wined3d_device_process_vertices(struct wined3d_device *device,
vs = state->shader[WINED3D_SHADER_TYPE_VERTEX]; state->shader[WINED3D_SHADER_TYPE_VERTEX] = NULL; - wined3d_stream_info_from_declaration(&stream_info, state, &device->adapter->d3d_info); + wined3d_stream_info_from_declaration(&stream_info, state, &device->adapter->d3d_info, true); state->shader[WINED3D_SHADER_TYPE_VERTEX] = vs;
/* We can't convert FROM a VBO, and vertex buffers used to source into diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h index 4d5f4765f57..ab5f2253462 100644 --- a/dlls/wined3d/wined3d_private.h +++ b/dlls/wined3d/wined3d_private.h @@ -1678,7 +1678,8 @@ struct wined3d_stream_info };
void wined3d_stream_info_from_declaration(struct wined3d_stream_info *stream_info, - const struct wined3d_state *state, const struct wined3d_d3d_info *d3d_info) DECLSPEC_HIDDEN; + const struct wined3d_state *state, const struct wined3d_d3d_info *d3d_info, + bool require_buffer) DECLSPEC_HIDDEN;
struct wined3d_direct_dispatch_parameters {
On Wed, 14 Apr 2021 at 13:37, Jan Sikorski jsikorski@codeweavers.com wrote:
Include these inputs in the Vulkan pipeline input descriptor. This prevents MoltenVK from rejecting the pipeline when they are consumed by the shader. We should probably also bind a null resource in that case to comply with Vulkan spec.
I don't think we particularly care about MoltenVK's behaviour, as such. The pertinent questions would probably be whether Direct3D has any defined behaviour across drivers for this (which implies tests), and what the Vulkan spec says.
On 14 Apr 2021, at 14:00, Henri Verbeet hverbeet@gmail.com wrote:
On Wed, 14 Apr 2021 at 13:37, Jan Sikorski jsikorski@codeweavers.com wrote:
Include these inputs in the Vulkan pipeline input descriptor. This prevents MoltenVK from rejecting the pipeline when they are consumed by the shader. We should probably also bind a null resource in that case to comply with Vulkan spec.
I don't think we particularly care about MoltenVK's behaviour, as such. The pertinent questions would probably be whether Direct3D has any defined behaviour across drivers for this (which implies tests), and what the Vulkan spec says.
According to the validation layer, it seems that in d3d it’s ok to not set a buffer, the IA will provide zeros on such inputs. Tests seem to confirm that, except for d3d10 on Vista, which crashes - by the way, what would be the correct way to handle that, if (vista) skip? In Vulkan I get a validation error on the shader module saying “Vertex shader consumes input at location # but not provided”, without a link to the spec. That said this patch only fixes the shader part, there still should be a buffer or NULL_HANDLE bound in the command buffer, relevant links: https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/vkCmdD... https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/vkCmdD... The second one seems to imply that we need to actually bind a zeroed buffer in case the shader reads it. I guess I might as well take a shot at the binding issue and then resend with the tests included..
- Janek
On Wed, 14 Apr 2021 at 17:46, Jan Sikorski jsikorski@codeweavers.com wrote:
According to the validation layer, it seems that in d3d it’s ok to not set a buffer, the IA will provide zeros on such inputs. Tests seem to confirm that, except for d3d10 on Vista, which crashes - by the way, what would be the correct way to handle that, if (vista) skip?
In principle, although you wouldn't be able to use GetVersion() or variants to detect Vista; you may have to e.g. query for the presence of d3d11 interfaces.
In Vulkan I get a validation error on the shader module saying “Vertex shader consumes input at location # but not provided”, without a link to the spec. That said this patch only fixes the shader part, there still should be a buffer or NULL_HANDLE bound in the command buffer, relevant links: https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/vkCmdD... https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/vkCmdD... The second one seems to imply that we need to actually bind a zeroed buffer in case the shader reads it. I guess I might as well take a shot at the binding issue and then resend with the tests included..
For what it's worth, we do have the wined3d_null_resources_vk bo that could be used for this, although it's not currently setup to be used as a vertex buffer.
-
Henri Verbeet -
Jan Sikorski