Skip to content

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 backend: 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.

@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 backend: vulkan Issues with Vulkan and removed backend: 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
@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

area: ecosystem Help the connected projects grow and prosper area: infrastructure Testing, building, coordinating issues backend: vulkan Issues with Vulkan

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enable GPU validation on DX12 and Vulkan

4 participants