Skip to content
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

Enable GPU-based validation for Vulkan #5046

Merged

Conversation

ErichDonGubler
Copy link
Member

@ErichDonGubler ErichDonGubler commented Jan 11, 2024

Connections
Link to the issues addressed by this PR, or dependent PRs in other repositories

Resolves #1709.

Description
Describe what problem this is solving, and how it's solved.

See #1709!

Testing
Explain how this change is tested.

Checklist

  • Run cargo fmt.
  • Run cargo clippy. If applicable, add:
    • --target wasm32-unknown-unknown
    • --target wasm32-unknown-emscripten
  • Run cargo xtask test to run tests.
  • Add change to CHANGELOG.md. See simple instructions inside file.

@ErichDonGubler ErichDonGubler self-assigned this Jan 11, 2024
@ErichDonGubler ErichDonGubler added the api: dx12 Issues with DX12 or DXGI label Jan 11, 2024
@ErichDonGubler ErichDonGubler changed the title WIP: feat(dx12): enable GPU-based validation for DX12 backend WIP: enable GPU-based validation for DX12 and Vulkan Jan 11, 2024
@ErichDonGubler ErichDonGubler force-pushed the gpu-based-validation branch 2 times, most recently from 695c7a8 to 7f66322 Compare January 11, 2024 19:39
@ErichDonGubler

This comment was marked as resolved.

@ErichDonGubler ErichDonGubler marked this pull request as ready for review January 11, 2024 20:58
@ErichDonGubler ErichDonGubler requested a review from a team as a code owner January 11, 2024 20:58
@ErichDonGubler ErichDonGubler changed the title WIP: enable GPU-based validation for DX12 and Vulkan Enable GPU-based validation for DX12 and Vulkan Jan 11, 2024
@cwfitzgerald

This comment was marked as resolved.

wgpu-hal/src/vulkan/instance.rs Outdated Show resolved Hide resolved
wgpu-types/src/lib.rs Outdated Show resolved Hide resolved
wgpu-hal/src/vulkan/instance.rs Outdated Show resolved Hide resolved
@ErichDonGubler ErichDonGubler force-pushed the gpu-based-validation branch 3 times, most recently from bfc0570 to 2dec66f Compare January 26, 2024 19:12
@ErichDonGubler ErichDonGubler changed the title Enable GPU-based validation for DX12 and Vulkan Enable GPU-based validation for Vulkan Jan 26, 2024
@ErichDonGubler

This comment was marked as resolved.

@ErichDonGubler

This comment was marked as resolved.

@ErichDonGubler ErichDonGubler force-pushed the gpu-based-validation branch 2 times, most recently from 703dd13 to 898c96f Compare January 26, 2024 19:32
@exrook

This comment was marked as resolved.

@cwfitzgerald

This comment was marked as resolved.

@ErichDonGubler

This comment was marked as resolved.

@ErichDonGubler ErichDonGubler added area: infrastructure Testing, building, coordinating issues area: ecosystem Help the connected projects grow and prosper api: vulkan Issues with Vulkan and removed api: dx12 Issues with DX12 or DXGI labels Feb 9, 2024
@ErichDonGubler ErichDonGubler dismissed cwfitzgerald’s stale review February 10, 2024 03:16

Concerns have been addressed.

@ErichDonGubler ErichDonGubler requested a review from a team February 10, 2024 03:16
@ErichDonGubler

This comment was marked as resolved.

This will be used shortly for checking if we should proceed with
enabling GPU-based validation.
…elper

This will be used shortly for checking if we should proceed with
enabling GPU-based validation.
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
@ErichDonGubler ErichDonGubler enabled auto-merge (rebase) February 12, 2024 15:06
@ErichDonGubler ErichDonGubler merged commit 95c026b into gfx-rs:trunk Feb 12, 2024
27 checks passed
@ErichDonGubler ErichDonGubler deleted the gpu-based-validation branch February 13, 2024 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: vulkan Issues with Vulkan area: ecosystem Help the connected projects grow and prosper area: infrastructure Testing, building, coordinating issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable GPU validation on DX12 and Vulkan
4 participants