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

👀 hot shader reloading #655

Merged
merged 7 commits into from
Jun 9, 2021
Merged

👀 hot shader reloading #655

merged 7 commits into from
Jun 9, 2021

Conversation

DJMcNab
Copy link
Contributor

@DJMcNab DJMcNab commented Jun 8, 2021

Add a (feature gated) implementation of hot shader reloading to SpirvBuilder

This attempts to handle initial compilation gracefully by watching the shader crate root recursively until we have more detailed dependency information

Unfortunately, notify uses a non-standard license (CC0) so we fail the cargo deny check. I will defer adding a commit which changes deny.toml to allow this to pass to the suitable Embarker (@XAMPPRocky ?)

It appears that if there was sufficient demand for it, notify would be willing to update their license ( notify-rs/notify#243 (comment))

In theory, the CC0 license they are under is probably already MIT/Apache compatible, so notify could avoid getting new permissions from the contributors, but it would still be wise to notify (ha) them.

(See e.g. this stackoverflow post (this is google's webcache since stackoverflow is down at the time of posting))

I'm also planning to integrate this into the example runner, but want to get the ball rolling on the license issues

@DJMcNab
Copy link
Contributor Author

DJMcNab commented Jun 8, 2021

I have produced a video showing how this works in the wgpu example runner:

2021-06-08.21-17-24.mp4

Copy link
Contributor

@khyperia khyperia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just the two things!

crates/spirv-builder/src/lib.rs Outdated Show resolved Hide resolved
examples/runners/wgpu/src/lib.rs Outdated Show resolved Hide resolved
@khyperia
Copy link
Contributor

khyperia commented Jun 9, 2021

Unfortunately, notify uses a non-standard license (CC0) so we fail the cargo deny check. I will defer adding a commit which changes deny.toml to allow this to pass to the suitable Embarker

Pushed the exception!

@DJMcNab DJMcNab marked this pull request as ready for review June 9, 2021 07:51
@khyperia khyperia merged commit 3bbe963 into EmbarkStudios:main Jun 9, 2021
@DJMcNab DJMcNab deleted the watch branch June 9, 2021 08:39
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 this pull request may close these issues.

2 participants