Twinklebear / oidn-rs

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

Should oidn expose the OIDNBuffer in a safe way? #29

Open Vecvec opened 3 months ago

Vecvec commented 3 months ago

Currently oidn provides OIDNBuffer, but we do not expose this, instead only providing vectors to interact with the api. It could be useful to expose a wrapper around these (and provide a similar api that took these instead). This could have the previous functions emulated (by directly writing to the buffer like done now).

If as_raw and from_raw functions are also added, users would be able to use the interoperability with graphical apis or these oidn functions could be provided (though I'm not sure safety could be guaranteed, so they would have to be unsafe)

The changes could look something like

pub fn albedo(&mut self, albedo: &[f32])

to

// different name so the other function can still exist
pub fn set_albedo_buffer(&mut self, albedo: &Buffer)

where the buffer is something like

pub struct Buffer(pub(crate) OIDNBuffer, pub(crate) usize);
Twinklebear commented 2 months ago

I think exposing this would make sense, and could help applications reduce data copies of their images since the current slice APIs will all copy the data into the filter's buffer member: https://github.com/Twinklebear/oidn-rs/blob/master/src/filter.rs#L86-L100 . If the app could provide an OIDN buffer instead, we wouldn't need to make this copy