Merge #1522

1522: Reduce feature flag surface for descriptor arrays. r=cwfitzgerald a=ElectronicRU DYNAMIC_INDEXING is checked by default, since WGPU doesn't allow for any kind of specialization constants and constant indexing is nigh useless. STORAGE_RESOURCE_BINDING_ARRAY is enabled iff the device supports: - both or neither of uniform and buffer arrays with dynamic indexing; - both or neither of sampled and storage images with dynamic indexing. NONUNIFORM_INDEXING is enabled iff for ALL types of descriptor arrays *that are supported for dynamic indexing*, nonuniform indexing is also allowed. These flags have a limitation in that some platforms (eg. https://vulkan.gpuinfo.org/displayreport.php?id=11692#features_core_12) may support a strange subset of those features (example device supporting non-uniform indexing for textures and storage buffers, but NOT for storage textures or uniform buffers). In that case feature will be reported as missing, even though it is partially present. In the author's opinion, this flag set is convenient for the users to query; however, due to aforementioned limitations, it would be desirable to obtain statistics about "edge-case" platforms and what percentage of reports they comprise. **Connections** Implementation of proposal outlined in #1515. Co-authored-by: Alex S <[email protected]>
gfx-rs · Jun 28, 2021 · aed736d · aed736d
2 parents ce753cf + b4b3dd7
commit aed736d
Show file tree

Hide file tree

Showing 8 changed files with 200 additions and 257 deletions.
diff --git a/player/tests/data/buffer-clear.ron b/player/tests/data/buffer-clear.ron
@@ -1,5 +1,5 @@
 (
-    features: (bits: 0x0000_0001_0000_0000),
+    features: (bits: 0x0000_0004_0000_0000),
     expectations: [
         (
             name: "basic",

diff --git a/player/tests/data/clear-buffer-image.ron b/player/tests/data/clear-buffer-image.ron
@@ -1,5 +1,5 @@
 (
-    features: (bits: 0x0000_0200_0000_0000),
+    features: (bits: 0x0000_0004_0000_0000),
     expectations: [
         (
             name: "Quad",
@@ -95,4 +95,4 @@
             )
         ]),
     ],
-)
+)
diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs
@@ -988,11 +988,20 @@ impl<A: HalApi> Device<A> {
                 Bt::Buffer {
                     ty: wgt::BufferBindingType::Storage { read_only },
                     ..
-                } => (Some(wgt::Features::BUFFER_BINDING_ARRAY), !read_only),
+                } => (
+                    Some(
+                        wgt::Features::BUFFER_BINDING_ARRAY
+                            | wgt::Features::STORAGE_RESOURCE_BINDING_ARRAY,
+                    ),
+                    !read_only,
+                ),
                 Bt::Sampler { .. } => (None, false),
