Skip to content

Commit

Permalink
WIP: feat(valid): enforce GPU_BASED_VALIDATIONVALIDATION
Browse files Browse the repository at this point in the history
Enforce in all back ends that `InstanceFlags::GPU_BASED_VALIDATION`
implies `InstanceFlags::GPU_BASED_VALIDATION`. ATOW, the GLES is the
only remaining implemented offender.

TODO: I want at least the "basic" fix before jumping to a new
convenience API.
  • Loading branch information
ErichDonGubler committed Feb 10, 2024
1 parent dded748 commit 29e4b2c
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 24 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ Bottom level categories:
- Eager release of GPU resources comes from device.trackers. By @bradwerth in [#5075](https://github.com/gfx-rs/wgpu/pull/5075)
- `wgpu-types`'s `trace` and `replay` features have been replaced by the `serde` feature. By @KirmesBude in [#5149](https://github.com/gfx-rs/wgpu/pull/5149)
- `wgpu-core`'s `serial-pass` feature has been removed. Use `serde` instead. By @KirmesBude in [#5149](https://github.com/gfx-rs/wgpu/pull/5149)
- Added `InstanceFlags::GPU_BASED_VALIDATION`, which enables GPU-based validation for shaders. This is currently only supported on the DX12 and Vulkan backends; other platforms ignore this flag, for now.
- Added `InstanceFlags::GPU_BASED_VALIDATION`, which enables GPU-based validation for shaders. This is currently only supported on the DX12 and Vulkan backends.
- When set, this flag implies `InstanceFlags::VALIDATION`.
- This has been added to the set of flags set by `InstanceFlags::debugging` and `InstanceFlags::from_build_config`. If you notice your graphics workloads running more slowly, this may be the culprit.
- As with other instance flags, this flag can be changed in calls to `InstanceFlags::with_env` with the new `WGPU_GPU_BASED_VALIDATION` environment variable.
Expand Down
19 changes: 19 additions & 0 deletions wgpu-hal/src/auxil/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,3 +136,22 @@ pub(crate) fn cstr_from_bytes_until_nul(bytes: &[std::os::raw::c_char]) -> Optio
None
}
}

pub(crate) struct RequestedValidation {
pub gpu_based_validation: bool,
}

impl RequestedValidation {
pub(crate) fn from_flags(instance_flags: wgt::InstanceFlags) -> Option<Self> {
let any_validation_requested = instance_flags
.intersects(wgt::InstanceFlags::VALIDATION | wgt::InstanceFlags::GPU_BASED_VALIDATION);
if any_validation_requested {
Some(Self {
gpu_based_validation: instance_flags
.intersects(wgt::InstanceFlags::GPU_BASED_VALIDATION),
})
} else {
None
}
}
}
20 changes: 8 additions & 12 deletions wgpu-hal/src/dx12/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use parking_lot::RwLock;
use winapi::shared::{dxgi1_5, minwindef};

use super::SurfaceTarget;
use crate::auxil::{self, dxgi::result::HResult as _};
use crate::auxil::{self, dxgi::result::HResult as _, RequestedValidation};
use std::{mem, sync::Arc};

impl Drop for super::Instance {
Expand All @@ -20,21 +20,17 @@ impl crate::Instance<super::Api> for super::Instance {
crate::InstanceError::with_source(String::from("failed to load d3d12.dll"), e)
})?;

