bevyengine/naga_oil

Alignment attribute is not applied when defined in modules.

Swoorup opened this issue · 7 comments

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.

The success case for brevity:

composer
    .make_naga_module(NagaModuleDescriptor {
        source: r#"
      struct S {
        x: f32
      }
      
      struct Valid {
        a: S,
        @align(16) b: f32 // valid: offset between a and b is 16 bytes
      }
      @group(0) @binding(1) var<uniform> valid: Valid;
    "#,
        file_path: "",
        ..Default::default()
    })
    .unwrap();

Because of this issue or something else I also run into other weird issue when used inside function from different file as well. It is perhaps treating the type separately due to the type contents as well.

 struct S {
  x: f32
}

struct Valid {
  a: S,
  @align(16) b: f32 // valid: offset between a and b is 16 bytes
}

fn weird_crap(v: Valid) {}
#import valid::{Valid, weird_crap, S};

fn test() {
  let s = S(1.0);
  let v = Valid(s, 2.9);
  weird_crap(v);
}
error: failed to build a valid final module: Function [2] 'test' is invalid
  ┌─ :4:11
  │  
4 │ ╭           fn test() {
5 │ │             let s = valid::S(1.0);
6 │ │             let v = valid::Valid(s, 2.9);
  │ │                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ naga::Expression [4]
7 │ │             valid::weird_crap(v);
  │ │             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ invalid function call
  │ ╰───────────────────────────────────────────────────^ naga::Function [2]
  │  
  = Call to [1] is invalid
  = Argument 0 value [4] doesn't match the type [3]


thread 'main' panicked at examples/pbr_compose_test.rs:316:10:

Probably related: gfx-rs/wgpu#4377

There are naga issues related to this (wgsl and glsl).

In bevy we still use dummy padding items (_padding: u32) instead of the align attribute to work around it.

I’ll investigate again though because this doesn’t seem exactly related to the output issues, it’s a pure naga issue.

on reflection, the backend issues are the direct cause - the including module is built from a generated header for the include for the target language (which includes the struct definition), and that header is generated incorrectly due to the backend issues.

on reflection, the backend issues are the direct cause - the including module is built from a generated header for the include for the target language (which includes the struct definition), and that header is generated incorrectly due to the backend issues.

afaik from my very rough understanding, naga oil works with the backend or the lower end of the wgsl from naga rather than the wgsl frontend. Doesn't it make more sense to work with naga::front::wgsl::parse::ast::TranslationUnit rather than naga::Module as it preserves some, if not all of the source information?

I assume information about alias issue is also lost at the backend?

naga_oil currently supports modules/shaders written in a mix of wgsl and glsl, and (if anybody wanted to add it) could support other naga-supported langauges as well. so i would much prefer fixing the naga issues directly rather than working around them and limiting the scope - from my brief look it doesn't seem like anything fundamentally hard to fix in naga.