diff --git a/src/back/glsl/mod.rs b/src/back/glsl/mod.rs index b8541e0d4d..11b4c2fd08 100644 --- a/src/back/glsl/mod.rs +++ b/src/back/glsl/mod.rs @@ -831,7 +831,9 @@ impl<'a, W: Write> Writer<'a, W> { // here, regardless of the version. if let Some(sampling) = sampling { if emit_interpolation_and_auxiliary { - write!(self.out, "{} ", glsl_sampling(sampling))?; + if let Some(qualifier) = glsl_sampling(sampling) { + write!(self.out, "{} ", qualifier)?; + } } } @@ -2225,11 +2227,11 @@ fn glsl_interpolation(interpolation: Interpolation) -> &'static str { } /// Return the GLSL auxiliary qualifier for the given sampling value. -fn glsl_sampling(sampling: Sampling) -> &'static str { +fn glsl_sampling(sampling: Sampling) -> Option<&'static str> { match sampling { - Sampling::Center => "", - Sampling::Centroid => "centroid", - Sampling::Sample => "sample", + Sampling::Center => None, + Sampling::Centroid => Some("centroid"), + Sampling::Sample => Some("sample"), } } diff --git a/src/front/spv/function.rs b/src/front/spv/function.rs index fef875d56d..15e969d5d3 100644 --- a/src/front/spv/function.rs +++ b/src/front/spv/function.rs @@ -361,6 +361,8 @@ impl> super::Parser { }); } + module.apply_common_default_interpolation(); + self.lookup_expression.clear(); self.lookup_sampled_image.clear(); Ok(()) diff --git a/src/front/wgsl/mod.rs b/src/front/wgsl/mod.rs index cea66700ca..8d4baa4c49 100644 --- a/src/front/wgsl/mod.rs +++ b/src/front/wgsl/mod.rs @@ -507,6 +507,10 @@ impl BindingParser { match (self.location, self.built_in, self.interpolation, self.sampling) { (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 + // 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 { location, interpolation, sampling })) } (None, Some(bi), None, None) => Ok(Some(crate::Binding::BuiltIn(bi))), @@ -2769,6 +2773,7 @@ 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/proc/interpolator.rs b/src/proc/interpolator.rs new file mode 100644 index 0000000000..3a5e7e8cb7 --- /dev/null +++ b/src/proc/interpolator.rs @@ -0,0 +1,119 @@ +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`]: super::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) + { + match types.get_mut(ty).inner { + // A struct. It's the individual members we care about, so recurse. + TypeInner::Struct { members: ref mut m, .. } => { + // 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::replace; + let mut members = replace(m, vec![]); + + 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, .. } => replace(m, members), + _ => unreachable!("ty must be a struct"), + }; + } + + // 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, .. } => { + // unwrap: all `EntryPoint` arguments or return values must either be structures + // or have a `Binding`. + let binding = binding.as_mut().unwrap(); + 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. + _ => { + // unwrap: all `EntryPoint` arguments or return values must either be structures + // or have a `Binding`. + let binding = binding.as_mut().unwrap(); + 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 a6d6809a47..27f5039f4e 100644 --- a/src/proc/mod.rs +++ b/src/proc/mod.rs @@ -1,5 +1,6 @@ //! Module processing functionality. +mod interpolator; mod namer; mod terminator; mod typifier; diff --git a/src/valid/interface.rs b/src/valid/interface.rs index c041169956..63541895b9 100644 --- a/src/valid/interface.rs +++ b/src/valid/interface.rs @@ -36,6 +36,8 @@ pub enum VaryingError { InvalidType(Handle), #[error("Interpolation is not valid")] InvalidInterpolation, + #[error("Interpolation must be specified on vertex shader outputs and fragment shader inputs")] + MissingInterpolation, #[error("BuiltIn {0:?} is not available at this stage")] InvalidBuiltInStage(crate::BuiltIn), #[error("BuiltIn type for {0:?} is invalid")] @@ -210,28 +212,33 @@ impl VaryingContext<'_> { if !self.location_mask.insert(location as usize) { 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, _ => false, }; - if !needs_interpolation && interpolation.is_some() { - return Err(VaryingError::InvalidInterpolation); - } - // It doesn't make sense to specify a sampling when - // `interpolation` is `Flat`, but SPIR-V and GLSL both - // explicitly tolerate such combinations of decorators / + + // It doesn't make sense to specify a sampling when `interpolation` is `Flat`, but + // SPIR-V and GLSL both explicitly tolerate such combinations of decorators / // qualifiers, so we won't complain about that here. let _ = sampling; + match ty_inner.scalar_kind() { - Some(crate::ScalarKind::Float) => {} - Some(_) - if needs_interpolation - && interpolation != Some(crate::Interpolation::Flat) => - { - return Err(VaryingError::InvalidInterpolation); + Some(crate::ScalarKind::Float) => { + if needs_interpolation && interpolation.is_none() { + return Err(VaryingError::MissingInterpolation); + } + } + Some(_) => { + if needs_interpolation && interpolation != Some(crate::Interpolation::Flat) { + return Err(VaryingError::InvalidInterpolation); + } } - Some(_) => {} None => return Err(VaryingError::InvalidType(self.ty)), } } diff --git a/tests/out/quad.Fragment.glsl b/tests/out/quad.Fragment.glsl index 7122843b3e..8062e3d200 100644 --- a/tests/out/quad.Fragment.glsl +++ b/tests/out/quad.Fragment.glsl @@ -9,7 +9,7 @@ struct VertexOutput { uniform highp sampler2D _group_0_binding_0; -layout(location = 0) in vec2 _vs2fs_location0; +smooth layout(location = 0) in vec2 _vs2fs_location0; layout(location = 0) out vec4 _fs2p_location0; void main() { diff --git a/tests/out/quad.Vertex.glsl b/tests/out/quad.Vertex.glsl index ccd93c498f..751d8c5925 100644 --- a/tests/out/quad.Vertex.glsl +++ b/tests/out/quad.Vertex.glsl @@ -9,7 +9,7 @@ struct VertexOutput { layout(location = 0) in vec2 _p2vs_location0; layout(location = 1) in vec2 _p2vs_location1; -layout(location = 0) out vec2 _vs2fs_location0; +smooth layout(location = 0) out vec2 _vs2fs_location0; void main() { vec2 pos = _p2vs_location0; diff --git a/tests/out/shadow.Fragment.glsl b/tests/out/shadow.Fragment.glsl index be7f69466e..b045c72508 100644 --- a/tests/out/shadow.Fragment.glsl +++ b/tests/out/shadow.Fragment.glsl @@ -18,8 +18,8 @@ readonly buffer Lights_block_1 { uniform highp sampler2DArrayShadow _group_0_binding_2; -layout(location = 0) in vec3 _vs2fs_location0; -layout(location = 1) in vec4 _vs2fs_location1; +smooth layout(location = 0) in vec3 _vs2fs_location0; +smooth layout(location = 1) in vec4 _vs2fs_location1; layout(location = 0) out vec4 _fs2p_location0; float fetch_shadow(uint light_id, vec4 homogeneous_coords) { diff --git a/tests/out/shadow.ron b/tests/out/shadow.ron index 8a4a267749..e99b3c0121 100644 --- a/tests/out/shadow.ron +++ b/tests/out/shadow.ron @@ -1520,7 +1520,7 @@ binding: Some(Location( location: 0, interpolation: Some(Perspective), - sampling: None, + sampling: Some(Center), )), ), ( @@ -1529,7 +1529,7 @@ binding: Some(Location( location: 1, interpolation: Some(Perspective), - sampling: None, + sampling: Some(Center), )), ), ], diff --git a/tests/out/skybox.Fragment.glsl b/tests/out/skybox.Fragment.glsl index 11f715ee74..c8a25b4488 100644 --- a/tests/out/skybox.Fragment.glsl +++ b/tests/out/skybox.Fragment.glsl @@ -9,7 +9,7 @@ struct VertexOutput { uniform highp samplerCube _group_0_binding_1; -layout(location = 0) in vec3 _vs2fs_location0; +smooth layout(location = 0) in vec3 _vs2fs_location0; layout(location = 0) out vec4 _fs2p_location0; void main() { diff --git a/tests/out/skybox.Vertex.glsl b/tests/out/skybox.Vertex.glsl index cc327f6a3f..e76121da33 100644 --- a/tests/out/skybox.Vertex.glsl +++ b/tests/out/skybox.Vertex.glsl @@ -12,7 +12,7 @@ uniform Data_block_0 { mat4x4 view; } _group_0_binding_0; -layout(location = 0) out vec3 _vs2fs_location0; +smooth layout(location = 0) out vec3 _vs2fs_location0; void main() { uint vertex_index = uint(gl_VertexID);