Signed-off-by: Jan Sikorski jsikorski@codeweavers.com --- v3: also test inputs without a corresponding entry in the input layout --- dlls/d3d11/tests/d3d11.c | 100 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 100 insertions(+)
diff --git a/dlls/d3d11/tests/d3d11.c b/dlls/d3d11/tests/d3d11.c index bf8837ab52c..35fe6458ab7 100644 --- a/dlls/d3d11/tests/d3d11.c +++ b/dlls/d3d11/tests/d3d11.c @@ -17157,6 +17157,105 @@ static void test_shader_stage_input_output_matching(void) release_test_context(&test_context); }
+static void test_unbound_streams(void) +{ + struct d3d11_test_context test_context; + ID3D11DeviceContext *context; + ID3D11PixelShader *ps; + ID3D11Device *device; + HRESULT hr; + + static const DWORD vs_code[] = + { +#if 0 + struct vs_ps + { + float4 position : SV_POSITION; + float4 color : COLOR0; + }; + + vs_ps vs_main(float4 position : POSITION, float4 color : COLOR0) + { + vs_ps result; + result.position = position; + result.color = color; + result.color.w = 1.0; + return result; + } +#endif + 0x43425844, 0x4a9efaec, 0xe2c6cdf5, 0x15dd28a7, 0xae68e320, 0x00000001, 0x00000154, 0x00000003, + 0x0000002c, 0x0000007c, 0x000000d0, 0x4e475349, 0x00000048, 0x00000002, 0x00000008, 0x00000038, + 0x00000000, 0x00000000, 0x00000003, 0x00000000, 0x00000f0f, 0x00000041, 0x00000000, 0x00000000, + 0x00000003, 0x00000001, 0x0000070f, 0x49534f50, 0x4e4f4954, 0x4c4f4300, 0xab00524f, 0x4e47534f, + 0x0000004c, 0x00000002, 0x00000008, 0x00000038, 0x00000000, 0x00000001, 0x00000003, 0x00000000, + 0x0000000f, 0x00000044, 0x00000000, 0x00000000, 0x00000003, 0x00000001, 0x0000000f, 0x505f5653, + 0x5449534f, 0x004e4f49, 0x4f4c4f43, 0xabab0052, 0x52444853, 0x0000007c, 0x00010040, 0x0000001f, + 0x0300005f, 0x001010f2, 0x00000000, 0x0300005f, 0x00101072, 0x00000001, 0x04000067, 0x001020f2, + 0x00000000, 0x00000001, 0x03000065, 0x001020f2, 0x00000001, 0x05000036, 0x001020f2, 0x00000000, + 0x00101e46, 0x00000000, 0x05000036, 0x00102072, 0x00000001, 0x00101246, 0x00000001, 0x05000036, + 0x00102082, 0x00000001, 0x00004001, 0x3f800000, 0x0100003e, + }; + + static const DWORD ps_code[] = + { +#if 0 + float4 ps_main(vs_ps input) : SV_TARGET + { + return input.color; + } +#endif + 0x43425844, 0xe2087fa6, 0xa35fbd95, 0x8e585b3f, 0x67890f54, 0x00000001, 0x000000f4, 0x00000003, + 0x0000002c, 0x00000080, 0x000000b4, 0x4e475349, 0x0000004c, 0x00000002, 0x00000008, 0x00000038, + 0x00000000, 0x00000001, 0x00000003, 0x00000000, 0x0000000f, 0x00000044, 0x00000000, 0x00000000, + 0x00000003, 0x00000001, 0x00000f0f, 0x505f5653, 0x5449534f, 0x004e4f49, 0x4f4c4f43, 0xabab0052, + 0x4e47534f, 0x0000002c, 0x00000001, 0x00000008, 0x00000020, 0x00000000, 0x00000000, 0x00000003, + 0x00000000, 0x0000000f, 0x545f5653, 0x45475241, 0xabab0054, 0x52444853, 0x00000038, 0x00000040, + 0x0000000e, 0x03001062, 0x001010f2, 0x00000001, 0x03000065, 0x001020f2, 0x00000000, 0x05000036, + 0x001020f2, 0x00000000, 0x00101e46, 0x00000001, 0x0100003e, + }; + + static const float white[4] = {1.0f, 1.0f, 1.0f, 1.0f}; + static const struct vec4 black = {0}; + + static const D3D11_INPUT_ELEMENT_DESC layout_desc[] = + { + {"POSITION", 0, DXGI_FORMAT_R32G32B32_FLOAT, 0, 0, D3D11_INPUT_PER_VERTEX_DATA, 0}, + {"COLOR", 0, DXGI_FORMAT_R32G32B32A32_FLOAT, 1, 16, D3D11_INPUT_PER_VERTEX_DATA, 0}, + }; + + if (!init_test_context(&test_context, NULL)) + return; + + device = test_context.device; + context = test_context.immediate_context; + + /* Create default input layout */ + draw_color_quad(&test_context, &black); + + hr = ID3D11Device_CreatePixelShader(device, ps_code, sizeof(ps_code), NULL, &ps); + ok(SUCCEEDED(hr), "Failed to create pixel shader, hr %#x.\n", hr); + + ID3D11DeviceContext_PSSetShader(context, ps, NULL, 0); + ID3D11DeviceContext_ClearRenderTargetView(context, test_context.backbuffer_rtv, white); + + /* Draw with default input layout */ + draw_quad_vs(&test_context, vs_code, sizeof(vs_code)); + check_texture_color(test_context.backbuffer, 0xff000000, 1); + + ID3D11InputLayout_Release(test_context.input_layout); + hr = ID3D11Device_CreateInputLayout(device, layout_desc, ARRAY_SIZE(layout_desc), + vs_code, sizeof(vs_code), &test_context.input_layout); + ok(SUCCEEDED(hr), "Failed to create input layout, hr %#x.\n", hr); + + ID3D11DeviceContext_ClearRenderTargetView(context, test_context.backbuffer_rtv, white); + /* Draw with matching input layout */ + draw_quad_vs(&test_context, vs_code, sizeof(vs_code)); + check_texture_color(test_context.backbuffer, 0xff000000, 1); + + ID3D11PixelShader_Release(ps); + release_test_context(&test_context); +} + static void test_shader_interstage_interface(void) { struct d3d11_test_context test_context; @@ -32520,6 +32619,7 @@ START_TEST(d3d11) queue_test(test_deferred_context_state); queue_test(test_deferred_context_swap_state); queue_test(test_deferred_context_rendering); + queue_test(test_unbound_streams);
run_queued_tests(); }
Signed-off-by: Jan Sikorski jsikorski@codeweavers.com --- v3: also test inputs without a corresponding entry in the input layout --- dlls/d3d10core/tests/d3d10core.c | 98 ++++++++++++++++++++++++++++++++ 1 file changed, 98 insertions(+)
diff --git a/dlls/d3d10core/tests/d3d10core.c b/dlls/d3d10core/tests/d3d10core.c index 927d79d90e9..a00ed87c706 100644 --- a/dlls/d3d10core/tests/d3d10core.c +++ b/dlls/d3d10core/tests/d3d10core.c @@ -18822,6 +18822,103 @@ static void test_dual_source_blend(void) release_test_context(&test_context); }
+static void test_unbound_streams(void) +{ + struct d3d10core_test_context test_context; + ID3D10PixelShader *ps; + ID3D10Device *device; + HRESULT hr; + + static const DWORD vs_code[] = + { +#if 0 + struct vs_ps + { + float4 position : SV_POSITION; + float4 color : COLOR0; + }; + + vs_ps vs_main(float4 position : POSITION, float4 color : COLOR0) + { + vs_ps result; + result.position = position; + result.color = color; + result.color.w = 1.0; + return result; + } +#endif + 0x43425844, 0x4a9efaec, 0xe2c6cdf5, 0x15dd28a7, 0xae68e320, 0x00000001, 0x00000154, 0x00000003, + 0x0000002c, 0x0000007c, 0x000000d0, 0x4e475349, 0x00000048, 0x00000002, 0x00000008, 0x00000038, + 0x00000000, 0x00000000, 0x00000003, 0x00000000, 0x00000f0f, 0x00000041, 0x00000000, 0x00000000, + 0x00000003, 0x00000001, 0x0000070f, 0x49534f50, 0x4e4f4954, 0x4c4f4300, 0xab00524f, 0x4e47534f, + 0x0000004c, 0x00000002, 0x00000008, 0x00000038, 0x00000000, 0x00000001, 0x00000003, 0x00000000, + 0x0000000f, 0x00000044, 0x00000000, 0x00000000, 0x00000003, 0x00000001, 0x0000000f, 0x505f5653, + 0x5449534f, 0x004e4f49, 0x4f4c4f43, 0xabab0052, 0x52444853, 0x0000007c, 0x00010040, 0x0000001f, + 0x0300005f, 0x001010f2, 0x00000000, 0x0300005f, 0x00101072, 0x00000001, 0x04000067, 0x001020f2, + 0x00000000, 0x00000001, 0x03000065, 0x001020f2, 0x00000001, 0x05000036, 0x001020f2, 0x00000000, + 0x00101e46, 0x00000000, 0x05000036, 0x00102072, 0x00000001, 0x00101246, 0x00000001, 0x05000036, + 0x00102082, 0x00000001, 0x00004001, 0x3f800000, 0x0100003e, + }; + + static const DWORD ps_code[] = + { +#if 0 + float4 ps_main(vs_ps input) : SV_TARGET + { + return input.color; + } +#endif + 0x43425844, 0xe2087fa6, 0xa35fbd95, 0x8e585b3f, 0x67890f54, 0x00000001, 0x000000f4, 0x00000003, + 0x0000002c, 0x00000080, 0x000000b4, 0x4e475349, 0x0000004c, 0x00000002, 0x00000008, 0x00000038, + 0x00000000, 0x00000001, 0x00000003, 0x00000000, 0x0000000f, 0x00000044, 0x00000000, 0x00000000, + 0x00000003, 0x00000001, 0x00000f0f, 0x505f5653, 0x5449534f, 0x004e4f49, 0x4f4c4f43, 0xabab0052, + 0x4e47534f, 0x0000002c, 0x00000001, 0x00000008, 0x00000020, 0x00000000, 0x00000000, 0x00000003, + 0x00000000, 0x0000000f, 0x545f5653, 0x45475241, 0xabab0054, 0x52444853, 0x00000038, 0x00000040, + 0x0000000e, 0x03001062, 0x001010f2, 0x00000001, 0x03000065, 0x001020f2, 0x00000000, 0x05000036, + 0x001020f2, 0x00000000, 0x00101e46, 0x00000001, 0x0100003e, + }; + + static const float white[4] = {1.0f, 1.0f, 1.0f, 1.0f}; + static const struct vec4 black = {0}; + + static const D3D10_INPUT_ELEMENT_DESC layout_desc[] = + { + {"POSITION", 0, DXGI_FORMAT_R32G32B32_FLOAT, 0, 0, D3D10_INPUT_PER_VERTEX_DATA, 0}, + {"COLOR", 0, DXGI_FORMAT_R32G32B32A32_FLOAT, 1, 16, D3D10_INPUT_PER_VERTEX_DATA, 0}, + }; + + if (!init_test_context(&test_context)) + return; + + device = test_context.device; + + /* Create default input layout */ + draw_color_quad(&test_context, &black); + + hr = ID3D10Device_CreatePixelShader(device, ps_code, sizeof(ps_code), &ps); + ok(SUCCEEDED(hr), "Failed to create pixel shader, hr %#x.\n", hr); + + ID3D10Device_PSSetShader(device, ps); + ID3D10Device_ClearRenderTargetView(device, test_context.backbuffer_rtv, white); + + /* Draw with default input layout */ + draw_quad_vs(&test_context, vs_code, sizeof(vs_code)); + check_texture_color(test_context.backbuffer, 0xff000000, 1); + + ID3D10InputLayout_Release(test_context.input_layout); + hr = ID3D10Device_CreateInputLayout(device, layout_desc, ARRAY_SIZE(layout_desc), + vs_code, sizeof(vs_code), &test_context.input_layout); + ok(SUCCEEDED(hr), "Failed to create input layout, hr %#x.\n", hr); + + ID3D10Device_ClearRenderTargetView(device, test_context.backbuffer_rtv, white); + /* Draw with matching input layout */ + draw_quad_vs(&test_context, vs_code, sizeof(vs_code)); + check_texture_color(test_context.backbuffer, 0xff000000, 1); + + ID3D10PixelShader_Release(ps); + release_test_context(&test_context); +} + START_TEST(d3d10core) { unsigned int argc, i; @@ -18946,6 +19043,7 @@ START_TEST(d3d10core) queue_test(test_color_mask); queue_test(test_independent_blend); queue_test(test_dual_source_blend); + queue_test(test_unbound_streams);
run_queued_tests();
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=89055
Your paranoid android.
=== w2008s64 (32 bit report) ===
d3d10core: d3d10core.c:18906: Test failed: Got 0xff0000ff, expected 0xff000000 at (0, 0), sub-resource 0.
=== w10pro64 (32 bit report) ===
d3d10core: d3d10core.c:4766: Test failed: Got unexpected IAVertices count: 0. d3d10core.c:4767: Test failed: Got unexpected IAPrimitives count: 0. d3d10core.c:4768: Test failed: Got unexpected VSInvocations count: 0. d3d10core.c:4771: Test failed: Got unexpected CInvocations count: 0. d3d10core.c:4772: Test failed: Got unexpected CPrimitives count: 0.
=== w1064_2qxl (64 bit report) ===
d3d10core: d3d10core.c:4651: Test failed: Got unexpected query result 0x0000000000000000.
=== debiant2 (32 bit WoW report) ===
d3d10core: d3d10core.c:13147: Test failed: Got {0x00000000, 0x80000000, 0x00000000, 0x00000000}, expected {0xffffffff, 0x00000000, 0x00000000, 0x00000000} at (0, 0), sub-resource 0. d3d10core.c:13147: Test failed: Got {0xffffffff, 0x00000001, 0x00000000, 0x00000000}, expected {0x00000000, 0x00000001, 0x00000000, 0x00000000} at (0, 0), sub-resource 0. d3d10core.c:13147: Test failed: Got {0x00000001, 0xffffffff, 0x00000000, 0x00000000}, expected {0x00000001, 0x00000000, 0x00000000, 0x00000000} at (0, 0), sub-resource 0.
Signed-off-by: Jan Sikorski jsikorski@codeweavers.com --- dlls/wined3d/device.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dlls/wined3d/device.c b/dlls/wined3d/device.c index 33b06b44c29..135fe31af9a 100644 --- a/dlls/wined3d/device.c +++ b/dlls/wined3d/device.c @@ -652,7 +652,7 @@ bool wined3d_device_vk_create_null_resources(struct wined3d_device_vk *device_vk vk_info = context_vk->vk_info;
usage = VK_BUFFER_USAGE_TRANSFER_DST_BIT | VK_BUFFER_USAGE_UNIFORM_TEXEL_BUFFER_BIT - | VK_BUFFER_USAGE_UNIFORM_BUFFER_BIT; + | VK_BUFFER_USAGE_UNIFORM_BUFFER_BIT | VK_BUFFER_USAGE_VERTEX_BUFFER_BIT; memory_type = VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT; if (!wined3d_context_vk_create_bo(context_vk, 16, usage, memory_type, &r->bo)) return false;
Since we're not using the nullDescriptor feature, we need to bind an actual buffer for all bindings that are accessed by the shader (VUID-vkCmdDraw-None-04007). If there's no buffer set for a stream, bind the null buffer instead.
Signed-off-by: Jan Sikorski jsikorski@codeweavers.com --- dlls/wined3d/context_vk.c | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-)
diff --git a/dlls/wined3d/context_vk.c b/dlls/wined3d/context_vk.c index 45133eabb69..e6dc5ea568b 100644 --- a/dlls/wined3d/context_vk.c +++ b/dlls/wined3d/context_vk.c @@ -2322,27 +2322,43 @@ static bool wined3d_context_vk_begin_render_pass(struct wined3d_context_vk *cont static void wined3d_context_vk_bind_vertex_buffers(struct wined3d_context_vk *context_vk, VkCommandBuffer vk_command_buffer, const struct wined3d_state *state, const struct wined3d_vk_info *vk_info) { + struct wined3d_device_vk *device_vk = wined3d_device_vk(context_vk->c.device); VkDeviceSize offsets[ARRAY_SIZE(state->streams)] = {0}; - VkBuffer buffers[ARRAY_SIZE(state->streams)]; + VkBuffer buffers[ARRAY_SIZE(state->streams)] = {0}; + struct wined3d_graphics_pipeline_key_vk *key; const struct wined3d_stream_state *stream; const VkDescriptorBufferInfo *buffer_info; + unsigned int i, first, count, binding; struct wined3d_buffer_vk *buffer_vk; struct wined3d_buffer *buffer; - unsigned int i, first, count;
- first = 0; - count = 0; - for (i = 0; i < ARRAY_SIZE(state->streams); ++i) + key = &context_vk->graphics.pipeline_key_vk; + for (i = 0; i < key->input_desc.vertexBindingDescriptionCount; ++i) { - stream = &state->streams[i]; - + binding = key->bindings[i].binding; + stream = &state->streams[binding]; if ((buffer = stream->buffer)) { buffer_vk = wined3d_buffer_vk(buffer); buffer_info = wined3d_buffer_vk_get_buffer_info(buffer_vk); wined3d_context_vk_reference_bo(context_vk, &buffer_vk->bo); - buffers[count] = buffer_info->buffer; - offsets[count] = buffer_info->offset + stream->offset; + } + else + { + buffer_info = &device_vk->null_resources_vk.buffer_info; + wined3d_context_vk_reference_bo(context_vk, &device_vk->null_resources_vk.bo); + } + + buffers[binding] = buffer_info->buffer; + offsets[binding] = buffer_info->offset + stream->offset; + } + + first = 0; + count = 0; + for (i = 0; i < ARRAY_SIZE(state->streams); ++i) + { + if (buffers[i]) + { ++count; continue; }
On Tue, 20 Apr 2021 at 15:08, Jan Sikorski jsikorski@codeweavers.com wrote:
Since we're not using the nullDescriptor feature, we need to bind an actual buffer for all bindings that are accessed by the shader (VUID-vkCmdDraw-None-04007). If there's no buffer set for a stream, bind the null buffer instead.
Signed-off-by: Jan Sikorski jsikorski@codeweavers.com
dlls/wined3d/context_vk.c | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-)
This seems fine, but one alternative I figured I'd mention would be to maintain a map of used streams in the wined3d_stream_info structure, analogous to the existing "use_map" we have for used elements/input attributes, and then using wined3d_bitmap_get_range() or wined3d_bit_scan() to iterate over them.
Signed-off-by: Jan Sikorski jsikorski@codeweavers.com --- dlls/wined3d/context_vk.c | 41 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+)
diff --git a/dlls/wined3d/context_vk.c b/dlls/wined3d/context_vk.c index e6dc5ea568b..47bbaed792a 100644 --- a/dlls/wined3d/context_vk.c +++ b/dlls/wined3d/context_vk.c @@ -2032,6 +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))) { + struct wined3d_shader_signature *signature = &state->shader[WINED3D_SHADER_TYPE_VERTEX]->input_signature; wined3d_stream_info_from_declaration(&stream_info, state, d3d_info); divisor_count = 0; for (i = 0, mask = 0, attribute_count = 0, binding_count = 0; i < ARRAY_SIZE(stream_info.elements); ++i) @@ -2071,6 +2072,46 @@ static bool wined3d_context_vk_update_graphics_pipeline_key(struct wined3d_conte } }
+ if (attribute_count < signature->element_count) + { + uint32_t null_binding; + for (i = 0; i < ARRAY_SIZE(state->streams); ++i) + { + if (!state->streams[i].buffer) + { + null_binding = i; + break; + } + } + + if (i == ARRAY_SIZE(state->streams)) + ERR("No streams left for a null buffer binding."); + else + { + VkVertexInputBindingDescription *b; + b = &key->bindings[binding_count++]; + b->binding = null_binding; + b->stride = 0; + b->inputRate = VK_VERTEX_INPUT_RATE_VERTEX; + + for (i = 0; i < signature->element_count; ++i) + { + struct wined3d_shader_signature_element *element = &signature->elements[i]; + uint32_t location = element->register_idx; + VkVertexInputAttributeDescription *a; + + if ((stream_info.use_map & (1 << location))) + continue; + + a = &key->attributes[attribute_count++]; + a->location = location; + a->binding = null_binding; + a->format = VK_FORMAT_R32G32B32A32_UINT; + a->offset = 0; + } + } + } + key->input_desc.pNext = NULL; key->input_desc.vertexBindingDescriptionCount = binding_count; key->input_desc.vertexAttributeDescriptionCount = attribute_count;
On Tue, 20 Apr 2021 at 15:08, Jan Sikorski jsikorski@codeweavers.com wrote:
@@ -2071,6 +2072,46 @@ static bool wined3d_context_vk_update_graphics_pipeline_key(struct wined3d_conte } }
if (attribute_count < signature->element_count){
I don't think that check is safe. Specifically, there may be multiple input signature elements for the same input register. (E.g., mapping v0.xy to "COLOUR0", and v0.zw to "COLOUR1".)
for (i = 0; i < signature->element_count; ++i){struct wined3d_shader_signature_element *element = &signature->elements[i];uint32_t location = element->register_idx;VkVertexInputAttributeDescription *a;if ((stream_info.use_map & (1 << location)))continue;a = &key->attributes[attribute_count++];a->location = location;a->binding = null_binding;a->format = VK_FORMAT_R32G32B32A32_UINT;a->offset = 0;}
Similarly, this may end up creating multiple attribute descriptions for the same input location if we don't keep track of which ones we've already processed here. We should probably skip elements with "sysval_semantic" set, although I wouldn't expect anything bad to happen if we don't.
Somewhat similar to my comment to patch 4/5, we could alternatively use the "shader->reg_maps.input_registers" bitmap by doing something like "uint32_t map = vs->reg_maps.input_registers & ~stream_info.use_map;", and then iterating over that map.
On 20 Apr 2021, at 19:25, Henri Verbeet hverbeet@gmail.com wrote:
On Tue, 20 Apr 2021 at 15:08, Jan Sikorski jsikorski@codeweavers.com wrote:
@@ -2071,6 +2072,46 @@ static bool wined3d_context_vk_update_graphics_pipeline_key(struct wined3d_conte } }
if (attribute_count < signature->element_count){I don't think that check is safe. Specifically, there may be multiple input signature elements for the same input register. (E.g., mapping v0.xy to "COLOUR0", and v0.zw to "COLOUR1".)
for (i = 0; i < signature->element_count; ++i){struct wined3d_shader_signature_element *element = &signature->elements[i];uint32_t location = element->register_idx;VkVertexInputAttributeDescription *a;if ((stream_info.use_map & (1 << location)))continue;a = &key->attributes[attribute_count++];a->location = location;a->binding = null_binding;a->format = VK_FORMAT_R32G32B32A32_UINT;a->offset = 0;}Similarly, this may end up creating multiple attribute descriptions for the same input location if we don't keep track of which ones we've already processed here. We should probably skip elements with "sysval_semantic" set, although I wouldn't expect anything bad to happen if we don't.
Somewhat similar to my comment to patch 4/5, we could alternatively use the "shader->reg_maps.input_registers" bitmap by doing something like "uint32_t map = vs->reg_maps.input_registers & ~stream_info.use_map;", and then iterating over that map.
Right, thanks for pointing that out. Another thing I noticed is that I should set the attribute format according to component type, or else the validation layer complains about mismatch. Luckily that’s straightforward and I don’t think I need to match number of components. There’s also an issue that wined3d_context_vk_bind_vertex_buffers() only gets called if STATE_STREAMSRC is dirty, while now a pipeline change might necessitate adding the null binding. Calling it each time the pipeline changes would be wasteful. It also smells fishy to invalidate STATE_STREAMSRC inside wined3d_context_vk_update_graphics_pipeline_key for that case, although I guess it would work. I can think of a few ways to resolve it, my favourite is to directly bind the null buffer next to vkCmdBindPipeline() if we need one (and perhaps if STATE_STREAMSRC is clean). We’d probably still need to do the thing in wined3d_context_vk_bind_vertex_buffers() in case someone binds a NULL vertex buffer. Although looks like now if STATE_STREAMSRC is dirty we always go to vkCmdBindPipeline(), so maybe not, but that's a bit brittle.
- Jan
On Wed, 21 Apr 2021 at 16:06, Jan Sikorski jsikorski@codeweavers.com wrote:
Calling it each time the pipeline changes would be wasteful. It also smells fishy to invalidate STATE_STREAMSRC inside wined3d_context_vk_update_graphics_pipeline_key for that case, although I guess it would work. I can think of a few ways to resolve it, my favourite is to directly bind the null buffer next to vkCmdBindPipeline() if we need one (and perhaps if STATE_STREAMSRC is clean). We’d probably still need to do the thing in wined3d_context_vk_bind_vertex_buffers() in case someone binds a NULL vertex buffer. Although looks like now if STATE_STREAMSRC is dirty we always go to vkCmdBindPipeline(), so maybe not, but that's a bit brittle.
It's perhaps unfortunate, but wined3d_stream_info_from_declaration() depends on information from the currently bound vertex buffers. Perhaps most notable for the Vulkan backend is the fact that we get attribute strides from there. It's probably possible to separate attribute and binding descriptions (although not necessarily trivial) but unless we also implement support for VK_EXT_extended_dynamic_state, it's still going to cause pipeline invalidation.
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=89054
Your paranoid android.
=== w2008s64 (32 bit report) ===
d3d11: d3d11.c:17243: Test failed: Got 0xffffffff, expected 0xff000000 at (0, 0, 0), sub-resource 0.
=== w1064 (32 bit report) ===
d3d11: d3d11.c:5811: Test failed: Got unexpected IAVertices count: 0. d3d11.c:5812: Test failed: Got unexpected IAPrimitives count: 0. d3d11.c:5813: Test failed: Got unexpected VSInvocations count: 0. d3d11.c:5816: Test failed: Got unexpected CInvocations count: 0. d3d11.c:5817: Test failed: Got unexpected CPrimitives count: 0.
=== debiant2 (32 bit French report) ===
d3d11: d3d11.c:9661: Test failed: d3d11.c:15096: Test marked todo: Got hr 0 for WRITE_DISCARD.
On 20 Apr 2021, at 15:20, Marvin testbot@winehq.org wrote:
=== w2008s64 (32 bit report) ===
d3d11: d3d11.c:17243: Test failed: Got 0xffffffff, expected 0xff000000 at (0, 0, 0), sub-resource 0.
Perhaps it doesn’t really makes sense to test that behavior after all, or only check it on wine, since it would be supported despite being invalid. I don’t know about any software that relies on it though.
On Tue, 20 Apr 2021 at 15:35, Jan Sikorski jsikorski@codeweavers.com wrote:
On 20 Apr 2021, at 15:20, Marvin testbot@winehq.org wrote:
=== w2008s64 (32 bit report) ===
d3d11: d3d11.c:17243: Test failed: Got 0xffffffff, expected 0xff000000 at (0, 0, 0), sub-resource 0.
Perhaps it doesn’t really makes sense to test that behavior after all, or only check it on wine, since it would be supported despite being invalid. I don’t know about any software that relies on it though.
I think the options are essentially: - Don't test that case at all, only include the original test. - Accept skipping the draw completely as a valid result here. - Accept skipping the draw completely as a broken() result here.
Those each have their advantages and disadvantages, and I don't have a clear preference, so just pick whichever one makes sense to you.
-
Henri Verbeet -
Jan Sikorski -
Marvin