-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Fix synchronization validation error in water
example on Vulkan
#5231
Comments
If [`VK_LAYER_KHRONOS_validation`] is present, and it supports [`VK_EXT_validation_features`], we can configure it with another instance creation info. element of type [`VkValidationFeaturesEXT`] to enable GPU-based validation. Wire `InstanceFlags::GPU_BASED_VALIDATION` to do this in the Vulkan backend. It's even already finding issues in our `examples`! But…we'd like to handle those later. So, I've broken that out to [a separate issue][examples-fixups]. It is apparent from this and the [DX12 implementation of GPU-based validation] that we will need to communicate clearly to users that `InstanceFlags::GPU_BASED_VALIDATION` implies `InstanceFlags::VALIDATION`. Not all backends enforce this yet; I have [split out this work][follow-up for flag implication]. Note that `VK_EXT_validation_features` has been deprecated in favor of the more general layer configuration mechanism offered by [`VK_EXT_layer_settings`]. [DX12 implementation of GPU-based validation]: gfx-rs#5146 [`VK_EXT_layer_settings`]: https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VK_EXT_layer_settings.html [`VK_EXT_validation_features`]: https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VK_EXT_validation_features.html [`VK_LAYER_KHRONOS_validation`]:https://vulkan.lunarg.com/doc/sdk/1.3.275.0/linux/khronos_validation_layer.html [`VkValidationFeaturesEXT`]: https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VkValidationFeaturesEXT.html [examples-fixups]: gfx-rs#5231 [follow-up for flag implication]: gfx-rs#5232
WIP: feat(vulkan): enable GPU-based validation for Vulkan backend If [`VK_LAYER_KHRONOS_validation`] is present, and it supports [`VK_EXT_validation_features`], we can configure it with another instance creation info. element of type [`VkValidationFeaturesEXT`] to enable GPU-based validation. Wire `InstanceFlags::GPU_BASED_VALIDATION` to do this in the Vulkan backend. It's even already finding issues in our `examples` and other tests! But…we'd like to handle those later, since this is so important for users. So, I've broken that out to separate issues. The instances we're aware of: * `water` is running into sync. validation issues: see gfx-rs#5231 * `wgpu_test::shader::struct_layout::uniform_input` is failing to instrument shaders now; see TODO It is apparent from this and the [DX12 implementation of GPU-based validation] that we will need to communicate clearly to users that `InstanceFlags::GPU_BASED_VALIDATION` implies `InstanceFlags::VALIDATION`. Not all backends enforce this yet; I have [split out this work][follow-up for flag implication]. Note that `VK_EXT_validation_features` has been deprecated in favor of the more general layer configuration mechanism offered by [`VK_EXT_layer_settings`]. [DX12 implementation of GPU-based validation]: gfx-rs#5146 [`VK_EXT_layer_settings`]: https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VK_EXT_layer_settings.html [`VK_EXT_validation_features`]: https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VK_EXT_validation_features.html [`VK_LAYER_KHRONOS_validation`]:https://vulkan.lunarg.com/doc/sdk/1.3.275.0/linux/khronos_validation_layer.html [`VkValidationFeaturesEXT`]: https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VkValidationFeaturesEXT.html [follow-up for flag implication]: gfx-rs#5232
feat(vulkan): enable GPU-based validation for Vulkan backend If [`VK_LAYER_KHRONOS_validation`] is present, and it supports [`VK_EXT_validation_features`], we can configure it with another instance creation info. element of type [`VkValidationFeaturesEXT`] to enable GPU-based validation. Wire `InstanceFlags::GPU_BASED_VALIDATION` to do this in the Vulkan backend. It's even already finding issues in our `examples` and other tests! But…we'd like to handle those later, since this is so important for users. So, I've broken that out to separate issues. The instances we're aware of: * `water` is running into sync. validation issues: see <gfx-rs#5231> * `wgpu_test::shader::struct_layout::uniform_input` is failing to instrument shaders now; see <gfx-rs#5245> It is apparent from this and the [DX12 implementation of GPU-based validation] that we will need to communicate clearly to users that `InstanceFlags::GPU_BASED_VALIDATION` implies `InstanceFlags::VALIDATION`. Not all backends enforce this yet; I have [split out this work][follow-up for flag implication]. Note that `VK_EXT_validation_features` has been deprecated in favor of the more general layer configuration mechanism offered by [`VK_EXT_layer_settings`]. [DX12 implementation of GPU-based validation]: gfx-rs#5146 [`VK_EXT_layer_settings`]: https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VK_EXT_layer_settings.html [`VK_EXT_validation_features`]: https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VK_EXT_validation_features.html [`VK_LAYER_KHRONOS_validation`]:https://vulkan.lunarg.com/doc/sdk/1.3.275.0/linux/khronos_validation_layer.html [`VkValidationFeaturesEXT`]: https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VkValidationFeaturesEXT.html [follow-up for flag implication]: gfx-rs#5232
If [`VK_LAYER_KHRONOS_validation`] is present, and it supports [`VK_EXT_validation_features`], we can configure it with another instance creation info. element of type [`VkValidationFeaturesEXT`] to enable GPU-based validation. Wire `InstanceFlags::GPU_BASED_VALIDATION` to do this in the Vulkan backend. It's even already finding issues in our `examples` and other tests! But…we'd like to handle those later, since this is so important for users. So, I've broken that out to separate issues. The instances we're aware of: * `water` is running into sync. validation issues: see <gfx-rs#5231> * `wgpu_test::shader::struct_layout::uniform_input` is failing to instrument shaders now; see <gfx-rs#5245> It is apparent from this and the [DX12 implementation of GPU-based validation] that we will need to communicate clearly to users that `InstanceFlags::GPU_BASED_VALIDATION` implies `InstanceFlags::VALIDATION`. Not all backends enforce this yet; I have [split out this work][follow-up for flag implication]. Note that `VK_EXT_validation_features` has been deprecated in favor of the more general layer configuration mechanism offered by [`VK_EXT_layer_settings`]. [DX12 implementation of GPU-based validation]: gfx-rs#5146 [`VK_EXT_layer_settings`]: https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VK_EXT_layer_settings.html [`VK_EXT_validation_features`]: https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VK_EXT_validation_features.html [`VK_LAYER_KHRONOS_validation`]:https://vulkan.lunarg.com/doc/sdk/1.3.275.0/linux/khronos_validation_layer.html [`VkValidationFeaturesEXT`]: https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VkValidationFeaturesEXT.html [follow-up for flag implication]: gfx-rs#5232
If [`VK_LAYER_KHRONOS_validation`] is present, and it supports [`VK_EXT_validation_features`], we can configure it with another instance creation info. element of type [`VkValidationFeaturesEXT`] to enable GPU-based validation. Wire `InstanceFlags::GPU_BASED_VALIDATION` to do this in the Vulkan backend. It's even already finding issues in our `examples` and other tests! But…we'd like to handle those later, since this is so important for users. So, I've broken that out to separate issues. The instances we're aware of: * `water` is running into sync. validation issues: see <#5231> * `wgpu_test::shader::struct_layout::uniform_input` is failing to instrument shaders now; see <#5245> It is apparent from this and the [DX12 implementation of GPU-based validation] that we will need to communicate clearly to users that `InstanceFlags::GPU_BASED_VALIDATION` implies `InstanceFlags::VALIDATION`. Not all backends enforce this yet; I have [split out this work][follow-up for flag implication]. Note that `VK_EXT_validation_features` has been deprecated in favor of the more general layer configuration mechanism offered by [`VK_EXT_layer_settings`]. [DX12 implementation of GPU-based validation]: #5146 [`VK_EXT_layer_settings`]: https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VK_EXT_layer_settings.html [`VK_EXT_validation_features`]: https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VK_EXT_validation_features.html [`VK_LAYER_KHRONOS_validation`]:https://vulkan.lunarg.com/doc/sdk/1.3.275.0/linux/khronos_validation_layer.html [`VkValidationFeaturesEXT`]: https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VkValidationFeaturesEXT.html [follow-up for flag implication]: #5232
In Fedora's
|
Expand the `water` test's expected error message substring to cover new validation errors reported by GPU-based validation starting in Fedora's vulkan-validation-layers-1.3.275.0-1.fc39.x86_64: ``` [2024-04-09T21:02:01Z ERROR wgpu_test::expectations] Validation Error: Validation Error: [ SYNC-HAZARD-WRITE-AFTER-WRITE ] Object 0: handle = 0x7f2fb53d44e0, type = VK_OBJECT_TYPE_QUEUE; | MessageID = 0x5c0ec5d6 | vkQueueSubmit(): Hazard WRITE_AFTER_WRITE for entry 7, VkCommandBuffer 0x7f2fb6fd5b40[], Submitted access info (submitted_usage: SYNC_LATE_FRAGMENT_TESTS_DEPTH_STENCIL_ATTACHMENT_WRITE, command: vkCmdEndRenderPass, seq_no: 3, renderpass: VkRenderPass 0x2d000000002d[], reset_no: 1). Access info (prior_usage: SYNC_IMAGE_LAYOUT_TRANSITION, write_barriers: SYNC_VERTEX_SHADER_SHADER_SAMPLED_READ|SYNC_VERTEX_SHADER_SHADER_STORAGE_READ|SYNC_VERTEX_SHADER_UNIFORM_READ|SYNC_FRAGMENT_SHADER_DEPTH_STENCIL_ATTACHMENT_READ|SYNC_FRAGMENT_SHADER_SHADER_SAMPLED_READ|SYNC_FRAGMENT_SHADER_SHADER_STORAGE_READ|SYNC_FRAGMENT_SHADER_UNIFORM_READ|SYNC_EARLY_FRAGMENT_TESTS_DEPTH_STENCIL_ATTACHMENT_READ|SYNC_LATE_FRAGMENT_TESTS_DEPTH_STENCIL_ATTACHMENT_READ|SYNC_COMPUTE_SHADER_SHADER_SAMPLED_READ|SYNC_COMPUTE_SHADER_SHADER_STORAGE_READ|SYNC_COMPUTE_SHADER_UNIFORM_READ, queue: VkQueue 0x7f2fb53d44e0[], submit: 0, batch: 0, batch_tag: 28, command: vkCmdPipelineBarrier, command_buffer: VkCommandBuffer 0x7f2fb6fd43c0[Main Command Encoder], seq_no: 1, VkImage 0x1c000000001c[Reflection Render Texture], VkImage 0x1f000000001f[Depth Buffer], reset_no: 1). ``` See also gfx-rs#5231.
Expand the `water` test's expected error message substring to cover new validation errors reported by GPU-based validation starting in Fedora's vulkan-validation-layers-1.3.275.0-1.fc39.x86_64: ``` [2024-04-09T21:02:01Z ERROR wgpu_test::expectations] Validation Error: Validation Error: [ SYNC-HAZARD-WRITE-AFTER-WRITE ] Object 0: handle = 0x7f2fb53d44e0, type = VK_OBJECT_TYPE_QUEUE; | MessageID = 0x5c0ec5d6 | vkQueueSubmit(): Hazard WRITE_AFTER_WRITE for entry 7, VkCommandBuffer 0x7f2fb6fd5b40[], Submitted access info (submitted_usage: SYNC_LATE_FRAGMENT_TESTS_DEPTH_STENCIL_ATTACHMENT_WRITE, command: vkCmdEndRenderPass, seq_no: 3, renderpass: VkRenderPass 0x2d000000002d[], reset_no: 1). Access info (prior_usage: SYNC_IMAGE_LAYOUT_TRANSITION, write_barriers: SYNC_VERTEX_SHADER_SHADER_SAMPLED_READ|SYNC_VERTEX_SHADER_SHADER_STORAGE_READ|SYNC_VERTEX_SHADER_UNIFORM_READ|SYNC_FRAGMENT_SHADER_DEPTH_STENCIL_ATTACHMENT_READ|SYNC_FRAGMENT_SHADER_SHADER_SAMPLED_READ|SYNC_FRAGMENT_SHADER_SHADER_STORAGE_READ|SYNC_FRAGMENT_SHADER_UNIFORM_READ|SYNC_EARLY_FRAGMENT_TESTS_DEPTH_STENCIL_ATTACHMENT_READ|SYNC_LATE_FRAGMENT_TESTS_DEPTH_STENCIL_ATTACHMENT_READ|SYNC_COMPUTE_SHADER_SHADER_SAMPLED_READ|SYNC_COMPUTE_SHADER_SHADER_STORAGE_READ|SYNC_COMPUTE_SHADER_UNIFORM_READ, queue: VkQueue 0x7f2fb53d44e0[], submit: 0, batch: 0, batch_tag: 28, command: vkCmdPipelineBarrier, command_buffer: VkCommandBuffer 0x7f2fb6fd43c0[Main Command Encoder], seq_no: 1, VkImage 0x1c000000001c[Reflection Render Texture], VkImage 0x1f000000001f[Depth Buffer], reset_no: 1). ``` See also gfx-rs#5231.
Expand the `water` test's expected error message substring to cover new validation errors reported by GPU-based validation starting in Fedora's vulkan-validation-layers-1.3.275.0-1.fc39.x86_64: ``` [2024-04-09T21:02:01Z ERROR wgpu_test::expectations] Validation Error: Validation Error: [ SYNC-HAZARD-WRITE-AFTER-WRITE ] Object 0: handle = 0x7f2fb53d44e0, type = VK_OBJECT_TYPE_QUEUE; | MessageID = 0x5c0ec5d6 | vkQueueSubmit(): Hazard WRITE_AFTER_WRITE for entry 7, VkCommandBuffer 0x7f2fb6fd5b40[], Submitted access info (submitted_usage: SYNC_LATE_FRAGMENT_TESTS_DEPTH_STENCIL_ATTACHMENT_WRITE, command: vkCmdEndRenderPass, seq_no: 3, renderpass: VkRenderPass 0x2d000000002d[], reset_no: 1). Access info (prior_usage: SYNC_IMAGE_LAYOUT_TRANSITION, write_barriers: SYNC_VERTEX_SHADER_SHADER_SAMPLED_READ|SYNC_VERTEX_SHADER_SHADER_STORAGE_READ|SYNC_VERTEX_SHADER_UNIFORM_READ|SYNC_FRAGMENT_SHADER_DEPTH_STENCIL_ATTACHMENT_READ|SYNC_FRAGMENT_SHADER_SHADER_SAMPLED_READ|SYNC_FRAGMENT_SHADER_SHADER_STORAGE_READ|SYNC_FRAGMENT_SHADER_UNIFORM_READ|SYNC_EARLY_FRAGMENT_TESTS_DEPTH_STENCIL_ATTACHMENT_READ|SYNC_LATE_FRAGMENT_TESTS_DEPTH_STENCIL_ATTACHMENT_READ|SYNC_COMPUTE_SHADER_SHADER_SAMPLED_READ|SYNC_COMPUTE_SHADER_SHADER_STORAGE_READ|SYNC_COMPUTE_SHADER_UNIFORM_READ, queue: VkQueue 0x7f2fb53d44e0[], submit: 0, batch: 0, batch_tag: 28, command: vkCmdPipelineBarrier, command_buffer: VkCommandBuffer 0x7f2fb6fd43c0[Main Command Encoder], seq_no: 1, VkImage 0x1c000000001c[Reflection Render Texture], VkImage 0x1f000000001f[Depth Buffer], reset_no: 1). ``` See also #5231.
water
example on Vulkan water
example on Vulkan
as i known , wgpu/examples/src/water/mod.rs Lines 207 to 209 in cc0ee7b
so why validation layer still report WAR ? |
I can reliably reproduce this error every single frame on Windows 11, NVIDIA RTX 2070 Mobile. The issue is fixed by either:
See https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VkAttachmentStoreOp.html. Any other VkAttachmentStoreOp than VK_ATTACHMENT_STORE_OP_NONE requires VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT . Seems to have been discussed here KhronosGroup/Vulkan-ValidationLayers#2349. The current situations is this I believe: KhronosGroup/Vulkan-ValidationLayers#2349 (comment) |
I did some researching, and I think we need to replace the following line wgpu/wgpu-hal/src/vulkan/conv.rs Line 462 in 0fc0b35
|
You need to use Store not Dont_care, but yes we need to either use None or suppress the error. |
@JMS55 can you try to fix this? |
No sorry. The dance to enable vulkan extensions within wgpu is hard to understand, I don't really want to go through that again. I had too much trouble figuring it out when doing u64 atomics and eventually just tried stuff until I got it running, but never really figured out how it's supposed to be setup. |
Alright. I was going to silence this validation error quickly, but this is harder to match on than I expected. If anyone is interested in adding this exception you'd need to silence the following, without silencing too many other validation errors.
|
GPU-based validation was implemented for the Vulkan backend in #5046 (🎉). With it, we began to run into a validation error for the
water
example:Because of the value of validation, that PR was merged without a complete fix to
water
, changing the outcome we expect of CI testing to failure. We should fix it, and adjust the expected test outcome of that example back to fully passing.Additional historical context: #5046 (comment)
The text was updated successfully, but these errors were encountered: