Require binding interpolation to be resolved by the front end.

When validating IR, verify that all `Binding`s for vertex shader outputs and
fragment shader inputs, whether directly in the argument or result, or applied
to a struct member, has specified an interpolation and sampling, not `None`.
This ensures that front ends explicitly state their policies, rather than
coasting through on back ends' default behavior.

In practice, all our front ends have very similar defaults, so provide a utility
function on `Module` to apply these rules. Use this utility function in the
SPIR-V and WGSL front ends; GLSL seems to already fill in interpolation as
required.
This commit is contained in:
Jim Blandy
2021-04-16 18:06:28 -07:00
committed by Dzmitry Malyshau
parent 0910af2718
commit 4f442ff8cc
12 changed files with 162 additions and 26 deletions

View File

@@ -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"),
}
}

View File

@@ -361,6 +361,8 @@ impl<I: Iterator<Item = u32>> super::Parser<I> {
});
}
module.apply_common_default_interpolation();
self.lookup_expression.clear();
self.lookup_sampled_image.clear();
Ok(())

View File

@@ -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);
}
}

119
src/proc/interpolator.rs Normal file
View File

@@ -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<Binding>,
ty: Handle<Type>,
types: &mut Arena<Type>)
{
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);
}
}
_ => (),
}
}
}
}

View File

@@ -1,5 +1,6 @@
//! Module processing functionality.
mod interpolator;
mod namer;
mod terminator;
mod typifier;

View File

@@ -36,6 +36,8 @@ pub enum VaryingError {
InvalidType(Handle<crate::Type>),
#[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)),
}
}

View File

@@ -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() {

View File

@@ -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;

View File

@@ -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) {

View File

@@ -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),
)),
),
],

View File

@@ -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() {

View File

@@ -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);