diff --git a/src/front/interpolator.rs b/src/front/interpolator.rs new file mode 100644 index 0000000000..491ca5a0bb --- /dev/null +++ b/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 => {} + } + } + } +} diff --git a/src/front/mod.rs b/src/front/mod.rs index 292acf3da3..56e000d62d 100644 --- a/src/front/mod.rs +++ b/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")] diff --git a/src/front/spv/function.rs b/src/front/spv/function.rs index 90ea81033e..0ce8e7686b 100644 --- a/src/front/spv/function.rs +++ b/src/front/spv/function.rs @@ -341,13 +341,8 @@ impl> super::Parser { 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> super::Parser { }); } - module.apply_common_default_interpolation(); - Ok(()) } } diff --git a/src/front/spv/mod.rs b/src/front/spv/mod.rs index 4869a47868..34b510d355 100644 --- a/src/front/spv/mod.rs +++ b/src/front/spv/mod.rs @@ -3725,7 +3725,7 @@ impl> Parser { 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; } @@ -3733,11 +3733,12 @@ impl> Parser { 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> Parser { } } + 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> Parser { (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> Parser { 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> Parser { } 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> Parser { 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, diff --git a/src/front/wgsl/mod.rs b/src/front/wgsl/mod.rs index bc013f1474..09c66bb63d 100644 --- a/src/front/wgsl/mod.rs +++ b/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); } } diff --git a/src/lib.rs b/src/lib.rs index 697264c516..afc93094d0 100644 --- a/src/lib.rs +++ b/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, diff --git a/src/proc/interpolator.rs b/src/proc/interpolator.rs deleted file mode 100644 index 3b550782c9..0000000000 --- a/src/proc/interpolator.rs +++ /dev/null @@ -1,149 +0,0 @@ -pub use crate::{Arena, Handle}; - -impl crate::Module { - /// Apply the usual default interpolation for vertex shader outputs and fragment shader inputs. - /// - /// For every [`Binding`] that is a vertex shader output or a fragment shader - /// input, and that has an `interpolation` or `sampling` of `None`, assign a - /// default interpolation and sampling as follows: - /// - /// - If the `Binding`'s type contains only 32-bit floating-point values or - /// vectors of such, default its interpolation to `Perspective` and its - /// sampling to `Center`. - /// - /// - Otherwise, mark its interpolation as `Flat`. - /// - /// When struct appear in input or output types, apply these rules to their - /// leaves, since those are the things that actually get assigned locations. - /// - /// This function is a utility front ends may use to satisfy the Naga IR's - /// requirement that all I/O `Binding`s from the vertex shader to the - /// fragment shader must have non-`None` `interpolation` values. This - /// requirement is meant to ensure that input languages' policies have been - /// applied appropriately. - /// - /// 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. - /// - /// [`Binding`]: crate::Binding - pub fn apply_common_default_interpolation(&mut self) { - use crate::{Binding, ScalarKind, Type, TypeInner}; - - /// Choose a default interpolation for a function argument or result. - /// - /// `binding` refers to the `Binding` whose type is `ty`. If `ty` is a struct, then it's the - /// bindings of the struct's members that we care about, and the binding of the struct - /// itself is meaningless, so `binding` should be `None`. - fn default_binding_or_struct( - binding: &mut Option, - ty: Handle, - types: &mut Arena, - ) { - let inner = &mut types.get_mut(ty).inner; - if let TypeInner::Struct { - members: ref mut m, .. - } = *inner - { - // A struct. It's the individual members we care about, so recurse. - - // To choose the right interpolations for `members`, we must consult other - // elements of `types`. But both `members` and the types it refers to are stored - // in `types`, and Rust won't let us mutate one element of the `Arena`'s `Vec` - // while reading others. - // - // So, temporarily swap the member list out its type, assign appropriate - // interpolations to its members, and then swap the list back in. - use std::mem; - let mut members = mem::take(m); - - for member in &mut members { - default_binding_or_struct(&mut member.binding, member.ty, types); - } - - // Swap the member list back in. It's essential that we call `types.get_mut` - // afresh here, rather than just using `m`: it's only because `m` was dead that - // we were able to pass `types` to the recursive call. - match types.get_mut(ty).inner { - TypeInner::Struct { - members: ref mut m, .. - } => mem::replace(m, members), - _ => unreachable!("ty must be a struct"), - }; - - return; - } - - // For all other types, a binding is required. Missing bindings will - // be caught during validation, but this processor is meant for use - // by front ends before validation, so just return for now. - let binding = match binding.as_mut() { - None => return, - Some(binding) => binding, - }; - - match *inner { - // Some interpolatable type. - // - // GLSL has 64-bit floats, but it won't interpolate them. WGSL and MSL only have - // 32-bit floats. SPIR-V has 16- and 64-bit float capabilities, but Vulkan is vague - // about what can and cannot be interpolated. - TypeInner::Scalar { - kind: ScalarKind::Float, - width: 4, - } - | TypeInner::Vector { - kind: ScalarKind::Float, - width: 4, - .. - } => { - if let Binding::Location { - ref mut interpolation, - ref mut sampling, - .. - } = *binding - { - if interpolation.is_none() { - *interpolation = Some(crate::Interpolation::Perspective); - } - if sampling.is_none() && *interpolation != Some(crate::Interpolation::Flat) - { - *sampling = Some(crate::Sampling::Center); - } - } - } - - // Some type that can't be interpolated. - _ => { - if let Binding::Location { - ref mut interpolation, - ref mut sampling, - .. - } = *binding - { - *interpolation = Some(crate::Interpolation::Flat); - *sampling = None; - } - } - } - } - - for ep in &mut self.entry_points { - let function = &mut ep.function; - match ep.stage { - crate::ShaderStage::Fragment => { - for arg in &mut function.arguments { - default_binding_or_struct(&mut arg.binding, arg.ty, &mut self.types); - } - } - crate::ShaderStage::Vertex => { - if let Some(result) = function.result.as_mut() { - default_binding_or_struct(&mut result.binding, result.ty, &mut self.types); - } - } - _ => (), - } - } - } -} diff --git a/src/proc/mod.rs b/src/proc/mod.rs index 867782ffe7..d672e905c4 100644 --- a/src/proc/mod.rs +++ b/src/proc/mod.rs @@ -1,7 +1,6 @@ //! Module processing functionality. mod index; -mod interpolator; mod layouter; mod namer; mod terminator; diff --git a/src/valid/interface.rs b/src/valid/interface.rs index 9253042af8..ff3f2401cd 100644 --- a/src/valid/interface.rs +++ b/src/valid/interface.rs @@ -256,10 +256,6 @@ impl VaryingContext<'_> { return Err(VaryingError::BindingCollision { location }); } - // Values passed from the vertex shader to the fragment shader must have their - // interpolation defaulted (i.e. not `None`) by the front end, as appropriate for - // that language. For anything other than floating-point scalars and vectors, the - // interpolation must be `Flat`. let needs_interpolation = match self.stage { crate::ShaderStage::Vertex => self.output, crate::ShaderStage::Fragment => !self.output, diff --git a/tests/out/hlsl/interface.hlsl b/tests/out/hlsl/interface.hlsl index affd807ecd..3c4334defb 100644 --- a/tests/out/hlsl/interface.hlsl +++ b/tests/out/hlsl/interface.hlsl @@ -13,7 +13,7 @@ struct VertexOutput { struct FragmentOutput { float depth : SV_Depth; uint sample_mask : SV_Coverage; - float color : SV_Target0; + linear float color : SV_Target0; }; groupshared uint output[1]; diff --git a/tests/out/ir/shadow.ron b/tests/out/ir/shadow.ron index 8c4e2df9ed..763aa28e9c 100644 --- a/tests/out/ir/shadow.ron +++ b/tests/out/ir/shadow.ron @@ -1527,8 +1527,8 @@ ty: 4, binding: Some(Location( location: 0, - interpolation: None, - sampling: None, + interpolation: Some(Perspective), + sampling: Some(Center), )), )), local_variables: [], diff --git a/tests/out/spv/interface.vertex.spvasm b/tests/out/spv/interface.vertex.spvasm index 9910b2bc5a..c51a4d563b 100644 --- a/tests/out/spv/interface.vertex.spvasm +++ b/tests/out/spv/interface.vertex.spvasm @@ -15,6 +15,7 @@ OpDecorate %15 ArrayStride 4 OpDecorate %18 BuiltIn VertexIndex OpDecorate %21 BuiltIn InstanceIndex OpDecorate %23 Location 10 +OpDecorate %23 Flat OpDecorate %25 BuiltIn Position OpDecorate %27 Location 1 OpDecorate %29 BuiltIn PointSize