Skip to content

update to 2.3.2#35

Merged
Twinklebear merged 14 commits intoTwinklebear:masterfrom
float3:master
Feb 22, 2025
Merged

update to 2.3.2#35
Twinklebear merged 14 commits intoTwinklebear:masterfrom
float3:master

Conversation

@float3
Copy link
Contributor

@float3 float3 commented Feb 11, 2025

No description provided.

@float3
Copy link
Contributor Author

float3 commented Feb 11, 2025

bumped version, also did some minor improvements to quality

@float3
Copy link
Contributor Author

float3 commented Feb 12, 2025

this fixes issue #32 and I believe issue #29 has been addressed previously as well?

@Vecvec
Copy link
Contributor

Vecvec commented Feb 14, 2025

Yes #29 was fixed, I forgot to link it in my PR.

@Twinklebear
Copy link
Owner

Thanks @float3 ! I should have some time to review this over the weekend

@float3
Copy link
Contributor Author

float3 commented Feb 14, 2025

I still wanted to see if I can expose

extern "C" { 
     pub fn oidnExecuteSYCLFilterAsync( 
         filter: OIDNFilter, 
         depEvents: *const sycl_event, 
         numDepEvents: ::std::os::raw::c_int, 
         doneEvent: *mut sycl_event, 
     ); 
 }

correctly

Copy link
Owner

@Twinklebear Twinklebear left a comment

Choose a reason for hiding this comment

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

This looks good! For oidnExecuteSYCLFilterAsync, it'd be interesting to think about wrapping it with some async/await semantics, I haven't been following Rust as closely but it sounded like there isn't async/await in the standard library? On another repo of mine someone added async support w/ feature flags to select between 2 popular libraries providing that functionality: Twinklebear/tobj#69

@float3
Copy link
Contributor Author

float3 commented Feb 18, 2025

Rust’s async/await is stable, but have to choose an executor, (for example Tokio or async-std)

@float3
Copy link
Contributor Author

float3 commented Feb 18, 2025

either way I want to save that for a future PR

@Vecvec
Copy link
Contributor

Vecvec commented Feb 19, 2025

I think there are a few other places that async exists in oidn, so maybe a issue for it is useful.

@float3
Copy link
Contributor Author

float3 commented Feb 19, 2025

yeah let's make a issue

@float3
Copy link
Contributor Author

float3 commented Feb 20, 2025

tested with my pathtracer, everything seems to work fine

@float3
Copy link
Contributor Author

float3 commented Feb 20, 2025

happy new rust edition day 🎉

Copy link
Owner

@Twinklebear Twinklebear left a comment

Choose a reason for hiding this comment

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

Separate issue for async sounds great, thanks for working on this pr @float3 !

@Twinklebear Twinklebear merged commit b150017 into Twinklebear:master Feb 22, 2025
9 checks passed
@float3 float3 mentioned this pull request Feb 22, 2025
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