From a842019b348618e4b67e87710fb143b58cf64511 Mon Sep 17 00:00:00 2001 From: Teodor Tanasoaia <28601907+teoxoy@users.noreply.github.com> Date: Wed, 18 Oct 2023 01:20:56 +0200 Subject: [PATCH] Vulkan API version refactors (#4252) --- wgpu-hal/src/vulkan/adapter.rs | 89 +++++++++++++++------------------ wgpu-hal/src/vulkan/device.rs | 6 +-- wgpu-hal/src/vulkan/instance.rs | 28 +++++------ wgpu-hal/src/vulkan/mod.rs | 9 +++- 4 files changed, 62 insertions(+), 70 deletions(-) diff --git a/wgpu-hal/src/vulkan/adapter.rs b/wgpu-hal/src/vulkan/adapter.rs index ef2f342084..5ad8e40023 100644 --- a/wgpu-hal/src/vulkan/adapter.rs +++ b/wgpu-hal/src/vulkan/adapter.rs @@ -78,7 +78,7 @@ impl PhysicalDeviceFeatures { /// /// `requested_features` should be the same as what was used to generate `enabled_extensions`. fn from_extensions_and_requested_features( - effective_api_version: u32, + device_api_version: u32, enabled_extensions: &[&'static CStr], requested_features: wgt::Features, downlevel_flags: wgt::DownlevelFlags, @@ -200,7 +200,7 @@ impl PhysicalDeviceFeatures { } else { None }, - imageless_framebuffer: if effective_api_version >= vk::API_VERSION_1_2 + imageless_framebuffer: if device_api_version >= vk::API_VERSION_1_2 || enabled_extensions.contains(&vk::KhrImagelessFramebufferFn::name()) { Some( @@ -211,7 +211,7 @@ impl PhysicalDeviceFeatures { } else { None }, - timeline_semaphore: if effective_api_version >= vk::API_VERSION_1_2 + timeline_semaphore: if device_api_version >= vk::API_VERSION_1_2 || enabled_extensions.contains(&vk::KhrTimelineSemaphoreFn::name()) { Some( @@ -222,7 +222,7 @@ impl PhysicalDeviceFeatures { } else { None }, - image_robustness: if effective_api_version >= vk::API_VERSION_1_3 + image_robustness: if device_api_version >= vk::API_VERSION_1_3 || enabled_extensions.contains(&vk::ExtImageRobustnessFn::name()) { Some( @@ -246,7 +246,7 @@ impl PhysicalDeviceFeatures { } else { None }, - multiview: if effective_api_version >= vk::API_VERSION_1_1 + multiview: if device_api_version >= vk::API_VERSION_1_1 || enabled_extensions.contains(&vk::KhrMultiviewFn::name()) { Some( @@ -279,7 +279,7 @@ impl PhysicalDeviceFeatures { } else { None }, - zero_initialize_workgroup_memory: if effective_api_version >= vk::API_VERSION_1_3 + zero_initialize_workgroup_memory: if device_api_version >= vk::API_VERSION_1_3 || enabled_extensions.contains(&vk::KhrZeroInitializeWorkgroupMemoryFn::name()) { Some( @@ -526,7 +526,7 @@ impl PhysicalDeviceFeatures { features.set( F::BGRA8UNORM_STORAGE, - supports_bgra8unorm_storage(instance, phd, caps.effective_api_version), + supports_bgra8unorm_storage(instance, phd, caps.device_api_version), ); (features, dl_flags) @@ -550,20 +550,12 @@ pub struct PhysicalDeviceCapabilities { maintenance_3: Option, descriptor_indexing: Option, driver: Option, - /// The effective driver api version supported by the physical device. + /// The device API version. /// - /// The Vulkan specification states the following in the documentation for VkPhysicalDeviceProperties: - /// > The value of apiVersion may be different than the version returned by vkEnumerateInstanceVersion; - /// > either higher or lower. In such cases, the application must not use functionality that exceeds - /// > the version of Vulkan associated with a given object. + /// Which is the version of Vulkan supported for device-level functionality. /// - /// For example, a Vulkan 1.1 instance cannot use functionality added in Vulkan 1.2 even if the physical - /// device supports Vulkan 1.2. - /// - /// This means that assuming that the apiVersion provided by VkPhysicalDeviceProperties is the actual - /// version we can use is incorrect. Instead the effective version is the lower of the instance version - /// and physical device version. - effective_api_version: u32, + /// It is associated with a `VkPhysicalDevice` and its children. + device_api_version: u32, } // This is safe because the structs have `p_next: *mut c_void`, which we null out/never read. @@ -592,7 +584,7 @@ impl PhysicalDeviceCapabilities { // Require `VK_KHR_swapchain` extensions.push(vk::KhrSwapchainFn::name()); - if self.effective_api_version < vk::API_VERSION_1_1 { + if self.device_api_version < vk::API_VERSION_1_1 { // Require either `VK_KHR_maintenance1` or `VK_AMD_negative_viewport_height` if self.supports_extension(vk::KhrMaintenance1Fn::name()) { extensions.push(vk::KhrMaintenance1Fn::name()); @@ -620,7 +612,7 @@ impl PhysicalDeviceCapabilities { } } - if self.effective_api_version < vk::API_VERSION_1_2 { + if self.device_api_version < vk::API_VERSION_1_2 { // Optional `VK_KHR_image_format_list` if self.supports_extension(vk::KhrImageFormatListFn::name()) { extensions.push(vk::KhrImageFormatListFn::name()); @@ -630,7 +622,7 @@ impl PhysicalDeviceCapabilities { if self.supports_extension(vk::KhrImagelessFramebufferFn::name()) { extensions.push(vk::KhrImagelessFramebufferFn::name()); // Require `VK_KHR_maintenance2` due to it being a dependency - if self.effective_api_version < vk::API_VERSION_1_1 { + if self.device_api_version < vk::API_VERSION_1_1 { extensions.push(vk::KhrMaintenance2Fn::name()); } } @@ -654,7 +646,7 @@ impl PhysicalDeviceCapabilities { if requested_features.contains(wgt::Features::SHADER_F16) { extensions.push(vk::KhrShaderFloat16Int8Fn::name()); // `VK_KHR_16bit_storage` requires `VK_KHR_storage_buffer_storage_class`, however we require that one already - if self.effective_api_version < vk::API_VERSION_1_1 { + if self.device_api_version < vk::API_VERSION_1_1 { extensions.push(vk::Khr16bitStorageFn::name()); } } @@ -663,7 +655,7 @@ impl PhysicalDeviceCapabilities { //extensions.push(vk::ExtSamplerFilterMinmaxFn::name()); } - if self.effective_api_version < vk::API_VERSION_1_3 { + if self.device_api_version < vk::API_VERSION_1_3 { // Optional `VK_EXT_image_robustness` if self.supports_extension(vk::ExtImageRobustnessFn::name()) { extensions.push(vk::ExtImageRobustnessFn::name()); @@ -788,19 +780,22 @@ impl super::InstanceShared { let mut capabilities = PhysicalDeviceCapabilities::default(); capabilities.supported_extensions = unsafe { self.raw.enumerate_device_extension_properties(phd).unwrap() }; - capabilities.properties = if let Some(ref get_device_properties) = - self.get_physical_device_properties - { + capabilities.properties = unsafe { self.raw.get_physical_device_properties(phd) }; + capabilities.device_api_version = capabilities.properties.api_version; + + if let Some(ref get_device_properties) = self.get_physical_device_properties { // Get these now to avoid borrowing conflicts later - let supports_descriptor_indexing = self.driver_api_version >= vk::API_VERSION_1_2 + let supports_maintenance3 = capabilities.device_api_version >= vk::API_VERSION_1_1 + || capabilities.supports_extension(vk::KhrMaintenance3Fn::name()); + let supports_descriptor_indexing = capabilities.device_api_version + >= vk::API_VERSION_1_2 || capabilities.supports_extension(vk::ExtDescriptorIndexingFn::name()); - let supports_driver_properties = self.driver_api_version >= vk::API_VERSION_1_2 + let supports_driver_properties = capabilities.device_api_version + >= vk::API_VERSION_1_2 || capabilities.supports_extension(vk::KhrDriverPropertiesFn::name()); let mut builder = vk::PhysicalDeviceProperties2KHR::builder(); - if self.driver_api_version >= vk::API_VERSION_1_1 - || capabilities.supports_extension(vk::KhrMaintenance3Fn::name()) - { + if supports_maintenance3 { capabilities.maintenance_3 = Some(vk::PhysicalDeviceMaintenance3Properties::default()); builder = builder.push_next(capabilities.maintenance_3.as_mut().unwrap()); @@ -824,15 +819,7 @@ impl super::InstanceShared { unsafe { get_device_properties.get_physical_device_properties2(phd, &mut properties2); } - properties2.properties - } else { - unsafe { self.raw.get_physical_device_properties(phd) } }; - - // Set the effective api version - capabilities.effective_api_version = self - .driver_api_version - .min(capabilities.properties.api_version); capabilities }; @@ -843,7 +830,7 @@ impl super::InstanceShared { let mut builder = vk::PhysicalDeviceFeatures2KHR::builder().features(core); // `VK_KHR_multiview` is promoted to 1.1 - if capabilities.effective_api_version >= vk::API_VERSION_1_1 + if capabilities.device_api_version >= vk::API_VERSION_1_1 || capabilities.supports_extension(vk::KhrMultiviewFn::name()) { let next = features @@ -905,7 +892,7 @@ impl super::InstanceShared { } // `VK_KHR_zero_initialize_workgroup_memory` is promoted to 1.3 - if capabilities.effective_api_version >= vk::API_VERSION_1_3 + if capabilities.device_api_version >= vk::API_VERSION_1_3 || capabilities.supports_extension(vk::KhrZeroInitializeWorkgroupMemoryFn::name()) { let next = features @@ -998,7 +985,7 @@ impl super::Instance { ); }; - if phd_capabilities.effective_api_version == vk::API_VERSION_1_0 + if phd_capabilities.device_api_version == vk::API_VERSION_1_0 && !phd_capabilities.supports_extension(vk::KhrStorageBufferStorageClassFn::name()) { log::warn!( @@ -1009,7 +996,7 @@ impl super::Instance { } if !phd_capabilities.supports_extension(vk::AmdNegativeViewportHeightFn::name()) && !phd_capabilities.supports_extension(vk::KhrMaintenance1Fn::name()) - && phd_capabilities.effective_api_version < vk::API_VERSION_1_1 + && phd_capabilities.device_api_version < vk::API_VERSION_1_1 { log::warn!( "viewport Y-flip is not supported, hiding adapter: {}", @@ -1030,7 +1017,7 @@ impl super::Instance { } let private_caps = super::PrivateCapabilities { - flip_y_requires_shift: phd_capabilities.effective_api_version >= vk::API_VERSION_1_1 + flip_y_requires_shift: phd_capabilities.device_api_version >= vk::API_VERSION_1_1 || phd_capabilities.supports_extension(vk::KhrMaintenance1Fn::name()), imageless_framebuffers: match phd_features.imageless_framebuffer { Some(features) => features.imageless_framebuffer == vk::TRUE, @@ -1038,7 +1025,7 @@ impl super::Instance { .imageless_framebuffer .map_or(false, |ext| ext.imageless_framebuffer != 0), }, - image_view_usage: phd_capabilities.effective_api_version >= vk::API_VERSION_1_1 + image_view_usage: phd_capabilities.device_api_version >= vk::API_VERSION_1_1 || phd_capabilities.supports_extension(vk::KhrMaintenance2Fn::name()), timeline_semaphores: match phd_features.timeline_semaphore { Some(features) => features.timeline_semaphore == vk::TRUE, @@ -1092,6 +1079,8 @@ impl super::Instance { .map_or(false, |ext| { ext.shader_zero_initialize_workgroup_memory == vk::TRUE }), + image_format_list: phd_capabilities.device_api_version >= vk::API_VERSION_1_2 + || phd_capabilities.supports_extension(vk::KhrImageFormatListFn::name()), }; let capabilities = crate::Capabilities { limits: phd_capabilities.to_wgpu_limits(), @@ -1165,7 +1154,7 @@ impl super::Adapter { features: wgt::Features, ) -> PhysicalDeviceFeatures { PhysicalDeviceFeatures::from_extensions_and_requested_features( - self.phd_capabilities.effective_api_version, + self.phd_capabilities.device_api_version, enabled_extensions, features, self.downlevel_flags, @@ -1219,7 +1208,7 @@ impl super::Adapter { &self.instance.raw, &raw_device, ))) - } else if self.phd_capabilities.effective_api_version >= vk::API_VERSION_1_2 { + } else if self.phd_capabilities.device_api_version >= vk::API_VERSION_1_2 { Some(super::ExtensionFn::Promoted) } else { None @@ -1760,14 +1749,14 @@ fn supports_format( fn supports_bgra8unorm_storage( instance: &ash::Instance, phd: vk::PhysicalDevice, - api_version: u32, + device_api_version: u32, ) -> bool { // See https://github.com/KhronosGroup/Vulkan-Docs/issues/2027#issuecomment-1380608011 // This check gates the function call and structures used below. // TODO: check for (`VK_KHR_get_physical_device_properties2` or VK1.1) and (`VK_KHR_format_feature_flags2` or VK1.3). // Right now we only check for VK1.3. - if api_version < vk::API_VERSION_1_3 { + if device_api_version < vk::API_VERSION_1_3 { return false; } diff --git a/wgpu-hal/src/vulkan/device.rs b/wgpu-hal/src/vulkan/device.rs index cb955e8318..f541fd1032 100644 --- a/wgpu-hal/src/vulkan/device.rs +++ b/wgpu-hal/src/vulkan/device.rs @@ -970,11 +970,7 @@ impl crate::Device for super::Device { wgt_view_formats = desc.view_formats.clone(); wgt_view_formats.push(desc.format); - if self.shared_instance().driver_api_version >= vk::API_VERSION_1_2 - || self - .enabled_device_extensions() - .contains(&vk::KhrImageFormatListFn::name()) - { + if self.shared.private_caps.image_format_list { vk_view_formats = desc .view_formats .iter() diff --git a/wgpu-hal/src/vulkan/instance.rs b/wgpu-hal/src/vulkan/instance.rs index 05218ba075..908f6e5064 100644 --- a/wgpu-hal/src/vulkan/instance.rs +++ b/wgpu-hal/src/vulkan/instance.rs @@ -177,8 +177,8 @@ impl super::InstanceShared { &self.raw } - pub fn driver_api_version(&self) -> u32 { - self.driver_api_version + pub fn instance_api_version(&self) -> u32 { + self.instance_api_version } pub fn extensions(&self) -> &[&'static CStr] { @@ -196,7 +196,7 @@ impl super::Instance { /// Return a vector of the names of instance extensions actually available /// on `entry` that wgpu would like to enable. /// - /// The `driver_api_version` argument should be the instance's Vulkan API + /// The `instance_api_version` argument should be the instance's Vulkan API /// version, as obtained from `vkEnumerateInstanceVersion`. This is the same /// space of values as the `VK_API_VERSION` constants. /// @@ -206,7 +206,7 @@ impl super::Instance { /// assumes that it has been enabled. pub fn desired_extensions( entry: &ash::Entry, - _driver_api_version: u32, + _instance_api_version: u32, flags: wgt::InstanceFlags, ) -> Result, crate::InstanceError> { let instance_extensions = entry @@ -282,9 +282,9 @@ impl super::Instance { /// # Safety /// /// - `raw_instance` must be created from `entry` - /// - `raw_instance` must be created respecting `driver_api_version`, `extensions` and `flags` + /// - `raw_instance` must be created respecting `instance_api_version`, `extensions` and `flags` /// - `extensions` must be a superset of `desired_extensions()` and must be created from the - /// same entry, driver_api_version and flags. + /// same entry, `instance_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 @@ -293,7 +293,7 @@ impl super::Instance { pub unsafe fn from_raw( entry: ash::Entry, raw_instance: ash::Instance, - driver_api_version: u32, + instance_api_version: u32, android_sdk_version: u32, debug_utils_user_data: Option, extensions: Vec<&'static CStr>, @@ -301,7 +301,7 @@ impl super::Instance { has_nv_optimus: bool, drop_guard: Option, ) -> Result { - log::info!("Instance version: 0x{:x}", driver_api_version); + log::info!("Instance version: 0x{:x}", instance_api_version); let debug_utils = if let Some(debug_callback_user_data) = debug_utils_user_data { if extensions.contains(&ext::DebugUtils::name()) { @@ -373,7 +373,7 @@ impl super::Instance { get_physical_device_properties, entry, has_nv_optimus, - driver_api_version, + instance_api_version, android_sdk_version, }), }) @@ -575,7 +575,7 @@ impl crate::Instance for super::Instance { let entry = unsafe { ash::Entry::load() }.map_err(|err| { crate::InstanceError::with_source(String::from("missing Vulkan entry points"), err) })?; - let driver_api_version = match entry.try_enumerate_instance_version() { + let instance_api_version = match entry.try_enumerate_instance_version() { // Vulkan 1.1+ Ok(Some(version)) => version, Ok(None) => vk::API_VERSION_1_0, @@ -595,7 +595,7 @@ impl crate::Instance for super::Instance { .engine_version(2) .api_version( // Vulkan 1.0 doesn't like anything but 1.0 passed in here... - if driver_api_version < vk::API_VERSION_1_1 { + if instance_api_version < vk::API_VERSION_1_1 { vk::API_VERSION_1_0 } else { // This is the max Vulkan API version supported by `wgpu-hal`. @@ -606,11 +606,11 @@ impl crate::Instance for super::Instance { // - If any were promoted in the new API version and the behavior has changed, we must handle the new behavior in addition to the old behavior. // - If any were obsoleted in the new API version, we must implement a fallback for the new API version // - If any are non-KHR-vendored, we must ensure the new behavior is still correct (since backwards-compatibility is not guaranteed). - vk::HEADER_VERSION_COMPLETE + vk::API_VERSION_1_3 }, ); - let extensions = Self::desired_extensions(&entry, driver_api_version, desc.flags)?; + let extensions = Self::desired_extensions(&entry, instance_api_version, desc.flags)?; let instance_layers = entry.enumerate_instance_layer_properties().map_err(|e| { log::info!("enumerate_instance_layer_properties: {:?}", e); @@ -720,7 +720,7 @@ impl crate::Instance for super::Instance { Self::from_raw( entry, vk_instance, - driver_api_version, + instance_api_version, android_sdk_version, debug_callback_user_data, extensions, diff --git a/wgpu-hal/src/vulkan/mod.rs b/wgpu-hal/src/vulkan/mod.rs index e08e27d486..5cdb7f11ca 100644 --- a/wgpu-hal/src/vulkan/mod.rs +++ b/wgpu-hal/src/vulkan/mod.rs @@ -112,7 +112,13 @@ pub struct InstanceShared { entry: ash::Entry, has_nv_optimus: bool, android_sdk_version: u32, - driver_api_version: u32, + /// The instance API version. + /// + /// Which is the version of Vulkan supported for instance-level functionality. + /// + /// It is associated with a `VkInstance` and its children, + /// except for a `VkPhysicalDevice` and its children. + instance_api_version: u32, } pub struct Instance { @@ -196,6 +202,7 @@ struct PrivateCapabilities { robust_buffer_access2: bool, robust_image_access2: bool, zero_initialize_workgroup_memory: bool, + image_format_list: bool, } bitflags::bitflags!(