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 support for generating OpTypeAccelerationStructureKHR #187

Closed
wants to merge 2 commits into from

Conversation

XAMPPRocky
Copy link
Contributor

This PR adds support for OpTypeAccelerationStructureKHR by adding a small pass to manually map reserved functions into another class, so that the methods for those ops can be properly generated.

See also EmbarkStudios/rust-gpu#563 which uses this change.

@MarijnS95
Copy link
Collaborator

MarijnS95 commented Mar 31, 2021

This looks very close to what is done in #175, albeit that catches every reserved OpType. How about we review and merge that first?

@XAMPPRocky
Copy link
Contributor Author

This looks very close to what is done in #175, albeit that catches every reserved OpType. How about we review and merge that first?

#175 as far as I can tell doesn't add a Builder::type_acceleration_structure_khr method, which is the important part of this PR that we need for generating those types in rust-gpu.

@MarijnS95
Copy link
Collaborator

#175 as far as I can tell doesn't add a Builder::type_acceleration_structure_khr method, which is the important part of this PR that we need for generating those types in rust-gpu.

I can pick that on top if you want, or rebase this PR to introduce that separately. At least we'll have more generic handling for reserved OpType instructions don't you think?

@XAMPPRocky
Copy link
Contributor Author

I can pick that on top if you want, or rebase this PR to introduce that separately. At least we'll have more generic handling for reserved OpType instructions don't you think?

Anything that makes that method available works for me.

@MarijnS95
Copy link
Collaborator

Allright, merged both solutions!

I like your approach more because it sets the type properly for every gen step, to add the missing method through gen_dr_builder_types. I hope that won't break things elsewhere when future changes come around, not sure what the deal is with these "reserved"-these-are-a-type-but-not-really classes anyway 🤔

@XAMPPRocky XAMPPRocky closed this Apr 1, 2021
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.

None yet

2 participants