From abef6aad2aef6b9fb8cc045c2354503c5f4c7ce6 Mon Sep 17 00:00:00 2001 From: Thierry Berger Date: Fri, 4 Oct 2024 12:13:48 +0200 Subject: [PATCH 1/5] add support for comments --- Cargo.toml | 4 ++++ src/compose/mod.rs | 17 +++++++++++++++-- src/compose/preprocess.rs | 13 ++++++++----- src/derive.rs | 19 ++++++++++++++++--- 4 files changed, 43 insertions(+), 10 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index cfaf4d6..bfc4a77 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -34,3 +34,7 @@ indexmap = "2" wgpu = { version = "22", features = ["naga-ir"] } futures-lite = "1" tracing-subscriber = { version = "0.3", features = ["std", "fmt"] } + +[patch.crates-io] +# naga = { path = "../wgpu/naga" } +naga = { git = "https://github.com/Vrixyz/wgpu.git", branch = "token-comment" } diff --git a/src/compose/mod.rs b/src/compose/mod.rs index af6672f..96692ff 100644 --- a/src/compose/mod.rs +++ b/src/compose/mod.rs @@ -570,7 +570,9 @@ impl Composer { } } - // build naga module for a given shader_def configuration. builds a minimal self-contained module built against headers for imports + /// Build naga module for a given shader_def configuration. + /// + /// Builds a minimal self-contained module built against headers for imports. fn create_module_ir( &self, name: &str, @@ -1160,6 +1162,8 @@ impl Composer { // todo figure out how to get span info for entrypoints } } + // Copy comments, should be done after each commented item has been imported. + module_builder.import_comments(&source_ir.comments); let module_ir = module_builder.into_module_with_entrypoints(); let mut header_ir: naga::Module = header_builder.into(); @@ -1281,6 +1285,8 @@ impl Composer { } } + derived.import_comments(&composable.module_ir.comments); + derived.clear_shader_source(); } @@ -1835,6 +1841,8 @@ static PREPROCESSOR: once_cell::sync::Lazy = once_cell::sync::Lazy::new(Preprocessor::default); /// Get module name and all required imports (ignoring shader_defs) from a shader string +/// +/// Returns nothing in case of error. The actual error will be displayed when the caller attempts to use the shader. pub fn get_preprocessor_data( source: &str, ) -> ( @@ -1847,7 +1855,7 @@ pub fn get_preprocessor_data( imports, defines, .. - }) = PREPROCESSOR.get_preprocessor_metadata(source, true) + }) = try_get_preprocessor_data(source) { ( name, @@ -1862,3 +1870,8 @@ pub fn get_preprocessor_data( Default::default() } } + +/// Get module name and all required imports (ignoring shader_defs) from a shader string +pub fn try_get_preprocessor_data(source: &str) -> Result { + PREPROCESSOR.get_preprocessor_metadata(source, true) +} diff --git a/src/compose/preprocess.rs b/src/compose/preprocess.rs index f9e0a04..bd206bd 100644 --- a/src/compose/preprocess.rs +++ b/src/compose/preprocess.rs @@ -225,10 +225,11 @@ impl Preprocessor { Ok((false, None)) } - // process #if[(n)?def]? / #else / #endif preprocessor directives, - // strip module name and imports - // also strip "#version xxx" - // replace items with resolved decorated names + /// - strip comments + /// - Process `#if[(n)?def]?` / `#else` / `#endif` preprocessor directives + /// - strip module name and imports + /// - strip `#version xxx` + /// - replace items with resolved decorated names pub fn preprocess( &self, shader_str: &str, @@ -242,7 +243,9 @@ impl Preprocessor { // this code broadly stolen from bevy_render::ShaderProcessor let mut lines = shader_str.lines(); - let mut lines = lines.replace_comments().zip(shader_str.lines()).peekable(); + let mut lines = lines //.replace_comments() + .zip(shader_str.lines()) + .peekable(); while let Some((mut line, original_line)) = lines.next() { let mut output = false; diff --git a/src/derive.rs b/src/derive.rs index 8aeef3e..78f8696 100644 --- a/src/derive.rs +++ b/src/derive.rs @@ -1,8 +1,9 @@ use indexmap::IndexMap; use naga::{ - Arena, AtomicFunction, Block, Constant, EntryPoint, Expression, Function, FunctionArgument, - FunctionResult, GatherMode, GlobalVariable, Handle, ImageQuery, LocalVariable, Module, - Override, SampleLevel, Span, Statement, StructMember, SwitchCase, Type, TypeInner, UniqueArena, + Arena, AtomicFunction, Block, Comments, Constant, EntryPoint, Expression, Function, + FunctionArgument, FunctionResult, GatherMode, GlobalVariable, Handle, ImageQuery, + LocalVariable, Module, Override, SampleLevel, Span, Statement, StructMember, SwitchCase, Type, + TypeInner, UniqueArena, }; use std::{cell::RefCell, rc::Rc}; @@ -30,6 +31,7 @@ pub struct DerivedModule<'a> { globals: Arena, functions: Arena, pipeline_overrides: Arena, + comments: Comments, } impl<'a> DerivedModule<'a> { @@ -807,6 +809,16 @@ impl<'a> DerivedModule<'a> { self.import_function(func, span) } + pub(crate) fn import_comments(&mut self, comments: &Comments) { + // Deconstructing to not miss a new property to map if we add more. + let Comments { types } = comments; + for comment in types.iter() { + self.comments + .types + .insert(*self.type_map.get(comment.0).unwrap(), comment.1.clone()); + } + } + pub fn into_module_with_entrypoints(mut self) -> naga::Module { let entry_points = self .shader @@ -842,6 +854,7 @@ impl<'a> From> for naga::Module { special_types: Default::default(), entry_points: Default::default(), overrides: derived.pipeline_overrides, + comments: derived.comments, } } } From 6d2c50a77c49258c18495ee6716d9f2d5c004dd2 Mon Sep 17 00:00:00 2001 From: Thierry Berger Date: Fri, 4 Oct 2024 15:29:37 +0200 Subject: [PATCH 2/5] expose a field + revert back preprocess comment removal --- src/compose/mod.rs | 2 +- src/compose/preprocess.rs | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/compose/mod.rs b/src/compose/mod.rs index 96692ff..19fb4c9 100644 --- a/src/compose/mod.rs +++ b/src/compose/mod.rs @@ -306,7 +306,7 @@ pub struct ImportDefinition { #[derive(Debug, Clone)] pub struct ImportDefWithOffset { - definition: ImportDefinition, + pub definition: ImportDefinition, offset: usize, } diff --git a/src/compose/preprocess.rs b/src/compose/preprocess.rs index bd206bd..ea5d236 100644 --- a/src/compose/preprocess.rs +++ b/src/compose/preprocess.rs @@ -243,9 +243,7 @@ impl Preprocessor { // this code broadly stolen from bevy_render::ShaderProcessor let mut lines = shader_str.lines(); - let mut lines = lines //.replace_comments() - .zip(shader_str.lines()) - .peekable(); + let mut lines = lines.replace_comments().zip(shader_str.lines()).peekable(); while let Some((mut line, original_line)) = lines.next() { let mut output = false; From 92e1a5bcb87b7228b1ba330dbac791c4d4d3f5fb Mon Sep 17 00:00:00 2001 From: Thierry Berger Date: Mon, 7 Oct 2024 16:20:56 +0200 Subject: [PATCH 3/5] add comment support for consts, global variables, functions and struct members --- src/derive.rs | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/src/derive.rs b/src/derive.rs index 78f8696..667fc5e 100644 --- a/src/derive.rs +++ b/src/derive.rs @@ -811,12 +811,40 @@ impl<'a> DerivedModule<'a> { pub(crate) fn import_comments(&mut self, comments: &Comments) { // Deconstructing to not miss a new property to map if we add more. - let Comments { types } = comments; + let Comments { + types, + functions, + constants, + global_variables, + struct_members, + } = comments; for comment in types.iter() { self.comments .types .insert(*self.type_map.get(comment.0).unwrap(), comment.1.clone()); } + for ((struct_handle, index), comment) in struct_members.iter() { + self.comments.struct_members.insert( + (*self.type_map.get(struct_handle).unwrap(), *index), + comment.clone(), + ); + } + for function in functions.iter() { + self.comments + .functions + .insert(function.0.to_string(), function.1.clone()); + } + for constant in constants.iter() { + self.comments + .constants + .insert(*self.const_map.get(constant.0).unwrap(), constant.1.clone()); + } + for global_variable in global_variables.iter() { + self.comments.global_variables.insert( + *self.global_map.get(global_variable.0).unwrap(), + global_variable.1.clone(), + ); + } } pub fn into_module_with_entrypoints(mut self) -> naga::Module { From 86b89c89bc013111ee9f47167a959d3b458e4b21 Mon Sep 17 00:00:00 2001 From: Thierry Berger Date: Tue, 8 Oct 2024 15:54:58 +0200 Subject: [PATCH 4/5] less unwraps +_better global support --- src/derive.rs | 36 ++++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/src/derive.rs b/src/derive.rs index 667fc5e..ef7e0af 100644 --- a/src/derive.rs +++ b/src/derive.rs @@ -819,15 +819,16 @@ impl<'a> DerivedModule<'a> { struct_members, } = comments; for comment in types.iter() { - self.comments - .types - .insert(*self.type_map.get(comment.0).unwrap(), comment.1.clone()); + if let Some(new_handle) = self.type_map.get(comment.0) { + self.comments.types.insert(*new_handle, comment.1.clone()); + } } for ((struct_handle, index), comment) in struct_members.iter() { - self.comments.struct_members.insert( - (*self.type_map.get(struct_handle).unwrap(), *index), - comment.clone(), - ); + if let Some(new_struct_handle) = self.type_map.get(struct_handle) { + self.comments + .struct_members + .insert((*new_struct_handle, *index), comment.clone()); + } } for function in functions.iter() { self.comments @@ -835,15 +836,22 @@ impl<'a> DerivedModule<'a> { .insert(function.0.to_string(), function.1.clone()); } for constant in constants.iter() { - self.comments - .constants - .insert(*self.const_map.get(constant.0).unwrap(), constant.1.clone()); + if let Some(new_handle) = self.const_map.get(constant.0) { + self.comments + .constants + .insert(*new_handle, constant.1.clone()); + } } for global_variable in global_variables.iter() { - self.comments.global_variables.insert( - *self.global_map.get(global_variable.0).unwrap(), - global_variable.1.clone(), - ); + if let Some(new_handle) = self.global_map.get(global_variable.0) { + self.comments + .global_variables + .insert(*new_handle, global_variable.1.clone()); + } else { + // NOTE: Could not migrate global variable, I'm not sure why this happens. + // It's probably because this module is not the owner of the global. + // Chances are that comment will be migrated by another module. + } } } From 6330223915aced811870f5f2ce39ebfbef84cd4f Mon Sep 17 00:00:00 2001 From: Thierry Berger Date: Thu, 10 Oct 2024 15:32:49 +0200 Subject: [PATCH 5/5] support for module comments --- src/compose/mod.rs | 3 --- src/derive.rs | 2 ++ 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/compose/mod.rs b/src/compose/mod.rs index 19fb4c9..6f94883 100644 --- a/src/compose/mod.rs +++ b/src/compose/mod.rs @@ -690,7 +690,6 @@ impl Composer { } })?, }; - Ok(IrBuildResult { module, start_offset, @@ -964,7 +963,6 @@ impl Composer { module_definition.name, source.len() ); - let IrBuildResult { module: mut source_ir, start_offset, @@ -1164,7 +1162,6 @@ impl Composer { } // Copy comments, should be done after each commented item has been imported. module_builder.import_comments(&source_ir.comments); - let module_ir = module_builder.into_module_with_entrypoints(); let mut header_ir: naga::Module = header_builder.into(); diff --git a/src/derive.rs b/src/derive.rs index ef7e0af..faee8f8 100644 --- a/src/derive.rs +++ b/src/derive.rs @@ -817,7 +817,9 @@ impl<'a> DerivedModule<'a> { constants, global_variables, struct_members, + module, } = comments; + self.comments.module = module.clone(); for comment in types.iter() { if let Some(new_handle) = self.type_map.get(comment.0) { self.comments.types.insert(*new_handle, comment.1.clone());