bevyengine/naga_oil

Update `naga_oil` to make use of `naga` frontend translation units.

DexterHill0 opened this issue · 16 comments

After using naga_oil myself and running into some issues, I think it would really benefit having access to naga internals. I did search for this feature request within WGPU, but I originally did not find any other issues, so I opened my own (gfx-rs/wgpu#6250). It turns out there is also a similar issue at gfx-rs/wgpu#5713, and a more fitting issue gfx-rs/wgpu#6132. In both my issue and 6132, the green light was given to make a PR to expose some of the naga frontend translation units for use in other projects. I realised in mine it would make more sense to first try and update naga_oil to use naga before making the PR so there is a clear list of features that need to be publicized.

I know this idea has been floating around some of this issues in this repo, is this definitely something that would still want to be implemented here? Although I couldn't start experimenting right now, I would be open to contributing to this change in the near future.

Yes definitely. @stefnotch has made some efforts in this direction already, though I’m not sure how far they’ve progressed.

The latest status on this is that we've started work on multiple additions to WGSL (imports, conditional compilation, IDE support, ...)

https://github.com/wgsl-tooling-wg/wesl-spec

We have a Discord server at https://discord.com/invite/Ng5FWmHuSv and would love to have you there! Please let us know what things you need, it'd be awesome if we can help with solving those problems.

Very cool, I will take a look at the server and spec soon!

However, (unless the info is on the server in which case let me know) what's the future plans for this crate? Are you going to go back to implementing it with naga? Are you going to make/use a different wgsl parser based on the custom spec? Wait for the spec to be officially added?

I think naga_oil would really benefit from naga and I'm happy to work on that if you wanted to focus on the spec or something else.

There are quite a few advanced features, like generics, that we're considering. Since parsing those with naga won't be practical, we will likely end up building a parser for a superset of WGSL.

As such, eventually naga_oil will be superseded by something newer. The big question is how long that'll take. So, if this sounds interesting to you, do join us and help us build a better WGSL for Bevy and co!

That makes sense. By the sounds of it though it doesn't seem like it will be very quick and in the project I'm working on I'd love to use naga_oil for its features but the current implementation wasn't working well for me. As a stepping stone between now and the future where this custom superset of wgsl is available, is it sensible to still update naga_oil?

@DexterHill0 I think it'd make sense to update naga oil as a stepping stone. However first the naga translation units need to be exposed in the first place.

Oh also, would be helpful to get a sense of your timelines, because this is assuming you want to make changes to naga oil yesterday...

@DexterHill0 I think it'd make sense to update naga oil as a stepping stone. However first the naga translation units need to be exposed in the first place.

That was what my original issue was for within naga itself but then I realised I wouldn't know what parts of naga to expose without first implementing the features into naga_oil.

Oh also, would be helpful to get a sense of your timelines, because this is assuming you want to make changes to naga oil yesterday...

Right now I'm 3/4s a way through another project so I wouldn't start work on it however by the end of the month I should have more time to focus on attempting something like this.

I've begun to make some changes to naga_oil to make use of the naga translation units, as well as naga to expose the required units. These changes are on my fork of naga_oil here and naga here.
I wanted to make a little bit of headway before I posted this because I wasn't sure if it was going to be feasible when I first took a look. When I first viewed the code it was pretty overwhelming and hard to get to grips with so I didn't want to immediately say I would go ahead and make the changes.
Everything has been fine otherwise and my main goal with the changes is to get it to a point where all the tests pass as they did before.

The only major issue so far is that I realised that the lexer I'm using is only for WGSL and so it would exclude GLSL functionality from the crate. GLSL has a very different lexing setup to WGSL which would make it hard to implement both. I don't think it's impossible but it would require some weirdness. How much of an issue would that be?

could you summarize what you're doing? i'm not entirely clear on why using naga internals requires a custom lexer.

it would be nice to maintain glsl even if not fully-featured. but whether we are willing to drop it would depend on the benefits i suppose.

could you summarize what you're doing? i'm not entirely clear on why using naga internals requires a custom lexer.

it would be nice to maintain glsl even if not fully-featured. but whether we are willing to drop it would depend on the benefits i suppose.

Sorry, now I read it back it does sound like I'm using a custom lexer. What I meant is that I'm just importing the lexer from the WGSL module within naga, and not the one for GLSL. The difference between the way the two languages get parsed is significant so, for now, I just went with using the WGSL lexer.
I can look into implementing GLSL after WGSL but the logic for everything else is much easier to get working with WGSL for now as it's a much simpler lexing setup in naga.

Going to be honest this is unfortunately beginning to look more and more unfeasable as time goes on. When I originally had this idea to use the lexer I was certain tokens were going to be much better than lines and regexes however at this stage I think lines and regexes ironically are a far more robust solution.

A massive advantage of lines and a huge weakness of the lexer is whitespace. The most major issues left to solve, and the ones that have been "solved", are to do with whitespace and the little flexibility the lexer provides with that. One more simple example is something like:

#import foobar

struct X {}

in the new system this would be considered foobar::struct::X as it's impossible to tell its separated by a line. The WGSL lexer does provide Token::Trivia but it's use is very limited as it not only includes whitespace and newlines, but comments as well.
The current issue I am facing is within Preprocessor::preprocess and how it handles padding the final string with spaces so any errors will match the original source spans correctly. The lexer is not going to allow for this padding and therefore all errors will no longer align with the original source.

There is also the other problem that a lot of this system would have to be duplicated to add the functionality of the GLSL lexer. I will be starting a new job at the end of this month and I don't think I could have it completed by then so I'm not sure where to take it. As much as I want this to work it may be a case of leaving it to a whole new project somewhere within the WESL spec.

@DexterHill0 I understand, it is surprisingly tough to get all the existing pieces to line up. I had previously written a whole parser for this, only to run into a few other architectural issues.

As for GLSL, I believe it is ok to not have perfect interoperability. For example, we could require WGSL bindings for GLSL modules.

  1. foo.glsl
  2. Create a foo.wgsl file, which contains stubs for everything
  3. Import from the foo.wgsl file
  4. Once it has all been parsed, we separately parse the foo.glsl file, and then replace the naga modules.

Alternatively, depending on naga's capabilities, one might be able to translate almost arbitrary GLSL code to WGSL code. Then we could simply tell people to automatically translate their existing GLSL code to WGSL.

Would you be interested in helping out with WESL? We're currently focusing on an MVP, which has a lot of tasks where we'd love to get some help wgsl-tooling-wg/wesl-spec#54

  • Spec: There are a few final bits that we need to get across the finish line. We absolutely appreciate feedback at this stage!
  • Documentation: Be it writing documentation, or giving us feedback from a user perspective. We also need to set up an mdbook webpage.
  • 3rd party tools: IDE support would be amazing! There are quite a few WGSL syntax highlighters and language servers out there. Polishing those would be amazing.
  • ...

@stefnotch I personally think for this project it's better to leave it as is since it would probably be easier to rewrite the whole thing and I think that's better off for WESL to do anyway. I'd definitely be happy to help there and there with WESL.

For helping out with the spec is it just a case of opening an issue in the spec repo or should it go through the Discord first?

For helping out with WESL, feel free to directly open issues on GitHub. The Discord is there for times when you want to ask a quick question, or when you think that a quick discussion would be worthwhile.

Thanks, I've run into a few things that might be nice so I'll definitely make sure to open an issue or two.