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.
This commit is contained in:
Jim Blandy
2021-09-22 12:08:04 -07:00
committed by Dzmitry Malyshau
parent 73f9d07207
commit d44e2ad207
12 changed files with 111 additions and 175 deletions

59
src/front/interpolator.rs Normal file
View File

@@ -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 => {}
}
}
}
}

View File

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

View File

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

View File

@@ -3725,7 +3725,7 @@ 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;
}
@@ -3733,11 +3733,12 @@ impl<I: Iterator<Item = u32>> Parser<I> {
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,

View File

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

View File

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

View File

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

View File

@@ -1,7 +1,6 @@
//! Module processing functionality.
mod index;
mod interpolator;
mod layouter;
mod namer;
mod terminator;

View File

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

View File

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

View File

@@ -1527,8 +1527,8 @@
ty: 4,
binding: Some(Location(
location: 0,
interpolation: None,
sampling: None,
interpolation: Some(Perspective),
sampling: Some(Center),
)),
)),
local_variables: [],

View File

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