[PATCH vkd3d 10/11] tests: Mark driver bugs as todos in tests.
From: Józef Kucia <jkucia(a)codeweavers.com> Allows running all tests cleanly on Intel and Nvidia. It might be a controversial change, but it should make development slightly easier. Signed-off-by: Józef Kucia <jkucia(a)codeweavers.com> --- tests/d3d12.c | 37 ++++++++++++++++++++++--------- tests/d3d12_crosstest.h | 48 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 73 insertions(+), 12 deletions(-) diff --git a/tests/d3d12.c b/tests/d3d12.c index 7e74fbfec2ae..2e980a43615c 100644 --- a/tests/d3d12.c +++ b/tests/d3d12.c @@ -5569,6 +5569,7 @@ static void test_draw_uav_only(void) D3D12_GPU_DESCRIPTOR_HANDLE gpu_handle; D3D12_ROOT_PARAMETER root_parameter; struct test_context_desc desc; + struct resource_readback rb; struct test_context context; ID3D12CommandQueue *queue; ID3D12Resource *resource; @@ -5650,7 +5651,10 @@ static void test_draw_uav_only(void) transition_resource_state(command_list, resource, D3D12_RESOURCE_STATE_UNORDERED_ACCESS, D3D12_RESOURCE_STATE_COPY_SOURCE); - check_sub_resource_uint(resource, 0, queue, command_list, 500, 0); + get_texture_readback_with_command_list(resource, 0, &rb, queue, command_list); + todo_if(is_radv_device(context.device)) + check_readback_data_uint(&rb, NULL, 500, 0); + release_resource_readback(&rb); ID3D12DescriptorHeap_Release(cpu_descriptor_heap); ID3D12DescriptorHeap_Release(descriptor_heap); @@ -7865,6 +7869,7 @@ static void test_shader_instructions(void) struct ivec4 i; } output; bool skip_on_warp; + bool skip_on_mesa; } tests[] = { @@ -7904,25 +7909,26 @@ static void test_shader_instructions(void) {&ps_if, {{0.0f}}, {{1.0f, 0.0f, 0.0f, 1.0f}}}, {&ps_if, {{1.0f}}, {{0.0f, 1.0f, 0.0f, 1.0f}}}, + /* FIXME: Ordered/unordered comparisons are broken on Mesa. */ {&ps_if_return, {{0.0f, 0.0f, 0.0f, 0.0f}}, {{0.0f, 0.0f, 0.0f, 0.0f}}}, - {&ps_if_return, {{ NAN, 0.0f, 0.0f, 0.0f}}, {{1.0f, 0.0f, 0.0f, 0.0f}}}, + {&ps_if_return, {{ NAN, 0.0f, 0.0f, 0.0f}}, {{1.0f, 0.0f, 0.0f, 0.0f}}, false, true}, {&ps_if_return, {{3.0f, 0.0f, 0.0f, 0.0f}}, {{0.0f, 0.0f, 0.0f, 0.0f}}}, {&ps_if_return, {{4.0f, 0.0f, 0.0f, 0.0f}}, {{1.0f, 0.0f, 0.0f, 0.0f}}}, - {&ps_if_return, {{4.0f, NAN, 0.0f, 0.0f}}, {{1.0f, 1.0f, 1.0f, 0.0f}}}, + {&ps_if_return, {{4.0f, NAN, 0.0f, 0.0f}}, {{1.0f, 1.0f, 1.0f, 0.0f}}, false, true}, {&ps_if_return, {{4.0f, 3.0f, 0.0f, 0.0f}}, {{1.0f, 0.0f, 0.0f, 0.0f}}}, {&ps_if_return, {{4.0f, 4.0f, 0.0f, 0.0f}}, {{1.0f, 1.0f, 1.0f, 0.0f}}}, - {&ps_if_return, {{4.0f, 4.0f, NAN, 0.0f}}, {{1.0f, 1.0f, 1.0f, 0.0f}}}, + {&ps_if_return, {{4.0f, 4.0f, NAN, 0.0f}}, {{1.0f, 1.0f, 1.0f, 0.0f}}, false, true}, {&ps_if_return, {{4.0f, 4.0f, 3.0f, 0.0f}}, {{1.0f, 1.0f, 1.0f, 0.0f}}}, {&ps_if_return, {{4.0f, 4.0f, 4.0f, 0.0f}}, {{1.0f, 1.0f, 0.0f, 0.0f}}}, {&ps_if_return, {{4.0f, 4.0f, 5.0f, 0.0f}}, {{1.0f, 1.0f, 0.0f, 0.0f}}}, - {&ps_if_return, {{4.0f, 4.0f, 0.0f, NAN}}, {{1.0f, 1.0f, 1.0f, 1.0f}}}, + {&ps_if_return, {{4.0f, 4.0f, 0.0f, NAN}}, {{1.0f, 1.0f, 1.0f, 1.0f}}, false, true}, {&ps_if_return, {{4.0f, 4.0f, 0.0f, 1.0f}}, {{1.0f, 1.0f, 1.0f, 0.0f}}}, {&ps_if_return, {{4.0f, 4.0f, 0.0f, 2.0f}}, {{1.0f, 1.0f, 1.0f, 0.0f}}}, {&ps_if_return, {{4.0f, 4.0f, 0.0f, 3.0f}}, {{1.0f, 1.0f, 1.0f, 0.0f}}}, {&ps_if_return, {{4.0f, 4.0f, 0.0f, 4.0f}}, {{1.0f, 1.0f, 1.0f, 0.0f}}}, {&ps_if_return, {{4.0f, 4.0f, 0.0f, 5.0f}}, {{1.0f, 1.0f, 1.0f, 1.0f}}}, {&ps_if_return, {{5.0f, 4.0f, 0.0f, 5.0f}}, {{1.0f, 1.0f, 1.0f, 0.0f}}}, - {&ps_if_return, {{ NAN, NAN, NAN, NAN}}, {{1.0f, 1.0f, 1.0f, 1.0f}}}, + {&ps_if_return, {{ NAN, NAN, NAN, NAN}}, {{1.0f, 1.0f, 1.0f, 1.0f}}, false, true}, {&ps_nested_if, {{0.0f, 0.0f, 0.0f}}, {{0.0f, 0.0f, 0.0f, 1.0f}}}, {&ps_nested_if, {{0.0f, 0.0f, 1.0f}}, {{1.0f, 0.0f, 0.0f, 1.0f}}}, @@ -8628,6 +8634,12 @@ static void test_shader_instructions(void) continue; } + if (tests[i].skip_on_mesa && is_mesa_device(context.device)) + { + skip("Skipping shader '%s' test on Mesa.\n", tests[i].ps->name); + continue; + } + if (current_ps != tests[i].ps) { if (context.pipeline_state) @@ -16103,7 +16115,7 @@ static void test_depth_stencil_sampling(void) destroy_depth_stencil(&ds); continue; } - if (is_amd_device(device)) + if (is_amd_windows_device(device)) { skip("Reads from depth/stencil shader resource views return stale values on some AMD drivers.\n"); destroy_depth_stencil(&ds); @@ -18611,7 +18623,8 @@ static void test_atomic_instructions(void) unsigned int expected = test->expected_result[j]; todo_if(test->i.x < 0 - && (!strcmp(instructions[j], "atomic_imax") || !strcmp(instructions[j], "atomic_imin"))) + && (!strcmp(instructions[j], "atomic_imax") || !strcmp(instructions[j], "atomic_imin")) + && !is_nvidia_device(device)) ok(value == expected, "Test %u: Got %#x (%d), expected %#x (%d) for '%s' " "with inputs (%u, %u), (%d), %#x (%d).\n", i, value, value, expected, expected, instructions[j], @@ -18630,7 +18643,7 @@ static void test_atomic_instructions(void) unsigned int value = get_readback_uint(&rb, j, 0, 0); unsigned int expected = test->expected_result[j]; - todo_if(test->i.x < 0 && todo_instruction) + todo_if(test->i.x < 0 && todo_instruction && !is_nvidia_device(device)) ok(value == expected, "Test %u: Got %#x (%d), expected %#x (%d) for '%s' " "with inputs (%u, %u), (%d), %#x (%d).\n", i, value, value, expected, expected, imm_instructions[j], @@ -24566,6 +24579,7 @@ static void test_coverage(void) ID3D12Resource *ms_render_target; ID3D12PipelineState *pso_resolve; struct test_context_desc desc; + struct resource_readback rb; struct test_context context; ID3D12DescriptorHeap *heap; ID3D12CommandQueue *queue; @@ -24702,7 +24716,10 @@ static void test_coverage(void) D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE, D3D12_RESOURCE_STATE_RENDER_TARGET); transition_resource_state(command_list, context.render_target, D3D12_RESOURCE_STATE_RENDER_TARGET, D3D12_RESOURCE_STATE_COPY_SOURCE); - check_sub_resource_uint(context.render_target, 0, queue, command_list, tests[i].expected_result, 0); + get_texture_readback_with_command_list(context.render_target, 0, &rb, queue, command_list); + todo_if((tests[i].expected_result & 0x3) != tests[i].expected_result && is_intel_mesa_device(context.device)) + check_readback_data_uint(&rb, NULL, tests[i].expected_result, 0); + release_resource_readback(&rb); reset_command_list(command_list, context.allocator); transition_resource_state(command_list, context.render_target, diff --git a/tests/d3d12_crosstest.h b/tests/d3d12_crosstest.h index d0019dc85042..e352cce6b324 100644 --- a/tests/d3d12_crosstest.h +++ b/tests/d3d12_crosstest.h @@ -296,7 +296,7 @@ static void init_adapter_info(void) IDXGIAdapter_Release(dxgi_adapter); } -static inline bool is_amd_device(ID3D12Device *device) +static inline bool is_amd_windows_device(ID3D12Device *device) { DXGI_ADAPTER_DESC desc = {0}; IDXGIFactory4 *factory; @@ -324,8 +324,29 @@ static inline bool is_amd_device(ID3D12Device *device) return desc.VendorId == 0x1002; } + +static inline bool is_intel_mesa_device(ID3D12Device *device) +{ + return false; +} + +static inline bool is_mesa_device(ID3D12Device *device) +{ + return false; +} + +static inline bool is_nvidia_device(ID3D12Device *device) +{ + return false; +} + +static inline bool is_radv_device(ID3D12Device *device) +{ + return false; +} #else static bool have_VK_KHR_driver_properties; +static VkDriverIdKHR vk_driver_id; static HRESULT create_vkd3d_instance(struct vkd3d_instance **instance) { @@ -484,16 +505,39 @@ static void init_adapter_info(void) trace("Driver name: %s, driver info: %s.\n", driver_properties.driverName, driver_properties.driverInfo); + vk_driver_id = driver_properties.driverID; + ID3D12Device_Release(device); done: vkd3d_instance_decref(instance); } -static inline bool is_amd_device(ID3D12Device *device) +static inline bool is_amd_windows_device(ID3D12Device *device) { return false; } + +static inline bool is_intel_mesa_device(ID3D12Device *device) +{ + return vk_driver_id == VK_DRIVER_ID_INTEL_OPEN_SOURCE_MESA_KHR; +} + +static inline bool is_mesa_device(ID3D12Device *device) +{ + return vk_driver_id == VK_DRIVER_ID_MESA_RADV_KHR + || vk_driver_id == VK_DRIVER_ID_INTEL_OPEN_SOURCE_MESA_KHR; +} + +static inline bool is_nvidia_device(ID3D12Device *device) +{ + return vk_driver_id == VK_DRIVER_ID_NVIDIA_PROPRIETARY_KHR; +} + +static inline bool is_radv_device(ID3D12Device *device) +{ + return vk_driver_id == VK_DRIVER_ID_MESA_RADV_KHR; +} #endif static ID3D12Device *create_device(void) -- 2.19.2
On Thu, 21 Feb 2019 at 15:10, Józef Kucia <joseph.kucia(a)gmail.com> wrote:
Allows running all tests cleanly on Intel and Nvidia. It might be a controversial change, but it should make development slightly easier.
I don't mind the concept (although in terms of terminology, I think I'd prefer something along the lines of "broken_if()" to distinguish it from actual vkd3d todo's), but doesn't this mean these will start failing if those bugs are ever fixed?
On Thu, Feb 21, 2019 at 8:49 PM Henri Verbeet <hverbeet(a)gmail.com> wrote:
On Thu, 21 Feb 2019 at 15:10, Józef Kucia <joseph.kucia(a)gmail.com> wrote:
Allows running all tests cleanly on Intel and Nvidia. It might be a controversial change, but it should make development slightly easier.
I don't mind the concept (although in terms of terminology, I think I'd prefer something along the lines of "broken_if()" to distinguish it from actual vkd3d todo's), but doesn't this mean these will start failing if those bugs are ever fixed?
Yes, these will start failing when driver bugs are fixed. We could guard these conditions with an env var or a command line option. Currently, it should be possible to disable those conditions by setting VKD3D_DISABLE_EXTENSIONS=VK_KHR_driver_properties. Other option is to make them special and count them as broken when a test fails, and treat them as success when a test passes. I agree that a separate name to mark driver-specific test failures could be a good idea. Unfortunately, broken() is taken. It's used for conditions which are accepted only on Windows. I'm not sure how to name them. A few random ideas: broken_driver_if(), driver_bug_if().
On Fri, 22 Feb 2019 at 02:51, Józef Kucia <joseph.kucia(a)gmail.com> wrote:
On Thu, Feb 21, 2019 at 8:49 PM Henri Verbeet <hverbeet(a)gmail.com> wrote:
On Thu, 21 Feb 2019 at 15:10, Józef Kucia <joseph.kucia(a)gmail.com> wrote:
Allows running all tests cleanly on Intel and Nvidia. It might be a controversial change, but it should make development slightly easier.
I don't mind the concept (although in terms of terminology, I think I'd prefer something along the lines of "broken_if()" to distinguish it from actual vkd3d todo's), but doesn't this mean these will start failing if those bugs are ever fixed?
Yes, these will start failing when driver bugs are fixed. We could guard these conditions with an env var or a command line option. Currently, it should be possible to disable those conditions by setting VKD3D_DISABLE_EXTENSIONS=VK_KHR_driver_properties. Other option is to make them special and count them as broken when a test fails, and treat them as success when a test passes.
I think we essentially want to just ignore the result if the condition is true, yes.
I agree that a separate name to mark driver-specific test failures could be a good idea. Unfortunately, broken() is taken. It's used for conditions which are accepted only on Windows. I'm not sure how to name them. A few random ideas: broken_driver_if(), driver_bug_if().
I don't have any great naming suggestions either.
participants (2)
-
Henri Verbeet -
Józef Kucia