From 0995c7161ed50e644c1d902fbf449dcbedbdd1b6 Mon Sep 17 00:00:00 2001 From: "Jasper St. Pierre" Date: Wed, 14 Jul 2021 08:55:58 -0700 Subject: [PATCH] [glsl-in] Keep around extra linkage variables WGSL will require this. Note that this still might cause some issues with multi-entry-point GLSL that I didn't know how to handle. That is, we will handle unused builtin inputs but not unused builtin outputs correctly right now. This is an existing issue though, not a regression. This is also provided as an option, but I feel like the more correct approach is to never strip linkage variables. We'll see though. --- cli/src/main.rs | 3 ++ src/front/glsl/ast.rs | 5 +++- src/front/glsl/functions.rs | 32 +++++++++++++++------ src/front/glsl/mod.rs | 3 +- src/front/glsl/parser_tests.rs | 2 +- src/front/glsl/variables.rs | 11 ++++++- tests/out/wgsl/210-bevy-2d-shader-frag.wgsl | 9 +++--- tests/out/wgsl/quad_glsl-frag.wgsl | 9 +++--- tests/snapshots.rs | 3 ++ 9 files changed, 57 insertions(+), 20 deletions(-) diff --git a/cli/src/main.rs b/cli/src/main.rs index 0415ddc4ff..00e6cf2a6d 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -166,6 +166,7 @@ fn main() { &naga::front::glsl::Options { entry_points, defines: Default::default(), + strip_unused_linkages: false, }, ) .unwrap_or_else(|err| { @@ -183,6 +184,7 @@ fn main() { &naga::front::glsl::Options { entry_points, defines: Default::default(), + strip_unused_linkages: false, }, ) .unwrap_or_else(|err| { @@ -200,6 +202,7 @@ fn main() { &naga::front::glsl::Options { entry_points, defines: Default::default(), + strip_unused_linkages: false, }, ) .unwrap_or_else(|err| { diff --git a/src/front/glsl/ast.rs b/src/front/glsl/ast.rs index 8ee5e004b4..8008864c7c 100644 --- a/src/front/glsl/ast.rs +++ b/src/front/glsl/ast.rs @@ -69,6 +69,7 @@ pub struct EntryArg { pub binding: Binding, pub handle: Handle, pub prologue: PrologueStage, + pub storage: StorageQualifier, } #[derive(Debug)] @@ -76,6 +77,7 @@ pub struct Program<'a> { pub version: u16, pub profile: Profile, pub entry_points: &'a FastHashMap, + pub strip_unused_linkages: bool, pub workgroup_size: [u32; 3], pub early_fragment_tests: bool, @@ -94,11 +96,12 @@ pub struct Program<'a> { } impl<'a> Program<'a> { - pub fn new(entry_points: &'a FastHashMap) -> Program<'a> { + pub fn new(entry_points: &'a FastHashMap, strip_unused_linkages: bool) -> Program<'a> { Program { version: 0, profile: Profile::Core, entry_points, + strip_unused_linkages, workgroup_size: [1; 3], early_fragment_tests: false, diff --git a/src/front/glsl/functions.rs b/src/front/glsl/functions.rs index 520bdef505..d515bbcc35 100644 --- a/src/front/glsl/functions.rs +++ b/src/front/glsl/functions.rs @@ -1,7 +1,7 @@ use crate::{ proc::ensure_block_returns, Arena, BinaryOperator, Block, Constant, ConstantInner, EntryPoint, Expression, Function, FunctionArgument, FunctionResult, Handle, ImageQuery, LocalVariable, - MathFunction, RelationalFunction, SampleLevel, ScalarKind, ScalarValue, Statement, + MathFunction, RelationalFunction, SampleLevel, ScalarKind, ScalarValue, ShaderStage, Statement, StructMember, SwizzleComponent, Type, TypeInner, VectorSize, }; @@ -1159,12 +1159,23 @@ impl Program<'_> { let mut expressions = Arena::new(); let mut body = Vec::new(); + let can_strip_stage_inputs = self.strip_unused_linkages || stage != ShaderStage::Fragment; + let can_strip_stage_outputs = self.strip_unused_linkages || stage != ShaderStage::Vertex; + for (i, arg) in self.entry_args.iter().enumerate() { - if function_arg_use[function.index()] + if arg.storage != StorageQualifier::Input { + continue; + } + + if !arg.prologue.contains(stage.into()) { + continue; + } + + let is_used = function_arg_use[function.index()] .get(i) - .map_or(true, |u| !u.contains(EntryArgUse::READ)) - || !arg.prologue.contains(stage.into()) - { + .map_or(false, |u| u.contains(EntryArgUse::READ)); + + if can_strip_stage_inputs && !is_used { continue; } @@ -1194,10 +1205,15 @@ impl Program<'_> { let mut components = Vec::new(); for (i, arg) in self.entry_args.iter().enumerate() { - if function_arg_use[function.index()] + if arg.storage != StorageQualifier::Output { + continue; + } + + let is_used = function_arg_use[function.index()] .get(i) - .map_or(true, |u| !u.contains(EntryArgUse::WRITE)) - { + .map_or(false, |u| u.contains(EntryArgUse::WRITE)); + + if can_strip_stage_outputs && !is_used { continue; } diff --git a/src/front/glsl/mod.rs b/src/front/glsl/mod.rs index 1db77c271a..474ab63f27 100644 --- a/src/front/glsl/mod.rs +++ b/src/front/glsl/mod.rs @@ -23,10 +23,11 @@ mod variables; pub struct Options { pub entry_points: FastHashMap, pub defines: FastHashMap, + pub strip_unused_linkages: bool, } pub fn parse_str(source: &str, options: &Options) -> Result { - let mut program = Program::new(&options.entry_points); + let mut program = Program::new(&options.entry_points, options.strip_unused_linkages); let lex = lex::Lexer::new(source, &options.defines); let mut parser = parser::Parser::new(&mut program, lex); diff --git a/src/front/glsl/parser_tests.rs b/src/front/glsl/parser_tests.rs index 4651638673..7c80013711 100644 --- a/src/front/glsl/parser_tests.rs +++ b/src/front/glsl/parser_tests.rs @@ -14,7 +14,7 @@ fn parse_program<'a>( source: &str, entry_points: &'a crate::FastHashMap, ) -> Result, ErrorKind> { - let mut program = Program::new(entry_points); + let mut program = Program::new(entry_points, true); let defines = crate::FastHashMap::default(); let lex = Lexer::new(source, &defines); let mut parser = parser::Parser::new(&mut program, lex); diff --git a/src/front/glsl/variables.rs b/src/front/glsl/variables.rs index cec46cdfff..98994bed2e 100644 --- a/src/front/glsl/variables.rs +++ b/src/front/glsl/variables.rs @@ -45,7 +45,7 @@ impl Program<'_> { return Ok(Some(global_var)); } - let mut add_builtin = |inner, builtin, mutable, prologue| { + let mut add_builtin = |inner, builtin, mutable, prologue, storage| { let ty = self .module .types @@ -66,6 +66,7 @@ impl Program<'_> { binding: Binding::BuiltIn(builtin), handle, prologue, + storage, }); self.global_variables.push(( @@ -101,6 +102,7 @@ impl Program<'_> { BuiltIn::Position, true, PrologueStage::empty(), + StorageQualifier::Output, ), "gl_FragCoord" => add_builtin( TypeInner::Vector { @@ -111,6 +113,7 @@ impl Program<'_> { BuiltIn::Position, false, PrologueStage::FRAGMENT, + StorageQualifier::Input, ), "gl_FragDepth" => add_builtin( TypeInner::Scalar { @@ -120,6 +123,7 @@ impl Program<'_> { BuiltIn::FragDepth, true, PrologueStage::empty(), + StorageQualifier::Output, ), "gl_VertexIndex" => add_builtin( TypeInner::Scalar { @@ -129,6 +133,7 @@ impl Program<'_> { BuiltIn::VertexIndex, false, PrologueStage::VERTEX, + StorageQualifier::Input, ), "gl_InstanceIndex" => add_builtin( TypeInner::Scalar { @@ -138,6 +143,7 @@ impl Program<'_> { BuiltIn::InstanceIndex, false, PrologueStage::VERTEX, + StorageQualifier::Input, ), "gl_GlobalInvocationID" => add_builtin( TypeInner::Vector { @@ -148,6 +154,7 @@ impl Program<'_> { BuiltIn::GlobalInvocationId, false, PrologueStage::COMPUTE, + StorageQualifier::Input, ), "gl_FrontFacing" => add_builtin( TypeInner::Scalar { @@ -157,6 +164,7 @@ impl Program<'_> { BuiltIn::FrontFacing, false, PrologueStage::FRAGMENT, + StorageQualifier::Input, ), _ => Ok(None), } @@ -440,6 +448,7 @@ impl Program<'_> { }, handle, prologue, + storage, }); if let Some(name) = name { diff --git a/tests/out/wgsl/210-bevy-2d-shader-frag.wgsl b/tests/out/wgsl/210-bevy-2d-shader-frag.wgsl index f7eff0a34e..d62e395b14 100644 --- a/tests/out/wgsl/210-bevy-2d-shader-frag.wgsl +++ b/tests/out/wgsl/210-bevy-2d-shader-frag.wgsl @@ -7,7 +7,7 @@ struct FragmentOutput { [[location(0)]] o_Target: vec4; }; -var v_Uv: vec2; +var v_Uv1: vec2; var o_Target: vec4; [[group(1), binding(0)]] var global: ColorMaterial_color; @@ -23,8 +23,9 @@ fn main1() { } [[stage(fragment)]] -fn main() -> FragmentOutput { +fn main([[location(0)]] v_Uv: vec2) -> FragmentOutput { + v_Uv1 = v_Uv; main1(); - let _e1: vec4 = o_Target; - return FragmentOutput(_e1); + let _e3: vec4 = o_Target; + return FragmentOutput(_e3); } diff --git a/tests/out/wgsl/quad_glsl-frag.wgsl b/tests/out/wgsl/quad_glsl-frag.wgsl index 9f8f2eaf25..6a52a6c236 100644 --- a/tests/out/wgsl/quad_glsl-frag.wgsl +++ b/tests/out/wgsl/quad_glsl-frag.wgsl @@ -2,7 +2,7 @@ struct FragmentOutput { [[location(0)]] o_color: vec4; }; -var v_uv: vec2; +var v_uv1: vec2; var o_color: vec4; fn main1() { @@ -11,8 +11,9 @@ fn main1() { } [[stage(fragment)]] -fn main() -> FragmentOutput { +fn main([[location(0)]] v_uv: vec2) -> FragmentOutput { + v_uv1 = v_uv; main1(); - let _e1: vec4 = o_color; - return FragmentOutput(_e1); + let _e3: vec4 = o_color; + return FragmentOutput(_e3); } diff --git a/tests/snapshots.rs b/tests/snapshots.rs index 11dd72e4bd..dae06f4712 100644 --- a/tests/snapshots.rs +++ b/tests/snapshots.rs @@ -457,6 +457,7 @@ fn convert_glsl_folder() { for entry in std::fs::read_dir(format!("{}/{}/glsl", root, BASE_DIR_IN)).unwrap() { let entry = entry.unwrap(); let file_name = entry.file_name().into_string().unwrap(); + if file_name.ends_with(".ron") { // No needed to validate ron files continue; @@ -498,11 +499,13 @@ fn convert_glsl_folder() { entry_points.insert("main".to_string(), stage); } + let strip_unused_linkages = entry_points.len() > 1; let module = naga::front::glsl::parse_str( &fs::read_to_string(entry.path()).expect("Couldn't find glsl file"), &naga::front::glsl::Options { entry_points, defines: Default::default(), + strip_unused_linkages: strip_unused_linkages, }, ) .unwrap();