search

twistedfall / opencv-rust

Rust bindings for OpenCV 3 & 4
MIT License
1.86k stars 144 forks source link

impl ToInputArray for Vector<BoxedRef<'_,Mat>> #555

Closed sampaioletti closed 2 months ago

sampaioletti commented 3 months ago

Would it be possible to implement something like....

impl ToInputArray for Vector<BoxedRef<'_,Mat>>

or more general i.e. impl ToInputArray for Vector<T> where T:...

Happy to do a PR, just wondering if that is idiomatic, seems like with the new boxedref returns it would be handy.

Usecase: crating a Vector from a set of Mat::roi and using in hconcat

PS sorry for the initial issue create, I hit the wrong button.

twistedfall commented 3 months ago

I think it makes perfect sense, thank you! If you feel like you can do the change I'm more than happy to review the PR, otherwise I will take a look at this in the coming days.

sampaioletti commented 3 months ago

So after my first time going through the repo (awesome btw) initially my thought would be I would have to impl VectorElement and VectorExtern for BoxedRef to exist in a Vector something about that feels off, or impossible, but might be the correct way. Am I better off going a different route of impl ToInputArray for Vec<BoxedRef<'_,Mat>> or can you see another direction.

twistedfall commented 3 months ago

I would have to impl VectorElement and VectorExtern for BoxedRef to exist in a Vector

That's the way I would take. Currently to be able to create a Vector<T> at all you need to have:

  1. Vector<T>: VectorExtern<T> — this is the glue that connects Rust Vector to its std::vector C++ counterpart.
  2. T: VectorElement — this is currently kind of hack to allow more efficient conversion of vectors of copy types to Rust Vec.

And necessary impls for cases where T is BoxedRef are not being generated at the moment.

The generation for 1 is happening from this template: https://github.com/twistedfall/opencv-rust/blob/master/binding-generator/src/writer/rust_native/vector.rs#L62 (tpl/vector/rust_extern.tpl.rs). For 2 it's https://github.com/twistedfall/opencv-rust/blob/master/binding-generator/src/writer/rust_native/vector.rs#L65-L69 (tpl/vector/rust_copy_non_bool.tpl.rs and tpl/vector/rust_non_copy_or_bool.tpl.rs).

After that the generation of proper ToInputArray is still needed and it happens from https://github.com/twistedfall/opencv-rust/blob/master/binding-generator/src/writer/rust_native/vector.rs#L71 (tpl/vector/rust_input_output_array.tpl.rs)

twistedfall commented 2 months ago

This functionality is now available in 0.91.2 release