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.

Sign up for GitHub

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

Don't report violations of VUID-vkCmdEndDebugUtilsLabelEXT-commandBuffer-01912 #3809

Merged
merged 1 commit into from
Jun 6, 2023
+134 −61
Merged

Don't report violations of VUID-vkCmdEndDebugUtilsLabelEXT-commandBuffer-01912 #3809

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
170 changes: 109 additions & 61 deletions wgpu-hal/src/vulkan/instance.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 {
Expand All @@ -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,
Expand Down Expand Up @@ -236,49 +252,69 @@ 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<super::DebugUtilsMessengerUserData>,
extensions: Vec<&'static CStr>,
flags: crate::InstanceFlags,
has_nv_optimus: bool,
drop_guard: Option<crate::DropGuard>,
) -> Result<Self, crate::InstanceError> {
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()) {
jimblandy marked this conversation as resolved.
Show resolved Hide resolved
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
};

Expand Down Expand Up @@ -543,31 +579,42 @@ impl crate::Instance<super::Api> 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 = {
Expand Down Expand Up @@ -619,6 +666,7 @@ impl crate::Instance<super::Api> for super::Instance {
vk_instance,
driver_api_version,
android_sdk_version,
debug_callback_user_data,
extensions,
desc.flags,
has_nv_optimus,
Expand Down
21 changes: 21 additions & 0 deletions wgpu-hal/src/vulkan/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<DebugUtilsMessengerUserData>,
}

/// 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 {
Expand Down