From b95346877abbed2e6a242c6bea836d9fed49802f Mon Sep 17 00:00:00 2001 From: Dzmitry Malyshau Date: Fri, 18 Dec 2020 00:33:22 -0500 Subject: [PATCH] Remove MemberOrigin in favor of spans --- Makefile | 2 +- src/back/msl/mod.rs | 27 ++++---- src/back/msl/writer.rs | 109 ++++++++------------------------- src/front/glsl/parser.rs | 4 +- src/front/spv/mod.rs | 31 +--------- src/front/wgsl/mod.rs | 14 ++--- src/front/wgsl/tests.rs | 20 +++--- src/lib.rs | 15 +---- tests/snapshots/in/boids.wgsl | 20 +++--- tests/snapshots/in/skybox.wgsl | 4 +- 10 files changed, 79 insertions(+), 167 deletions(-) diff --git a/Makefile b/Makefile index 910db15430..db45f23620 100644 --- a/Makefile +++ b/Makefile @@ -6,7 +6,7 @@ all: clean: rm *.metal *.air *.metallib *.vert *.frag *.comp -%.metal: test-data/%.wgsl $(wildcard src/*.rs src/**/*.rs examples/*.rs) +%.metal: tests/snapshots/in/%.wgsl $(wildcard src/*.rs src/**/*.rs examples/*.rs) cargo run --example convert --features wgsl-in,msl-out -- $< $@ %.air: %.metal diff --git a/src/back/msl/mod.rs b/src/back/msl/mod.rs index cc08869e74..053704303f 100644 --- a/src/back/msl/mod.rs +++ b/src/back/msl/mod.rs @@ -57,20 +57,15 @@ pub enum Error { IO(IoError), Utf8(FromUtf8Error), Type(ResolveError), - UnexpectedLocation, - MissingBinding(Handle), MissingBindTarget(BindSource), InvalidImageAccess(crate::StorageAccess), - MutabilityViolation(Handle), BadName(String), - UnexpectedGlobalType(Handle), UnimplementedBindTarget(BindTarget), UnsupportedCompose(Handle), UnsupportedBinaryOp(crate::BinaryOperator), UnexpectedSampleLevel(crate::SampleLevel), UnsupportedCall(String), UnsupportedDynamicArrayLength, - UnableToReturnValue(Handle), /// The source IR is not valid. Validation, } @@ -115,12 +110,12 @@ impl Options { fn resolve_binding( &self, stage: crate::ShaderStage, - binding: &crate::Binding, + var: &crate::GlobalVariable, mode: LocationMode, ) -> Result { - match *binding { - crate::Binding::BuiltIn(built_in) => Ok(ResolvedBinding::BuiltIn(built_in)), - crate::Binding::Location(index) => match mode { + match var.binding { + Some(crate::Binding::BuiltIn(built_in)) => Ok(ResolvedBinding::BuiltIn(built_in)), + Some(crate::Binding::Location(index)) => match mode { LocationMode::VertexInput => Ok(ResolvedBinding::Attribute(index)), LocationMode::FragmentOutput => Ok(ResolvedBinding::Color(index)), LocationMode::Intermediate => Ok(ResolvedBinding::User { @@ -131,9 +126,15 @@ impl Options { }, index, }), - LocationMode::Uniform => Err(Error::UnexpectedLocation), + LocationMode::Uniform => { + log::error!( + "Unexpected Binding::Location({}) for the Uniform mode", + index + ); + Err(Error::Validation) + } }, - crate::Binding::Resource { group, binding } => { + Some(crate::Binding::Resource { group, binding }) => { let source = BindSource { stage, group, @@ -145,6 +146,10 @@ impl Options { .map(ResolvedBinding::Resource) .ok_or(Error::MissingBindTarget(source)) } + None => { + log::error!("Missing binding for {:?}", var.name); + Err(Error::Validation) + } } } } diff --git a/src/back/msl/writer.rs b/src/back/msl/writer.rs index feaa010986..671c8d1667 100644 --- a/src/back/msl/writer.rs +++ b/src/back/msl/writer.rs @@ -1,4 +1,4 @@ -use super::{keywords::RESERVED, Error, LocationMode, Options, ResolvedBinding, TranslationInfo}; +use super::{keywords::RESERVED, Error, LocationMode, Options, TranslationInfo}; use crate::{ arena::Handle, proc::{EntryPointIndex, NameKey, Namer, ResolveContext, Typifier}, @@ -740,18 +740,7 @@ impl Writer { for (index, member) in members.iter().enumerate() { let member_name = &self.names[&NameKey::StructMember(handle, index as u32)]; let base_name = &self.names[&NameKey::Type(member.ty)]; - write!(self.out, "\t{} {}", base_name, member_name)?; - match member.origin { - crate::MemberOrigin::Empty => {} - crate::MemberOrigin::BuiltIn(built_in) => { - ResolvedBinding::BuiltIn(built_in) - .try_fmt_decorated(&mut self.out, "")?; - } - crate::MemberOrigin::Offset(_) => { - //TODO - } - } - writeln!(self.out, ";")?; + writeln!(self.out, "\t{} {};", base_name, member_name)?; } write!(self.out, "}}")?; } @@ -932,7 +921,6 @@ impl Writer { crate::ShaderStage::Vertex | crate::ShaderStage::Fragment => { // make dedicated input/output structs writeln!(self.out, "struct {} {{", location_input_name)?; - for ((handle, var), &usage) in module.global_variables.iter().zip(&fun.global_usage) { @@ -941,36 +929,20 @@ impl Writer { { continue; } - // if it's a struct, lift all the built-in contents up to the root - if let crate::TypeInner::Struct { - block: _, - ref members, - } = module.types[var.ty].inner - { - for (index, member) in members.iter().enumerate() { - if let crate::MemberOrigin::BuiltIn(built_in) = member.origin { - let name = - &self.names[&NameKey::StructMember(var.ty, index as u32)]; - let ty_name = &self.names[&NameKey::Type(member.ty)]; - write!(self.out, "\t{} {}", ty_name, name)?; - ResolvedBinding::BuiltIn(built_in) - .try_fmt_decorated(&mut self.out, ";\n")?; - } - } - } else if let Some(ref binding @ crate::Binding::Location(_)) = var.binding - { - let tyvar = TypedGlobalVariable { - module, - names: &self.names, - handle, - usage: crate::GlobalUse::empty(), - }; - let resolved = options.resolve_binding(stage, binding, in_mode)?; - - write!(self.out, "\t")?; - tyvar.try_fmt(&mut self.out)?; - resolved.try_fmt_decorated(&mut self.out, ";\n")?; + if let Some(crate::Binding::BuiltIn(_)) = var.binding { + // MSL disallows built-ins in input structs + continue; } + let tyvar = TypedGlobalVariable { + module, + names: &self.names, + handle, + usage: crate::GlobalUse::empty(), + }; + write!(self.out, "\t")?; + tyvar.try_fmt(&mut self.out)?; + let resolved = options.resolve_binding(stage, var, in_mode)?; + resolved.try_fmt_decorated(&mut self.out, ";\n")?; } writeln!(self.out, "}};")?; @@ -983,43 +955,17 @@ impl Writer { { continue; } - // if it's a struct, lift all the built-in contents up to the root - if let crate::TypeInner::Struct { - block: _, - ref members, - } = module.types[var.ty].inner - { - for (index, member) in members.iter().enumerate() { - let name = - &self.names[&NameKey::StructMember(var.ty, index as u32)]; - let ty_name = &self.names[&NameKey::Type(member.ty)]; - match member.origin { - crate::MemberOrigin::Empty => {} - crate::MemberOrigin::BuiltIn(built_in) => { - write!(self.out, "\t{} {}", ty_name, name)?; - ResolvedBinding::BuiltIn(built_in) - .try_fmt_decorated(&mut self.out, ";\n")?; - } - crate::MemberOrigin::Offset(_) => { - //TODO - } - } - } - } else { - let tyvar = TypedGlobalVariable { - module, - names: &self.names, - handle, - usage: crate::GlobalUse::empty(), - }; - write!(self.out, "\t")?; - tyvar.try_fmt(&mut self.out)?; - if let Some(ref binding) = var.binding { - let resolved = options.resolve_binding(stage, binding, out_mode)?; - resolved.try_fmt_decorated(&mut self.out, "")?; - } - writeln!(self.out, ";")?; - } + + let tyvar = TypedGlobalVariable { + module, + names: &self.names, + handle, + usage: crate::GlobalUse::empty(), + }; + write!(self.out, "\t")?; + tyvar.try_fmt(&mut self.out)?; + let resolved = options.resolve_binding(stage, var, out_mode)?; + resolved.try_fmt_decorated(&mut self.out, ";\n")?; } writeln!(self.out, "}};")?; @@ -1062,8 +1008,7 @@ impl Writer { } _ => LocationMode::Uniform, }; - let resolved = - options.resolve_binding(stage, var.binding.as_ref().unwrap(), loc_mode)?; + let resolved = options.resolve_binding(stage, var, loc_mode)?; let tyvar = TypedGlobalVariable { module, names: &self.names, diff --git a/src/front/glsl/parser.rs b/src/front/glsl/parser.rs index 05086ff653..0328060bb5 100644 --- a/src/front/glsl/parser.rs +++ b/src/front/glsl/parser.rs @@ -10,7 +10,7 @@ pomelo! { Arena, BinaryOperator, Binding, Block, Constant, ConstantInner, EntryPoint, Expression, Function, GlobalVariable, Handle, Interpolation, - LocalVariable, MemberOrigin, SampleLevel, ScalarKind, + LocalVariable, SampleLevel, ScalarKind, Statement, StorageAccess, StorageClass, StructMember, SwitchCase, Type, TypeInner, UnaryOperator, }; @@ -745,7 +745,7 @@ pomelo! { if let Some(ty) = t { sdl.iter().map(|name| StructMember { name: Some(name.clone()), - origin: MemberOrigin::Empty, + span: None, ty, }).collect() } else { diff --git a/src/front/spv/mod.rs b/src/front/spv/mod.rs index 4328b635d9..8b0bf98e02 100644 --- a/src/front/spv/mod.rs +++ b/src/front/spv/mod.rs @@ -204,32 +204,6 @@ impl Decoration { _ => None, } } - - fn get_origin(&self) -> Result { - match *self { - Decoration { - location: Some(_), .. - } - | Decoration { - desc_set: Some(_), .. - } - | Decoration { - desc_index: Some(_), - .. - } => Err(Error::MissingDecoration(spirv::Decoration::Offset)), - Decoration { - built_in: Some(built_in), - offset: None, - .. - } => Ok(crate::MemberOrigin::BuiltIn(built_in)), - Decoration { - built_in: None, - offset: Some(offset), - .. - } => Ok(crate::MemberOrigin::Offset(offset)), - _ => Ok(crate::MemberOrigin::Empty), - } - } } bitflags::bitflags! { @@ -2028,6 +2002,7 @@ impl> Parser { inst.expect_at_least(2)?; let id = self.next()?; let parent_decor = self.future_decor.remove(&id); + let block_decor = parent_decor.as_ref().and_then(|decor| decor.block.clone()); let mut members = Vec::with_capacity(inst.wc as usize - 2); let mut member_type_ids = Vec::with_capacity(members.capacity()); @@ -2039,15 +2014,13 @@ impl> Parser { .future_member_decor .remove(&(id, i)) .unwrap_or_default(); - let origin = decor.get_origin()?; members.push(crate::StructMember { name: decor.name, - origin, + span: None, //TODO ty, }); } - let block_decor = parent_decor.as_ref().and_then(|decor| decor.block.clone()); let inner = crate::TypeInner::Struct { block: block_decor.is_some(), members, diff --git a/src/front/wgsl/mod.rs b/src/front/wgsl/mod.rs index ec69381f2b..5235f8e5d2 100644 --- a/src/front/wgsl/mod.rs +++ b/src/front/wgsl/mod.rs @@ -82,8 +82,6 @@ pub enum Error<'a> { UnknownFunction(&'a str), #[error("unknown storage format: `{0}`")] UnknownStorageFormat(&'a str), - #[error("missing offset for structure member `{0}`")] - MissingMemberOffset(&'a str), #[error("array stride must not be 0")] ZeroStride, #[error("not a composite type: {0:?}")] @@ -1131,7 +1129,7 @@ impl Parser { let mut members = Vec::new(); lexer.expect(Token::Paren('{'))?; loop { - let mut offset = !0; + let mut span = 0; if lexer.skip(Token::DoubleParen('[')) { self.scopes.push(Scope::Decoration); let mut ready = true; @@ -1143,9 +1141,10 @@ impl Parser { Token::Separator(',') if !ready => { ready = true; } - Token::Word("offset") if ready => { + Token::Word("span") if ready => { lexer.expect(Token::Paren('('))?; - offset = lexer.next_uint_literal()?; + //Note: 0 is not handled + span = lexer.next_uint_literal()?; lexer.expect(Token::Paren(')'))?; ready = false; } @@ -1159,15 +1158,12 @@ impl Parser { Token::Paren('}') => return Ok(members), other => return other.unexpected("field name"), }; - if offset == !0 { - return Err(Error::MissingMemberOffset(name)); - } lexer.expect(Token::Separator(':'))?; let (ty, _access) = self.parse_type_decl(lexer, None, type_arena, const_arena)?; lexer.expect(Token::Separator(';'))?; members.push(crate::StructMember { name: Some(name.to_owned()), - origin: crate::MemberOrigin::Offset(offset), + span: NonZeroU32::new(span), ty, }); } diff --git a/src/front/wgsl/tests.rs b/src/front/wgsl/tests.rs index e761e6b299..c8b812497a 100644 --- a/src/front/wgsl/tests.rs +++ b/src/front/wgsl/tests.rs @@ -23,13 +23,6 @@ fn parse_types() { parse_str("var t: texture_multisampled_2d;").unwrap(); parse_str("var t: [[access(write)]] texture_storage_1d;").unwrap(); parse_str("var t: [[access(read)]] texture_storage_3d;").unwrap(); - parse_str( - " - [[block]] struct Foo { [[offset(0)]] x: i32; }; - var s: [[access(read_write)]] Foo; - ", - ) - .unwrap(); } #[test] @@ -46,6 +39,19 @@ fn parse_type_cast() { .unwrap(); } +#[test] +fn parse_struct() { + parse_str( + " + [[block]] struct Foo { x: i32; }; + struct Bar { [[span(16)]] x: vec2; }; + struct Empty {}; + var s: [[access(read_write)]] Foo; + ", + ) + .unwrap(); +} + #[test] fn parse_standard_fun() { parse_str( diff --git a/src/lib.rs b/src/lib.rs index 262fd02645..b9e06d0d41 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -201,19 +201,6 @@ pub enum ArraySize { Dynamic, } -/// Describes where a struct member is placed. -#[derive(Clone, Copy, Debug, Hash, Eq, Ord, PartialEq, PartialOrd)] -#[cfg_attr(feature = "serialize", derive(Serialize))] -#[cfg_attr(feature = "deserialize", derive(Deserialize))] -pub enum MemberOrigin { - /// Member is local to the shader. - Empty, - /// Built-in shader variable. - BuiltIn(BuiltIn), - /// Offset within the struct. - Offset(u32), -} - /// The interpolation qualifier of a binding or struct field. #[derive(Clone, Copy, Debug, Hash, Eq, Ord, PartialEq, PartialOrd)] #[cfg_attr(feature = "serialize", derive(Serialize))] @@ -245,7 +232,7 @@ pub enum Interpolation { #[cfg_attr(feature = "deserialize", derive(Deserialize))] pub struct StructMember { pub name: Option, - pub origin: MemberOrigin, + pub span: Option, pub ty: Handle, } diff --git a/tests/snapshots/in/boids.wgsl b/tests/snapshots/in/boids.wgsl index 022827774b..3d60518671 100644 --- a/tests/snapshots/in/boids.wgsl +++ b/tests/snapshots/in/boids.wgsl @@ -39,24 +39,24 @@ fn main() { # compute shader [[block]] struct Particle { - [[offset(0)]] pos : vec2; - [[offset(8)]] vel : vec2; + [[span(8)]] pos : vec2; + [[span(8)]] vel : vec2; }; [[block]] struct SimParams { - [[offset(0)]] deltaT : f32; - [[offset(4)]] rule1Distance : f32; - [[offset(8)]] rule2Distance : f32; - [[offset(12)]] rule3Distance : f32; - [[offset(16)]] rule1Scale : f32; - [[offset(20)]] rule2Scale : f32; - [[offset(24)]] rule3Scale : f32; + deltaT : f32; + rule1Distance : f32; + rule2Distance : f32; + rule3Distance : f32; + rule1Scale : f32; + rule2Scale : f32; + rule3Scale : f32; }; [[block]] struct Particles { - [[offset(0)]] particles : [[stride(16)]] array; + particles : [[stride(16)]] array; }; [[group(0), binding(0)]] var params : SimParams; diff --git a/tests/snapshots/in/skybox.wgsl b/tests/snapshots/in/skybox.wgsl index 78de8bacd1..42436f13b8 100644 --- a/tests/snapshots/in/skybox.wgsl +++ b/tests/snapshots/in/skybox.wgsl @@ -5,8 +5,8 @@ var out_position: vec4; #[[block]] struct Data { - [[offset(0)]] proj_inv: mat4x4; - [[offset(64)]] view: mat4x4; + proj_inv: mat4x4; + view: mat4x4; }; [[group(0), binding(0)]] var r_data: Data;