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 vulkan indirect command structs #34

Merged
merged 4 commits into from
Oct 14, 2024

Conversation

Firestar99
Copy link
Contributor

@Firestar99 Firestar99 commented Oct 13, 2024

  • Add the *IndirectCommand structs from vulkan, mostly copied them from ash.
  • Add optional bytemuck dependency to mark plain structs as Pod

@@ -25,3 +26,4 @@ glam = { workspace = true, default-features = true }

[features]
default = []
bytemuck = ["dep:bytemuck", "glam/bytemuck"]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added bytemuck as an optional dependency to mark all the *IndirectCommand structs as Pod, as most graphics APIs will require this. Should we make the bytemuck dependency required, eg. not hidden behind a feature?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think keeping bytemuck optional is good. It's true that most graphics projects will have it somewhere in the tree, but I don't think we should be deciding how folks marshal data to the GPU.

@Firestar99 Firestar99 marked this pull request as draft October 13, 2024 11:55
Copy link
Collaborator

@LegNeato LegNeato left a comment

Choose a reason for hiding this comment

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

Seems reasonable to me!

@Firestar99 Firestar99 marked this pull request as ready for review October 14, 2024 11:10
@Firestar99 Firestar99 merged commit 666f1fc into Rust-GPU:main Oct 14, 2024
7 checks passed
@Firestar99 Firestar99 deleted the indirect_command branch October 14, 2024 11:35
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.

3 participants