From c8c48afba7c2f0994844dd70cd7367cba8d89bcc Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Thu, 22 Dec 2022 19:06:57 +0100 Subject: [PATCH 1/5] document and improve extension detection --- wgpu-hal/src/vulkan/adapter.rs | 105 ++++++++++++++++++++++---------- wgpu-hal/src/vulkan/instance.rs | 24 ++++++-- 2 files changed, 90 insertions(+), 39 deletions(-) diff --git a/wgpu-hal/src/vulkan/adapter.rs b/wgpu-hal/src/vulkan/adapter.rs index cf5c8eeae7..93b6b7cb1b 100644 --- a/wgpu-hal/src/vulkan/adapter.rs +++ b/wgpu-hal/src/vulkan/adapter.rs @@ -172,9 +172,7 @@ impl PhysicalDeviceFeatures { //.shader_resource_residency(requested_features.contains(wgt::Features::SHADER_RESOURCE_RESIDENCY)) .geometry_shader(requested_features.contains(wgt::Features::SHADER_PRIMITIVE_INDEX)) .build(), - descriptor_indexing: if enabled_extensions - .contains(&vk::ExtDescriptorIndexingFn::name()) - { + descriptor_indexing: if requested_features.intersects(indexing_features()) { Some( vk::PhysicalDeviceDescriptorIndexingFeaturesEXT::builder() .shader_sampled_image_array_non_uniform_indexing( @@ -229,7 +227,9 @@ impl PhysicalDeviceFeatures { } else { None }, - image_robustness: if enabled_extensions.contains(&vk::ExtImageRobustnessFn::name()) { + image_robustness: if effective_api_version >= vk::API_VERSION_1_3 + || enabled_extensions.contains(&vk::ExtImageRobustnessFn::name()) + { Some( vk::PhysicalDeviceImageRobustnessFeaturesEXT::builder() .robust_image_access(private_caps.robust_image_access) @@ -412,7 +412,7 @@ impl PhysicalDeviceFeatures { //if caps.supports_extension(vk::ExtSamplerFilterMinmaxFn::name()) { features.set( F::MULTI_DRAW_INDIRECT_COUNT, - caps.supports_extension(khr::DrawIndirectCount::name()), + caps.supports_extension(vk::KhrDrawIndirectCountFn::name()), ); features.set( F::CONSERVATIVE_RASTERIZATION, @@ -554,78 +554,118 @@ impl PhysicalDeviceCapabilities { fn get_required_extensions(&self, requested_features: wgt::Features) -> Vec<&'static CStr> { let mut extensions = Vec::new(); - extensions.push(khr::Swapchain::name()); + // Note that quite a few extensions depend on the `VK_KHR_get_physical_device_properties2` instance extension. + // We enable `VK_KHR_get_physical_device_properties2` unconditionally (if available). + + // Require `VK_KHR_swapchain` + extensions.push(vk::KhrSwapchainFn::name()); if self.effective_api_version < vk::API_VERSION_1_1 { - extensions.push(vk::KhrMaintenance1Fn::name()); - extensions.push(vk::KhrMaintenance2Fn::name()); + // Require either `VK_KHR_maintenance1` or `VK_AMD_negative_viewport_height` + if self.supports_extension(vk::KhrMaintenance1Fn::name()) { + extensions.push(vk::KhrMaintenance1Fn::name()); + } else { + // `VK_AMD_negative_viewport_height` is obsoleted by `VK_KHR_maintenance1` and must not be enabled alongside it + extensions.push(vk::AmdNegativeViewportHeightFn::name()); + } + + // Optional `VK_KHR_maintenance2` + if self.supports_extension(vk::KhrMaintenance2Fn::name()) { + extensions.push(vk::KhrMaintenance2Fn::name()); + } - // `VK_KHR_storage_buffer_storage_class` required for Naga on Vulkan 1.0 devices + // Require `VK_KHR_storage_buffer_storage_class` extensions.push(vk::KhrStorageBufferStorageClassFn::name()); - // Below Vulkan 1.1 we can get multiview from an extension + // Require `VK_KHR_multiview` if the associated feature was requested if requested_features.contains(wgt::Features::MULTIVIEW) { extensions.push(vk::KhrMultiviewFn::name()); } - - // `VK_AMD_negative_viewport_height` is obsoleted by `VK_KHR_maintenance1` and must not be enabled alongside `VK_KHR_maintenance1` or a 1.1+ device. - if !self.supports_extension(vk::KhrMaintenance1Fn::name()) { - extensions.push(vk::AmdNegativeViewportHeightFn::name()); - } } if self.effective_api_version < vk::API_VERSION_1_2 { + // Optional `VK_KHR_imageless_framebuffer` if self.supports_extension(vk::KhrImagelessFramebufferFn::name()) { extensions.push(vk::KhrImagelessFramebufferFn::name()); - extensions.push(vk::KhrImageFormatListFn::name()); // Required for `KhrImagelessFramebufferFn` + // Require `VK_KHR_image_format_list` due to it being a dependency + extensions.push(vk::KhrImageFormatListFn::name()); + // Require `VK_KHR_maintenance2` due to it being a dependency + if self.effective_api_version < vk::API_VERSION_1_1 { + extensions.push(vk::KhrMaintenance2Fn::name()); + } } - // This extension is core in Vulkan 1.2 + // Optional `VK_KHR_driver_properties` if self.supports_extension(vk::KhrDriverPropertiesFn::name()) { extensions.push(vk::KhrDriverPropertiesFn::name()); } - extensions.push(vk::ExtSamplerFilterMinmaxFn::name()); - extensions.push(vk::KhrTimelineSemaphoreFn::name()); + // Optional `VK_KHR_timeline_semaphore` + if self.supports_extension(vk::KhrTimelineSemaphoreFn::name()) { + extensions.push(vk::KhrTimelineSemaphoreFn::name()); + } + // Require `VK_EXT_descriptor_indexing` if one of the associated features was requested if requested_features.intersects(indexing_features()) { extensions.push(vk::ExtDescriptorIndexingFn::name()); + // Require `VK_KHR_maintenance3` due to it being a dependency if self.effective_api_version < vk::API_VERSION_1_1 { extensions.push(vk::KhrMaintenance3Fn::name()); } } + // Require `VK_KHR_shader_float16_int8` and `VK_KHR_16bit_storage` if the associated feature was requested + if requested_features.contains(wgt::Features::SHADER_FLOAT16) { + 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 { + extensions.push(vk::Khr16bitStorageFn::name()); + } + } + //extensions.push(vk::KhrSamplerMirrorClampToEdgeFn::name()); //extensions.push(vk::ExtSamplerFilterMinmaxFn::name()); } + if self.effective_api_version < vk::API_VERSION_1_3 { + // Optional `VK_EXT_image_robustness` + if self.supports_extension(vk::ExtImageRobustnessFn::name()) { + extensions.push(vk::ExtImageRobustnessFn::name()); + } + } + + // Optional `VK_EXT_robustness2` + if self.supports_extension(vk::ExtRobustness2Fn::name()) { + extensions.push(vk::ExtRobustness2Fn::name()); + } + + // Require `VK_KHR_draw_indirect_count` if the associated feature was requested // Even though Vulkan 1.2 has promoted the extension to core, we must require the extension to avoid // large amounts of spaghetti involved with using PhysicalDeviceVulkan12Features. if requested_features.contains(wgt::Features::MULTI_DRAW_INDIRECT_COUNT) { extensions.push(vk::KhrDrawIndirectCountFn::name()); } + // Require `VK_EXT_conservative_rasterization` if the associated feature was requested if requested_features.contains(wgt::Features::CONSERVATIVE_RASTERIZATION) { extensions.push(vk::ExtConservativeRasterizationFn::name()); } + // Require `VK_EXT_depth_clip_enable` if the associated feature was requested if requested_features.contains(wgt::Features::DEPTH_CLIP_CONTROL) { extensions.push(vk::ExtDepthClipEnableFn::name()); } + // Require `VK_KHR_portability_subset` on macOS/iOS #[cfg(any(target_os = "macos", target_os = "ios"))] extensions.push(vk::KhrPortabilitySubsetFn::name()); + // Require `VK_EXT_texture_compression_astc_hdr` if the associated feature was requested if requested_features.contains(wgt::Features::TEXTURE_COMPRESSION_ASTC_HDR) { extensions.push(vk::ExtTextureCompressionAstcHdrFn::name()); } - if requested_features.contains(wgt::Features::SHADER_FLOAT16) { - extensions.push(vk::KhrShaderFloat16Int8Fn::name()); - extensions.push(vk::Khr16bitStorageFn::name()); - } - extensions } @@ -762,13 +802,12 @@ impl super::InstanceShared { self.get_physical_device_properties { // Get these now to avoid borrowing conflicts later - let supports_descriptor_indexing = - capabilities.supports_extension(vk::ExtDescriptorIndexingFn::name()); - let supports_driver_properties = capabilities.properties.api_version - >= vk::API_VERSION_1_2 + let supports_descriptor_indexing = self.driver_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 || capabilities.supports_extension(vk::KhrDriverPropertiesFn::name()); - let mut builder = vk::PhysicalDeviceProperties2::builder(); + let mut builder = vk::PhysicalDeviceProperties2KHR::builder(); if supports_descriptor_indexing { let next = capabilities @@ -1404,10 +1443,10 @@ impl crate::Adapter for super::Adapter { Tfc::SAMPLED_LINEAR, features.contains(vk::FormatFeatureFlags::SAMPLED_IMAGE_FILTER_LINEAR), ); - flags.set( - Tfc::SAMPLED_MINMAX, - features.contains(vk::FormatFeatureFlags::SAMPLED_IMAGE_FILTER_MINMAX), - ); + // flags.set( + // Tfc::SAMPLED_MINMAX, + // features.contains(vk::FormatFeatureFlags::SAMPLED_IMAGE_FILTER_MINMAX), + // ); flags.set( Tfc::STORAGE | Tfc::STORAGE_READ_WRITE, features.contains(vk::FormatFeatureFlags::STORAGE_IMAGE), diff --git a/wgpu-hal/src/vulkan/instance.rs b/wgpu-hal/src/vulkan/instance.rs index 0f828e3a26..5dd52f6dde 100644 --- a/wgpu-hal/src/vulkan/instance.rs +++ b/wgpu-hal/src/vulkan/instance.rs @@ -153,6 +153,7 @@ impl super::Instance { pub fn required_extensions( entry: &ash::Entry, + driver_api_version: u32, flags: crate::InstanceFlags, ) -> Result, crate::InstanceError> { let instance_extensions = entry @@ -164,6 +165,8 @@ impl super::Instance { // Check our extensions against the available extensions let mut extensions: Vec<&'static CStr> = Vec::new(); + + // VK_KHR_surface extensions.push(khr::Surface::name()); // Platform-specific WSI extensions @@ -172,29 +175,40 @@ impl super::Instance { not(target_os = "android"), not(target_os = "macos") )) { + // VK_KHR_xlib_surface extensions.push(khr::XlibSurface::name()); + // VK_KHR_xcb_surface extensions.push(khr::XcbSurface::name()); + // VK_KHR_wayland_surface extensions.push(khr::WaylandSurface::name()); } if cfg!(target_os = "android") { + // VK_KHR_android_surface extensions.push(khr::AndroidSurface::name()); } if cfg!(target_os = "windows") { + // VK_KHR_win32_surface extensions.push(khr::Win32Surface::name()); } if cfg!(target_os = "macos") { + // VK_EXT_metal_surface extensions.push(ext::MetalSurface::name()); } if flags.contains(crate::InstanceFlags::DEBUG) { + // VK_EXT_debug_utils extensions.push(ext::DebugUtils::name()); } - extensions.push(vk::KhrGetPhysicalDeviceProperties2Fn::name()); - + // VK_EXT_swapchain_colorspace // Provid wide color gamut extensions.push(vk::ExtSwapchainColorspaceFn::name()); + // VK_KHR_get_physical_device_properties2 + if driver_api_version < vk::API_VERSION_1_1 { + extensions.push(vk::KhrGetPhysicalDeviceProperties2Fn::name()); + } + // Only keep available extensions. extensions.retain(|&ext| { if instance_extensions.iter().any(|inst_ext| { @@ -262,10 +276,8 @@ impl super::Instance { None }; - // We can't use any of Vulkan-1.1+ abilities on Vk 1.0 instance, - // so disabling this query helps. let get_physical_device_properties = if driver_api_version >= vk::API_VERSION_1_1 - && extensions.contains(&khr::GetPhysicalDeviceProperties2::name()) + || extensions.contains(&khr::GetPhysicalDeviceProperties2::name()) { log::info!("Enabling device properties2"); Some(khr::GetPhysicalDeviceProperties2::new( @@ -519,7 +531,7 @@ impl crate::Instance for super::Instance { }, ); - let extensions = Self::required_extensions(&entry, desc.flags)?; + let extensions = Self::required_extensions(&entry, driver_api_version, desc.flags)?; let instance_layers = entry.enumerate_instance_layer_properties().map_err(|e| { log::info!("enumerate_instance_layer_properties: {:?}", e); From 61d375ed0db2e6df18963537b59e478dcba0e2a4 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Thu, 22 Dec 2022 19:29:37 +0100 Subject: [PATCH 2/5] add changelog entry --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ba0e7d3cf5..a637394457 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -183,6 +183,10 @@ Additionally `Surface::get_default_config` now returns an Option and returns Non - Fixed WebGL not displaying srgb targets correctly if a non-screen filling viewport was previously set. By @Wumpf in [#3093](https://github.com/gfx-rs/wgpu/pull/3093) - Fix disallowing multisampling for float textures if otherwise supported. By @Wumpf in [#3183](https://github.com/gfx-rs/wgpu/pull/3183) +#### Vulkan + +- Document and improve extension detection. By @teoxoy in [#3327](https://github.com/gfx-rs/wgpu/pull/3327) + #### deno-webgpu - Let `setVertexBuffer` and `setIndexBuffer` calls on From e54ae89b940ddb11e77da9031529ab9ace04236b Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Wed, 4 Jan 2023 18:45:19 +0100 Subject: [PATCH 3/5] request `VK_KHR_get_physical_device_properties2` unconditionally --- wgpu-hal/src/vulkan/instance.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/wgpu-hal/src/vulkan/instance.rs b/wgpu-hal/src/vulkan/instance.rs index 5dd52f6dde..910d7cae06 100644 --- a/wgpu-hal/src/vulkan/instance.rs +++ b/wgpu-hal/src/vulkan/instance.rs @@ -153,7 +153,7 @@ impl super::Instance { pub fn required_extensions( entry: &ash::Entry, - driver_api_version: u32, + _driver_api_version: u32, flags: crate::InstanceFlags, ) -> Result, crate::InstanceError> { let instance_extensions = entry @@ -205,9 +205,11 @@ impl super::Instance { extensions.push(vk::ExtSwapchainColorspaceFn::name()); // VK_KHR_get_physical_device_properties2 - if driver_api_version < vk::API_VERSION_1_1 { - extensions.push(vk::KhrGetPhysicalDeviceProperties2Fn::name()); - } + // Even though the extension was promoted to Vulkan 1.1, we still require the extension + // so that we don't have to conditionally use the functions provided by the 1.1 instance + // if driver_api_version < vk::API_VERSION_1_1 { + extensions.push(vk::KhrGetPhysicalDeviceProperties2Fn::name()); + // } // Only keep available extensions. extensions.retain(|&ext| { From 3da7be59f1c6d58864a0069da3da5d7421eb32f7 Mon Sep 17 00:00:00 2001 From: Teodor Tanasoaia <28601907+teoxoy@users.noreply.github.com> Date: Thu, 5 Jan 2023 11:01:00 +0100 Subject: [PATCH 4/5] remove commented code --- wgpu-hal/src/vulkan/instance.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/wgpu-hal/src/vulkan/instance.rs b/wgpu-hal/src/vulkan/instance.rs index 910d7cae06..a8b4399437 100644 --- a/wgpu-hal/src/vulkan/instance.rs +++ b/wgpu-hal/src/vulkan/instance.rs @@ -207,9 +207,7 @@ impl super::Instance { // VK_KHR_get_physical_device_properties2 // Even though the extension was promoted to Vulkan 1.1, we still require the extension // so that we don't have to conditionally use the functions provided by the 1.1 instance - // if driver_api_version < vk::API_VERSION_1_1 { extensions.push(vk::KhrGetPhysicalDeviceProperties2Fn::name()); - // } // Only keep available extensions. extensions.retain(|&ext| { From 1e966d834f8a234f49a8c3ca758c760e3a457296 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Thu, 5 Jan 2023 11:05:08 +0100 Subject: [PATCH 5/5] remove vk 1.1 check --- wgpu-hal/src/vulkan/instance.rs | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/wgpu-hal/src/vulkan/instance.rs b/wgpu-hal/src/vulkan/instance.rs index a8b4399437..bb0e333098 100644 --- a/wgpu-hal/src/vulkan/instance.rs +++ b/wgpu-hal/src/vulkan/instance.rs @@ -276,17 +276,16 @@ impl super::Instance { None }; - let get_physical_device_properties = if driver_api_version >= vk::API_VERSION_1_1 - || extensions.contains(&khr::GetPhysicalDeviceProperties2::name()) - { - log::info!("Enabling device properties2"); - Some(khr::GetPhysicalDeviceProperties2::new( - &entry, - &raw_instance, - )) - } else { - None - }; + let get_physical_device_properties = + if extensions.contains(&khr::GetPhysicalDeviceProperties2::name()) { + log::info!("Enabling device properties2"); + Some(khr::GetPhysicalDeviceProperties2::new( + &entry, + &raw_instance, + )) + } else { + None + }; Ok(Self { shared: Arc::new(super::InstanceShared {