if desc
.flags
.intersects(wgt::InstanceFlags::VALIDATION | wgt::InstanceFlags::GPU_BASED_VALIDATION)
{
if let Some(requested_validation) = RequestedValidation::from_flags(desc.flags) {
let RequestedValidation {
gpu_based_validation,
} = requested_validation;

// Enable debug layer
match lib_main.get_debug_interface() {
Ok(pair) => match pair.into_result() {
Ok(debug_controller) => {
if desc.flags.intersects(wgt::InstanceFlags::VALIDATION) {
debug_controller.enable_layer();
}
if desc
.flags
.intersects(wgt::InstanceFlags::GPU_BASED_VALIDATION)
{
debug_controller.enable_layer();
if gpu_based_validation {
#[allow(clippy::collapsible_if)]
if !debug_controller.enable_gpu_based_validation() {
log::warn!("Failed to enable GPU-based validation");
Expand Down
11 changes: 6 additions & 5 deletions wgpu-hal/src/gles/egl.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::auxil::RequestedValidation;
use glow::HasContext;
use parking_lot::{Mutex, MutexGuard, RwLock};

Expand Down Expand Up @@ -738,6 +739,8 @@ impl crate::Instance<super::Api> for Instance {
#[cfg(Emscripten)]
let egl1_5: Option<&Arc<EglInstance>> = Some(&egl);

let requested_validation = RequestedValidation::from_flags(self.flags);

let (display, display_owner, wsi_kind) =
if let (Some(library), Some(egl)) = (wayland_library, egl1_5) {
log::info!("Using Wayland platform");
Expand Down Expand Up @@ -769,7 +772,7 @@ impl crate::Instance<super::Api> for Instance {
EGL_PLATFORM_ANGLE_NATIVE_PLATFORM_TYPE_ANGLE as khronos_egl::Attrib,
EGL_PLATFORM_X11_KHR as khronos_egl::Attrib,
EGL_PLATFORM_ANGLE_DEBUG_LAYERS_ENABLED as khronos_egl::Attrib,
usize::from(desc.flags.contains(wgt::InstanceFlags::VALIDATION)),
usize::from(requested_validation.is_some()),
khronos_egl::ATTRIB_NONE,
];
let display = unsafe {
Expand Down Expand Up @@ -800,9 +803,7 @@ impl crate::Instance<super::Api> for Instance {
(display, None, WindowKind::Unknown)
};

if desc.flags.contains(wgt::InstanceFlags::VALIDATION)
&& client_ext_str.contains("EGL_KHR_debug")
{
if requested_validation.is_some() && client_ext_str.contains("EGL_KHR_debug") {
log::debug!("Enabling EGL debug output");
let function: EglDebugMessageControlFun = {
let addr = egl.get_proc_address("eglDebugMessageControlKHR").unwrap();
Expand Down Expand Up @@ -960,7 +961,7 @@ impl crate::Instance<super::Api> for Instance {
});
}

if self.flags.contains(wgt::InstanceFlags::VALIDATION) && gl.supports_debug() {
if RequestedValidation::from_flags(self.flags).is_some() && gl.supports_debug() {
log::debug!("Enabling GLES debug output");
unsafe { gl.enable(glow::DEBUG_OUTPUT) };
unsafe { gl.debug_message_callback(super::gl_debug_message_callback) };
Expand Down
11 changes: 5 additions & 6 deletions wgpu-hal/src/vulkan/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -667,9 +667,10 @@ impl crate::Instance<super::Api> for super::Instance {
Ok(found)
})
};
let should_enable_gpu_based_validation = desc
.flags
.intersects(wgt::InstanceFlags::GPU_BASED_VALIDATION)
let requested_validation = crate::auxil::RequestedValidation::from_flags(desc.flags);
let should_enable_gpu_based_validation = requested_validation
.as_ref()
.map_or(false, |rv| rv.gpu_based_validation)
&& validation_features_are_enabled()
.transpose()?
.unwrap_or(false);
Expand All @@ -684,9 +685,7 @@ impl crate::Instance<super::Api> for super::Instance {

// Request validation layer if asked.
let mut debug_utils = None;
if desc.flags.intersects(wgt::InstanceFlags::VALIDATION)
|| should_enable_gpu_based_validation
{
if requested_validation.is_some() {
if let Some(layer_properties) = validation_layer_properties {
layers.push(validation_layer_name);

Expand Down

0 comments on commit 29e4b2c

Please sign in to comment.