Allow unconsumed inputs in fragment shaders by removing them from vertex

outputs when generating HLSL.

Fixes gfx-rs#3748

* Add naga::back::hlsl::FragmentEntryPoint for providing information
  about the fragment entry point when generating vertex entry points via
  naga::back::hlsl::Writer::write. Vertex outputs not consumed by the
  fragment entry point are omitted in the final output struct.
* Add naga snapshot test for this new feature,
* Remove Features::SHADER_UNUSED_VERTEX_OUTPUT,
  StageError::InputNotConsumed, and associated validation logic.
* Make wgpu dx12 backend pass fragment shader info when generating
  vertex HLSL.
* Add wgpu regression test for allowing unconsumed inputs.

CHANGELOG.md

@@ -68,8 +68,18 @@ By @stefnotch in [#5410](https://github.com/gfx-rs/wgpu/pull/5410)
 
 #### General
 
+- Unconsumed vertex outputs are now always allowed. Removed `StageError::InputNotConsumed`, `Features::SHADER_UNUSED_VERTEX_OUTPUT`, and associated validation. By @Imberflur in [#5531](https://github.com/gfx-rs/wgpu/pull/5531)
+
 #### Naga
 
+- In hlsl-out, allow passing information about the fragment entry point to omit vertex outputs that are not in the fragment inputs. By @Imberflur in [#5531](https://github.com/gfx-rs/wgpu/pull/5531)
+
+  ```diff
+  let writer: naga::back::hlsl::Writer = /* ... */;
+  -writer.write(&module, &module_info);
+  +writer.write(&module, &module_info, None);
+  ```
+
 ### Bug Fixes
 
 ## v0.20.0 (2024-04-28)

deno_webgpu/lib.rs

@@ -360,9 +360,6 @@ fn deserialize_features(features: &wgpu_types::Features) -> Vec<&'static str> {
     if features.contains(wgpu_types::Features::SHADER_EARLY_DEPTH_TEST) {
         return_features.push("shader-early-depth-test");
     }
-    if features.contains(wgpu_types::Features::SHADER_UNUSED_VERTEX_OUTPUT) {
-        return_features.push("shader-unused-vertex-output");
-    }
 
     return_features
 }
@@ -648,10 +645,6 @@ impl From<GpuRequiredFeatures> for wgpu_types::Features {
             wgpu_types::Features::SHADER_EARLY_DEPTH_TEST,
             required_features.0.contains("shader-early-depth-test"),
         );
-        features.set(
-            wgpu_types::Features::SHADER_UNUSED_VERTEX_OUTPUT,
-            required_features.0.contains("shader-unused-vertex-output"),
-        );
 
         features
     }

naga-cli/src/bin/naga.rs

@@ -777,7 +777,7 @@ fn write_output(
 
             let mut buffer = String::new();
             let mut writer = hlsl::Writer::new(&mut buffer, &params.hlsl);
-            writer.write(&module, &info).unwrap_pretty();
+            writer.write(&module, &info, None).unwrap_pretty();
             fs::write(output_path, buffer)?;
         }
         "wgsl" => {

naga/benches/criterion.rs

@@ -226,7 +226,7 @@ fn backends(c: &mut Criterion) {
             let mut string = String::new();
             for &(ref module, ref info) in inputs.iter() {
                 let mut writer = naga::back::hlsl::Writer::new(&mut string, &options);
-                let _ = writer.write(module, info); // may fail on unimplemented things
+                let _ = writer.write(module, info, None); // may fail on unimplemented things
                 string.clear();
             }
         });

naga/src/back/hlsl/mod.rs

@@ -287,6 +287,35 @@ impl Wrapped {
     }
 }
 
+/// A fragment entry point to be considered when generating HLSL for the output interface of vertex
+/// entry points.
+///
+/// This is provided as an optional paramter to [`Writer::write`].
+///
+/// If this is provided, vertex outputs will be removed if they are not inputs of this fragment
+/// entry point. This is necessary for generating correct HLSL when some of the vertex shader
+/// outputs are not consumed by the fragment shader.
+pub struct FragmentEntryPoint<'a> {
+    module: &'a crate::Module,
+    func: &'a crate::Function,
+}
+
+impl<'a> FragmentEntryPoint<'a> {
+    /// Returns `None` if the entry point with the provided name can't be found or isn't a fragment
+    /// entry point.
+    pub fn new(module: &'a crate::Module, ep_name: &'a str) -> Option<Self> {
+        module
+            .entry_points
+            .iter()
+            .find(|ep| ep.name == ep_name)
+            .filter(|ep| ep.stage == crate::ShaderStage::Fragment)
+            .map(|ep| Self {
+                module,
+                func: &ep.function,
+            })
+    }
+}
+
 pub struct Writer<'a, W> {
     out: W,
     names: crate::FastHashMap<proc::NameKey, String>,

