don't panic if naga parsing of shader source fails #5034
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nical
requested changes
Jan 10, 2024
67503e7 to
7148796
Compare
waynr
commented
Jan 11, 2024
nical
requested changes
Jan 11, 2024
7148796 to
0061d54
Compare
|
@waynr could you rebase the PR on latest trunk? so the new CI jobs can run. |
|
Ah, nvm, they didn't run because the clippy jobs are failing. |
0061d54 to
73d6fae
Compare
teoxoy
reviewed
Jan 19, 2024
teoxoy
approved these changes
Jan 19, 2024
|
@nical do you want to take another look? |
cwfitzgerald
requested changes
Jan 21, 2024
e31179a to
4d53a97
Compare
7159d78 to
f8bc24b
Compare
a1phyr
reviewed
Jan 22, 2024
Co-authored-by: Alphyr <47725341+a1phyr@users.noreply.github.com>
|
This should be ready to go. Pinging @nical & @cwfitzgerald for pending reviews. |
nical
approved these changes
Jan 23, 2024
cwfitzgerald
approved these changes
Jan 23, 2024
|
Thanks for working through this with me! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Connections
#5019
fixes #2545
Description
In my application shaders are meant to be hot-reloaded on filesystem changes.
If there is a syntax issue with the newly-updated shader source
nagaparsingof the file can fail. In current trunk, the
Resultreturned bynagaparsingis simply unwrapped.
This PR removes the
nagaparse unwrap and instead bubbles theResultupthrough the call stack so to be handled by the calling code.
I am opening this PR prior to updating the changelog and getting
cargo xtask testto run just to start discussion (should have commits addressing thoseshortly after opening it).
Testing
I've updated the examples and test cases that were already calling
Device.create_shader_module.Checklist
cargo fmt.cargo clippy. If applicable, add:--target wasm32-unknown-unknown--target wasm32-unknown-emscriptencargo xtask testto run tests.CHANGELOG.md. See simple instructions inside file.