Test And Normalize Vertex Behavior on All Backends (#4723)

Co-authored-by: teoxoy <28601907+teoxoy@users.noreply.github.com>
gfx-rs · Nov 21, 2023 · 1df98d9 · 1df98d9
1 parent a820a3f
commit 1df98d9
Show file tree

Hide file tree

Showing 37 changed files with 1,068 additions and 503 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -40,9 +40,17 @@ Bottom level categories:
 
 ## Unreleased
 
+### New Features
+
+#### General
+- Added `DownlevelFlags::VERTEX_AND_INSTANCE_INDEX_RESPECTS_RESPECTIVE_FIRST_VALUE_IN_INDIRECT_DRAW` to know if `@builtin(vertex_index)` and `@builtin(instance_index)` will respect the `first_vertex` / `first_instance` in indirect calls. If this is not present, both will always start counting from 0. Currently enabled on all backends except DX12. By @cwfitzgerald in [#4722](https://github.com/gfx-rs/wgpu/pull/4722)
+
+#### OpenGL
+- `@builtin(instance_index)` now properly reflects the range provided in the draw call instead of always counting from 0. By @cwfitzgerald in [#4722](https://github.com/gfx-rs/wgpu/pull/4722).
+
 ### Changes
 
-- Arcanization of wgpu core resources: 
+- Arcanization of wgpu core resources:
 Removed Token and LifeTime related management
 Removed RefCount and MultiRefCount in favour of using only Arc internal reference count
 Removing mut from resources and added instead internal members locks on demand or atomics operations
@@ -65,6 +73,7 @@ By @gents83 in [#3626](https://github.com/gfx-rs/wgpu/pull/3626) and tnx also to
 
 - Log vulkan validation layer messages during instance creation and destruction: By @exrook in [#4586](https://github.com/gfx-rs/wgpu/pull/4586)
 - `TextureFormat::block_size` is deprecated, use `TextureFormat::block_copy_size` instead: By @wumpf in [#4647](https://github.com/gfx-rs/wgpu/pull/4647)
+- Rename of `DispatchIndirect`, `DrawIndexedIndirect`, and `DrawIndirect` types in the `wgpu::util` module to `DispatchIndirectArgs`, `DrawIndexedIndirectArgs`, and `DrawIndirectArgs`. By @cwfitzgerald in [#4723](https://github.com/gfx-rs/wgpu/pull/4723).
 
 #### Safe `Surface` creation
 

diff --git a/d3d12/src/command_list.rs b/d3d12/src/command_list.rs
@@ -213,29 +213,29 @@ impl GraphicsCommandList {
         &self,
         num_vertices: VertexCount,
         num_instances: InstanceCount,
-        start_vertex: VertexCount,
-        start_instance: InstanceCount,
+        first_vertex: VertexCount,
+        first_instance: InstanceCount,
     ) {
         unsafe {
-            self.DrawInstanced(num_vertices, num_instances, start_vertex, start_instance);
+            self.DrawInstanced(num_vertices, num_instances, first_vertex, first_instance);
         }
     }
 
     pub fn draw_indexed(
         &self,
         num_indices: IndexCount,
         num_instances: InstanceCount,
-        start_index: IndexCount,
+        first_index: IndexCount,
         base_vertex: VertexOffset,
-        start_instance: InstanceCount,
+        first_instance: InstanceCount,
     ) {
         unsafe {
             self.DrawIndexedInstanced(
                 num_indices,
                 num_instances,
-                start_index,
+                first_index,
                 base_vertex,
-                start_instance,
+                first_instance,
             );
         }
     }

diff --git a/naga/src/back/glsl/features.rs b/naga/src/back/glsl/features.rs
@@ -1,5 +1,6 @@
 use super::{BackendResult, Error, Version, Writer};
 use crate::{
+    back::glsl::{Options, WriterFlags},
     AddressSpace, Binding, Expression, Handle, ImageClass, ImageDimension, Interpolation, Sampling,
     Scalar, ScalarKind, ShaderStage, StorageFormat, Type, TypeInner,
 };
@@ -43,6 +44,10 @@ bitflags::bitflags! {
         const IMAGE_SIZE = 1 << 20;
         /// Dual source blending
         const DUAL_SOURCE_BLENDING = 1 << 21;
+        /// Instance index
+        ///
+        /// We can always support this, either through the language or a polyfill
+        const INSTANCE_INDEX = 1 << 22;
     }
 }
 
@@ -63,6 +68,11 @@ impl FeaturesManager {
         self.0 |= features
     }
 
+    /// Checks if the list of features [`Features`] contains the specified [`Features`]
+    pub fn contains(&mut self, features: Features) -> bool {
+        self.0.contains(features)
+    }
+
     /// Checks that all required [`Features`] are available for the specified
     /// [`Version`] otherwise returns an [`Error::MissingFeatures`].
     pub fn check_availability(&self, version: Version) -> BackendResult {
@@ -129,86 +139,86 @@ impl FeaturesManager {
     /// # Notes
     /// This won't check for feature availability so it might output extensions that aren't even
     /// supported.[`check_availability`](Self::check_availability) will check feature availability
-    pub fn write(&self, version: Version, mut out: impl Write) -> BackendResult {
-        if self.0.contains(Features::COMPUTE_SHADER) && !version.is_es() {
+    pub fn write(&self, options: &Options, mut out: impl Write) -> BackendResult {
+        if self.0.contains(Features::COMPUTE_SHADER) && !options.version.is_es() {
             // https://www.khronos.org/registry/OpenGL/extensions/ARB/ARB_compute_shader.txt
             writeln!(out, "#extension GL_ARB_compute_shader : require")?;
         }
 
-        if self.0.contains(Features::BUFFER_STORAGE) && !version.is_es() {
+        if self.0.contains(Features::BUFFER_STORAGE) && !options.version.is_es() {
             // https://www.khronos.org/registry/OpenGL/extensions/ARB/ARB_shader_storage_buffer_object.txt
             writeln!(
                 out,
                 "#extension GL_ARB_shader_storage_buffer_object : require"
             )?;
         }
 
-        if self.0.contains(Features::DOUBLE_TYPE) && version < Version::Desktop(400) {
+        if self.0.contains(Features::DOUBLE_TYPE) && options.version < Version::Desktop(400) {
             // https://www.khronos.org/registry/OpenGL/extensions/ARB/ARB_gpu_shader_fp64.txt
             writeln!(out, "#extension GL_ARB_gpu_shader_fp64 : require")?;
         }
 
         if self.0.contains(Features::CUBE_TEXTURES_ARRAY) {
-            if version.is_es() {
+            if options.version.is_es() {
                 // https://www.khronos.org/registry/OpenGL/extensions/EXT/EXT_texture_cube_map_array.txt
                 writeln!(out, "#extension GL_EXT_texture_cube_map_array : require")?;
-            } else if version < Version::Desktop(400) {
+            } else if options.version < Version::Desktop(400) {
                 // https://www.khronos.org/registry/OpenGL/extensions/ARB/ARB_texture_cube_map_array.txt
                 writeln!(out, "#extension GL_ARB_texture_cube_map_array : require")?;
             }
         }
 
-        if self.0.contains(Features::MULTISAMPLED_TEXTURE_ARRAYS) && version.is_es() {
+        if self.0.contains(Features::MULTISAMPLED_TEXTURE_ARRAYS) && options.version.is_es() {
             // https://www.khronos.org/registry/OpenGL/extensions/OES/OES_texture_storage_multisample_2d_array.txt
             writeln!(
                 out,
                 "#extension GL_OES_texture_storage_multisample_2d_array : require"
             )?;
         }
 
-        if self.0.contains(Features::ARRAY_OF_ARRAYS) && version < Version::Desktop(430) {
+        if self.0.contains(Features::ARRAY_OF_ARRAYS) && options.version < Version::Desktop(430) {
             // https://www.khronos.org/registry/OpenGL/extensions/ARB/ARB_arrays_of_arrays.txt
             writeln!(out, "#extension ARB_arrays_of_arrays : require")?;
         }
 
         if self.0.contains(Features::IMAGE_LOAD_STORE) {
-            if self.0.contains(Features::FULL_IMAGE_FORMATS) && version.is_es() {
+            if self.0.contains(Features::FULL_IMAGE_FORMATS) && options.version.is_es() {
                 // https://www.khronos.org/registry/OpenGL/extensions/NV/NV_image_formats.txt
                 writeln!(out, "#extension GL_NV_image_formats : require")?;
             }
 
-            if version < Version::Desktop(420) {
+            if options.version < Version::Desktop(420) {
                 // https://www.khronos.org/registry/OpenGL/extensions/ARB/ARB_shader_image_load_store.txt
                 writeln!(out, "#extension GL_ARB_shader_image_load_store : require")?;
             }
         }
 
         if self.0.contains(Features::CONSERVATIVE_DEPTH) {
-            if version.is_es() {
+            if options.version.is_es() {
                 // https://www.khronos.org/registry/OpenGL/extensions/EXT/EXT_conservative_depth.txt
                 writeln!(out, "#extension GL_EXT_conservative_depth : require")?;
             }
 
-            if version < Version::Desktop(420) {
+            if options.version < Version::Desktop(420) {
                 // https://www.khronos.org/registry/OpenGL/extensions/ARB/ARB_conservative_depth.txt
                 writeln!(out, "#extension GL_ARB_conservative_depth : require")?;
             }
         }
 
         if (self.0.contains(Features::CLIP_DISTANCE) || self.0.contains(Features::CULL_DISTANCE))
-            && version.is_es()
+            && options.version.is_es()
         {
             // https://www.khronos.org/registry/OpenGL/extensions/EXT/EXT_clip_cull_distance.txt
             writeln!(out, "#extension GL_EXT_clip_cull_distance : require")?;
         }
 
-        if self.0.contains(Features::SAMPLE_VARIABLES) && version.is_es() {
+        if self.0.contains(Features::SAMPLE_VARIABLES) && options.version.is_es() {
             // https://www.khronos.org/registry/OpenGL/extensions/OES/OES_sample_variables.txt
             writeln!(out, "#extension GL_OES_sample_variables : require")?;
         }
 
         if self.0.contains(Features::MULTI_VIEW) {
-            if let Version::Embedded { is_webgl: true, .. } = version {
+            if let Version::Embedded { is_webgl: true, .. } = options.version {
                 // https://www.khronos.org/registry/OpenGL/extensions/OVR/OVR_multiview2.txt
                 writeln!(out, "#extension GL_OVR_multiview2 : require")?;
             } else {
@@ -225,15 +235,22 @@ impl FeaturesManager {
             )?;
         }
 
-        if self.0.contains(Features::TEXTURE_LEVELS) && version < Version::Desktop(430) {
+        if self.0.contains(Features::TEXTURE_LEVELS) && options.version < Version::Desktop(430) {
             // https://www.khronos.org/registry/OpenGL/extensions/ARB/ARB_texture_query_levels.txt
             writeln!(out, "#extension GL_ARB_texture_query_levels : require")?;
         }
-        if self.0.contains(Features::DUAL_SOURCE_BLENDING) && version.is_es() {
+        if self.0.contains(Features::DUAL_SOURCE_BLENDING) && options.version.is_es() {
             // https://registry.khronos.org/OpenGL/extensions/EXT/EXT_blend_func_extended.txt
             writeln!(out, "#extension GL_EXT_blend_func_extended : require")?;
         }
 
+        if self.0.contains(Features::INSTANCE_INDEX) {
+            if options.writer_flags.contains(WriterFlags::DRAW_PARAMETERS) {
+                // https://registry.khronos.org/OpenGL/extensions/ARB/ARB_shader_draw_parameters.txt
+                writeln!(out, "#extension GL_ARB_shader_draw_parameters : require")?;
+            }
+        }
+
         Ok(())
     }
 }
@@ -490,6 +507,9 @@ impl<'a, W> Writer<'a, W> {
                             crate::BuiltIn::ViewIndex => {
                                 self.features.request(Features::MULTI_VIEW)
                             }
+                            crate::BuiltIn::InstanceIndex => {
+                                self.features.request(Features::INSTANCE_INDEX)
+                            }
                             _ => {}
                         },
                         Binding::Location {

diff --git a/naga/src/back/glsl/keywords.rs b/naga/src/back/glsl/keywords.rs
@@ -480,4 +480,5 @@ pub const RESERVED_KEYWORDS: &[&str] = &[
     // Naga utilities:
     super::MODF_FUNCTION,
     super::FREXP_FUNCTION,
+    super::FIRST_INSTANCE_BINDING,
 ];