naga/src/back/hlsl/writer.rs

@@ -4,7 +4,7 @@ use super::{
         WrappedZeroValue,
     },
     storage::StoreValue,
-    BackendResult, Error, Options,
+    BackendResult, Error, FragmentEntryPoint, Options,
 };
 use crate::{
     back,
@@ -28,6 +28,7 @@ pub(crate) const INSERT_BITS_FUNCTION: &str = "naga_insertBits";
 struct EpStructMember {
     name: String,
     ty: Handle<crate::Type>,
+    // TODO: log error if binding is none?
     // technically, this should always be `Some`
     binding: Option<crate::Binding>,
     index: u32,
@@ -196,6 +197,7 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
         &mut self,
         module: &Module,
         module_info: &valid::ModuleInfo,
+        fragment_entry_point: Option<&FragmentEntryPoint<'_>>,
     ) -> Result<super::ReflectionInfo, Error> {
         if !module.overrides.is_empty() {
             return Err(Error::Override);
@@ -296,7 +298,13 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
         // Write all entry points wrapped structs
         for (index, ep) in module.entry_points.iter().enumerate() {
             let ep_name = self.names[&NameKey::EntryPoint(index as u16)].clone();
-            let ep_io = self.write_ep_interface(module, &ep.function, ep.stage, &ep_name)?;
+            let ep_io = self.write_ep_interface(
+                module,
+                &ep.function,
+                ep.stage,
+                &ep_name,
+                fragment_entry_point,
+            )?;
             self.entry_point_io.push(ep_io);
         }
 
@@ -504,6 +512,7 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
         writeln!(self.out, "}};")?;
         writeln!(self.out)?;
 
+        // See ordering notes on EntryPointInterface fields
         match shader_stage.1 {
             Io::Input => {
                 // bring back the original order
@@ -537,6 +546,8 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
         for arg in func.arguments.iter() {
             match module.types[arg.ty].inner {
                 TypeInner::Struct { ref members, .. } => {
+                    // TODO: what about nested structs? Is that possible? Maybe try an unwrap on
+                    // the binding?
                     for member in members.iter() {
                         let name = self.namer.call_or(&member.name, "member");
                         let index = fake_members.len() as u32;
@@ -573,10 +584,10 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
         result: &crate::FunctionResult,
         stage: ShaderStage,
         entry_point_name: &str,
+        frag_ep: Option<&FragmentEntryPoint<'_>>,
     ) -> Result<EntryPointBinding, Error> {
         let struct_name = format!("{stage:?}Output_{entry_point_name}");
 
-        let mut fake_members = Vec::new();
         let empty = [];
         let members = match module.types[result.ty].inner {
             TypeInner::Struct { ref members, .. } => members,
@@ -586,14 +597,60 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
             }
         };
 
-        for member in members.iter() {
+        // Gather list of fragment input locations. We use this below to remove user-defined
+        // varyings from VS outputs that aren't in the FS inputs. This makes the VS interface match
+        // as long as the FS inputs are a subset of the VS outputs. This is only applied if the
+        // writer is supplied with information about the fragment entry point.
+        let fs_input_locs = if let (Some(frag_ep), ShaderStage::Vertex) = (frag_ep, stage) {
+            let mut fs_input_locs = Vec::new();
+            for arg in frag_ep.func.arguments.iter() {
+                let mut push_if_location = |binding: &Option<crate::Binding>| {
+                    match *binding {
+                        Some(crate::Binding::Location { location, .. }) => {
+                            fs_input_locs.push(location)
+                        }
+                        Some(crate::Binding::BuiltIn(_)) => {}
+                        // Log error?
+                        None => {}
+                    }
+                };
+                match frag_ep.module.types[arg.ty].inner {
+                    TypeInner::Struct { ref members, .. } => {
+                        // TODO: nesting?
+                        for member in members.iter() {
+                            push_if_location(&member.binding);
+                        }
+                    }
+                    _ => push_if_location(&arg.binding),
+                }
+            }
+            fs_input_locs.sort();
+            Some(fs_input_locs)
+        } else {
+            None
+        };
+
+        let mut fake_members = Vec::new();
+        for (index, member) in members.iter().enumerate() {
+            if let Some(ref fs_input_locs) = fs_input_locs {
+                match member.binding {
+                    Some(crate::Binding::Location { location, .. }) => {
+                        if fs_input_locs.binary_search(&location).is_err() {
+                            continue;
+                        }
+                    }
+                    Some(crate::Binding::BuiltIn(_)) => {}
+                    // Log error?
+                    None => {}
+                }
+            }
+
             let member_name = self.namer.call_or(&member.name, "member");
-            let index = fake_members.len() as u32;
             fake_members.push(EpStructMember {
                 name: member_name,
                 ty: member.ty,
                 binding: member.binding.clone(),
-                index,
+                index: index as u32,
             });
         }
 
@@ -609,6 +666,7 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
         func: &crate::Function,
         stage: ShaderStage,
         ep_name: &str,
+        frag_ep: Option<&FragmentEntryPoint<'_>>,
     ) -> Result<EntryPointInterface, Error> {
         Ok(EntryPointInterface {
             input: if !func.arguments.is_empty()
@@ -624,7 +682,7 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
             },
             output: match func.result {
                 Some(ref fr) if fr.binding.is_none() && stage == ShaderStage::Vertex => {
-                    Some(self.write_ep_output_struct(module, fr, stage, ep_name)?)
+                    Some(self.write_ep_output_struct(module, fr, stage, ep_name, frag_ep)?)
                 }
                 _ => None,
             },

naga/tests/in/unconsumed_vertex_outputs_frag.param.ron

@@ -0,0 +1,2 @@
+(
+)

naga/tests/in/unconsumed_vertex_outputs_frag.wgsl

@@ -0,0 +1,13 @@
+// Out of order to test sorting.
+struct FragmentIn {
+    @location(1) value: f32,
+    @location(3) value2: f32,
+    @builtin(position) position: vec4<f32>,
+    // @location(0) unused_value: f32,
+    // @location(2) unused_value2: vec4<f32>,
+}
+
+@fragment
+fn fs_main(v_out: FragmentIn) -> @location(0) vec4<f32> {
+    return vec4<f32>(v_out.value, v_out.value, v_out.value2, v_out.value2);
+}

naga/tests/in/unconsumed_vertex_outputs_vert.param.ron

@@ -0,0 +1,2 @@
+(
+)

naga/tests/in/unconsumed_vertex_outputs_vert.wgsl

@@ -0,0 +1,13 @@
+// Out of order to test sorting.
+struct VertexOut {
+    @builtin(position) position: vec4<f32>,
+    @location(1) value: f32,
+    @location(2) unused_value2: vec4<f32>,
+    @location(0) unused_value: f32,
+    @location(3) value2: f32,
+}
+
+@vertex
+fn vs_main() -> VertexOut {
+    return VertexOut(vec4(1.0), 1.0, vec4(2.0), 1.0, 0.5);
+}

naga/tests/out/hlsl/unconsumed_vertex_outputs_frag.hlsl

@@ -0,0 +1,17 @@
+struct FragmentIn {
+    float value : LOC1;
+    float value2_ : LOC3;
+    float4 position : SV_Position;
+};
+
+struct FragmentInput_fs_main {
+    float value : LOC1;
+    float value2_ : LOC3;
+    float4 position : SV_Position;
+};
+
+float4 fs_main(FragmentInput_fs_main fragmentinput_fs_main) : SV_Target0
+{
+    FragmentIn v_out = { fragmentinput_fs_main.value, fragmentinput_fs_main.value2_, fragmentinput_fs_main.position };
+    return float4(v_out.value, v_out.value, v_out.value2_, v_out.value2_);
+}

naga/tests/out/hlsl/unconsumed_vertex_outputs_frag.ron

@@ -0,0 +1,12 @@
+(
+    vertex:[
+    ],
+    fragment:[
+        (
+            entry_point:"fs_main",
+            target_profile:"ps_5_1",
+        ),
+    ],
+    compute:[
+    ],
+)

naga/tests/out/hlsl/unconsumed_vertex_outputs_vert.hlsl

@@ -0,0 +1,30 @@
+struct VertexOut {
+    float4 position : SV_Position;
+    float value : LOC1;
+    float4 unused_value2_ : LOC2;
+    float unused_value : LOC0;
+    float value2_ : LOC3;
+};
+
+struct VertexOutput_vs_main {
+    float value : LOC1;
+    float value2_ : LOC3;
+    float4 position : SV_Position;
+};
+
+VertexOut ConstructVertexOut(float4 arg0, float arg1, float4 arg2, float arg3, float arg4) {
+    VertexOut ret = (VertexOut)0;
+    ret.position = arg0;
+    ret.value = arg1;
+    ret.unused_value2_ = arg2;
+    ret.unused_value = arg3;
+    ret.value2_ = arg4;
+    return ret;
+}
+
+VertexOutput_vs_main vs_main()
+{
+    const VertexOut vertexout = ConstructVertexOut((1.0).xxxx, 1.0, (2.0).xxxx, 1.0, 0.5);
+    const VertexOutput_vs_main vertexout_1 = { vertexout.value, vertexout.value2_, vertexout.position };
+    return vertexout_1;
+}

naga/tests/out/hlsl/unconsumed_vertex_outputs_vert.ron

@@ -0,0 +1,12 @@
+(
+    vertex:[
+        (
+            entry_point:"vs_main",
+            target_profile:"vs_5_1",
+        ),
+    ],
+    fragment:[
+    ],
+    compute:[
+    ],
+)