bevyengine / naga_oil

Apache License 2.0
120 stars 31 forks source link

pipeline-overridable constants #102

Open 9SMTM6 opened 1 month ago

9SMTM6 commented 1 month ago

Support for these has been added to wgpu. Release 0.20.0 supports them on metal, vulkan and dx12 (i think), release 22.0.0 supports them, at least in some fashion, on the web.

At least in my current application I get errors if I try to use pipeline-overrideable constants. In all fairness, I use naga_oil indirectly, through wgsl_bindgen. But as far as I can tell, I get the error through here and originally coming from here.

That error is coming from naga, as you can see. The reason I make this issue here, is that I'm not entirely sure whether solving that issue upstream will solve it. If we look at e.g. what they did in the spir-v backend, then it appears to me, that they replaced the override with its actual value. Note that this code seems to have changed a lot in the meantime, probably due to the refactor in the 22.0 release.

But as I understand it, as a preprocessor, naga_oil would not actually want to replace these values yet.

Ideally I would want pipeline-overridable constants to work, but honestly, this seems a bit beyond me at this point, at the very least I don't know what would be required to achieve that.

If you tell me that it would help you if I make a PR to naga that does similar things as it does in the spir-v code then I think I'm perfectly able to do that. But AFAICT this would not solve this issue for naga oil.

robtfm commented 1 month ago

my understanding (though i haven't checked if anything changed for 22.0) is that pipeline constants are implemented in wgpu via search/replace rather than taking advantage of any driver capabilities. i.e. they work exactly the same way as naga_oil's preprocessor defines. so we haven't prioritized exposing the wgpu functionality.

can you use preprocessor defines instead?

9SMTM6 commented 1 month ago

is that pipeline constants are implemented in wgpu via search/replace rather than taking advantage of any driver capabilities.

That is my understanding as well.

they work exactly the same way as naga_oil's preprocessor defines [...] can you use preprocessor defines instead?

I wasn't aware of these. Hmm. The way I understand them from the Readme description however, it seems they're intended to be defined in the shader source, with no tooling to inject them from host code, as is intended with overridable constants. Certainly I don't find any API for something like that with wgsl_bindgen, but that may simply be because its not been something that was wanted.

wgsl_bindgen is also intended to run during compile time, so if all that that is supposed to be done in their tooling, it would require a bunch of work on their end. I would like to note that pipeline overridable constants would just work if naga_oil (whether its its fault or nagas) would just ignore them for now, since then you'd just get a shader in the end that retains its pipeline overridable constants, which can then be provided on pipeline creation, as is intended.

But yeah, of course I can see, how with other workflows they would be an acceptable replacement - eg. if you use naga_oil in a more dynamic fashion, as seems to be the purpose of the compose layer. I would still note that it'd be great if you would eventually normalize to the standard, but you had that sort of issue before the standard was ready and made a solution that worked for you, and see no need to rush. Especially with how naga does things it probably won't be easy either.

I'll ultimately also be able to find some solution for this. If you think its out of the scope of this project feel free to close it, otherwise I'm gonna leave it open since I think it'd be great if you could eventually get there.