-
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
Enable GPU-based validation for Vulkan #5046
Merged
ErichDonGubler
merged 4 commits into
gfx-rs:trunk
from
erichdongubler-mozilla:gpu-based-validation
Feb 12, 2024
Merged
Enable GPU-based validation for Vulkan #5046
ErichDonGubler
merged 4 commits into
gfx-rs:trunk
from
erichdongubler-mozilla:gpu-based-validation
Feb 12, 2024
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
695c7a8
to
7f66322
Compare
This comment was marked as resolved.
This comment was marked as resolved.
1d63f0e
to
95dd5b2
Compare
This comment was marked as resolved.
This comment was marked as resolved.
cwfitzgerald
previously requested changes
Jan 12, 2024
3 tasks
bfc0570
to
2dec66f
Compare
4 tasks
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
703dd13
to
898c96f
Compare
2 tasks
ErichDonGubler
commented
Jan 26, 2024
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
898c96f
to
0bac57d
Compare
ErichDonGubler
commented
Feb 1, 2024
0bac57d
to
db05c18
Compare
4bc6b3e
to
57d20d9
Compare
57d20d9
to
575f184
Compare
6 tasks
617c9e2
to
fcaad33
Compare
This comment was marked as resolved.
This comment was marked as resolved.
fcaad33
to
c3df032
Compare
Concerns have been addressed.
This comment was marked as resolved.
This comment was marked as resolved.
b595c1d
to
dded748
Compare
teoxoy
approved these changes
Feb 12, 2024
dded748
to
8f7e720
Compare
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
8f7e720
to
3a5f583
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
area: ecosystem
Help the connected projects grow and prosper
area: infrastructure
Testing, building, coordinating issues
backend: vulkan
Issues with Vulkan
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Connections
Link to the issues addressed by this PR, or dependent PRs in other repositories
Resolves #1709.
Conflicts with Debug printf in shaders #4297 without further API design (see this conversation).Description
Describe what problem this is solving, and how it's solved.
See #1709!
Testing
Explain how this change is tested.
Checklist
cargo fmt
.cargo clippy
. If applicable, add:--target wasm32-unknown-unknown
--target wasm32-unknown-emscripten
cargo xtask test
to run tests.CHANGELOG.md
. See simple instructions inside file.