-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Implement Clone
for ShaderModuleDescriptor
#2902
Conversation
21c7f40
to
688aae8
Compare
688aae8
to
964e9d6
Compare
Now that gfx-rs/naga#2013 was merged, I updated the relevant parts to depend on it. To explain this PR a bit further, personally it doesn't make sense to me without #2903. This PR can further refine the process by not having the user build With this PR: const SHADER: &'static ShaderModuleDescriptor = ...;
// The module itself is not cloned here, because it uses `Cow::Borrowed(..)`.
device.create_shader_module(SHADER.clone()); Without this PR: const MODULE: &'static Module = ...;
// Now you have to write it out, even though it could be part of the const.
device.create_shader_module(ShaderModuleDescriptor {
label: None,
source: ShaderSource::Naga(Cow::Borrowed(MODULE)),
}); I must say after writing out this example and already locally using #2903 successfully, I don't think the quality-of-life improvement this PR offers is good. Calling Maybe somebody else can see the value in this, otherwise feel free to close. |
964e9d6
to
e16b5ce
Compare
Closing in favor of #2903 |
@cwfitzgerald just wanted to make sure you take a second look at this after #2890 was merged. I can rebase this PR if it is still desired. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes please rebase, then we're g2g
@cwfitzgerald done, you probably have to reopen for CI to run. |
Did you push? Force pushes should show up in the event log (and trigger CI) |
Yeah I pushed, I think nothing shows up because the PR is closed. I can open another one? |
Huh, yeah go ahead, I can't re-open it either |
Checklist
cargo clippy
.RUSTFLAGS=--cfg=web_sys_unstable_apis cargo clippy --target wasm32-unknown-unknown
if applicable.Connections
Depends on gfx-rs/naga#2013.
Description
I am playing around with pre-compiled shaders and encountered an issue with storing them in
static
s,ShaderSource::Naga
needs to own it, so cloning solves the problem.Testing
None.