Twinklebear / oidn-rs

Rust bindings to Intel's OpenImageDenoise Library
MIT License
24 stars 12 forks source link

About the Sync impl on RayTracing #4

Closed TomCrypto closed 4 years ago

TomCrypto commented 4 years ago

The Sync impl on RayTracing is pretty useless considering all methods on the filter take &mut self. On the other hand a Send impl would be far more useful since that's what the thread-safety in OIDN is intended for; to have multiple filters on the same device driven by different threads. In any case it probably doesn't make sense to drive a single filter from multiple threads, they'd just scribble all over one another's changes to the filter settings.

So in principle (untested) it should be okay from an API safety point of view for the RayTracing filter to just take a &Device instead of a &mut Device, and be Send instead of Sync?

Don't know if this change is really worth considering since I seriously doubt many people are using more than one filter at a time, but food for thought anyway.

Twinklebear commented 4 years ago

Yea, Send is the right one to have actually, multiple threads using/modifying the same filter isn't valid. This is also changed in 8ee7c3b104a14cb5943b8a51be6eddb774676da1