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

Fixes to the Optix crate #157

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Schmiedium
Copy link
Contributor

Some new traits needed to be derived for MotionFlags, InstanceFlags, and some others. Additonally, from here: https://docs.rs/bitflags/latest/bitflags/, it looks we need to use the impl syntax for the bit flag structs to work with the custom derive macros from cust.

Also fixed a warning about an elided lifetime

@LegNeato
Copy link
Contributor

Note you are going to want to rebase on #156 as it touches a ton of stuff.

@LegNeato
Copy link
Contributor

(it's just about ready to land)

@Schmiedium Schmiedium changed the title Should all be working now Fixes to the Optix crate Mar 13, 2025
@Schmiedium
Copy link
Contributor Author

Gotcha, will do! Thanks for letting me know. I'll keep this open and rebase on top once that drops, if that works

@Schmiedium Schmiedium requested a review from frjnn as a code owner March 13, 2025 18:22
@Schmiedium Schmiedium force-pushed the fix_optix_compile_errors branch from 5dfb476 to 7e7007d Compare March 13, 2025 18:29
@Schmiedium
Copy link
Contributor Author

Rebased on top of the recent changes from #156, checked that everything still works

Copy link
Contributor

@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.

I think the MotionFlags doc comments need to be moved out of the impl and on the struct like BuildFlags has them?

It would be nice to have all of these documented in general but keeping the staus quo is all that is required. Thanks for the PR!

@Schmiedium
Copy link
Contributor Author

Schmiedium commented Mar 13, 2025

Understood, I can reorganize that and push up the changes shortly. I can work on documenting the rest of them as well, that might take a minute as I figure out the specifics of what they do. Thanks for reviewing!

@LegNeato
Copy link
Contributor

looks like this overlaps with #159

@Schmiedium
Copy link
Contributor Author

Okay, should I close this one out then? I'd prefer not to duplicate work in the future

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