Another dynamic image type approach

[jerry73204]

I stumbled on writing an image type over slicesm with dynamic pixel type depending on the runtime context. To be exact, the function returns the enum below.

pub enum ColorImage<'a> {
    Bgr8(ImageBuffer<Bgr<u8>, &'a [u8]>),
    Bgra8(ImageBuffer<Bgra<u8>, &'a [u8]>),
    Rgb8(ImageBuffer<Rgb<u8>, &'a [u8]>),
    Rgba8(ImageBuffer<Rgba<u8>, &'a [u8]>),
}

Though we have DynamicImage, it assumes Vec<_> as underlying buffer. It incurs a large image copy if my function returns DynamicImage, while my function is called several times. Hence, I choose to roll my own enum. That means I would implement GenericImage and GenericImageView manually on the enum for later use. Otherwise, I cannot avoid the match hell.

Here I'm considering an alternative: Oppsed from DynamicImage being an enum over several variants of ImageBuffer, suppose we can derive an image of dynamic pixel and buffer, instead of enums?

enum DynPixel { /* ... */ }
enum DynBuffer { /* ... */ }
type DynamicImage = ImageBuffer<DynPixel, DynBuffer>;

impl GenericImage on DynamicImage { /* ... */ }

In this way, I see we cannot avoid the great refactoring over the crate. Nevertheless, it's more like an once for all solution and gains more flexibility.

[HeroicKatora]

Have you had a look at the flat::FlatSamples as an alternative to using ImageBuffer and DynamicImage? It's main restriction is that the channel type is part of the type. That is, it allows using the same buffer for both Rgb<u8> and Rgba<u8> images but not Rgb<u16>. To use these buffers as a GenericImage you need to supply the Pixel type at a slightly later pointer when calling as_view. Note that ImageBuffer contains a method to view its content as a FlatSamples.

I'm afraid I can't follow all of what you've written.

Oppsed from DynamicImage being an enum over several variants of ImageBuffer, suppose we can derive an image of dynamic pixel and buffer, instead of enums?

In particular, it's very unclear to me how your draft would incorporate type information that's required to access pixels of an image. It needs to be supplied at some point, and currently is part of the type parameter of ImageBuffer and then supplied to the GenericImage implementation. If the type parameter were replaced by an attribute then it would be lost. Without that associated Pixel type, the GenericImage trait could not contain any method that deals with the precise channel type.

[jerry73204]

I see FlatSamples. In fact, I have a more worse scenario when writing the dynamic image for realsense-rust crate. It mixes up images of distinct channels, distinct pixel types and has slice storage. In this way, I can either 1) convert to DynamicImage implying a copy or 2) dive into the match {} hell.

That's where my argument stems from. The current type design might be too picky if some information is only given in runtime over distinct channels, pixel types and storage types. I'm still open to any solutions that could resolve this issue.

I think we can make more clarifications for further discussion.

• Is GenericImage{,View} designed to provide pixel information statically? If yes, the DynamicImage should not assume Pixel = color::Rgba<u8> here.
• What's the best practice to work with dynamic pixels and channels in current design (without match hell)?

My proposal using dynamic pixel might be a solution, but not limited to. I look forward a design that is better than adding more variants in DynamicImage because the all possible combinations just explode. One of pratical application I could find is nalgebra's Matrix. It works with static sized and dynamic sized matrices.

TL;DR: I think we need better support on dynamic images. nalgebra's approach might be a reference.

[HeroicKatora]

nalgebra's approach might be a reference.

nalgebra unfortunately does not attempt to solve the channel type problem, the size constraint is simpler (I think? Can you make clear how your intend to solve types in a similar manner?). I also don't like how hard it makes to read the documentation.

Is GenericImage{,View} designed to provide pixel information statically? If yes, the DynamicImage should not assume Pixel = color::Rgba here.

It was designed to do that. It failed, I do agreed, but no better replacement has been made available either. At least it enabled passing both an ImageBuffer as well as a SubImage to methods that previously required many more copies. It also allowed constraining images to all gray scale types but still keep method generic over those. Those two should be possible with every successor interface.

I would like to point out that a new dynamic interface does not have to replace the currently available DynamicImage. Since 0.23 the ImageDecoder interface should be generic enough to be used by other buffer implementations, and not only those implemented within image. There is an ongoing experiment with another buffer implementation in image-canvas where your requirements could hugely influence the design so that it might provide more of what you are looking for.

[jerry73204]

Thanks you for pointing out image-canvas. The thread would diverges to talks on philosophy of design, so it's fine to close it.

Suppose the next step is to fix the DynamicImage's Pixel = Rgba. The type does not harm, but it might affect the correctness.

[HeroicKatora]

Thread closed as by your request, if I understood correctly, feel free to reopen or create a new issue if there is development in the matter. Some final maintainer note on the planned direction of development:

Suppose the next step is to fix the DynamicImage's Pixel = Rgba. The type does not harm, but it might affect the correctness.

As that is a major break, changing anything about the current DynamicImage is not my priority. Instead, I'd encourage first developing a demonstrably better replacement, then we'll deprecate methods that rely on it as a hard coded in/out type, then deprecating the type itself, and only then removing it in (hopefully) 0.24.