diff --git a/CHANGELOG.md b/CHANGELOG.md index f0b42fb18d..b9146ab30b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,10 @@ Bottom level categories: - Increase the `max_storage_buffers_per_shader_stage` and `max_storage_textures_per_shader_stage` limits based on what the hardware supports. by @Elabajaba in [#3798]https://github.com/gfx-rs/wgpu/pull/3798 +#### Vulkan + +- Work around [Vulkan-ValidationLayers#5671](https://github.com/KhronosGroup/Vulkan-ValidationLayers/issues/5671) by ignoring reports of violations of [VUID-vkCmdEndDebugUtilsLabelEXT-commandBuffer-01912](https://registry.khronos.org/vulkan/specs/1.3-extensions/html/vkspec.html#VUID-vkCmdEndDebugUtilsLabelEXT-commandBuffer-01912). By @jimblandy in [#3809](https://github.com/gfx-rs/wgpu/pull/3809). + ### Documentation #### General diff --git a/wgpu-hal/src/vulkan/instance.rs b/wgpu-hal/src/vulkan/instance.rs index 101f303c16..8ae477da9f 100644 --- a/wgpu-hal/src/vulkan/instance.rs +++ b/wgpu-hal/src/vulkan/instance.rs @@ -14,15 +14,39 @@ unsafe extern "system" fn debug_utils_messenger_callback( message_severity: vk::DebugUtilsMessageSeverityFlagsEXT, message_type: vk::DebugUtilsMessageTypeFlagsEXT, callback_data_ptr: *const vk::DebugUtilsMessengerCallbackDataEXT, - _user_data: *mut c_void, + user_data: *mut c_void, ) -> vk::Bool32 { - const VUID_VKSWAPCHAINCREATEINFOKHR_IMAGEEXTENT_01274: i32 = 0x7cd0911d; use std::borrow::Cow; if thread::panicking() { return vk::FALSE; } + let cd = unsafe { &*callback_data_ptr }; + let user_data = unsafe { &*(user_data as *mut super::DebugUtilsMessengerUserData) }; + + const VUID_VKCMDENDDEBUGUTILSLABELEXT_COMMANDBUFFER_01912: i32 = 0x56146426; + if cd.message_id_number == VUID_VKCMDENDDEBUGUTILSLABELEXT_COMMANDBUFFER_01912 { + // https://github.com/KhronosGroup/Vulkan-ValidationLayers/issues/5671 + // Versions 1.3.240 through 1.3.250 return a spurious error here if + // the debug range start and end appear in different command buffers. + let khronos_validation_layer = + std::ffi::CStr::from_bytes_with_nul(b"Khronos Validation Layer\0").unwrap(); + if user_data.validation_layer_description.as_ref() == khronos_validation_layer + && user_data.validation_layer_spec_version >= vk::make_api_version(0, 1, 3, 240) + && user_data.validation_layer_spec_version <= vk::make_api_version(0, 1, 3, 250) + { + return vk::FALSE; + } + } + + // Silence Vulkan Validation error "VUID-VkSwapchainCreateInfoKHR-imageExtent-01274" + // - it's a false positive due to the inherent racy-ness of surface resizing + const VUID_VKSWAPCHAINCREATEINFOKHR_IMAGEEXTENT_01274: i32 = 0x7cd0911d; + if cd.message_id_number == VUID_VKSWAPCHAINCREATEINFOKHR_IMAGEEXTENT_01274 { + return vk::FALSE; + } + let level = match message_severity { vk::DebugUtilsMessageSeverityFlagsEXT::VERBOSE => log::Level::Debug, vk::DebugUtilsMessageSeverityFlagsEXT::INFO => log::Level::Info, @@ -31,8 +55,6 @@ unsafe extern "system" fn debug_utils_messenger_callback( _ => log::Level::Warn, }; - let cd = unsafe { &*callback_data_ptr }; - let message_id_name = if cd.p_message_id_name.is_null() { Cow::from("") } else { @@ -44,12 +66,6 @@ unsafe extern "system" fn debug_utils_messenger_callback( unsafe { CStr::from_ptr(cd.p_message) }.to_string_lossy() }; - // Silence Vulkan Validation error "VUID-VkSwapchainCreateInfoKHR-imageExtent-01274" - // - it's a false positive due to the inherent racy-ness of surface resizing - if cd.message_id_number == VUID_VKSWAPCHAINCREATEINFOKHR_IMAGEEXTENT_01274 { - return vk::FALSE; - } - let _ = std::panic::catch_unwind(|| { log::log!( level, @@ -236,12 +252,16 @@ impl super::Instance { /// - `extensions` must be a superset of `required_extensions()` and must be created from the /// same entry, driver_api_version and flags. /// - `android_sdk_version` is ignored and can be `0` for all platforms besides Android + /// + /// If `debug_utils_user_data` is `Some`, then the validation layer is + /// available, so create a [`vk::DebugUtilsMessengerEXT`]. #[allow(clippy::too_many_arguments)] pub unsafe fn from_raw( entry: ash::Entry, raw_instance: ash::Instance, driver_api_version: u32, android_sdk_version: u32, + debug_utils_user_data: Option, extensions: Vec<&'static CStr>, flags: crate::InstanceFlags, has_nv_optimus: bool, @@ -249,36 +269,52 @@ impl super::Instance { ) -> Result { log::info!("Instance version: 0x{:x}", driver_api_version); - let debug_utils = if extensions.contains(&ext::DebugUtils::name()) { - log::info!("Enabling debug utils"); - let extension = ext::DebugUtils::new(&entry, &raw_instance); - // having ERROR unconditionally because Vk doesn't like empty flags - let mut severity = vk::DebugUtilsMessageSeverityFlagsEXT::ERROR; - if log::max_level() >= log::LevelFilter::Debug { - severity |= vk::DebugUtilsMessageSeverityFlagsEXT::VERBOSE; - } - if log::max_level() >= log::LevelFilter::Info { - severity |= vk::DebugUtilsMessageSeverityFlagsEXT::INFO; - } - if log::max_level() >= log::LevelFilter::Warn { - severity |= vk::DebugUtilsMessageSeverityFlagsEXT::WARNING; + let debug_utils = if let Some(debug_callback_user_data) = debug_utils_user_data { + if extensions.contains(&ext::DebugUtils::name()) { + log::info!("Enabling debug utils"); + // Move the callback data to the heap, to ensure it will never be + // moved. + let callback_data = Box::new(debug_callback_user_data); + + let extension = ext::DebugUtils::new(&entry, &raw_instance); + // having ERROR unconditionally because Vk doesn't like empty flags + let mut severity = vk::DebugUtilsMessageSeverityFlagsEXT::ERROR; + if log::max_level() >= log::LevelFilter::Debug { + severity |= vk::DebugUtilsMessageSeverityFlagsEXT::VERBOSE; + } + if log::max_level() >= log::LevelFilter::Info { + severity |= vk::DebugUtilsMessageSeverityFlagsEXT::INFO; + } + if log::max_level() >= log::LevelFilter::Warn { + severity |= vk::DebugUtilsMessageSeverityFlagsEXT::WARNING; + } + let user_data_ptr: *const super::DebugUtilsMessengerUserData = &*callback_data; + let vk_info = vk::DebugUtilsMessengerCreateInfoEXT::builder() + .flags(vk::DebugUtilsMessengerCreateFlagsEXT::empty()) + .message_severity(severity) + .message_type( + vk::DebugUtilsMessageTypeFlagsEXT::GENERAL + | vk::DebugUtilsMessageTypeFlagsEXT::VALIDATION + | vk::DebugUtilsMessageTypeFlagsEXT::PERFORMANCE, + ) + .pfn_user_callback(Some(debug_utils_messenger_callback)) + .user_data(user_data_ptr as *mut _); + let messenger = + unsafe { extension.create_debug_utils_messenger(&vk_info, None) }.unwrap(); + Some(super::DebugUtils { + extension, + messenger, + callback_data, + }) + } else { + log::info!("Debug utils not enabled: extension not listed"); + None } - let vk_info = vk::DebugUtilsMessengerCreateInfoEXT::builder() - .flags(vk::DebugUtilsMessengerCreateFlagsEXT::empty()) - .message_severity(severity) - .message_type( - vk::DebugUtilsMessageTypeFlagsEXT::GENERAL - | vk::DebugUtilsMessageTypeFlagsEXT::VALIDATION - | vk::DebugUtilsMessageTypeFlagsEXT::PERFORMANCE, - ) - .pfn_user_callback(Some(debug_utils_messenger_callback)); - let messenger = - unsafe { extension.create_debug_utils_messenger(&vk_info, None) }.unwrap(); - Some(super::DebugUtils { - extension, - messenger, - }) } else { + log::info!( + "Debug utils not enabled: \ + debug_utils_user_data not passed to Instance::from_raw" + ); None }; @@ -543,31 +579,42 @@ impl crate::Instance for super::Instance { crate::InstanceError })?; - let nv_optimus_layer = CStr::from_bytes_with_nul(b"VK_LAYER_NV_optimus\0").unwrap(); - let has_nv_optimus = instance_layers.iter().any(|inst_layer| { - cstr_from_bytes_until_nul(&inst_layer.layer_name) == Some(nv_optimus_layer) - }); + fn find_layer<'layers>( + instance_layers: &'layers [vk::LayerProperties], + name: &CStr, + ) -> Option<&'layers vk::LayerProperties> { + instance_layers + .iter() + .find(|inst_layer| cstr_from_bytes_until_nul(&inst_layer.layer_name) == Some(name)) + } - // Check requested layers against the available layers - let layers = { - let mut layers: Vec<&'static CStr> = Vec::new(); - if desc.flags.contains(crate::InstanceFlags::VALIDATION) { - layers.push(CStr::from_bytes_with_nul(b"VK_LAYER_KHRONOS_validation\0").unwrap()); + let nv_optimus_layer = CStr::from_bytes_with_nul(b"VK_LAYER_NV_optimus\0").unwrap(); + let has_nv_optimus = find_layer(&instance_layers, nv_optimus_layer).is_some(); + + let mut layers: Vec<&'static CStr> = Vec::new(); + + // Request validation layer if asked. + let mut debug_callback_user_data = None; + if desc.flags.contains(crate::InstanceFlags::VALIDATION) { + let validation_layer_name = + CStr::from_bytes_with_nul(b"VK_LAYER_KHRONOS_validation\0").unwrap(); + if let Some(layer_properties) = find_layer(&instance_layers, validation_layer_name) { + layers.push(validation_layer_name); + debug_callback_user_data = Some(super::DebugUtilsMessengerUserData { + validation_layer_description: cstr_from_bytes_until_nul( + &layer_properties.description, + ) + .unwrap() + .to_owned(), + validation_layer_spec_version: layer_properties.spec_version, + }); + } else { + log::warn!( + "InstanceFlags::VALIDATION requested, but unable to find layer: {}", + validation_layer_name.to_string_lossy() + ); } - - // Only keep available layers. - layers.retain(|&layer| { - if instance_layers.iter().any(|inst_layer| { - cstr_from_bytes_until_nul(&inst_layer.layer_name) == Some(layer) - }) { - true - } else { - log::warn!("Unable to find layer: {}", layer.to_string_lossy()); - false - } - }); - layers - }; + } #[cfg(target_os = "android")] let android_sdk_version = { @@ -619,6 +666,7 @@ impl crate::Instance for super::Instance { vk_instance, driver_api_version, android_sdk_version, + debug_callback_user_data, extensions, desc.flags, has_nv_optimus, diff --git a/wgpu-hal/src/vulkan/mod.rs b/wgpu-hal/src/vulkan/mod.rs index 27200dc4e0..6bea143359 100644 --- a/wgpu-hal/src/vulkan/mod.rs +++ b/wgpu-hal/src/vulkan/mod.rs @@ -75,6 +75,27 @@ impl crate::Api for Api { struct DebugUtils { extension: ext::DebugUtils, messenger: vk::DebugUtilsMessengerEXT, + + /// Owning pointer to the debug messenger callback user data. + /// + /// `InstanceShared::drop` destroys the debug messenger before + /// dropping this, so the callback should never receive a dangling + /// user data pointer. + #[allow(dead_code)] + callback_data: Box, +} + +/// User data needed by `instance::debug_utils_messenger_callback`. +/// +/// When we create the [`vk::DebugUtilsMessengerEXT`], the `pUserData` +/// pointer refers to one of these values. +#[derive(Debug)] +pub struct DebugUtilsMessengerUserData { + /// Validation layer description, from `vk::LayerProperties`. + validation_layer_description: std::ffi::CString, + + /// Validation layer specification version, from `vk::LayerProperties`. + validation_layer_spec_version: u32, } pub struct InstanceShared {