-
Notifications
You must be signed in to change notification settings - Fork 42
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
Standardize #include #83
Comments
As for the Ideally, we'd write up a specification, and define a WGSL extension for this. This it to avoid having include statements get interpreted differently by different tools. // Tells language servers which extension is going to be used.
// Theoretically, someone else could define a different wgsl importing extension, with a different name.
enable wgsl_imports;
// Now this an unambiguous meaning
#import my_module; See also gpuweb/gpuweb#4576 (comment) for how to not do it. (me complaining about GLSL tools) Questions that aren't immediately clear to me from the readme:
|
i'm completely open to adding/exposing features to aid integration directly. i'm not very keen on writing a formal spec, but happy to describe the current implementation (and potentially change the implementation if a spec does evolve). briefly:
anywhere is valid provided the import begins on its own line, but they are explicitly not scoped. imports are collected by the preprocessor and apply for the whole file, regardless of the "scope" of the import, whether it is declared before/after the usage, etc.
comments are stripped before preprocessor declarations are parsed, so
it should not affect the imported items. the imports are technically done by building a naga module for each source individually/wihtout context, and then munging them together at the end.
as you've said. other things could potentially be supported, but each would need investigation.
yes, individually without context
yep, and variables, functions, consts, etc
we can expose this (we discussed that in another issue i think), but the current scheme is there are some non-obvious criteria for this to produce valid identifier names in each target language (e.g. double underscores are not allowed in wgsl, naga doesn't handle items ending in digits or underscores well, putting the item first potentially helps with cache lookups, etc), and obviously if you make a proper spec the pre-marker would need to change.
i'm normally on the bevy discord in rendering-dev (you might need to ping me though), and other people there are more or less familiar with naga oil as well. |
First of all, thank you so much for taking your time to respond to all of this!
Very understandable. I'm willing to do the work of writing up a specification.
Alrighty. Would it be reasonable if the specification said something along the lines "top of a file only", and
I think there's a name mangling scheme where one doesn't need a disallowed string. For example, using the ``{item}{pre_marker}{b64(module)}{post_marker}` scheme, we could always decode it by going backwards
Now if
Sweet, I just joined that Discord :) Looking forward to seeing where this goes, hopefully it'll end up being rather useful for the lovely bevy engie. |
The problem if you don’t have a magic marker is that users can make a raw item that happens to parse as a decorated item. I don’t see any way to avoid that without denying (part of) the marker. This could probably be managed with ast context, but I specifically tried to avoid that (naga doesn’t facilitate it currently, and it raises the bar for other language support significantly).
Sounds very reasonable to me. |
are imports preprocessor aware? can you conditionally import a file? do imported files take on the same defines as the first file? |
Yes - see eg the import of VertexOutput in bevy’s pbr_fragment.wgsl
Yes, as above. Naga_oil also tree-shakes though, so you often don’t need to.
Yes, there is one set of defines applied through the whole module build, otherwise chaos would ensue. |
@robtfm If we mangle every identifier[1], then I think we do not need a magic marker. This does mean mangling everything in the main module as well. This makes specifying the entry point for a WGSL shader a bit more annoying. I'll properly write it up later, and I'll also look at what Rust and Swift are doing for their name mangling, just so that I don't miss any essential details. There's also an alternative mangling scheme, which does "escaping" instead. e.g. All identifiers in the main module are valid as is, except if we encounter something that could have a special meaning, then we escape it. Including escaping an escape sequence. @compute
fn main_X_NAGA_OIL_MOD_X() {
} [1] Yes, every identifier. Globals can be used inside a function? Well, we'd have to mangle every variable inside a function. I suppose the escaping strategy would be useful there, since it only needs to mangle a few things. |
escaping won't work with the current status quo, because the modules need to be compiled by naga which enforces wgsl identifier rules (UAX31) - anything we can use, users can use too, unless we choose to deny something to users that's otherwise valid. so practically, escaping would have to be the same as using a magic marker and denying it to users. mangling everything would work i think. we would also be able to undo that mangling on the top-level names (at least the entry points) once we've built the all pieces to naga, to avoid the ux issue. might be quite a lot slower though.. |
Counterintuitively, there is an escaping scheme that should work under that constraint. (Very convenient for us) We pick some escape character, like
To decode it, we first look at the stuff at the end. Afterwards, we undo the escaping to get the original identifier. As in, whenever the user writes
|
@robtfm I've been thinking about Essentially, when a language server sees
|
There’s an issue about making a cli which links to a simple one I knocked together. That shows how I would expect the resolution to work, in a lsp context you presumably have a known project root to search from, and additional folders could be configured from plugin settings (at least in vscode, I guess other containers would have a similar mechanism) |
Given the way resolution works there’s no way to avoid searching all files, but I guess you could generate a static map upfront and track changes if that’s too slow for building dynamically. |
@robtfm Could we change how resolution will work in the future, to avoid searching all files? |
The main reason for this setup was that bevy needed to reference statically included shaders that didn’t have any associated path, so needed to be “self-identifying”. That’s changed now so that they can (optionally) have a path, and will probably change more as asset-sources improve. But disallowing the |
I understand. Would a reasonable path forward be: The specification won't have anything like |
If you’re sure it’s too onerous for servers to support then ok. Someone did actually post a working lsp to the bevy discord yesterday though… |
There is one environment that I'd like to support, where the Essentially, there one would specify the shader, and then a bit of Javascript would go over it to find all the other shaders that it can import. However, one can only do The other thing that I have to keep in mind is making it possible for multiple tools to deal with Of course keeping the existing behaviour intact is also important, hence my approach
|
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:
#import
PolyMeilex/vscode-wgsl#25 - wgsl-analyzer will need to re-implement the#include
algorithmWould 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? :)
The text was updated successfully, but these errors were encountered: