Skip to content

Commit

Permalink
vulkan: use effective api version for determining device features (#3011
Browse files Browse the repository at this point in the history
)
  • Loading branch information
i509VCB authored Sep 20, 2022
1 parent f877a8a commit 65e4051
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 14 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ SurfaceConfiguration {
#### 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

Expand Down
46 changes: 32 additions & 14 deletions wgpu-hal/src/vulkan/adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -511,6 +511,20 @@ pub struct PhysicalDeviceCapabilities {
supported_extensions: Vec<vk::ExtensionProperties>,
properties: vk::PhysicalDeviceProperties,
descriptor_indexing: Option<vk::PhysicalDeviceDescriptorIndexingPropertiesEXT>,
/// 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.
Expand All @@ -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());

Expand All @@ -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`
Expand All @@ -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());
}
}
Expand Down Expand Up @@ -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
};

Expand All @@ -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
Expand Down Expand Up @@ -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!(
Expand All @@ -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: {}",
Expand All @@ -925,15 +943,15 @@ 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,
None => phd_features
.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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 65e4051

Please sign in to comment.