Simplify interpolation defaulting.

Replace `Module::apply_common_default_interpolation` with a simpler function
that handles a single `Binding` at a time. In exchange for the simplicity, the
function must be called at each point function arguments, function results, and
struct members are prepared. (Any missed spots will be caught by the verifier.)

This approach no longer requires mutating types in the arena, a prerequisite for
properly handling type identity.

Applying defaults to struct members when the struct declaration is parsed does
have a disadvantage, compared to the old whole-module pass: at struct parse
time, we don't yet know which pipeline stages the struct will be used in. The
best we can do is apply defaults to anything with a `Location` binding. This
causes needless qualifiers to appear in some output. However, it seems that our
back end languages all tolerate such qualifiers.

src/front/interpolator.rs

@@ -0,0 +1,59 @@
+//! Interpolation defaults.
+
+impl crate::Binding {
+    /// Apply the usual default interpolation for `ty` to `binding`.
+    ///
+    /// This function is a utility front ends may use to satisfy the Naga IR's
+    /// requirement, meant to ensure that input languages' policies have been
+    /// applied appropriately, that all I/O `Binding`s from the vertex shader to the
+    /// fragment shader must have non-`None` `interpolation` values.
+    ///
+    /// All the shader languages Naga supports have similar rules:
+    /// perspective-correct, center-sampled interpolation is the default for any
+    /// binding that can vary, and everything else either defaults to flat, or
+    /// requires an explicit flat qualifier/attribute/what-have-you.
+    ///
+    /// If `binding` is not a [`Location`] binding, or if its [`interpolation`] is
+    /// already set, then make no changes. Otherwise, set `binding`'s interpolation
+    /// and sampling to reasonable defaults depending on `ty`, the type of the value
+    /// being interpolated:
+    ///
+    /// - If `ty` is a floating-point scalar, vector, or matrix type, then
+    ///   default to [`Perspective`] interpolation and [`Center`] sampling.
+    ///
+    /// - If `ty` is an integral scalar or vector, then default to [`Flat`]
+    ///   interpolation, which has no associated sampling.
+    ///
+    /// - For any other types, make no change. Such types are not permitted as
+    ///   user-defined IO values, and will probably be flagged by the verifier
+    ///
+    /// When structs appear in input or output types, each member ought to have its
+    /// own [`Binding`], so structs are simply covered by the third case.
+    ///
+    /// [`Binding`]: crate::Binding
+    /// [`Location`]: crate::Binding::Location
+    /// [`interpolation`]: crate::Binding::Location::interpolation
+    /// [`Perspective`]: crate::Interpolation::Perspective
+    /// [`Flat`]: crate::Interpolation::Flat
+    /// [`Center`]: crate::Sampling::Center
+    pub fn apply_default_interpolation(&mut self, ty: &crate::TypeInner) {
+        if let crate::Binding::Location {
+            location: _,
+            interpolation: ref mut interpolation @ None,
+            ref mut sampling,
+        } = *self
+        {
+            match ty.scalar_kind() {
+                Some(crate::ScalarKind::Float) => {
+                    *interpolation = Some(crate::Interpolation::Perspective);
+                    *sampling = Some(crate::Sampling::Center);
+                }
+                Some(crate::ScalarKind::Sint) | Some(crate::ScalarKind::Uint) => {
+                    *interpolation = Some(crate::Interpolation::Flat);
+                    *sampling = None;
+                }
+                Some(_) | None => {}
+            }
+        }
+    }
+}

src/front/mod.rs

@@ -1,5 +1,7 @@
 //! Parsers which load shaders into memory.
 
+mod interpolator;
+
 #[cfg(feature = "glsl-in")]
 pub mod glsl;
 #[cfg(feature = "spv-in")]

src/front/spv/function.rs

@@ -341,13 +341,8 @@ impl<I: Iterator<Item = u32>> super::Parser<I> {
 
                     let mut arg = arg.clone();
                     if ep.stage == crate::ShaderStage::Fragment {
-                        if let Some(crate::Binding::Location {
-                            interpolation: ref mut interpolation @ None,
-                            ..
-                        }) = arg.binding
-                        {
-                            *interpolation = Some(crate::Interpolation::Perspective);
-                            // default
+                        if let Some(ref mut binding) = arg.binding {
+                            binding.apply_default_interpolation(&module.types[arg.ty].inner);
                         }
                     }
                     function.arguments.push(arg);
@@ -516,8 +511,6 @@ impl<I: Iterator<Item = u32>> super::Parser<I> {
             });
         }
 
-        module.apply_common_default_interpolation();
-
         Ok(())
     }
 }

src/front/spv/mod.rs

