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

spirv-builder should declare dependencies on Cargo.toml files. #404

Open
eddyb opened this issue Feb 1, 2021 · 3 comments
Open

spirv-builder should declare dependencies on Cargo.toml files. #404

eddyb opened this issue Feb 1, 2021 · 3 comments
Labels
s: needs update This issue needs a follow up or investigation whether it still applies. t: enhancement A new feature or improvement to an existing one.

Comments

@eddyb
Copy link
Contributor

eddyb commented Feb 1, 2021

For #403 I changed the workspace Cargo.toml, but that didn't result in a rebuild of sky-shader, whereas something like this does cause a rebuild (of example-runner-wgpu, in this example):

[profile.dev.package."example-runner-wgpu"]
opt-level = 3

So I think we need spirv-builder to get the workspace Cargo.toml (and Cargo.lock, I suppose) paths from Cargo somehow, and also list those out as "rebuild if changed".

@eddyb eddyb added the t: enhancement A new feature or improvement to an existing one. label Feb 1, 2021
@khyperia
Copy link
Contributor

khyperia commented Dec 8, 2021

Hm, thinking about this, is this something that should be fixed in rust-gpu, with manually finding the Cargo.toml somehow (or even more somehow, extracting from cargo), or something that cargo should do when ingesting rustc's .d file and outputting its custom cargo-mangled .d file? (the difference between deps/compute_shader.d and libcompute_shader.d) - if cargo automatically included the relevant files in its .d file, rust-gpu would "just work".

@oisyn oisyn added the s: needs update This issue needs a follow up or investigation whether it still applies. label Nov 15, 2022
@oisyn
Copy link
Contributor

oisyn commented Nov 16, 2022

There currently is no example or test within rust-gpu that actually uses spirv-builder

@eddyb
Copy link
Contributor Author

eddyb commented Nov 25, 2022

that actually uses spirv-builder

To be clear, there is, but not from a build script (except for... Android and wasm, I think?)

And that's because of the hot reloading feature. IMO we should do both: build them in the build script, and have hot reloading as an optional Cargo package feature, so people can know what they can ignore from the example, when they don't want to implement hot reloading themselves (and ideally all the logic that ash and wgpu examples want to share should be in one shared crate).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
s: needs update This issue needs a follow up or investigation whether it still applies. t: enhancement A new feature or improvement to an existing one.
Projects
None yet
Development

No branches or pull requests

4 participants
@eddyb @khyperia @oisyn and others