Skip to content

Commit

Permalink
Deterministic output (#75)
Browse files Browse the repository at this point in the history 
fix OS-level shader caching by switching to `IndexMap` to maintain
naga's deterministic output.
  • Loading branch information
@robtfm
robtfm committed Feb 2, 2024
1 parent 983d31f commit 33e57e4
Show file tree
Hide file tree
Showing 11 changed files with 117 additions and 171 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "naga_oil"
version = "0.12.0"
version = "0.13.0"
edition = "2021"
license = "MIT OR Apache-2.0"
description = "a crate for combining and manipulating shaders using naga IR"
Expand Down
21 changes: 11 additions & 10 deletions src/compose/mod.rs
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use indexmap::IndexMap;
/// the compose module allows construction of shaders from modules (which are themselves shaders).
///
/// it does this by treating shaders as modules, and
Expand Down Expand Up @@ -235,7 +236,7 @@ pub struct ComposableModule {
pub virtual_functions: HashSet<String>,
// overriding functions defined in this module
// target function -> Vec<replacement functions>
pub override_functions: HashMap<String, Vec<String>>,
pub override_functions: IndexMap<String, Vec<String>>,
// naga module, built against headers for any imports
module_ir: naga::Module,
// headers in different shader languages, used for building modules/shaders that import this module
Expand Down Expand Up @@ -397,7 +398,7 @@ const DECORATION_OVERRIDE_PRE: &str = "X_naga_oil_vrt_X";
struct IrBuildResult {
module: naga::Module,
start_offset: usize,
override_functions: HashMap<String, Vec<String>>,
override_functions: IndexMap<String, Vec<String>>,
}

impl Composer {
Expand Down Expand Up @@ -583,7 +584,7 @@ impl Composer {
ShaderLanguage::Glsl => String::from("#version 450\n"),
};

let mut override_functions: HashMap<String, Vec<String>> = HashMap::default();
let mut override_functions: IndexMap<String, Vec<String>> = IndexMap::default();
let mut added_imports: HashSet<String> = HashSet::new();
let mut header_module = DerivedModule::default();

Expand Down Expand Up @@ -611,7 +612,7 @@ impl Composer {
if !module.override_functions.is_empty() {
for (original, replacements) in &module.override_functions {
match override_functions.entry(original.clone()) {
Entry::Occupied(o) => {
indexmap::map::Entry::Occupied(o) => {
let existing = o.into_mut();
let new_replacements: Vec<_> = replacements
.iter()
Expand All @@ -620,7 +621,7 @@ impl Composer {
.collect();
existing.extend(new_replacements);
}
Entry::Vacant(v) => {
indexmap::map::Entry::Vacant(v) => {
v.insert(replacements.clone());
}
}
Expand Down Expand Up @@ -723,7 +724,7 @@ impl Composer {
})?,
};

let recompiled_types: HashMap<_, _> = recompiled
let recompiled_types: IndexMap<_, _> = recompiled
.types
.iter()
.flat_map(|(h, ty)| ty.name.as_deref().map(|name| (name, h)))
Expand Down Expand Up @@ -869,7 +870,7 @@ impl Composer {
});

// record and rename override functions
let mut local_override_functions: HashMap<String, String> = Default::default();
let mut local_override_functions: IndexMap<String, String> = Default::default();

#[cfg(not(feature = "override_any"))]
let mut override_error = None;
Expand Down Expand Up @@ -992,7 +993,7 @@ impl Composer {
}

// rename and record owned items (except types which can't be mutably accessed)
let mut owned_constants = HashMap::new();
let mut owned_constants = IndexMap::new();
for (h, c) in source_ir.constants.iter_mut() {
if let Some(name) = c.name.as_mut() {
if !name.contains(DECORATION_PRE) {
Expand All @@ -1002,7 +1003,7 @@ impl Composer {
}
}

let mut owned_vars = HashMap::new();
let mut owned_vars = IndexMap::new();
for (h, gv) in source_ir.global_variables.iter_mut() {
if let Some(name) = gv.name.as_mut() {
if !name.contains(DECORATION_PRE) {
Expand All @@ -1013,7 +1014,7 @@ impl Composer {
}
}

let mut owned_functions = HashMap::new();
let mut owned_functions = IndexMap::new();
for (h_f, f) in source_ir.functions.iter_mut() {
if let Some(name) = f.name.as_mut() {
if !name.contains(DECORATION_PRE) {
Expand Down
34 changes: 17 additions & 17 deletions src/compose/parse_imports.rs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::collections::HashMap;
use indexmap::IndexMap;

use super::{
tokenizer::{Token, Tokenizer},
Expand All @@ -7,7 +7,7 @@ use super::{

pub fn parse_imports<'a>(
input: &'a str,
declared_imports: &mut HashMap<String, Vec<String>>,
declared_imports: &mut IndexMap<String, Vec<String>>,
) -> Result<(), (&'a str, usize)> {
let mut tokens = Tokenizer::new(input, false).peekable();

Expand Down Expand Up @@ -116,8 +116,8 @@ pub fn parse_imports<'a>(
pub fn substitute_identifiers(
input: &str,
offset: usize,
declared_imports: &HashMap<String, Vec<String>>,
used_imports: &mut HashMap<String, ImportDefWithOffset>,
declared_imports: &IndexMap<String, Vec<String>>,
used_imports: &mut IndexMap<String, ImportDefWithOffset>,
allow_ambiguous: bool,
) -> Result<String, usize> {
let tokens = Tokenizer::new(input, true);
Expand Down Expand Up @@ -193,8 +193,8 @@ pub fn substitute_identifiers(
}

#[cfg(test)]
fn test_parse(input: &str) -> Result<HashMap<String, Vec<String>>, (&str, usize)> {
let mut declared_imports = HashMap::default();
fn test_parse(input: &str) -> Result<IndexMap<String, Vec<String>>, (&str, usize)> {
let mut declared_imports = IndexMap::default();
parse_imports(input, &mut declared_imports)?;
Ok(declared_imports)
}
Expand All @@ -206,7 +206,7 @@ fn import_tokens() {
";
assert_eq!(
test_parse(input),
Ok(HashMap::from_iter([(
Ok(IndexMap::from_iter([(
"b".to_owned(),
vec!("a::b".to_owned())
)]))
Expand All @@ -217,7 +217,7 @@ fn import_tokens() {
";
assert_eq!(
test_parse(input),
Ok(HashMap::from_iter([
Ok(IndexMap::from_iter([
("b".to_owned(), vec!("a::b".to_owned())),
("c".to_owned(), vec!("a::c".to_owned())),
]))
Expand All @@ -228,7 +228,7 @@ fn import_tokens() {
";
assert_eq!(
test_parse(input),
Ok(HashMap::from_iter([
Ok(IndexMap::from_iter([
("d".to_owned(), vec!("a::b".to_owned())),
("c".to_owned(), vec!("a::c".to_owned())),
]))
Expand All @@ -239,7 +239,7 @@ fn import_tokens() {
";
assert_eq!(
test_parse(input),
Ok(HashMap::from_iter([
Ok(IndexMap::from_iter([
("c".to_owned(), vec!("a::b::c".to_owned())),
("d".to_owned(), vec!("a::b::d".to_owned())),
("e".to_owned(), vec!("a::e".to_owned())),
Expand All @@ -251,7 +251,7 @@ fn import_tokens() {
";
assert_eq!(
test_parse(input),
Ok(HashMap::from_iter([
Ok(IndexMap::from_iter([
("c".to_owned(), vec!("a::b::c".to_owned())),
("d".to_owned(), vec!("a::b::d".to_owned())),
("e".to_owned(), vec!("e".to_owned())),
Expand All @@ -263,7 +263,7 @@ fn import_tokens() {
";
assert_eq!(
test_parse(input),
Ok(HashMap::from_iter([
Ok(IndexMap::from_iter([
("a".to_owned(), vec!("a".to_owned())),
("b".to_owned(), vec!("b".to_owned())),
]))
Expand All @@ -274,7 +274,7 @@ fn import_tokens() {
";
assert_eq!(
test_parse(input),
Ok(HashMap::from_iter([
Ok(IndexMap::from_iter([
("c".to_owned(), vec!("a::b::c".to_owned())),
("d".to_owned(), vec!("a::b::d".to_owned())),
]))
Expand All @@ -285,7 +285,7 @@ fn import_tokens() {
";
assert_eq!(
test_parse(input),
Ok(HashMap::from_iter([(
Ok(IndexMap::from_iter([(
"c".to_owned(),
vec!("a::b::c".to_owned())
),]))
Expand All @@ -296,7 +296,7 @@ fn import_tokens() {
";
assert_eq!(
test_parse(input),
Ok(HashMap::from_iter([
Ok(IndexMap::from_iter([
("d".to_owned(), vec!("a::b::c::d".to_owned())),
("e".to_owned(), vec!("a::b::c::e".to_owned())),
("f".to_owned(), vec!("a::b::f".to_owned())),
Expand All @@ -317,7 +317,7 @@ fn import_tokens() {
";
assert_eq!(
test_parse(input),
Ok(HashMap::from_iter([
Ok(IndexMap::from_iter([
("d".to_owned(), vec!("a::b::c::d".to_owned())),
("e".to_owned(), vec!("a::b::c::e".to_owned())),
("f".to_owned(), vec!("a::b::f".to_owned())),
Expand All @@ -331,7 +331,7 @@ fn import_tokens() {
"#;
assert_eq!(
test_parse(input),
Ok(HashMap::from_iter([
Ok(IndexMap::from_iter([
(
"a".to_owned(),
vec!(r#""path//with\ all sorts of .stuff"::a"#.to_owned())
Expand Down
9 changes: 5 additions & 4 deletions src/compose/preprocess.rs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::collections::{HashMap, HashSet};

use indexmap::IndexMap;
use regex::Regex;

use super::{
Expand Down Expand Up @@ -234,8 +235,8 @@ impl Preprocessor {
shader_defs: &HashMap<String, ShaderDefValue>,
validate_len: bool,
) -> Result<PreprocessOutput, ComposerErrorInner> {
let mut declared_imports = HashMap::new();
let mut used_imports = HashMap::new();
let mut declared_imports = IndexMap::new();
let mut used_imports = IndexMap::new();
let mut scope = Scope::new();
let mut final_string = String::new();
let mut offset = 0;
Expand Down Expand Up @@ -390,8 +391,8 @@ impl Preprocessor {
shader_str: &str,
allow_defines: bool,
) -> Result<PreprocessorMetaData, ComposerErrorInner> {
let mut declared_imports = HashMap::default();
let mut used_imports = HashMap::default();
let mut declared_imports = IndexMap::default();
let mut used_imports = IndexMap::default();
let mut name = None;
let mut offset = 0;
let mut defines = HashMap::default();
Expand Down
26 changes: 1 addition & 25 deletions src/compose/test.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,6 @@ mod test {
naga::back::wgsl::WriterFlags::EXPLICIT_TYPES,
)
.unwrap();
let mut wgsl: Vec<_> = wgsl.lines().collect();
wgsl.sort();
let wgsl = wgsl.join("\n");

// println!("{}", wgsl);
// let mut f = std::fs::File::create("dup_import.txt").unwrap();
Expand Down Expand Up @@ -338,21 +335,11 @@ mod test {
)
.unwrap();

// unfortunately glsl variables are emitted in random order...
// so this is better than nothing
let mut wgsl: Vec<_> = wgsl.lines().collect();
wgsl.sort();
let wgsl = wgsl.join("\n");

// let mut f = std::fs::File::create("wgsl_call_glsl.txt").unwrap();
// f.write_all(wgsl.as_bytes()).unwrap();
// drop(f);

// assert_eq!(wgsl, include_str!("tests/expected/wgsl_call_glsl.txt"));

// actually it's worse than that ... glsl output seems volatile over struct names
// i suppose at least we are testing that it doesn't throw any errors ..?
let _ = wgsl;
output_eq!(wgsl, "tests/expected/wgsl_call_glsl.txt");
}

#[cfg(feature = "glsl")]
Expand Down Expand Up @@ -852,11 +839,6 @@ mod test {
)
.unwrap();

// println!("{}", wgsl);
let mut wgsl = wgsl.lines().collect::<Vec<_>>();
wgsl.sort();
let wgsl = wgsl.join("\n");

// let mut f = std::fs::File::create("item_import_test.txt").unwrap();
// f.write_all(wgsl.as_bytes()).unwrap();
// drop(f);
Expand Down Expand Up @@ -923,9 +905,6 @@ mod test {
naga::back::wgsl::WriterFlags::EXPLICIT_TYPES,
)
.unwrap();
let mut wgsl: Vec<_> = wgsl.lines().collect();
wgsl.sort();
let wgsl = wgsl.join("\n");

// let mut f = std::fs::File::create("bad_identifiers.txt").unwrap();
// f.write_all(wgsl.as_bytes()).unwrap();
Expand Down Expand Up @@ -992,9 +971,6 @@ mod test {
naga::back::wgsl::WriterFlags::EXPLICIT_TYPES,
)
.unwrap();
let mut wgsl: Vec<_> = wgsl.lines().collect();
wgsl.sort();
let wgsl = wgsl.join("\n");

// let mut f = std::fs::File::create("dup_struct_import.txt").unwrap();
// f.write_all(wgsl.as_bytes()).unwrap();
Expand Down
50 changes: 25 additions & 25 deletions src/compose/tests/expected/bad_identifiers.txt
View file
Original file line number Diff line number Diff line change
@@ -1,39 +1,39 @@
struct IsFineX_naga_oil_mod_XON2HE5LDORZQX {
fine: f32,
}

struct Isbad_X_naga_oil_mod_XON2HE5LDORZQX {
fine_member: f32,
}

const fineX_naga_oil_mod_XMNXW443UOMX: f32 = 1f;
const bad_X_naga_oil_mod_XMNXW443UOMX: f32 = 1f;

var<private> fineX_naga_oil_mod_XM5WG6YTBNRZQX: f32 = 1f;
var<private> bad_X_naga_oil_mod_XM5WG6YTBNRZQX: f32 = 1f;

fn fineX_naga_oil_mod_XMZXHGX(in: f32) -> f32 {
return in;
}

fn bad_X_naga_oil_mod_XMZXHGX(in_1: f32) -> f32 {
return in_1;
}

fn main() -> f32 {
var d: IsFineX_naga_oil_mod_XON2HE5LDORZQX;
var e: Isbad_X_naga_oil_mod_XON2HE5LDORZQX;


d.fine = 3f;
e.fine_member = 4f;
fine: f32,
fine_member: f32,
let _e1: f32 = fineX_naga_oil_mod_XMZXHGX(1f);
let _e20: f32 = d.fine;
let _e23: f32 = e.fine_member;
let _e3: f32 = bad_X_naga_oil_mod_XMZXHGX(2f);
let b: f32 = (_e1 + _e3);
let _e6: f32 = fineX_naga_oil_mod_XM5WG6YTBNRZQX;
let _e8: f32 = bad_X_naga_oil_mod_XM5WG6YTBNRZQX;
let b: f32 = (_e1 + _e3);
let c: f32 = (_e6 + _e8);
d.fine = 3f;
e.fine_member = 4f;
let _e20: f32 = d.fine;
let _e23: f32 = e.fine_member;
return ((((2f + b) + c) + _e20) + _e23);
return in;
return in_1;
var d: IsFineX_naga_oil_mod_XON2HE5LDORZQX;
var e: Isbad_X_naga_oil_mod_XON2HE5LDORZQX;
const bad_X_naga_oil_mod_XMNXW443UOMX: f32 = 1f;
const fineX_naga_oil_mod_XMNXW443UOMX: f32 = 1f;
fn bad_X_naga_oil_mod_XMZXHGX(in_1: f32) -> f32 {
fn fineX_naga_oil_mod_XMZXHGX(in: f32) -> f32 {
fn main() -> f32 {
struct IsFineX_naga_oil_mod_XON2HE5LDORZQX {
struct Isbad_X_naga_oil_mod_XON2HE5LDORZQX {
var<private> bad_X_naga_oil_mod_XM5WG6YTBNRZQX: f32 = 1f;
var<private> fineX_naga_oil_mod_XM5WG6YTBNRZQX: f32 = 1f;
}
}
}
}
}

Loading

0 comments on commit 33e57e4

Please sign in to comment.