@@ -3725,19 +3725,20 @@ impl<I: Iterator<Item = u32>> Parser<I> {
             span = crate::front::align_up(span, self.layouter[ty].alignment.get());
             alignment = self.layouter[ty].alignment.get().max(alignment);
 
-            let binding = decor.io_binding().ok();
+            let mut binding = decor.io_binding().ok();
             if let Some(offset) = decor.offset {
                 span = offset;
             }
             let offset = span;
 
             span += self.layouter[ty].size;
 
+            let inner = &module.types[ty].inner;
             if let crate::TypeInner::Matrix {
                 columns,
                 rows,
                 width,
-            } = module.types[ty].inner
+            } = *inner
             {
                 if let Some(stride) = decor.matrix_stride {
                     let rounded_rows = if rows > crate::VectorSize::Bi {
@@ -3757,6 +3758,9 @@ impl<I: Iterator<Item = u32>> Parser<I> {
                 }
             }
 
+            if let Some(ref mut binding) = binding {
+                binding.apply_default_interpolation(inner);
+            }
             members.push(crate::StructMember {
                 name: decor.name,
                 ty,
@@ -4181,7 +4185,7 @@ impl<I: Iterator<Item = u32>> Parser<I> {
                 (Variable::Global, var)
             }
             ExtendedClass::Input => {
-                let binding = dec.io_binding()?;
+                let mut binding = dec.io_binding()?;
                 let mut unsigned_ty = effective_ty;
                 if let crate::Binding::BuiltIn(built_in) = binding {
                     let needs_inner_uint = match built_in {
@@ -4222,6 +4226,9 @@ impl<I: Iterator<Item = u32>> Parser<I> {
                     ty: effective_ty,
                     init: None,
                 };
+
+                binding.apply_default_interpolation(&module.types[unsigned_ty].inner);
+
                 let inner = Variable::Input(crate::FunctionArgument {
                     name: dec.name,
                     ty: unsigned_ty,
@@ -4231,7 +4238,7 @@ impl<I: Iterator<Item = u32>> Parser<I> {
             }
             ExtendedClass::Output => {
                 // For output interface blocks, this would be a structure.
-                let binding = dec.io_binding().ok();
+                let mut binding = dec.io_binding().ok();
                 let init = match binding {
                     Some(crate::Binding::BuiltIn(built_in)) => {
                         match null::generate_default_built_in(
@@ -4297,6 +4304,9 @@ impl<I: Iterator<Item = u32>> Parser<I> {
                     ty: effective_ty,
                     init,
                 };
+                if let Some(ref mut binding) = binding {
+                    binding.apply_default_interpolation(&module.types[effective_ty].inner);
+                }
                 let inner = Variable::Output(crate::FunctionResult {
                     ty: effective_ty,
                     binding,

src/front/wgsl/mod.rs

@@ -1085,7 +1085,7 @@ impl BindingParser {
             (None, None, None, None) => Ok(None),
             (Some(location), None, interpolation, sampling) => {
                 // Before handing over the completed `Module`, we call
-                // `apply_common_default_interpolation` to ensure that the interpolation and
+                // `apply_default_interpolation` to ensure that the interpolation and
                 // sampling have been explicitly specified on all vertex shader output and fragment
                 // shader input user bindings, so leaving them potentially `None` here is fine.
                 Ok(Some(crate::Binding::Location {
@@ -2704,10 +2704,14 @@ impl Parser {
             alignment = alignment.max(align);
             offset = range.end;
 
+            let mut binding = bind_parser.finish(bind_span)?;
+            if let Some(ref mut binding) = binding {
+                binding.apply_default_interpolation(&type_arena[ty].inner);
+            }
             members.push(crate::StructMember {
                 name: Some(name.to_owned()),
                 ty,
-                binding: bind_parser.finish(bind_span)?,
+                binding,
                 offset: range.start,
             });
         }
@@ -3818,7 +3822,7 @@ impl Parser {
                     ExpectedToken::Token(Token::Separator(',')),
                 ));
             }
-            let binding = self.parse_varying_binding(lexer)?;
+            let mut binding = self.parse_varying_binding(lexer)?;
             let (param_name, param_name_span, param_type, _access) =
                 self.parse_variable_ident_decl(lexer, &mut module.types, &mut module.constants)?;
             let param_index = arguments.len() as u32;
@@ -3833,6 +3837,9 @@ impl Parser {
                     is_reference: false,
                 },
             );
+            if let Some(ref mut binding) = binding {
+                binding.apply_default_interpolation(&module.types[param_type].inner);
+            }
             arguments.push(crate::FunctionArgument {
                 name: Some(param_name.to_string()),
                 ty: param_type,
@@ -3842,9 +3849,12 @@ impl Parser {
         }
         // read return type
         let result = if lexer.skip(Token::Arrow) && !lexer.skip(Token::Word("void")) {
-            let binding = self.parse_varying_binding(lexer)?;
+            let mut binding = self.parse_varying_binding(lexer)?;
             let (ty, _access) =
                 self.parse_type_decl(lexer, None, &mut module.types, &mut module.constants)?;
+            if let Some(ref mut binding) = binding {
+                binding.apply_default_interpolation(&module.types[ty].inner);
+            }
             Some(crate::FunctionResult { ty, binding })
         } else {
             None
@@ -4122,7 +4132,6 @@ impl Parser {
                         log::error!("Reached the end of file, but scopes are not closed");
                         return Err(Error::Other.as_parse_error(lexer.source));
                     };
-                    module.apply_common_default_interpolation();
                     return Ok(module);
                 }
             }

src/lib.rs

@@ -710,7 +710,23 @@ pub enum ConstantInner {
 pub enum Binding {
     /// Built-in shader variable.
     BuiltIn(BuiltIn),
+
     /// Indexed location.
+    ///
+    /// Values passed from the [`Vertex`] stage to the [`Fragment`] stage must
+    /// have their `interpolation` defaulted (i.e. not `None`) by the front end
+    /// as appropriate for that language.
+    ///
+    /// For other stages, we permit interpolations even though they're ignored.
+    /// When a front end is parsing a struct type, it usually doesn't know what
+    /// stages will be using it for IO, so it's easiest if it can apply the
+    /// defaults to anything with a `Location` binding, just in case.
+    ///
+    /// For anything other than floating-point scalars and vectors, the
+    /// interpolation must be `Flat`.
+    ///
+    /// [`Vertex`]: crate::ShaderStage::Vertex
+    /// [`Fragment`]: crate::ShaderStage::Fragment
     Location {
         location: u32,
         interpolation: Option<Interpolation>,