From a2ecd8ecff40b55d48962a88d886048401ce6510 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Capucho?= Date: Sun, 6 Jun 2021 18:49:34 +0100 Subject: [PATCH] [glsl-in] Refractor global and blocks handling --- src/front/glsl/ast.rs | 26 ++----- src/front/glsl/parser.rs | 91 +++++------------------ src/front/glsl/variables.rs | 141 +++++++++++++++++++++++------------- 3 files changed, 120 insertions(+), 138 deletions(-) diff --git a/src/front/glsl/ast.rs b/src/front/glsl/ast.rs index 11b0652c2c..9759635617 100644 --- a/src/front/glsl/ast.rs +++ b/src/front/glsl/ast.rs @@ -21,6 +21,7 @@ pub enum GlobalLookupKind { pub struct GlobalLookup { pub kind: GlobalLookupKind, pub entry_arg: Option, + pub mutable: bool, } #[derive(Debug, PartialEq, Eq, Hash)] @@ -84,7 +85,6 @@ pub struct Program<'a> { pub lookup_type: FastHashMap>, pub global_variables: Vec<(String, GlobalLookup)>, - pub constants: Vec<(String, Handle)>, pub entry_args: Vec, pub entries: Vec<(String, ShaderStage, Handle)>, @@ -107,7 +107,6 @@ impl<'a> Program<'a> { lookup_function: FastHashMap::default(), lookup_type: FastHashMap::default(), global_variables: Vec::new(), - constants: Vec::new(), entry_args: Vec::new(), entries: Vec::new(), @@ -225,7 +224,7 @@ impl<'function> Context<'function> { scopes: vec![FastHashMap::default()], lookup_global_var_exps: FastHashMap::with_capacity_and_hasher( - program.constants.len() + program.global_variables.len(), + program.global_variables.len(), Default::default(), ), typifier: Typifier::new(), @@ -235,23 +234,15 @@ impl<'function> Context<'function> { emitter: Emitter::default(), }; - for &(ref name, handle) in program.constants.iter() { - let expr = this.expressions.append(Expression::Constant(handle)); - let var = VariableReference { - expr, - load: None, - mutable: false, - entry_arg: None, - }; - - this.lookup_global_var_exps.insert(name.into(), var); - } - this.emit_start(); for &(ref name, lookup) in program.global_variables.iter() { this.emit_flush(body); - let GlobalLookup { kind, entry_arg } = lookup; + let GlobalLookup { + kind, + entry_arg, + mutable, + } = lookup; let (expr, load) = match kind { GlobalLookupKind::Variable(v) => { let res = ( @@ -280,8 +271,7 @@ impl<'function> Context<'function> { } else { None }, - // TODO: respect constant qualifier - mutable: true, + mutable, entry_arg, }; diff --git a/src/front/glsl/parser.rs b/src/front/glsl/parser.rs index fa50d99419..0caa16dd1a 100644 --- a/src/front/glsl/parser.rs +++ b/src/front/glsl/parser.rs @@ -12,8 +12,8 @@ use super::{ }; use crate::{ arena::Handle, Arena, ArraySize, BinaryOperator, Block, Constant, ConstantInner, Expression, - Function, FunctionResult, GlobalVariable, ResourceBinding, ScalarValue, Statement, - StorageAccess, StorageClass, StructMember, SwitchCase, Type, TypeInner, UnaryOperator, + Function, FunctionResult, ResourceBinding, ScalarValue, Statement, StorageClass, StructMember, + SwitchCase, Type, TypeInner, UnaryOperator, }; use core::convert::TryFrom; use std::{iter::Peekable, mem}; @@ -715,7 +715,7 @@ impl<'source, 'program, 'options> Parser<'source, 'program, 'options> { match token.value { TokenValue::Identifier(ty_name) => { if self.bump_if(TokenValue::LeftBrace).is_some() { - self.parse_block_declaration(&qualifiers, ty_name) + self.parse_block_declaration(&qualifiers, ty_name, token.meta) } else { //TODO: declaration // type_qualifier IDENTIFIER SEMICOLON @@ -761,55 +761,8 @@ impl<'source, 'program, 'options> Parser<'source, 'program, 'options> { &mut self, qualifiers: &[(TypeQualifier, SourceMetadata)], ty_name: String, + mut meta: SourceMetadata, ) -> Result { - let mut class = StorageClass::Private; - let mut binding = None; - let mut layout = None; - - for &(ref qualifier, meta) in qualifiers { - match *qualifier { - TypeQualifier::StorageQualifier(StorageQualifier::StorageClass(c)) => { - if StorageClass::PushConstant == class && c == StorageClass::Uniform { - // Ignore the Uniform qualifier if the class was already set to PushConstant - continue; - } else if StorageClass::Private != class { - return Err(ErrorKind::SemanticError( - meta, - "Cannot use more than one storage qualifier per declaration".into(), - )); - } - - class = c; - } - TypeQualifier::ResourceBinding(ref r) => { - if binding.is_some() { - return Err(ErrorKind::SemanticError( - meta, - "Cannot use more than one storage qualifier per declaration".into(), - )); - } - - binding = Some(r.clone()); - } - TypeQualifier::Layout(ref l) => { - if layout.is_some() { - return Err(ErrorKind::SemanticError( - meta, - "Cannot use more than one storage qualifier per declaration".into(), - )); - } - - layout = Some(l); - } - _ => { - return Err(ErrorKind::SemanticError( - meta, - "Qualifier not supported in block declarations".into(), - )); - } - } - } - let mut members = Vec::new(); let span = self.parse_struct_declaration_list(&mut members)?; self.expect(TokenValue::RightBrace)?; @@ -828,7 +781,7 @@ impl<'source, 'program, 'options> Parser<'source, 'program, 'options> { TokenValue::Semicolon => None, TokenValue::Identifier(name) => { if let Some(size) = self.parse_array_specifier()? { - ty = self.program.module.types.append(Type { + ty = self.program.module.types.fetch_or_append(Type { name: None, inner: TypeInner::Array { base: ty, @@ -846,25 +799,15 @@ impl<'source, 'program, 'options> Parser<'source, 'program, 'options> { } _ => return Err(ErrorKind::InvalidToken(token)), }; + meta = meta.union(&token.meta); - let handle = self.program.module.global_variables.append(GlobalVariable { - name: name.clone(), - class, - binding, + let handle = self.program.add_global_var(VarDeclaration { + qualifiers, ty, + name, init: None, - storage_access: StorageAccess::empty(), - }); - - if let Some(k) = name { - self.program.global_variables.push(( - k, - GlobalLookup { - kind: GlobalLookupKind::Variable(handle), - entry_arg: None, - }, - )); - } + meta, + })?; for (i, k) in members .into_iter() @@ -876,6 +819,7 @@ impl<'source, 'program, 'options> Parser<'source, 'program, 'options> { GlobalLookup { kind: GlobalLookupKind::BlockSelect(handle, i), entry_arg: None, + mutable: true, }, )); } @@ -1556,7 +1500,7 @@ impl<'source, 'program, 'options> Parser<'source, 'program, 'options> { let decl = VarDeclaration { qualifiers: &qualifiers, ty, - name, + name: Some(name), init: None, meta: meta.union(&end_meta), }; @@ -1740,13 +1684,18 @@ impl<'ctx, 'fun> DeclarationContext<'ctx, 'fun> { let decl = VarDeclaration { qualifiers: &self.qualifiers, ty, - name, + name: Some(name), init, meta, }; match self.external { - true => program.add_global_var(self.ctx, self.body, decl), + true => { + let handle = program.add_global_var(decl)?; + Ok(self + .ctx + .add_expression(Expression::GlobalVariable(handle), self.body)) + } false => program.add_local_var(self.ctx, self.body, decl), } } diff --git a/src/front/glsl/variables.rs b/src/front/glsl/variables.rs index 85530b9cb8..03dd2ddfde 100644 --- a/src/front/glsl/variables.rs +++ b/src/front/glsl/variables.rs @@ -11,7 +11,7 @@ use super::token::SourceMetadata; pub struct VarDeclaration<'a> { pub qualifiers: &'a [(TypeQualifier, SourceMetadata)], pub ty: Handle, - pub name: String, + pub name: Option, pub init: Option>, pub meta: SourceMetadata, } @@ -57,6 +57,7 @@ impl Program<'_> { GlobalLookup { kind: GlobalLookupKind::Variable(handle), entry_arg: Some(idx), + mutable, }, )); ctx.arg_use.push(EntryArgUse::empty()); @@ -222,8 +223,6 @@ impl Program<'_> { pub fn add_global_var( &mut self, - ctx: &mut Context, - body: &mut Block, VarDeclaration { qualifiers, ty, @@ -231,7 +230,7 @@ impl Program<'_> { init, meta, }: VarDeclaration, - ) -> Result, ErrorKind> { + ) -> Result, ErrorKind> { let mut storage = StorageQualifier::StorageClass(StorageClass::Private); let mut interpolation = None; let mut binding = None; @@ -242,7 +241,12 @@ impl Program<'_> { for &(ref qualifier, meta) in qualifiers { match *qualifier { TypeQualifier::StorageQualifier(s) => { - if StorageQualifier::StorageClass(StorageClass::Private) != storage { + if StorageQualifier::StorageClass(StorageClass::PushConstant) == storage + && s == StorageQualifier::StorageClass(StorageClass::Uniform) + { + // Ignore the Uniform qualifier if the class was already set to PushConstant + continue; + } else if StorageQualifier::StorageClass(StorageClass::Private) != storage { return Err(ErrorKind::SemanticError( meta, "Cannot use more than one storage qualifier per declaration".into(), @@ -255,7 +259,8 @@ impl Program<'_> { if interpolation.is_some() { return Err(ErrorKind::SemanticError( meta, - "Cannot use more than one storage qualifier per declaration".into(), + "Cannot use more than one interpolation qualifier per declaration" + .into(), )); } @@ -265,7 +270,7 @@ impl Program<'_> { if binding.is_some() { return Err(ErrorKind::SemanticError( meta, - "Cannot use more than one storage qualifier per declaration".into(), + "Cannot use more than one binding per declaration".into(), )); } @@ -275,7 +280,7 @@ impl Program<'_> { if location.is_some() { return Err(ErrorKind::SemanticError( meta, - "Cannot use more than one storage qualifier per declaration".into(), + "Cannot use more than one binding per declaration".into(), )); } @@ -285,7 +290,7 @@ impl Program<'_> { if sampling.is_some() { return Err(ErrorKind::SemanticError( meta, - "Cannot use more than one storage qualifier per declaration".into(), + "Cannot use more than one sampling qualifier per declaration".into(), )); } @@ -295,7 +300,7 @@ impl Program<'_> { if layout.is_some() { return Err(ErrorKind::SemanticError( meta, - "Cannot use more than one storage qualifier per declaration".into(), + "Cannot use more than one layout qualifier per declaration".into(), )); } @@ -311,10 +316,17 @@ impl Program<'_> { } if binding.is_some() && storage != StorageQualifier::StorageClass(StorageClass::Uniform) { - return Err(ErrorKind::SemanticError( - meta, - "binding requires uniform or buffer storage qualifier".into(), - )); + match storage { + StorageQualifier::StorageClass(StorageClass::PushConstant) + | StorageQualifier::StorageClass(StorageClass::Uniform) + | StorageQualifier::StorageClass(StorageClass::Storage) => {} + _ => { + return Err(ErrorKind::SemanticError( + meta, + "binding requires uniform or buffer storage qualifier".into(), + )) + } + } } if (sampling.is_some() || interpolation.is_some()) && location.is_none() { @@ -325,7 +337,8 @@ impl Program<'_> { } if let Some(location) = location { - let prologue = if let StorageQualifier::Input = storage { + let input = storage == StorageQualifier::Input; + let prologue = if input { PrologueStage::all() } else { PrologueStage::empty() @@ -339,7 +352,7 @@ impl Program<'_> { }); let handle = self.module.global_variables.append(GlobalVariable { - name: Some(name.clone()), + name: name.clone(), class: StorageClass::Private, binding: None, ty, @@ -358,23 +371,40 @@ impl Program<'_> { prologue, }); - self.global_variables.push(( - name, - GlobalLookup { - kind: GlobalLookupKind::Variable(handle), - entry_arg: Some(idx), - }, - )); + if let Some(name) = name { + self.global_variables.push(( + name, + GlobalLookup { + kind: GlobalLookupKind::Variable(handle), + entry_arg: Some(idx), + mutable: !input, + }, + )); + } - return Ok(ctx.add_expression(Expression::GlobalVariable(handle), body)); + return Ok(handle); } else if let StorageQualifier::Const = storage { - let handle = init.ok_or_else(|| { - ErrorKind::SemanticError(meta, "Constant must have a initializer".into()) - })?; + let handle = self.module.global_variables.append(GlobalVariable { + name: name.clone(), + class: StorageClass::Private, + binding: None, + ty, + init, + storage_access: StorageAccess::empty(), + }); - self.constants.push((name, handle)); + if let Some(name) = name { + self.global_variables.push(( + name, + GlobalLookup { + kind: GlobalLookupKind::Variable(handle), + entry_arg: None, + mutable: false, + }, + )); + } - return Ok(ctx.add_expression(Expression::Constant(handle), body)); + return Ok(handle); } let (class, storage_access) = match self.module.types[ty].inner { @@ -389,17 +419,23 @@ impl Program<'_> { }, ), TypeInner::Sampler { .. } => (StorageClass::Handle, StorageAccess::empty()), - _ => ( - match storage { - StorageQualifier::StorageClass(class) => class, - _ => StorageClass::Private, - }, - StorageAccess::empty(), - ), + _ => { + if let StorageQualifier::StorageClass(StorageClass::Storage) = storage { + (StorageClass::Storage, StorageAccess::all()) + } else { + ( + match storage { + StorageQualifier::StorageClass(class) => class, + _ => StorageClass::Private, + }, + StorageAccess::empty(), + ) + } + } }; let handle = self.module.global_variables.append(GlobalVariable { - name: Some(name.clone()), + name: name.clone(), class, binding, ty, @@ -407,15 +443,18 @@ impl Program<'_> { storage_access, }); - self.global_variables.push(( - name, - GlobalLookup { - kind: GlobalLookupKind::Variable(handle), - entry_arg: None, - }, - )); + if let Some(name) = name { + self.global_variables.push(( + name, + GlobalLookup { + kind: GlobalLookupKind::Variable(handle), + entry_arg: None, + mutable: true, + }, + )); + } - Ok(ctx.add_expression(Expression::GlobalVariable(handle), body)) + Ok(handle) } pub fn add_local_var( @@ -431,8 +470,10 @@ impl Program<'_> { }: VarDeclaration, ) -> Result, ErrorKind> { #[cfg(feature = "glsl-validate")] - if ctx.lookup_local_var_current_scope(&name).is_some() { - return Err(ErrorKind::VariableAlreadyDeclared(name)); + if let Some(ref name) = name { + if ctx.lookup_local_var_current_scope(name).is_some() { + return Err(ErrorKind::VariableAlreadyDeclared(name.clone())); + } } let mut mutable = true; @@ -459,13 +500,15 @@ impl Program<'_> { } let handle = ctx.locals.append(LocalVariable { - name: Some(name.clone()), + name: name.clone(), ty, init, }); let expr = ctx.add_expression(Expression::LocalVariable(handle), body); - ctx.add_local_var(name, expr, mutable); + if let Some(name) = name { + ctx.add_local_var(name, expr, mutable); + } Ok(expr) }