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

vulkan: use effective api version for determining device features #3011

Merged
merged 1 commit into from
Sep 20, 2022
Merged

vulkan: use effective api version for determining device features #3011

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
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
46 changes: 32 additions & 14 deletions wgpu-hal/src/vulkan/adapter.rs
View file
Open in desktop
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