bevyengine / naga_oil Goto Github PK

View Code? Open in Web Editor NEW

113.0 113.0 32.0 378 KB

License: Apache License 2.0

Rust 93.75% GLSL 0.31% WGSL 5.95%

naga_oil's People

Contributors

Stargazers

Watchers

naga_oil's Issues

Make `naga_oil` `stringify!` compatible.

naga_oil not being stringify! compatible severely hurts the viability of macro generated shaders.

To make naga_oil compatible we need

Treat # import as #import.
Ignore line breaks and use expression based parsing

Questions around modules: type inside the module named same as the module.

This is more of a design question.

If I have a type inside a module which is named the same as the module itself, I'd want to be able to import it twice.

One for importing the actual type directly;
Other for importing the module but fully qualifying when using any of the functions inside the module.

The following currently doesn't work and I get the error. Only approach I have found is to name the struct differently or use pascal casing.

error: required import 'fp64::fp64' not found
  ┌─ :8:20
  │
8 │             return fp64::add(a, b);
  │                    ^
  │
  = missing import 'fp64::fp64'

composer
    .add_composable_module(ComposableModuleDescriptor {
        source: r#"
      struct fp64 {
        high: f32,
        low: f32
      }

      fn add(a: fp64, b: fp64) -> fp64 {
        let high= a.high + b.high;
        let low= a.low + b.low;
        return fp64(high, low);
      }
    "#,
        file_path: "fp64.wgsl",
        as_name: Some("fp64".to_owned()),
        ..Default::default()
    })
    .unwrap();

let module = composer
    .make_naga_module(NagaModuleDescriptor {
        source: r#"
      #import fp64;
      #import fp64::fp64;

      fn test() -> fp64 {
        let a = fp64(0.0, 0.0);
        let b = fp64(0.0, 0.0);
        return fp64::add(a, b);
      }

      @group(0) @binding(1) var<uniform> bounds: fp64;
    "#,
        file_path: "",
        ..Default::default()
    })
    .map_err(|err| println!("{}", err.emit_to_string(&composer)))
    .unwrap();

What would be best approach to organize otherwise?

Bind group bindings

When I use compute shaders, I need to create bind group layouts on the Rust side that match the shader. This is a redundant definition and needs to be kept in sync with Rust source.

I'm wondering if this could be automated to have one source of truth for the binding groups and their layout.

For example, could naga parse @group(0) @binding(0) WGSL directives and expose them as some Bevy type like BindGroupLayout?

or do the inverse, and take BindGroupLayouts when parsing a shader, and inject bind group definitions based on the Rust side?

This would be particularly nice if it supported structs in uniforms, because struct definitions have to be manually kept in sync between Rust and shaders.

Item substitution doesn’t respect shader defs

Conditionally importing the same item on different cases causes the wrong item to be substituted, as substitution happens before preprocessing.

From nisevoid on discord:
Like this my shader works (if EFFECT_ID is 3)

#if EFFECT_ID == 3
    #import "shaders/skills/railgun_trail.wgsl" frag, vert
#endif

It still works now (still EFFECT_ID 3):

#if EFFECT_ID == 2
    #import "shaders/skills/slash.wgsl" frag, vert
#else if EFFECT_ID == 3
    #import "shaders/skills/railgun_trail.wgsl" frag, vert
#endif

But now it doesn't (still EFFECT_ID 3):

#if EFFECT_ID == 3
    #import "shaders/skills/railgun_trail.wgsl" frag, vert
#else if EFFECT_ID == 4
    #import "shaders/skills/magic_arrow.wgsl" frag, vert
#endif

It also doesn't if I use the second snippet and swap the ifs around. Same thing with separate if blocks instead of else ifs. Same thing happens on any other effect as long as another follows it, it'll say something along the lines of missing import '"shaders/skills/railgun_trail.wgsl"'. Same thing happens with non-local imports. It even breaks if I put an import after it that isn't in an if ... I'm not entirely sure what is even going wrong here 🤔

Expose error labels or source locations in `ComposerError`

There's some useful logic buried in ComposerError::emit_to_string_internal for calculating labels and notes to create a diagnostic for the error.

It would be useful for tooling integration if it could be made part of naga_oil's public API somehow, similar to how naga exposes ParseError::labels, ParseError::location.

#import "file" without list of names should err/warn

Import with a file path, but without a list of names, "fails" silently:

#import "common.wgsl"

This syntax is very similar to C's #include that just imports everything, so I did not think of specifying individual type names, and was puzzled why this directive doesn't seem to do anything, and doesn't even cause a compilation error.

Standardize #include

I've noticed that quite a few projects have started using naga oil. 🎉 This has lead to reasonable feature requests in WGSL language servers and WGSL tools:

PolyMeilex/vscode-wgsl#25 - wgsl-analyzer will need to re-implement the #include algorithm
wgsl-analyzer/wgsl-analyzer#88 - straightforward integration of naga_oil
ScanMountGoat/wgsl_to_wgpu#40 - naga_oil currently doesn't provide the required type info
https://github.com/dannymcgee/vscode-wgsl - doesn't have a feature request yet, but has a PR

