Skip to content

Commit

Permalink
Only execute #define if current scope is accepting lines (bevyengin…
Browse files Browse the repository at this point in the history
…e#7798)

# Objective

While working on bevyengine#7784, I noticed that a `#define VAR` in a `.wgsl` file is always effective, even if it its scope is not accepting lines. 

Example:
```c
#define A
#ifndef A
#define B
#endif
```

Currently, `B` will be defined although it shouldn't. This PR fixes that. 

## Solution

Move the branch responsible for `#define` lines into the last else branch, which is only evaluated if the current scope is accepting lines.
  • Loading branch information
geieredgar authored and Shfty committed Mar 19, 2023
1 parent 49f74af commit 250c64a
Showing 1 changed file with 48 additions and 20 deletions.
68 changes: 48 additions & 20 deletions crates/bevy_render/src/render_resource/shader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -556,26 +556,6 @@ impl ShaderProcessor {
let current_valid = scopes.last().unwrap().is_accepting_lines();

scopes.push(Scope::new(current_valid && new_scope));
} else if let Some(cap) = self.define_regex.captures(line) {
let def = cap.get(1).unwrap();
let name = def.as_str().to_string();

if let Some(val) = cap.get(2) {
if let Ok(val) = val.as_str().parse::<u32>() {
shader_defs_unique.insert(name.clone(), ShaderDefVal::UInt(name, val));
} else if let Ok(val) = val.as_str().parse::<i32>() {
shader_defs_unique.insert(name.clone(), ShaderDefVal::Int(name, val));
} else if let Ok(val) = val.as_str().parse::<bool>() {
shader_defs_unique.insert(name.clone(), ShaderDefVal::Bool(name, val));
} else {
return Err(ProcessShaderError::InvalidShaderDefDefinitionValue {
shader_def_name: name,
value: val.as_str().to_string(),
});
}
} else {
shader_defs_unique.insert(name.clone(), ShaderDefVal::Bool(name, true));
}
} else if let Some(cap) = self.else_ifdef_regex.captures(line) {
// When should we accept the code in an
//
Expand Down Expand Up @@ -680,6 +660,26 @@ impl ShaderProcessor {
.is_match(line)
{
// ignore import path lines
} else if let Some(cap) = self.define_regex.captures(line) {
let def = cap.get(1).unwrap();
let name = def.as_str().to_string();

if let Some(val) = cap.get(2) {
if let Ok(val) = val.as_str().parse::<u32>() {
shader_defs_unique.insert(name.clone(), ShaderDefVal::UInt(name, val));
} else if let Ok(val) = val.as_str().parse::<i32>() {
shader_defs_unique.insert(name.clone(), ShaderDefVal::Int(name, val));
} else if let Ok(val) = val.as_str().parse::<bool>() {
shader_defs_unique.insert(name.clone(), ShaderDefVal::Bool(name, val));
} else {
return Err(ProcessShaderError::InvalidShaderDefDefinitionValue {
shader_def_name: name,
value: val.as_str().to_string(),
});
}
} else {
shader_defs_unique.insert(name.clone(), ShaderDefVal::Bool(name, true));
}
} else {
let mut line_with_defs = line.to_string();
for capture in self.def_regex.captures_iter(line) {
Expand Down Expand Up @@ -2503,6 +2503,34 @@ defined at end
assert_eq!(result.get_wgsl_source().unwrap(), EXPECTED);
}

#[test]
fn process_shader_define_only_in_accepting_scopes() {
#[rustfmt::skip]
const WGSL: &str = r"
#define GUARD
#ifndef GUARD
#define GUARDED
#endif
#ifdef GUARDED
This should not be part of the result
#endif
";

#[rustfmt::skip]
const EXPECTED: &str = r"
";
let processor = ShaderProcessor::default();
let result = processor
.process(
&Shader::from_wgsl(WGSL),
&[],
&HashMap::default(),
&HashMap::default(),
)
.unwrap();
assert_eq!(result.get_wgsl_source().unwrap(), EXPECTED);
}

#[test]
fn process_shader_define_in_shader_with_value() {
#[rustfmt::skip]
Expand Down

0 comments on commit 250c64a

Please sign in to comment.