Skip to content
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

Update to naga 0.13 #33

Closed
Elabajaba opened this issue Jul 21, 2023 · 6 comments · Fixed by #40
Closed

Update to naga 0.13 #33

Elabajaba opened this issue Jul 21, 2023 · 6 comments · Fixed by #40

Comments

@Elabajaba
Copy link
Contributor

I had a quick look at this, and it seemed confusing (especially since naga doesn't have a changelog yet, and gfx-rs/naga#2266 seems to have changed a bunch of stuff).

@mockersf
Copy link
Member

mockersf commented Jul 21, 2023

Doing this update far from the next Bevy release would mean it would be harder to publish a patch that would be picked up by the current version of Bevy. That said, I don't know if there are already breaking changes merged.

And not doing it means it will be harder to update Bevy to the latest version of wgpu...

@Elabajaba
Copy link
Contributor Author

Doing this update far from the next Bevy release would mean it would be harder to publish a patch that would be picked up by the current version of Bevy.

Agreed

And not doing it means it will be harder to update Bevy to the latest version of wgpu...

It's not possible to update bevy to wgpu 0.17 without first updating naga_oil

@robtfm
Copy link
Collaborator

robtfm commented Jul 21, 2023

i assume there is no intention to bump wgpu for 0.11.1?

if there is i'll prioritise this, otherwise i'll look later (if nobody beats me to it).

there are already breaking changes in naga_oil from #32, which i hope will be used in the bevy point release as well.

@Elabajaba
Copy link
Contributor Author

Elabajaba commented Jul 21, 2023

i assume there is no intention to bump wgpu for 0.11.1?

I don't think we've ever bumped it for a patch release?

I'd definitely prioritize any fixes for eg. stuff that Griffin or others on discord have run into over this for now.

@VitalyAnkh
Copy link
Contributor

The development version of bevy should use a development version of naga_oil, or naga_oil lives in the same repository with bevy's crates. The latter won't stop people from using naga_oil as a standalone tool.

@JMS55
Copy link
Contributor

JMS55 commented Jul 23, 2023

I would appreciate a version of naga_oil compatible with naga 0.13, and keeping up with upstream naga regardless of bevy version in the future.

When developing bevy, I often need to test a git version of wgpu/naga, which is impossible without first updating naga_oil :(.

robtfm added a commit that referenced this issue Aug 26, 2023
fixes #33

this should not merged until bevy 0.11.1 is out. builds on @VitalyAnkh's
partial migration, with additionally :

- use `Rc<RefCell>` for `const_expressions` and `const_expr_map` - this
is basically unavoidable, as `DerivedModule::import_expression` needs
access to both the `const_expressions` and the arena to import into,
which may also be `const_expressions`
- don't emit `Expression::Literal` and `Expression::ZeroValue`s
- enforce `const_expression` uniqueness with a custom impl of
`PartialEq` for `Expression`s, to ensure that the uniqueness test for
globals and consts still passes (else we end up with duplicated items,
they no longer test as equal as they refer to different `init`
expressions). this could be removed if the `PartialEq` derive on
`Expression` in naga is made externally available.
- a basic port of the `prune` module which just leaves all
`const_expressions` present

the tests pass, but i haven't tried integrating into bevy (which will
require bevy to use wgpu 0.17).

---------

Co-authored-by: VitalyR <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants