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

Add more SpirvBuilder fields as cli arguments #15

Open
schell opened this issue Nov 13, 2024 · 8 comments
Open

Add more SpirvBuilder fields as cli arguments #15

schell opened this issue Nov 13, 2024 · 8 comments
Assignees
Labels
enhancement New feature or request priority

Comments

@schell
Copy link
Collaborator

schell commented Nov 13, 2024

This would allow use to further customize compilation.

From #12 .

@schell schell added the enhancement New feature or request label Nov 13, 2024
@schell schell added the priority label Dec 8, 2024
@schell
Copy link
Collaborator Author

schell commented Dec 8, 2024

Without this, users cannot set Vulkan extensions.

@tombh
Copy link
Contributor

tombh commented Dec 21, 2024

What do you think about also defining all of these in the [package.metadata.rust-gpu] section of the shader's Cargo.toml? I was thinking that they would take lower precedence than the same config set on the CLI.

And so related to that, what do you think if we always built like that? Therefore there'd be no need for the cargo gpu toml subcommmand (we'd still need all the code of course, just that because it'd always be run there'd be no reason to manually invoke it). Then the sources of config would be (ordered from lowest precedence to highest):

  1. Defaults (included those defined by the spirv-std and the rust-gpu it points to).
  2. [package.metadata.rust-gpu] fields.
  3. CLI arguments.

@schell
Copy link
Collaborator Author

schell commented Dec 21, 2024

Yes, I'm in agreement with all of that. 👍

@schell
Copy link
Collaborator Author

schell commented Dec 21, 2024

@tombh would you like to take this one?

@tombh
Copy link
Contributor

tombh commented Dec 22, 2024

Sure ✨

@tombh
Copy link
Contributor

tombh commented Dec 22, 2024

I'm making a start on this and I noticed that spirv-builder::SpirvBuilder::new() takes a target as its second argument. Does that do something different from the target_spec() method?

@schell
Copy link
Collaborator Author

schell commented Dec 22, 2024

They are related, if path_to_target_spec is None, it will fall back to target with an expected path.

@tombh
Copy link
Contributor

tombh commented Dec 22, 2024

Great, I'll make sure they're both supported then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request priority
Projects
Status: Ready
Development

No branches or pull requests

2 participants