-
Notifications
You must be signed in to change notification settings - Fork 958
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 - compile with context #5713
Comments
I would, of course, be willing to dive into naga and provide a PR for this. Pointers on roughly where to look are always appreciated. |
This is the general flow when you call wgpu/naga/src/front/wgsl/mod.rs Lines 39 to 41 in 4902e47
The parser should be able to handle missing global items just fine and the resulting For a first attempt at this, I would:
This would mean that you have to hold onto all original |
Thank you! I've now had a chance to look at this, and there are a two different options regarding how to implement it.
I think the second option could be even simpler to implement and maintain. Should I take a stab at it, or could there be any major issues that I'm overlooking? |
You can try the 2nd approach but I think the first will be more straight-forward (it's not clear to me what needs to happen for the 2nd approach to work without actually doing it). |
I found an interesting issue. In WGSL, one can redefine built-ins at any point. For example: Like compute.toys does, I want to import/inject a struct with the mouse position struct Mouse { pos: vec2f }
@group(0) @binding(0) var<uniform> mouse: Mouse; into a shader fn main() -> vec3 {
return vec3(mouse.pos.xy);
} That would work. But the moment the shader author adds an alias fn main() -> vec3 {
return vec3(mouse.pos.xy, 1.0);
}
alias vec2f = vec2<i32> the provided mouse struct would break. Despite it being part of the context, and not part of the shader source code. I think, as with Rust's macros, that this should be prevented. As in, "compile with context" should be hygienic. The context can only add definitions for subsequent code, it cannot be changed by subsequent code. This should be relatively do-able, one just has to always refer to the resolved built-in types in the provided context. |
Why should imports be compared to rust macros instead of rust imports? The examples above would be a non-issue if you had to specify what you wanted to import. Even in rust you can do There are also plans to add a wgsl namespace to the WGSL spec which would contain all built-ins. |
@stefnotch I talked with @jimblandy about this and we think that the approach in #5713 (comment) is the way to go. |
@teoxoy Thank you! Then trying out the approach from said comment will be one of the next things to try out for me :) Currently, I am also exploring the alternative variant of writing a WGSL parser from scratch. Essentially, that gives me a chance to try building a minimal implementation of "WGSL imports". Afterwards, I can take those ideas and figure out how to combine that with naga. |
Is your feature request related to a problem? Please describe.
Background - naga_oil
I have been working on naga_oil, which is a WGSL preprocessor, for a while. One important feature that naga_oil implements is shader imports.
For shader imports, we independently compile shaders using naga, and then correctly link them together. This works nicely, but comes with the limitation that we need to generate "header files" for the naga compiler.
For example, when we want to compile
we first look at the
foo
file, and generate code along the lines ofAnd this scales really badly. As the import tree gets larger, we can end up with massive shaders that end up including almost everything else, just in case the compilation needs it.
So we're doing
and repeat that, until we've compiled all shaders
Describe the solution you'd like
It would be much nicer if we could do
This request is asking for a way of pre-populating a naga module with some entries. I think this would be a useful building block for other tools, such as naga_oil.
Describe alternatives you've considered
Writing our own WGSL parser for performance, or accepting O(n^2) compile times hit for Bevy's shaders.
Additional context
There might also additional use cases for this, such as incremental compilation. But that's a different story.
The text was updated successfully, but these errors were encountered: