From 56eb7f66811215bda1fe5fb4a1ae6488230d9959 Mon Sep 17 00:00:00 2001 From: i509VCB Date: Sun, 4 Sep 2022 23:15:00 -0500 Subject: [PATCH] vulkan: use effective api version for determining device features wgpu wrongly assumes that the instance version will always be the maximum value. This assumption can be broken by using Instance::from_raw. This pull request fixes that issue by calculating the "effective" api version the device supports, which is the lower of the instance version and physical device version. More information can be found in the Vulkan specification's documentation for VkPhysicalDeviceProperties. --- CHANGELOG.md | 2 ++ wgpu-hal/src/vulkan/adapter.rs | 46 +++++++++++++++++++++++----------- 2 files changed, 34 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3555702a5a..1b074d9386 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -97,6 +97,8 @@ the same every time it is rendered, we now warn if it is missing. #### Vulkan - Remove use of Vulkan12Features/Properties types. By @i509VCB in [#2936](https://github.com/gfx-rs/wgpu/pull/2936) - Provide a means for `wgpu` users to access `vk::Queue` and the queue index. By @anlumo in [#2950](https://github.com/gfx-rs/wgpu/pull/2950) +- Use the use effective api version for determining device features instead of wrongly assuming `VkPhysicalDeviceProperties.apiVersion` + is the actual version of the device. By @i509VCB in [#3011](https://github.com/gfx-rs/wgpu/pull/3011) ### Performance diff --git a/wgpu-hal/src/vulkan/adapter.rs b/wgpu-hal/src/vulkan/adapter.rs index 0a3afb690e..ae17684b49 100644 --- a/wgpu-hal/src/vulkan/adapter.rs +++ b/wgpu-hal/src/vulkan/adapter.rs @@ -72,7 +72,7 @@ impl PhysicalDeviceFeatures { /// /// `requested_features` should be the same as what was used to generate `enabled_extensions`. fn from_extensions_and_requested_features( - api_version: u32, + effective_api_version: u32, enabled_extensions: &[&'static CStr], requested_features: wgt::Features, downlevel_flags: wgt::DownlevelFlags, @@ -207,7 +207,7 @@ impl PhysicalDeviceFeatures { } else { None }, - imageless_framebuffer: if api_version >= vk::API_VERSION_1_2 + imageless_framebuffer: if effective_api_version >= vk::API_VERSION_1_2 || enabled_extensions.contains(&vk::KhrImagelessFramebufferFn::name()) { Some( @@ -218,7 +218,7 @@ impl PhysicalDeviceFeatures { } else { None }, - timeline_semaphore: if api_version >= vk::API_VERSION_1_2 + timeline_semaphore: if effective_api_version >= vk::API_VERSION_1_2 || enabled_extensions.contains(&vk::KhrTimelineSemaphoreFn::name()) { Some( @@ -262,7 +262,7 @@ impl PhysicalDeviceFeatures { } else { None }, - multiview: if api_version >= vk::API_VERSION_1_1 + multiview: if effective_api_version >= vk::API_VERSION_1_1 || enabled_extensions.contains(&vk::KhrMultiviewFn::name()) { Some( @@ -511,6 +511,20 @@ pub struct PhysicalDeviceCapabilities { supported_extensions: Vec, properties: vk::PhysicalDeviceProperties, descriptor_indexing: Option, + /// The effective driver api version supported by the physical device. + /// + /// 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. + /// + /// 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, } // This is safe because the structs have `p_next: *mut c_void`, which we null out/never read. @@ -534,7 +548,7 @@ impl PhysicalDeviceCapabilities { extensions.push(khr::Swapchain::name()); - if self.properties.api_version < vk::API_VERSION_1_1 { + if self.effective_api_version < vk::API_VERSION_1_1 { extensions.push(vk::KhrMaintenance1Fn::name()); extensions.push(vk::KhrMaintenance2Fn::name()); @@ -552,7 +566,7 @@ impl PhysicalDeviceCapabilities { } } - if self.properties.api_version < vk::API_VERSION_1_2 { + if self.effective_api_version < vk::API_VERSION_1_2 { if self.supports_extension(vk::KhrImagelessFramebufferFn::name()) { extensions.push(vk::KhrImagelessFramebufferFn::name()); extensions.push(vk::KhrImageFormatListFn::name()); // Required for `KhrImagelessFramebufferFn` @@ -564,7 +578,7 @@ impl PhysicalDeviceCapabilities { if requested_features.intersects(indexing_features()) { extensions.push(vk::ExtDescriptorIndexingFn::name()); - if self.properties.api_version < vk::API_VERSION_1_1 { + if self.effective_api_version < vk::API_VERSION_1_1 { extensions.push(vk::KhrMaintenance3Fn::name()); } } @@ -755,6 +769,10 @@ impl super::InstanceShared { 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 }; @@ -765,7 +783,7 @@ impl super::InstanceShared { let mut builder = vk::PhysicalDeviceFeatures2KHR::builder().features(core); // `VK_KHR_multiview` is promoted to 1.1 - if capabilities.properties.api_version >= vk::API_VERSION_1_1 + if capabilities.effective_api_version >= vk::API_VERSION_1_1 || capabilities.supports_extension(vk::KhrMultiviewFn::name()) { let next = features @@ -893,7 +911,7 @@ impl super::Instance { ); }; - if phd_capabilities.properties.api_version == vk::API_VERSION_1_0 + if phd_capabilities.effective_api_version == vk::API_VERSION_1_0 && !phd_capabilities.supports_extension(vk::KhrStorageBufferStorageClassFn::name()) { log::warn!( @@ -904,7 +922,7 @@ impl super::Instance { } if !phd_capabilities.supports_extension(vk::AmdNegativeViewportHeightFn::name()) && !phd_capabilities.supports_extension(vk::KhrMaintenance1Fn::name()) - && phd_capabilities.properties.api_version < vk::API_VERSION_1_1 + && phd_capabilities.effective_api_version < vk::API_VERSION_1_1 { log::warn!( "viewport Y-flip is not supported, hiding adapter: {}", @@ -925,7 +943,7 @@ impl super::Instance { } let private_caps = super::PrivateCapabilities { - flip_y_requires_shift: phd_capabilities.properties.api_version >= vk::API_VERSION_1_1 + flip_y_requires_shift: phd_capabilities.effective_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, @@ -933,7 +951,7 @@ impl super::Instance { .imageless_framebuffer .map_or(false, |ext| ext.imageless_framebuffer != 0), }, - image_view_usage: phd_capabilities.properties.api_version >= vk::API_VERSION_1_1 + image_view_usage: phd_capabilities.effective_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, @@ -1039,7 +1057,7 @@ impl super::Adapter { uab_types: super::UpdateAfterBindTypes, ) -> PhysicalDeviceFeatures { PhysicalDeviceFeatures::from_extensions_and_requested_features( - self.phd_capabilities.properties.api_version, + self.phd_capabilities.effective_api_version, enabled_extensions, features, self.downlevel_flags, @@ -1093,7 +1111,7 @@ impl super::Adapter { &self.instance.raw, &raw_device, ))) - } else if self.phd_capabilities.properties.api_version >= vk::API_VERSION_1_2 { + } else if self.phd_capabilities.effective_api_version >= vk::API_VERSION_1_2 { Some(super::ExtensionFn::Promoted) } else { None