kornelski / imgref

A trivial Rust struct for interchange of pixel buffers with width, height & stride
https://lib.rs/crates/imgref
Apache License 2.0
59 stars 6 forks source link

Calling `to_owned` on `ImgRef` returns another `ImgRef` intead of `ImgVec` #22

Open CBenoit opened 6 months ago

CBenoit commented 6 months ago

Reproducer:

fn  foo(img: ImgRef<'_, RGB8>) -> ImgVec<RGB8> {
    img.to_owned()
}

The above code will not compile:

   |
95 | fn foo(img: ImgRef<'_, RGB8>) -> ImgVec<RGB8> {
   |                                  ------------ expected `imgref::Img<std::vec::Vec<rgb::RGB<u8>>>` because of return type
96 |     img.to_owned()
   |     ^^^^^^^^^^^^^^ expected `Img<Vec<RGB<u8>>>`, found `Img<&[RGB<u8>]>`
   |
   = note: expected struct `imgref::Img<std::vec::Vec<rgb::RGB<u8>>>`
              found struct `imgref::Img<&[rgb::RGB<u8>]>`

As seen in the error, to_owned had no effect.

kpreid commented 1 month ago

I just hit this myself. The problem is in the bounds on the definition:

impl<T> Img<T> where T: ToOwned {
    pub fn to_owned(&self) -> Img<T::Owned> {

T here is the container type, which we would like to be the slice reference &[P] (where P is our pixel type), but the relevant ToOwned implementation to copy a slice is implemented for [P], not &[P]. Calling <&[P] as ToOwned>::to_owned() just clones the slice reference. Here is a function definition that does what was presumably wanted:

pub fn img_to_owned<T, P>(this: &imgref::Img<&T>) -> imgref::Img<T::Owned>
where
    T: AsRef<[P]> + ToOwned,
    T::Owned: AsRef<[P]>,
{
    this.map_buf(ToOwned::to_owned)
}

Unfortunately, because it's technically possible to call Img::to_owned(), but it doesn't return the type it's supposed to, changing it to fix this bug is technically a semver breaking change.

Applications in need of a workaround could write .map_buf(ToOwned::to_owned) or .map_buf(<[_]>::to_vec).