-                Bt::Texture { .. } => (Some(wgt::Features::SAMPLED_TEXTURE_BINDING_ARRAY), false),
+                Bt::Texture { .. } => (Some(wgt::Features::TEXTURE_BINDING_ARRAY), false),
                 Bt::StorageTexture { access, .. } => (
-                    Some(wgt::Features::STORAGE_TEXTURE_BINDING_ARRAY),
+                    Some(
+                        wgt::Features::TEXTURE_BINDING_ARRAY
+                            | wgt::Features::STORAGE_RESOURCE_BINDING_ARRAY,
+                    ),
                     match access {
                         wgt::StorageTextureAccess::ReadOnly => false,
                         wgt::StorageTextureAccess::WriteOnly => true,

diff --git a/wgpu-hal/src/metal/adapter.rs b/wgpu-hal/src/metal/adapter.rs
@@ -848,22 +848,18 @@ impl super::PrivateCapabilities {
             | F::TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES;
 
         features.set(
-            F::SAMPLED_TEXTURE_BINDING_ARRAY
-                | F::SAMPLED_TEXTURE_ARRAY_DYNAMIC_INDEXING
-                | F::SAMPLED_TEXTURE_ARRAY_NON_UNIFORM_INDEXING,
+            F::TEXTURE_BINDING_ARRAY | F::RESOURCE_BINDING_ARRAY_NON_UNIFORM_INDEXING,
             self.msl_version >= MTLLanguageVersion::V2_0 && self.supports_arrays_of_textures,
         );
         //// XXX: this is technically not true, as read-only storage images can be used in arrays
         //// on precisely the same conditions that sampled textures can. But texel fetch from a
         //// sampled texture is a thing; should we bother introducing another feature flag?
-        features.set(
-            F::STORAGE_TEXTURE_BINDING_ARRAY
-                | F::STORAGE_TEXTURE_ARRAY_DYNAMIC_INDEXING
-                | F::STORAGE_TEXTURE_ARRAY_NON_UNIFORM_INDEXING,
-            self.msl_version >= MTLLanguageVersion::V2_2
-                && self.supports_arrays_of_textures
-                && self.supports_arrays_of_textures_write,
-        );
+        if self.msl_version >= MTLLanguageVersion::V2_2
+            && self.supports_arrays_of_textures
+            && self.supports_arrays_of_textures_write
+        {
+            features.insert(F::STORAGE_RESOURCE_BINDING_ARRAY);
+        }
         features.set(
             F::ADDRESS_MODE_CLAMP_TO_BORDER,
             self.sampler_clamp_to_border,

diff --git a/wgpu-hal/src/vulkan/adapter.rs b/wgpu-hal/src/vulkan/adapter.rs
@@ -11,9 +11,7 @@ use std::{ffi::CStr, mem, ptr, sync::Arc};
 
 //TODO: const fn?
 fn indexing_features() -> wgt::Features {
-    wgt::Features::UNIFORM_BUFFER_ARRAY_DYNAMIC_INDEXING
-        | wgt::Features::SAMPLED_TEXTURE_ARRAY_DYNAMIC_INDEXING
-        | wgt::Features::STORAGE_BUFFER_ARRAY_DYNAMIC_INDEXING
+    wgt::Features::BUFFER_BINDING_ARRAY | wgt::Features::TEXTURE_BINDING_ARRAY
 }
 
 /// Aggregate of the `vk::PhysicalDevice*Features` structs used by `gfx`.
@@ -106,17 +104,19 @@ impl PhysicalDeviceFeatures {
                 //.shader_image_gather_extended(
                 //.shader_storage_image_extended_formats(
                 .shader_uniform_buffer_array_dynamic_indexing(
-                    requested_features
-                        .contains(wgt::Features::UNIFORM_BUFFER_ARRAY_DYNAMIC_INDEXING),
+                    requested_features.contains(wgt::Features::BUFFER_BINDING_ARRAY),
                 )
+                .shader_storage_buffer_array_dynamic_indexing(requested_features.contains(
+                    wgt::Features::BUFFER_BINDING_ARRAY
+                        | wgt::Features::STORAGE_RESOURCE_BINDING_ARRAY,
+                ))
                 .shader_sampled_image_array_dynamic_indexing(
-                    requested_features
-                        .contains(wgt::Features::SAMPLED_TEXTURE_ARRAY_DYNAMIC_INDEXING),
-                )
-                .shader_storage_buffer_array_dynamic_indexing(
-                    requested_features
-                        .contains(wgt::Features::STORAGE_BUFFER_ARRAY_DYNAMIC_INDEXING),
+                    requested_features.contains(wgt::Features::TEXTURE_BINDING_ARRAY),
                 )
+                .shader_storage_buffer_array_dynamic_indexing(requested_features.contains(
+                    wgt::Features::TEXTURE_BINDING_ARRAY
+                        | wgt::Features::STORAGE_RESOURCE_BINDING_ARRAY,
+                ))
                 //.shader_storage_image_array_dynamic_indexing(
                 //.shader_clip_distance(requested_features.contains(wgt::Features::SHADER_CLIP_DISTANCE))
                 //.shader_cull_distance(requested_features.contains(wgt::Features::SHADER_CULL_DISTANCE))
@@ -135,18 +135,30 @@ impl PhysicalDeviceFeatures {
                         .descriptor_indexing(requested_features.intersects(indexing_features()))
                         .shader_sampled_image_array_non_uniform_indexing(
                             requested_features.contains(
-                                wgt::Features::SAMPLED_TEXTURE_ARRAY_NON_UNIFORM_INDEXING,
+                                wgt::Features::TEXTURE_BINDING_ARRAY
+                                    | wgt::Features::RESOURCE_BINDING_ARRAY_NON_UNIFORM_INDEXING,
                             ),
                         )
                         .shader_storage_image_array_non_uniform_indexing(
                             requested_features.contains(
-                                wgt::Features::STORAGE_TEXTURE_ARRAY_NON_UNIFORM_INDEXING,
+                                wgt::Features::TEXTURE_BINDING_ARRAY
+                                    | wgt::Features::STORAGE_RESOURCE_BINDING_ARRAY
+                                    | wgt::Features::RESOURCE_BINDING_ARRAY_NON_UNIFORM_INDEXING,
                             ),
                         )
                         //.shader_storage_buffer_array_non_uniform_indexing(
                         .shader_uniform_buffer_array_non_uniform_indexing(
-                            requested_features
-                                .contains(wgt::Features::UNIFORM_BUFFER_ARRAY_NON_UNIFORM_INDEXING),
+                            requested_features.contains(
+                                wgt::Features::BUFFER_BINDING_ARRAY
+                                    | wgt::Features::RESOURCE_BINDING_ARRAY_NON_UNIFORM_INDEXING,
+                            ),
+                        )
+                        .shader_storage_buffer_array_non_uniform_indexing(
+                            requested_features.contains(
+                                wgt::Features::BUFFER_BINDING_ARRAY
+                                    | wgt::Features::STORAGE_RESOURCE_BINDING_ARRAY
+                                    | wgt::Features::RESOURCE_BINDING_ARRAY_NON_UNIFORM_INDEXING,
+                            ),
                         )
                         .runtime_descriptor_array(
                             requested_features.contains(wgt::Features::UNSIZED_BINDING_ARRAY),
@@ -165,18 +177,30 @@ impl PhysicalDeviceFeatures {
                     vk::PhysicalDeviceDescriptorIndexingFeaturesEXT::builder()
                         .shader_sampled_image_array_non_uniform_indexing(
                             requested_features.contains(
-                                wgt::Features::SAMPLED_TEXTURE_ARRAY_NON_UNIFORM_INDEXING,
+                                wgt::Features::TEXTURE_BINDING_ARRAY
+                                    | wgt::Features::RESOURCE_BINDING_ARRAY_NON_UNIFORM_INDEXING,
                             ),
                         )
                         .shader_storage_image_array_non_uniform_indexing(
                             requested_features.contains(
-                                wgt::Features::STORAGE_TEXTURE_ARRAY_NON_UNIFORM_INDEXING,
+                                wgt::Features::TEXTURE_BINDING_ARRAY
+                                    | wgt::Features::STORAGE_RESOURCE_BINDING_ARRAY
+                                    | wgt::Features::RESOURCE_BINDING_ARRAY_NON_UNIFORM_INDEXING,
                             ),
                         )
                         //.shader_storage_buffer_array_non_uniform_indexing(
                         .shader_uniform_buffer_array_non_uniform_indexing(
-                            requested_features
-                                .contains(wgt::Features::UNIFORM_BUFFER_ARRAY_NON_UNIFORM_INDEXING),
+                            requested_features.contains(
+                                wgt::Features::BUFFER_BINDING_ARRAY
+                                    | wgt::Features::RESOURCE_BINDING_ARRAY_NON_UNIFORM_INDEXING,
+                            ),
+                        )
+                        .shader_storage_buffer_array_non_uniform_indexing(
+                            requested_features.contains(
+                                wgt::Features::BUFFER_BINDING_ARRAY
+                                    | wgt::Features::STORAGE_RESOURCE_BINDING_ARRAY
+                                    | wgt::Features::RESOURCE_BINDING_ARRAY_NON_UNIFORM_INDEXING,
+                            ),
                         )
                         .runtime_descriptor_array(
                             requested_features.contains(wgt::Features::UNSIZED_BINDING_ARRAY),
@@ -207,9 +231,6 @@ impl PhysicalDeviceFeatures {
             | F::MAPPABLE_PRIMARY_BUFFERS
             | F::PUSH_CONSTANTS
             | F::ADDRESS_MODE_CLAMP_TO_BORDER
-            | F::SAMPLED_TEXTURE_BINDING_ARRAY
-            | F::STORAGE_TEXTURE_BINDING_ARRAY
-            | F::BUFFER_BINDING_ARRAY
             | F::TIMESTAMP_QUERY
             | F::PIPELINE_STATISTICS_QUERY
             | F::TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES;
@@ -250,21 +271,28 @@ impl PhysicalDeviceFeatures {
         //if self.core.shader_image_gather_extended != 0 {
         //if self.core.shader_storage_image_extended_formats != 0 {
         features.set(
-            F::UNIFORM_BUFFER_ARRAY_DYNAMIC_INDEXING,
+            F::BUFFER_BINDING_ARRAY,
             self.core.shader_uniform_buffer_array_dynamic_indexing != 0,
         );
         features.set(
-            F::SAMPLED_TEXTURE_ARRAY_DYNAMIC_INDEXING,
+            F::TEXTURE_BINDING_ARRAY,
             self.core.shader_sampled_image_array_dynamic_indexing != 0,
         );
-        features.set(
-            F::STORAGE_TEXTURE_ARRAY_DYNAMIC_INDEXING,
-            self.core.shader_storage_image_array_dynamic_indexing != 0,
-        );
-        features.set(
-            F::STORAGE_BUFFER_ARRAY_DYNAMIC_INDEXING,
-            self.core.shader_storage_buffer_array_dynamic_indexing != 0,
-        );
+        if Self::all_features_supported(
+            &features,
+            &[
+                (
+                    F::BUFFER_BINDING_ARRAY,
+                    self.core.shader_storage_buffer_array_dynamic_indexing,
+                ),
+                (
+                    F::TEXTURE_BINDING_ARRAY,
+                    self.core.shader_storage_image_array_dynamic_indexing,
+                ),
+            ],
+        ) {
+            features.insert(F::STORAGE_RESOURCE_BINDING_ARRAY);
+        }
         //if self.core.shader_storage_image_array_dynamic_indexing != 0 {
         //if self.core.shader_clip_distance != 0 {
         //if self.core.shader_cull_distance != 0 {
@@ -284,15 +312,29 @@ impl PhysicalDeviceFeatures {
         );
 
         if let Some(ref vulkan_1_2) = self.vulkan_1_2 {
-            if vulkan_1_2.shader_sampled_image_array_non_uniform_indexing != 0 {
-                features |= F::SAMPLED_TEXTURE_ARRAY_NON_UNIFORM_INDEXING;
-            }
-            if vulkan_1_2.shader_storage_image_array_non_uniform_indexing != 0 {
-                features |= F::STORAGE_TEXTURE_ARRAY_NON_UNIFORM_INDEXING;
-            }
-            //if vulkan_1_2.shader_storage_buffer_array_non_uniform_indexing != 0 {
-            if vulkan_1_2.shader_uniform_buffer_array_non_uniform_indexing != 0 {
-                features |= F::UNIFORM_BUFFER_ARRAY_NON_UNIFORM_INDEXING;
+            const STORAGE: F = F::STORAGE_RESOURCE_BINDING_ARRAY;
+            if Self::all_features_supported(
+                &features,
+                &[
+                    (
+                        F::BUFFER_BINDING_ARRAY,
+                        vulkan_1_2.shader_uniform_buffer_array_non_uniform_indexing,
+                    ),
+                    (
+                        F::TEXTURE_BINDING_ARRAY,
+                        vulkan_1_2.shader_sampled_image_array_non_uniform_indexing,
+                    ),
+                    (
+                        F::BUFFER_BINDING_ARRAY | STORAGE,
+                        vulkan_1_2.shader_storage_buffer_array_non_uniform_indexing,
+                    ),
+                    (
+                        F::TEXTURE_BINDING_ARRAY | STORAGE,
+                        vulkan_1_2.shader_storage_image_array_non_uniform_indexing,
+                    ),
+                ],
+            ) {
+                features.insert(F::RESOURCE_BINDING_ARRAY_NON_UNIFORM_INDEXING);
             }
             if vulkan_1_2.runtime_descriptor_array != 0 {
                 features |= F::UNSIZED_BINDING_ARRAY;
@@ -305,15 +347,29 @@ impl PhysicalDeviceFeatures {
         }
 
         if let Some(ref descriptor_indexing) = self.descriptor_indexing {
-            if descriptor_indexing.shader_sampled_image_array_non_uniform_indexing != 0 {
-                features |= F::SAMPLED_TEXTURE_ARRAY_NON_UNIFORM_INDEXING;
-            }
-            if descriptor_indexing.shader_storage_image_array_non_uniform_indexing != 0 {
-                features |= F::STORAGE_TEXTURE_ARRAY_NON_UNIFORM_INDEXING;
-            }
-            //if descriptor_indexing.shader_storage_buffer_array_non_uniform_indexing != 0 {
-            if descriptor_indexing.shader_uniform_buffer_array_non_uniform_indexing != 0 {
-                features |= F::UNIFORM_BUFFER_ARRAY_NON_UNIFORM_INDEXING;
+            const STORAGE: F = F::STORAGE_RESOURCE_BINDING_ARRAY;
+            if Self::all_features_supported(
+                &features,
+                &[
+                    (
+                        F::BUFFER_BINDING_ARRAY,
+                        descriptor_indexing.shader_uniform_buffer_array_non_uniform_indexing,
+                    ),
+                    (
+                        F::TEXTURE_BINDING_ARRAY,
+                        descriptor_indexing.shader_sampled_image_array_non_uniform_indexing,
+                    ),
+                    (
+                        F::BUFFER_BINDING_ARRAY | STORAGE,
+                        descriptor_indexing.shader_storage_buffer_array_non_uniform_indexing,
+                    ),
+                    (
+                        F::TEXTURE_BINDING_ARRAY | STORAGE,
+                        descriptor_indexing.shader_storage_image_array_non_uniform_indexing,
+                    ),
+                ],
+            ) {
+                features.insert(F::RESOURCE_BINDING_ARRAY_NON_UNIFORM_INDEXING);
             }
             if descriptor_indexing.runtime_descriptor_array != 0 {
                 features |= F::UNSIZED_BINDING_ARRAY;
@@ -322,6 +378,15 @@ impl PhysicalDeviceFeatures {
 
         (features, dl_flags)
     }
+
+    fn all_features_supported(
+        features: &wgt::Features,
+        implications: &[(wgt::Features, vk::Bool32)],
+    ) -> bool {
+        implications
+            .iter()
+            .all(|&(flag, support)| !features.contains(flag) || support != 0)
+    }
 }
 
 /// Information gathered about a physical device capabilities.