Would it be possible to fully document how the #include algorithm works? And, potentially fix any rough edges that it might have at the moment?

I would be willing to help with this. Is there a preferred place for communication? :)

Confusing shader_defs field in ComposableModuleDescriptor and NagaModuleDescriptor

What's the difference between shader_defs in ComposableModuleDescriptor and that in NagaModuleDescriptor? I can understand why pass defs when calling make_naga_module, but what do we do with the defs when calling add_composable_module? Should I keep them the same during these two calls? As I tested, only the defs past to make_naga_module took effect.

#[derive(Default)]
pub struct ComposableModuleDescriptor<'a> {
    pub source: &'a str,
    pub file_path: &'a str,
    pub language: ShaderLanguage,
    pub as_name: Option<String>,
    pub additional_imports: &'a [ImportDefinition],
    pub shader_defs: HashMap<String, ShaderDefValue>,
}

#[derive(Default)]
pub struct NagaModuleDescriptor<'a> {
    pub source: &'a str,
    pub file_path: &'a str,
    pub shader_type: ShaderType,
    pub shader_defs: HashMap<String, ShaderDefValue>,
    pub additional_imports: &'a [ImportDefinition],
}

Panic on unwrap in `ensure_imports`

In my project, I'm getting the following panic:

thread 'Compute Task Pool (3)' panicked at C:\Users\Johan\.cargo\registry\src\index.crates.io-6f17d22bba15001f\naga_oil-0.8.1\src\compose\mod.rs:1356:59:
called `Option::unwrap()` on a `None` value
stack backtrace:
   0: std::panicking::begin_panic_handler
             at /rustc/180dffba142c47240ca0d93096ce90b9fd97c8d7/library\std\src\panicking.rs:617
   1: core::panicking::panic_fmt
             at /rustc/180dffba142c47240ca0d93096ce90b9fd97c8d7/library\core\src\panicking.rs:67
   2: core::panicking::panic
             at /rustc/180dffba142c47240ca0d93096ce90b9fd97c8d7/library\core\src\panicking.rs:117
   3: naga_oil::compose::Composer::ensure_import
             at C:\Users\Johan\.cargo\registry\src\index.crates.io-6f17d22bba15001f\naga_oil-0.8.1\src\compose\mod.rs:1336
   4: naga_oil::compose::Composer::ensure_imports<core::iter::adapters::map::Map<core::slice::iter::Iter<naga_oil::compose::ImportDefWithOffset>,naga_oil::compose::impl$8::make_naga_module::closure_env$5> >
             at C:\Users\Johan\.cargo\registry\src\index.crates.io-6f17d22bba15001f\naga_oil-0.8.1\src\compose\mod.rs:1365
   5: naga_oil::compose::Composer::make_naga_module
             at C:\Users\Johan\.cargo\registry\src\index.crates.io-6f17d22bba15001f\naga_oil-0.8.1\src\compose\mod.rs:1679
   6: bevy_render::render_resource::pipeline_cache::ShaderCache::get
             at C:\Users\Johan\.cargo\registry\src\index.crates.io-6f17d22bba15001f\bevy_render-0.11.0\src\render_resource\pipeline_cache.rs:320
   7: bevy_render::render_resource::pipeline_cache::PipelineCache::process_queue
             at C:\Users\Johan\.cargo\registry\src\index.crates.io-6f17d22bba15001f\bevy_render-0.11.0\src\render_resource\pipeline_cache.rs:793
   8: bevy_render::render_resource::pipeline_cache::PipelineCache::process_pipeline_queue_system
             at C:\Users\Johan\.cargo\registry\src\index.crates.io-6f17d22bba15001f\bevy_render-0.11.0\src\render_resource\pipeline_cache.rs:825
   9: core::ops::function::FnMut::call_mut
             at /rustc/180dffba142c47240ca0d93096ce90b9fd97c8d7\library\core\src\ops\function.rs:166
  10: core::ops::function::impls::impl$3::call_mut
             at /rustc/180dffba142c47240ca0d93096ce90b9fd97c8d7\library\core\src\ops\function.rs:294
  11: bevy_ecs::system::function_system::impl$11::run::call_inner
             at C:\Users\Johan\.cargo\registry\src\index.crates.io-6f17d22bba15001f\bevy_ecs-0.11.0\src\system\function_system.rs:622
  12: bevy_ecs::system::function_system::impl$11::run
             at C:\Users\Johan\.cargo\registry\src\index.crates.io-6f17d22bba15001f\bevy_ecs-0.11.0\src\system\function_system.rs:625
  13: bevy_ecs::system::function_system::impl$6::run_unsafe<void (*)(bevy_ecs::change_detection::ResMut<bevy_render::render_resource::pipeline_cache::PipelineCache>),void (*)(bevy_ecs::change_detection::ResMut<bevy_render::render_resource::pipeline_cache::Pipel
             at C:\Users\Johan\.cargo\registry\src\index.crates.io-6f17d22bba15001f\bevy_ecs-0.11.0\src\system\function_system.rs:460
  14: core::panic::unwind_safe::impl$26::poll
             at /rustc/180dffba142c47240ca0d93096ce90b9fd97c8d7\library\core\src\panic\unwind_safe.rs:296
  15: futures_lite::future::impl$14::poll::closure$0
             at C:\Users\Johan\.cargo\registry\src\index.crates.io-6f17d22bba15001f\futures-lite-1.13.0\src\future.rs:626
  16: core::panic::unwind_safe::impl$23::call_once<enum2$<core::task::poll::Poll<tuple$<> > >,futures_lite::future::impl$14::poll::closure_env$0<core::panic::unwind_safe::AssertUnwindSafe<enum2$<bevy_ecs::schedule::executor::multi_threaded::impl$3::spawn_system
             at /rustc/180dffba142c47240ca0d93096ce90b9fd97c8d7\library\core\src\panic\unwind_safe.rs:271
  17: futures_lite::future::impl$14::poll
             at C:\Users\Johan\.cargo\registry\src\index.crates.io-6f17d22bba15001f\futures-lite-1.13.0\src\future.rs:626
  18: async_executor::impl$5::spawn::async_block$0
             at C:\Users\Johan\.cargo\registry\src\index.crates.io-6f17d22bba15001f\async-executor-1.5.1\src\lib.rs:145
  19: async_task::raw::RawTask<enum2$<async_executor::impl$5::spawn::async_block_env$0<enum2$<core::result::Result<tuple$<>,alloc::boxed::Box<dyn$<core::any::Any,core::marker::Send>,alloc::alloc::Global> > >,futures_lite::future::CatchUnwind<core::panic::unwind
             at C:\Users\Johan\.cargo\registry\src\index.crates.io-6f17d22bba15001f\async-task-4.4.0\src\raw.rs:563
  20: futures_lite::future::impl$12::poll
             at C:\Users\Johan\.cargo\registry\src\index.crates.io-6f17d22bba15001f\futures-lite-1.13.0\src\future.rs:529
  21: async_executor::impl$5::run::async_fn$0<enum2$<core::result::Result<tuple$<>,async_channel::RecvError> >,futures_lite::future::Or<enum2$<bevy_tasks::task_pool::impl$2::new_internal::closure$0::closure$0::closure$0::closure$0::async_block_env$0>,async_chan
             at C:\Users\Johan\.cargo\registry\src\index.crates.io-6f17d22bba15001f\async-executor-1.5.1\src\lib.rs:243
  22: futures_lite::future::block_on::closure$0
             at C:\Users\Johan\.cargo\registry\src\index.crates.io-6f17d22bba15001f\futures-lite-1.13.0\src\future.rs:89
  23: std::thread::local::LocalKey<core::cell::RefCell<tuple$<parking::Parker,core::task::wake::Waker> > >::try_with
             at /rustc/180dffba142c47240ca0d93096ce90b9fd97c8d7\library\std\src\thread\local.rs:270
  24: std::thread::local::LocalKey<core::cell::RefCell<tuple$<parking::Parker,core::task::wake::Waker> > >::with
             at /rustc/180dffba142c47240ca0d93096ce90b9fd97c8d7\library\std\src\thread\local.rs:246
  25: futures_lite::future::block_on<enum2$<core::result::Result<tuple$<>,async_channel::RecvError> >,enum2$<async_executor::impl$5::run::async_fn_env$0<enum2$<core::result::Result<tuple$<>,async_channel::RecvError> >,futures_lite::future::Or<enum2$<bevy_tasks:
             at C:\Users\Johan\.cargo\registry\src\index.crates.io-6f17d22bba15001f\futures-lite-1.13.0\src\future.rs:79
  26: bevy_tasks::task_pool::impl$2::new_internal::closure$0::closure$0::closure$0::closure$0
             at C:\Users\Johan\.cargo\registry\src\index.crates.io-6f17d22bba15001f\bevy_tasks-0.11.0\src\task_pool.rs:171
  27: std::panicking::try::do_call
             at /rustc/180dffba142c47240ca0d93096ce90b9fd97c8d7\library\std\src\panicking.rs:524
  28: std::panicking::try
             at /rustc/180dffba142c47240ca0d93096ce90b9fd97c8d7\library\std\src\panicking.rs:488
  29: std::panic::catch_unwind
             at /rustc/180dffba142c47240ca0d93096ce90b9fd97c8d7\library\std\src\panic.rs:142
  30: bevy_tasks::task_pool::impl$2::new_internal::closure$0::closure$0::closure$0
             at C:\Users\Johan\.cargo\registry\src\index.crates.io-6f17d22bba15001f\bevy_tasks-0.11.0\src\task_pool.rs:165
  31: std::thread::local::LocalKey<async_executor::LocalExecutor>::try_with
             at /rustc/180dffba142c47240ca0d93096ce90b9fd97c8d7\library\std\src\thread\local.rs:270
  32: std::thread::local::LocalKey<async_executor::LocalExecutor>::with
             at /rustc/180dffba142c47240ca0d93096ce90b9fd97c8d7\library\std\src\thread\local.rs:246
  33: bevy_tasks::task_pool::impl$2::new_internal::closure$0::closure$0
             at C:\Users\Johan\.cargo\registry\src\index.crates.io-6f17d22bba15001f\bevy_tasks-0.11.0\src\task_pool.rs:158
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
Encountered a panic in system `bevy_render::render_resource::pipeline_cache::PipelineCache::process_pipeline_queue_system`!
2023-09-09T08:41:57.642548Z ERROR matchbox_socket::webrtc_socket::socket: Socket(Io(Os { code: 10093, kind: Uncategorized, message: "Either the application has not called WSAStartup, or WSAStartup failed." }))
error: process didn't exit successfully: `target\debug\cargo_space.exe --dev --next 1 --no-ldtk --n 0` (exit code: 101)

I don't have a minimal test case, and it's entirely possible that I'm doing something weird or unsupported, but it kind of looks like the code is making incorrect assumptions. Would perhaps be better to make it an expect at least? And give the user some hint as to what they did wrong?

naga_oil/src/compose/mod.rs

Line 1355 in b8bcd03

// we've already ensured imports exist when they were added

naga_oil/src/compose/mod.rs

Line 1356 in b8bcd03

let module_set = self.module_sets.get(import).unwrap();

naga_oil does not support preprocessor definitions that end in numbers

Reported in bevyengine/bevy#10237

@IceSentry believes this is a naga_oil problem, not a naga limitation.

Naga invariant error is confusing

We can at least call out the most common case of ending an imported item with a number.

Ideally find a way to remap so that users don’t need to worry about it

Handle naga's SpecialTypes

Ray tracing APIs do not currently work with naga_oil. DerivedModule throws out special_type info.

GLSL / WGSL composition example

Hi! I'm interested on using naga-oil to combine GLSL functions inside a WGSL shader. Is there any example or documentation for that? What would be the right place to start with it?

Thanks in advance

Double underscore is reserved in glsl

Not sure if this is naga or naga_oil at fault, maybe both as it's technically valid wgsl and should probably be converted to something compliant in glsl, but using naga_oil compose adds lots of double underscores to identifiers which will break when loaded in webgl.

Example error:

 panicked at 'wgpu error: Validation Error

Caused by:
    In Device::create_render_pipeline
    Internal error in VERTEX shader: ERROR: 0:6: '_naga_oil__mod__MNXW23LPNY__member__Globals' : identifiers containing two consecutive underscores (__) are reserved as possible future keywords
ERROR: 0:9: '_naga_oil__mod__MNXW23LPNY__member__Transforms' : identifiers containing two consecutive underscores (__) are reserved as possible future keywords
ERROR: 0:12: '_naga_oil__mod__MNXW23LPNY__member__ColorTransforms' : identifiers containing two consecutive underscores (__) are reserved as possible future keywords
ERROR: 0:16: '_naga_oil__mod__MNXW23LPNY__member__TextureTransforms' : identifiers containing two consecutive underscores (__) are reserved as possible future keywords
ERROR: 0:19: '_naga_oil__mod__MNXW23LPNY__member__PushConstants' : identifiers containing two consecutive underscores (__) are reserved as possible future keywords
ERROR: 0:23: '_naga_oil__mod__MNXW23LPNY__member__VertexInput' : identifiers containing two consecutive underscores (__) are reserved as possible future keywords
ERROR: 0:31: '_naga_oil__mod__MNXW23LPNY__member__Globals_block_0Vertex' : identifiers containing two consecutive underscores (__) are reserved as possible future keywords
ERROR: 0:33: '_naga_oil__mod__MNXW23LPNY__member__Transforms_block_1Vertex' : identifiers containing two consecutive underscores (__) are reserved as possible future keywords
ERROR: 0:39: '_naga_oil__mod__MNXW23LPNY__member__linear_to_srgb' : identifiers containing two consecutive underscores (__) are reserved as possible future keywords
ERROR: 0:53: '_naga_oil__mod__MNXW23LPNY__member__srgb_to_linear' : identifiers containing two consecutive underscores (__) are reserved as possible future keywords

Invalid escape character

Running Bevy 0.11.2 on an ArchLinux x64 is throwing an error when trying to build the following regex:

static RE_COMMENT: once_cell::sync::Lazy<Regex> =
    once_cell::sync::Lazy::new(|| Regex::new(r"(\/\/|\/\*|\*\/)").unwrap());

Shouldn't it be r"(//|/\*|\*/)" ?

#defines can't be used after comments / whitespace

#defines need to be at the start of the top-level shader, even a commented line or a non-empty pure whitespace line stops them from being allowed.

comments and whitespace should not disallow following #defines

Composable module identifiers error

Bevy version : 0.12.1

With new update my old wgsl shader code in broaken:

struct Poly6 {
  c0:vec3<f32>,
  c1:vec3<f32>,
  c2:vec3<f32>,
  c3:vec3<f32>,
  c4:vec3<f32>,
  c5:vec3<f32>,
  c6:vec3<f32>,

}

Error message:

**error: Composable module identifiers must not require substitution according to naga writeback rules:  c0**

Import under ifdef unexpected behavior

imported module is being validated even though it should never be imported

fn main() -> u32 {
#ifdef USE_A
    return a::func();
#else
    return 0u;
#endif
}

#define_import_path a

fn func() -> u32 {
    incorrect;
}

    #[test]
    fn conditional_import2() {
        let mut composer = Composer::default();

        composer
            .add_composable_module(ComposableModuleDescriptor {
                source: include_str!("tests/conditional_import2/mod.wgsl"),
                file_path: "tests/conditional_import2/mod.wgsl",
                ..Default::default()
            })
            .unwrap();

        composer
            .make_naga_module(NagaModuleDescriptor {
                source: include_str!("tests/conditional_import2/top.wgsl"),
                file_path: "tests/conditional_import2/top.wgsl",
                ..Default::default()
            })
            .map_err(|err| println!("{}", err.emit_to_string(&composer)))
            .unwrap();
    }

results in

error: expected assignment or increment/decrement, found ';'
  ┌─ tests/conditional_import2/mod.wgsl:4:14
  │
4 │     incorrect;
  │              ^ expected assignment or increment/decrement
  │
  = expected assignment or increment/decrement, found ';'

I have a branch with this test here: https://github.com/arcashka/naga_oil/blob/failing_import_test/src/compose/test.rs#L1098

This example is silly looking because I tried to make it as minimal as possible. I found this issue when I used shader defs as an index for tonemapping lut bindings.

Conditional import

#ifdef TONEMAP_IN_SHADER
#import bevy_core_pipeline::tonemapping::tone_mapping
#endif

in tonemapping_shared.wgsl i had

@group(0) @binding(#TONEMAPPING_LUT_TEXTURE_BINDING_INDEX) var dt_lut_texture: texture_3d<f32>;
@group(0) @binding(#TONEMAPPING_LUT_SAMPLER_BINDING_INDEX) var dt_lut_sampler: sampler;

which resulted in

error: expected expression, found '#'
   ┌─ /home/arcashka/Documents/projects/other/bevy/crates/bevy_core_pipeline/src/tonemapping/tonemapping_shared.wgsl:13:20
   │
13 │ @group(0) @binding(#TONEMAPPING_LUT_TEXTURE_BINDING_INDEX) var dt_lut_texture: texture_3d<f32>;
   │                    ^ expected expression
   │
   = expected expression, found '#'

when TONEMAP_IN_SHADER is undefined

Imports clobber field names in locally defined structs

#import dep::{foo, bar}

struct Example {
    bar: u32,
}

fn test() {
    let example = Example(0u);
    foo(example.bar);
}

This code produces this error:

error: invalid field accessor `bar`
  ┌─ shaders/test.wgsl:9:39
  │
9 │     dep::foo(example.bar);
  │                                       ^^^ invalid accessor
  │
  = invalid field accessor `bar`

If I amend the code to not import a symbol named bar, the error goes away. But it took a lot more debugging than I'd have liked to figure out that that's what was happening.

prune returns wrong checktype for WorkGroupUniformLoad

here

Wgsl `alias` does not work when using from module

It appears that you can't use wgsl alias from a composable module. It works when using you are using from the make module file though.

composer
  .add_composable_module(ComposableModuleDescriptor {
      source: "alias Vec2 = vec2<f32>;",
      file_path: "valid.wgsl",
      as_name: Some("valid".to_owned()),
      ..Default::default()
  })
  .unwrap();

let module = composer
    .make_naga_module(NagaModuleDescriptor {
        source: r#"
      #import valid::Vec2;

      @group(0) @binding(1) var<uniform> position: Vec2;
    "#,
        file_path: "",
        ..Default::default()
    })
    .map_err(|err| println!("{}", err.emit_to_string(&composer)))
    .unwrap();

Here you get the error:

error: Composable module identifiers must not require substitution according to naga writeback rules: `Vec2`
  ┌─ valid.wgsl:1:1
  │
1 │ 
  │ ^ Composable module identifiers must not require substitution according to naga writeback rules: `Vec2`

Allow building `ShaderDefValue` from a string key alone

It's fairly common to use in shaders a simple boolean flag #if XXX to activate a feature, without caring much about its value. Currently it's rather verbose because there's no existing conversion, e.g.:

let mut shader_defs = std::collections::HashMap::<String, ShaderDefValue>::new();
shader_defs.insert("LOCAL_SPACE_SIMULATION".into(), ShaderDefValue::Bool(true));

Bevy's own ShaderDefVal on the other hand has a bunch of existing conversions from string types, which make it a lot more convenient to work with:

https://docs.rs/bevy/latest/bevy/render/render_resource/enum.ShaderDefVal.html

I transitioned a piece of code from using Bevy's specialization pipeline etc. which used ShaderDefVal, to using naga_oil to validate shader code without having to bring the entire system-based infrastructure (because the code has some #if), and found it was more verbose. Nothing critical, but this would make usage smoother.

Expose internals for library consumption

I am currently working on a library which uses naga_oil to generate bindings of shaders utilizing naga oil's features. The library statically generates rust bindings given the shader entry points.

There are few things which would have been nicer if naga_oil exposed for consumption.

Path parsing basically given a shader content, figure out what are the imports of the file. At the moment, it is a semi handwritten + uses parse_imports from naga to parse further.
Demangle functions. Basically had to reduplicate the logic to figure out the original import path, so I can generate the equivalent bindings preserving the heirarchy.

Item imports affect too many places

If you import an item from “module” called “view” with a member also called “view”, the member gets renamed to “module::view”

Also importing an item called “fragment” or “vertex” makes it impossible to use the @Fragment or @vertex annotations.

Need to add more context to the substitution function compose.rs@476 to avoid this

Update to naga 0.13

I had a quick look at this, and it seemed confusing (especially since naga doesn't have a changelog yet, and gfx-rs/naga#2266 seems to have changed a bunch of stuff).

Unrecognized escape sequence

I tried to run bevy 3D examples in my Windows laptop but these errors are given:

thread 'main' panicked at C:\Users\Jasper\.cargo\registry\src\rsproxy.cn-0dccff568467c15b\naga_oil-0.9.0\src\compose\comment_strip_iter.rs:6:67:
called `Result::unwrap()` on an `Err` value: Syntax(
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
regex parse error:
    (\/\/|\/\*|\*\/)
     ^^
error: unrecognized escape sequence
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
)

Question? Am I doing imports/defines incorrectly or is this a bug?

forgive the screengrab:

I have this error:

2023-10-05T08:32:45.075565Z ERROR bevy_render::render_resource::pipeline_cache: failed to process shader:
error: required import 'shadplay::common' not found
  ┌─ shaders/myshader_2d.wgsl:3:1
  │
3 │ #import shadplay::common
  │ ^
  │
  = missing import 'shadplay::common'

from this frag:

// myrustproject/assets/shaders/myshader.wgsl
#import bevy_pbr::mesh_vertex_output MeshVertexOutput
#import shadplay::common::common

@fragment
fn fragment(in: MeshVertexOutput) -> @location(0) vec4<f32> {
    var fragColor: vec4<f32> = vec4<f32>(0.0080);
    var uv = (in.uv.xy * 2.0)- 1.0;
    var col = vec4f(0.220);

    return col;
}

and the common.wgsl is:

// myrustproject/assets/shaders/common.wgsl
#define_import_path shadplay::common

/// Clockwise by `theta`
fn rotate2D(theta: f32) -> mat2x2<f32> {
    let c = cos(theta);
    let s = sin(theta);
    return mat2x2<f32>(c, s, -s, c);
}

My paths and files are there...

#MYPROJECT DIR
├──  src/
│   ├──  lib.rs
         /* snip */
├── assets/
│   └── shaders
│       ├── common.wgsl
│       ├── myshader.wgsl
│       ├── myshader_2d.wgsl

I think i've copied everything exactly per you example, and even pulled this project to see it working -- so all my paths etc are correct, what gives?
The readme mentions Compose -- is there setup on the bevy side of my code that needs to do something to make the path discovery work?

Can't import constants from/into glsl modules

It looks like if either my imported module, or my importing module, or both, use GLSL, then I get an error when trying to use constants from the module

Error: Composer error: [Error { kind: UnknownVariable("_naga_oil__mod__MNXW23LPNY__member__my_constant"), meta: Span { start: 199, end: 246 } }]

The source file I used:
common.glsl

#define_import_path common

const float my_constant = 0.5;
float my_function() { return 0.75; }

void main() {}

test.glsl

#version 450
#import common

out vec4 out_color;

void main() { out_color = vec4(1, common::my_constant, 0, 1); }

(I'm not sure if we're meant to be able to mix wgsl modules & glsl ones together but in any case, the error happens when using glsl only)

Better error reporting in release/non-validating

When using non-validating, the error messages don’t report file/line correctly. We could at least let the user know that a debug/validating build will report better info

Define in module produce error

Inside a GLSL module I'm doing

#ifndef FNC_ADD
#define FNC_ADD
float add(in add a, in add b) {
    return a + b;
}
#endif

Produce the following error

ComposerError { 
    inner: DefineInModule(100), 
    source: Constructing { 
        path: "_module.glsl", 
        source: "..."
    }
}

The line that produce the error in question is

...
#define FNC_ADD
...

Is there a way of using the following pattern without errors?

#ifndef SOME_DEFINE
#define SOME_DEFINE
...
#endif

Update to naga 0.12.0

wgpu 0.16.0 came out and with it, naga 0.12. Can you update naga_oil to use it please?

I had a go at updating naga_oil to it myself but I hit a bump when it came to gfx-rs/naga#2264 and I wasn't confident enough in knowing the code of this library to continue. Otherwise the changes weren't that difficult.

Thanks!

missing shader def literal should report appropriate error

currently trying to use a shader def literal when the def isn't defined gives a strange error:

error: expected expression, found '#'
  ┌─ crates/bevy_pbr/src/render/pbr_bindings.wgsl:6:101
  │
6 │ var<uniform> materials: array<bevy_pbr::pbr_types::StandardMaterial, #{MATERIAL_BUFFER_BATCH_SIZE}u>;
  │                                                                                                     ^ expected expression
  │
  = expected expression, found '#'

it should instead return a specific error pointing out the missing def.

Repo is missing git tags for released crates.io versions

Assetpath imports fail with a define_import_path directive

Bevy specific but probably a naga_oil bug

If a shader is imported by path (“shaders/my.wgsl”) and also contains a #define_import_path directive, the imported module doesn’t use the import path name correctly

Naga_oil breaks when importing a struct with a field name ending in an underscore

I've modified one of the example shaders to demonstrate the issue: Aaron1011@d9e8cd2
Running cargo run --example pbr_compose_test produces the following output:

running 1000 full composer builds (no caching)
error: invalid field accessor `myfield_`
    ┌─ examples/bevy_pbr_wgsl/pbr_lighting.wgsl:286:24
    │
286 │     let myfield_ = val.myfield_;
    │                        ^^^^^^^^ invalid accessor
    │
    = invalid field accessor `myfield_`

Renaming both the field definition and usage to myfield_a (or anything that doesn't end in an underscore) allows the example to run successfully.

using imported consts in let statements can fail

using an imported constant in a let statement within a module causes the constant to be added to both header files. importing both modules into another module/shader causes a duplicate definition parse error.

    #[test]
    fn import_in_decl() {
        let mut composer = Composer::default();
        composer
            .add_composable_module(ComposableModuleDescriptor {
                source: include_str!("tests/const_in_decl/consts.wgsl"),
                file_path: "tests/const_in_decl/consts.wgsl",
                ..Default::default()
            })
            .unwrap();
        composer
            .add_composable_module(ComposableModuleDescriptor {
                source: include_str!("tests/const_in_decl/bind.wgsl"),
                file_path: "tests/const_in_decl/bind.wgsl",
                ..Default::default()
            })
            .unwrap();
        let _module = composer
            .make_naga_module(NagaModuleDescriptor {
                source: include_str!("tests/const_in_decl/top.wgsl"),
                file_path: "tests/const_in_decl/top.wgsl",
                ..Default::default()
            })
            .unwrap();
    }

consts.wgsl:

#define_import_path consts

let X: u32 = 1u;

bind.wgsl:

#define_import_path bind

#import consts

var<private> arr: array<u32, consts::X>;

top.wgsl:

#import consts
#import bind

fn main() -> f32 {
    return f32(bind::arr[0]);
}

results in :

thread 'compose::test::test::import_in_decl' panicked at 'called `Result::unwrap()` on an `Err` value: ComposerError { inner: WgslParseError(ParseError { message: "redefinition of `_naga_oil__mod__MNXW443UOM__member__X`", labels: [(58..95, "redefinition of `_naga_oil__mod__MNXW443UOM__member__X`"), (4..41, "previous definition of `_naga_oil__mod__MNXW443UOM__member__X`")], notes: [] }), source: Constructing { path: "tests/const_in_decl/top.wgsl", source: "#import consts\n#import bind\n\nfn main() -> f32 {\n    return f32(_naga_oil__mod__MJUW4ZA__member__arr[0]);\n} \n", offset: 210 } }', src\compose\test.rs:179:14

unwrap None when creating a naga module with missing import

imports are not verified when used in a top level shader.

    fn missing_import_in_shader() {
        let mut composer = Composer::default();

        let error = composer
            .make_naga_module(NagaModuleDescriptor {
                source: include_str!("tests/error_test/include.wgsl"),
                file_path: "tests/error_test/include.wgsl",
                ..Default::default()
            })
            .err()
            .unwrap();
    }

running 1 test
thread 'compose::test::test::missing_import_in_shader' panicked at 'called `Option::unwrap()` on a `None` value', src\compose\mod.rs:1271:59

update to naga 10

check if there are any changes in the IR format that will require modifications here

Alignment attribute is not applied when defined in modules.

This example is straight from the w3 examples which follows the valid path.
It appears that the align(16) attribute is not applied when the struct is defined in a composable module rather than naga module.

EDIT: I am not sure this issue also extends to @size attribute.

 composer
     .add_composable_module(ComposableModuleDescriptor {
        source: r#"
          struct S {
            x: f32
          }
        
          struct Valid {
            a: S,
           @align(16) b: f32 // valid: offset between a and b is 16 bytes
          }
        "#,
        file_path: "valid.wgsl",
        as_name: Some("valid".to_owned()),
        ..Default::default()
    })
    .unwrap();

composer
    .make_naga_module(NagaModuleDescriptor {
        source: r#"
            #import valid::Valid;
            @group(0) @binding(1) var<uniform> valid: Valid;
        "#,
        file_path: "",
        ..Default::default()
      })
      .map_err(|err| println!("{}", err.emit_to_string(&composer)))
      .unwrap();

The above fails with

error: failed to build a valid final module: Global variable [1] 'valid' is invalid
  ┌─ :3:33
  │
3 │           @group(0) @binding(1) var<uniform> valid: valid::Valid;
  │                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ naga::GlobalVariable [1]
  │
  = Alignment requirements for address space Uniform are not met by [4]
  = The struct member[1] offset 4 must be at least 16

This works however when everything is defined in a single file.

tracing macros in naga_oil could have finer-grained controls

It's hard to debug bevy's renderer without having recourse to reading the tracing logs. Welp. I previously said it was relevant to bevy, but I don't think so.

Here is my suggestion. I think one of the two should make using logging easier:

Split naga in more modules, so that the TRACE macros have different modules, and it is possible to silence specific messages
Use DEBUG log level for the "with defs" message.

`atomicCompareExchangeWeak` is processed incorrectly by `compose`.

Issue: When using this crate to parse a wgsl file containing an atomicCompareExchangeWeak instruction, the second argument and result type are switched with other types and expressions in scope when the call is performed.

Versions: (all latest at time of writing)

naga_oil = "0.9"
naga = { version = "0.13", features = ["wgsl-in", "wgsl-out"] }

Shader (wgsl):

@group(0) @binding(0) var<storage, read_write> foo: atomic<u32>;

@compute
@workgroup_size(256,1,1)
fn bar(@builtin(global_invocation_id) global_id: vec3<u32>) {
    atomicCompareExchangeWeak(&foo, 0u, global_id.x);
}

Rust:

fn main() {
    let mut composer = naga_oil::compose::Composer::non_validating();
    let module = composer
        .make_naga_module(NagaModuleDescriptor {
            source: include_str!("main.wgsl"),
            file_path: "main.wgsl",
            shader_type: naga_oil::compose::ShaderType::Wgsl,
            shader_defs: HashMap::default(),
            additional_imports: &[],
        })
        .unwrap();

    let info = naga::valid::Validator::new(
        naga::valid::ValidationFlags::empty(),
        naga::valid::Capabilities::empty(),
    )
    .validate(&module)
    .unwrap();

    let res = naga::back::wgsl::write_string(
        &module,
        &info,
        naga::back::wgsl::WriterFlags::EXPLICIT_TYPES,
    )
    .unwrap();

    println!("{}", res);
}

Result:

struct gen___atomic_compare_exchange_resultUint4_ {
    old_value: u32,
    exchanged: bool,
}

@group(0) @binding(0) 
var<storage, read_write> foo: atomic<u32>;

@compute @workgroup_size(256, 1, 1) 
fn bar(@builtin(global_invocation_id) global_id: vec3<u32>) {
    let _e3: vec3<u32> = atomicCompareExchangeWeak((&foo), (&foo), global_id.x);
    return;
}

Note that both the type of _e3, and the second argument of atomicCompareExchangeWeak, are incorrect.

Performing a parse and write cycle with plain naga does not have this issue, so the type and argument switching occurs in this crate.

effective defs don't include #if defs

we record the "effective shader defs" (the defs which can alter the generated shader) for modules in order to know when we can reuse cached modules - if all defs in a module's the effective defs match those in an existing cached version then that cached version will be used.

the effective shader defs include those used in #ifdef and #ifndef (and the #else variants) but not the defs used in #if def == value. this could lead to incorrect reuse of a cached module when a new variant should be generated.

Potential overflow in ModuleKey::from_members

https://github.com/robtfm/naga_oil/blob/66f25d5cdd68d9e1c62eb449a1356f9effddb025/src/compose/mod.rs#L152
This will overflow if there are more than 64 items in universe, which seems quite possible seeing as this gets passed the list of all shader defs that can affect a module.

invalid color codes in errors

with naga_oil 0.10, error are not colored correctly

ANSI output used with non-ANSI terminals, NO_COLOR

I'm running my Bevy projects via Sublime Text, which doesn't have a real terminal, only a super basic plaintext log.

The problem is that naga_oil uses ANSI colors on non-WASM targets unconditionally, without checking whether the terminal is compatible. In my case TERM is undefined, and I'm running std::env::set_var("NO_COLOR", "1") in main, but I still get ANSI color output that makes WGSL errors completely unreadable:

"//" in import declarations are incorrectly recognized as comments

Consider this piece of shader code:

#import "embedded://path/to/shader.wgsl"::foo

Currently, naga_oil recognizes the "//" part of the import path as comments and replaces the line with #import "embedded:, so this import declaration becomes invalid. This breaks shader import in Bevy with asset sources.

Naga Oil needs a CLI

It would be nice if there were a command-line tool that would process shader templates into plain WGSL.

bevyengine / naga_oil Goto Github PK

naga_oil's People

Contributors

Stargazers

Watchers

Forkers

naga_oil's Issues

Recommend Projects

Recommend Topics

Recommend Org