Image: implement GenericImageView::save

[lovasoa]

Hello, and thank you for this great library!

The feature

This is a feature request for something that is already doable, but could be easier: saving a GenericImageView, that is adding a method :

pub fn save<Q: AsRef<Path>>(&self, path: Q) -> Result<()>

to GenericImageView

My specific use case for this functionality is ...

I am implementing the GenericImageView trait on structs, and I currently have to either implement a save method myself, or instruct the users of the type that they have to manually create a DynamicImage if they want to save the image.

This is more generally applicable to ...

It would notably allow using save() on SubImage

[fintelia]

I think that any implementation of save will ultimately have to go through image::save_buffer since that is roughly the interface that the encoders themselves take. Since GenericImageView doesn't provide a way to directly get a byte slice of the data backing the image, this would mean that any generic implementation of save would first require pixel by pixel copying of the image data into another buffer.

It is a kind of unfortunate consequence of the interface for GenericImageView being so generic that even most of the rest of this library insists on taking inputs with more well defined layouts.

[fintelia]

Also, are you able to share the code you're working on? I'd be curious if a trait like this would be better for most uses:

trait ImageView<P: Pixel> {
    fn dimensions(&self) -> (u32, u32);
    fn image_data(&mut self) -> &mut [u8];

    fn get_pixel(&self, x: u32, y: u32) -> P {...}
    fn set_pixel(&mut self, x: u32, y: u32, p: P) {...}
    ... 
}

[HeroicKatora]

@fintelia Pretty sure that image_data(&mut self) should not be mut in ImageView. But overall, if saving is first copying to a dedicated byte buffer then so be it. If we add save to the trait then it can be specialized in impl to avoid that copy.

Not sure if that is worth it, but it might be.

[HeroicKatora]

Related, I wouldn't add

pub fn save<Q: AsRef<Path>>(&self, path: Q) -> Result<()>

but

pub fn write(&self, path: impl std::io::Write) -> Result<()>

and provide save as an extension that opens the path as a file. Else we have all the troubles of providing a memory-backed interface we had with open again. And we have to find a better way for providing the file format.

[fintelia]

I wonder if the right function to add is:

```rust
fn GenericImageView::to_image_buffer(&self) -> ImageBuffer ````

The remainder of the functions then could then be added on ImageBuffer (possibly with helpers on GenericImageView).

[HeroicKatora]

Here was a bad idea about using flat::View[Mut]

Putting the actual methods on ImageBuffer sounds like a good plan. It already has save but is missing the memory interface. The best part would be that it is not duplicated all across so that if we ever need more configuration (such as format specific flags) there is a central type for that.

[lovasoa]

I agree that a to_image_buffer method would be more generally useful !

My use case is for my rust implementation of seam carving. Here is the part with the implementation of GenericImage. Don't hesitate to notify me if you think there is something wrong with how I am using the image crate :)

[theotherphil]

(Not relevant to this issue, but an FYI for @lovasoa. The imageproc crate has a not-very-polished implementation of seam carving: https://github.com/image-rs/imageproc/blob/master/src/seam_carving.rs If your version is better (faster, more accurate, more flexible) then I'd love to get your input into improving the version we have.)

[lovasoa]

@theotherphil I just started my own implementation, but it looks like we took different paths: I use dijkstra for finding seams, and I don't resize the image until all the seams have been removed (using a specialized intermediary structure to store the carved image). It will be interesting to do a comparison of the two, but there are still a few things I want to implement.