Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

naga support #99

Open
stefnotch opened this issue Aug 1, 2024 · 3 comments
Open

naga support #99

stefnotch opened this issue Aug 1, 2024 · 3 comments

Comments

@stefnotch
Copy link
Contributor

Eventually, we might need proper support from naga to be able to handle all WGSL constructs. (The hard alternative is rolling our own parser.)

Currently, naga_oil generates headers and uses them to compile modules individually.

However, generating WGSL headers breaks down when they include a predeclared type which gets redefined.

For example, a simple header could be

// foo.wgsl
alias LightValue = f32;
fn light() -> LightValue { return 1.0f; }

Then, if we parse code like this, it leads to an issue

// main.wgsl
#import foo::{light};

// Affects the header
alias f32 = f64;

I believe that this is very hard to fix under the current model. We currently generate WGSL code for headers, and then pass the whole code to naga.

If WGSL were slightly more expressive, then this would also no longer be an issue. For example gpuweb/gpuweb#4308 would let us fix this.

@ncthbrt
Copy link

ncthbrt commented Aug 27, 2024

The right way to treat this would be to also mangle the aliases surely? Wouldn't that avoid the problem?

@stefnotch
Copy link
Contributor Author

stefnotch commented Aug 27, 2024

Yes, that's what should happen in the generated output.

But, naga-oil currently fully relies on naga to do the shader parsing. As in, to parse a module, we roughly do

let mut module_string = String::new();
module_string.push_str(&composed_header); // Can depend on f32
module_string.push_str(&source); // Can alias f32. Breaks the header. :(

let module = naga::front::wgsl::parse_str(&module_string);

@ncthbrt
Copy link

ncthbrt commented Aug 27, 2024

I see. Yeah you're right and precisely why I want to move away from that model.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants