Add a new sample for VK_EXT_device_fault #1447
Conversation
|
|
melihmete
changed the title
|
I see a bunch of compilation errors when I try to build this: |
|
@gary-sweet All fixed and pushed the changes. |
|
Can you move the shaders to a subdirectory called "glsl"? We support different shading languages and the goal is for all samples to have shaders in different shading languages (glsl, slang and/or hlsl). |
| const std::vector<VkPushConstantRange> ranges = { | ||
| vkb::initializers::push_constant_range(graphics ? VK_SHADER_STAGE_VERTEX_BIT : VK_SHADER_STAGE_COMPUTE_BIT, | ||
| graphics ? sizeof(PushVertex) : sizeof(PushCompute), 0), | ||
| }; |
There was a problem hiding this comment.
Why do you use a std::vector here, when you have just one element?
There was a problem hiding this comment.
Hi, this sample is built on top of buffer_device_address so that snippet is implemented in buffer_device_address extension sample.
| memory_barrier.dst_access_mask = VK_ACCESS_INDEX_READ_BIT; | ||
| memory_barrier.src_stage_mask = VK_PIPELINE_STAGE_TRANSFER_BIT; | ||
| memory_barrier.dst_stage_mask = VK_PIPELINE_STAGE_VERTEX_INPUT_BIT; | ||
| cmd->buffer_memory_barrier(*index_buffer, 0, VK_WHOLE_SIZE, memory_barrier); |
There was a problem hiding this comment.
Is this memory barrier needed at all, considering that you wait_idle a few lines later anyways?
There was a problem hiding this comment.
That section is also same with buffer_device_address extension sample.
| // So we incorrectly call wait_idle here, so we can get the GPU in error state, and we can query it for device_fault before an exception is thrown. | ||
| VkResult error = get_device().get_queue_by_present(0).wait_idle(); | ||
| check_device_fault(error); | ||
| ApiVulkanSample::submit_frame(); |
There was a problem hiding this comment.
Maybe, instead of this slightly hacky approach:
try
{
ApiVulkanSample::submit_frame();
}
catch (std::exception const &e)
{
vk::DeviceLostError const *device_lost_error = reinterpret_cast<vk::DeviceLostError const *>(&e);
if (device_lost_error)
{
check_device_fault(VK_ERROR_DEVICE_LOST);
}
}
There was a problem hiding this comment.
Great hack! Changed it and tested it.
| VkResult error = get_device().get_queue_by_present(0).wait_idle(); | ||
| check_device_fault(error); | ||
| ApiVulkanSample::submit_frame(); | ||
|
|
There was a problem hiding this comment.
The comments above are outdated with this change.
| if (device_lost_error) | ||
| { | ||
| check_device_fault(VK_ERROR_DEVICE_LOST); | ||
| } |
There was a problem hiding this comment.
Unfortunately, my suggested code snippet is not correct: the reinterpret_cast would always give you some non-nullptr.
I think, the best you could do here is something like
catch (std::runtime_error const &e)
{
if (strcmp("Detected Vulkan error: ERROR_DEVICE_LOST", e.what()) == 0)
{
check_device_fault(VK_ERROR_DEVICE_LOST);
}
std::rethrow_exception(std::current_exception());
}
There was a problem hiding this comment.
catch (std::exception const &e)
{
vk::DeviceLostError const *device_lost_error = reinterpret_cast<vk::DeviceLostError const *>(&e);
if (device_lost_error)
{
check_device_fault(VK_ERROR_DEVICE_LOST);
}
}
Actually previous snippet I've tested reports the error as VK_ERROR_DEVICE_LOST and I can retrieve the log I need. Second snippet you've suggested is not working for the purpose of this extension.
There was a problem hiding this comment.
Actually previous snippet I've tested reports the error as VK_ERROR_DEVICE_LOST and I can retrieve the log I need.
Sure. But it would identify every error as a device lost error.
What does not work with the strcmp-based approach? What does e.what() look like on your end?
There was a problem hiding this comment.
In the previous version logcat error itself VkResult VK_ERROR_DEVICE_LOST but with changed version, in the framework currently reported as "Detected Vulkan error: ERROR_INITIALIZATION_FAILED" and not logging VK_ERROR_DEVICE_LOST.
|
This does build ok for me now, and correctly reports as not-supported. I can't say any more than that though. |
|
@SaschaWillems Hello, I've just received errors regarding Quality Checks / Copyright Headers Check in the files I haven't modified. Should I fix them or is it related to CI needs a fixing? |
|
As per yesterdays call we'll ignore the CI failure caused by files you didn't touch. We'll try to make that CI step more robust/less error prone. |
5 participants
Description
When developing Vulkan applications, understanding and handling GPU errors is crucial. Currently, traditional graphics debugging methods do not give detailed information about GPU faults.
The VK_EXT_device_fault extension provides detailed information when ERROR_DEVICE_LOST occur, while the VK_EXT_device_address_binding_report extension helps monitor GPU memory usage by reporting
allocated and bound/unbound addresses in Vulkan application.
General Checklist:
Please ensure the following points are checked:
Note: The Samples CI runs a number of checks including:
If this PR contains framework changes:
batchcommand line argument to make sure all samples still work properlySample Checklist
If your PR contains a new or modified sample, these further checks must be carried out in addition to the General Checklist: