-
Notifications
You must be signed in to change notification settings - Fork 85
Add naga-wgsl target v2
#493
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
base: target-decl
Are you sure you want to change the base?
Conversation
5fcf85a to
846794b
Compare
| use-installed-tools = ["spirv-tools/use-installed-tools", "naga"] | ||
| # If enabled will compile and link the C++ code for the spirv tools, the compiled | ||
| # version is preferred if both this and `use-installed-tools` are enabled | ||
| use-compiled-tools = ["spirv-tools/use-compiled-tools"] | ||
| use-compiled-tools = ["spirv-tools/use-compiled-tools", "naga"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, this effectively makes naga non-optional if I'm reading it correctly. Is it intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's from the old PR, added with comment
wgsl: enable naga feature by default, cargo-gpu can't handle it
So this was more a hack than anything else. I'll have a look what the compile time impact is, and if it's actually significant (which next to spirv-tools-sys may not be), look into making it optional and supported by cargo-gpu.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if naga should really be a dependency of either of those two features. Why not let user specify it explicitly instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cargo-gpu can't handle it
we'd need to build the infra in cargo-gpu first
Requires #491
Supersedes #280
Adds
spirv-unknown-naga-wgsltarget for built-in naga transpilation to wgsl. Further naga targets could be added easily.Quite a lot compiletest still fail with the naga target.