Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Encoder/Decoder supported ColorTypes for ImageFormat #2210

Open
iganev opened this issue Apr 18, 2024 · 3 comments
Open

Encoder/Decoder supported ColorTypes for ImageFormat #2210

iganev opened this issue Apr 18, 2024 · 3 comments

Comments

@iganev
Copy link

iganev commented Apr 18, 2024

Working on a piece of code that converts images from one format to another while applying some other options provided by DynamicImage. Just realized (the hard way) that if I try to blindly convert an Rgba8 png to jpg the Jpeg encoder would scream at me for a wrong ColorType.

To prevent the encoder from screaming at me I did something like:

                let format = ImageFormat::from_extension(ext).unwrap_or(ImageFormat::Jpeg);

                let img = match format {
                    ImageFormat::Png => match img.color() {
                        ColorType::L8
                        | ColorType::La8
                        | ColorType::Rgb8
                        | ColorType::Rgba8
                        | ColorType::L16
                        | ColorType::La16
                        | ColorType::Rgb16
                        | ColorType::Rgba16 => img,
                        _ => img.to_rgba8().into(),
                    },
                    ImageFormat::Jpeg => match img.color() {
                        ColorType::L8 | ColorType::Rgb8 => img,
                        _ => img.to_rgb8().into(),
                    },
                    ImageFormat::Gif => match img.color() {
                        ColorType::Rgb8 | ColorType::Rgba8 => img,
                        _ => img.to_rgba8().into(),
                    },
                    ImageFormat::WebP => match img.color() {
                        ColorType::L8 | ColorType::La8 | ColorType::Rgb8 | ColorType::Rgba8 => img,
                        _ => img.to_rgba8().into(),
                    },
                    // unlikely to be supported as output formats so I won't bother
                    // ImageFormat::Pnm => todo!(),
                    // ImageFormat::Tiff => todo!(),
                    // ImageFormat::Tga => todo!(),
                    // ImageFormat::Dds => todo!(),
                    // ImageFormat::Bmp => todo!(),
                    // ImageFormat::Ico => todo!(),
                    // ImageFormat::Hdr => todo!(),
                    // ImageFormat::OpenExr => todo!(),
                    // ImageFormat::Farbfeld => todo!(),
                    // ImageFormat::Avif => todo!(),
                    // ImageFormat::Qoi => todo!(),
                    _ => img,
                };

Before trying to save the image. This is pretty verbose, error-prone and most likely subject to changes.

It would be super nice to have 1-2 methods in ImageFormat to get a list of supported ColorTypes and perhaps a default (fallback) ColorType.

Then, maybe if DynamicImage offers a to_color(color: ColorType) I could condense that verbose check into 2-3 lines of code and let the image crate dictate when ColorType support for different encoders/decoders change.

I could implement the above to contribute, if deemed appropriate.

@fintelia
Copy link
Contributor

Most of these color type restrictions are actually caused by limitations of the underlying image format. PNGs cannot store floating point data, WebP cannot hold 16-bit per sample images, JPEG cannot represent alpha, etc. Our encoders aren't going to add support for any of those in a future version. If they tried to include any of that data, other libraries would rightfully consider the resulting files to be corrupt!

JPEG's inability to store alpha has unfortunately drawn a bunch of attention lately because we switched from silently dropping the alpha channel to returning an error instead (with a short span in between where trying to encode a RGBA jpeg would produce a corrupt image ☹️).

That said, I do like the idea of programmatically exposing the list of supported color types for each encoder and the DynamicImage::to_color method (though not fully sold on the name). One implementation note is that encoders actually take an ExtendedColorType rather than a ColorType.

For decoders, I don't think supported color types makes as much sense. Some of our decoders don't support all image format features (meaning there are some images they cannot decode) but that often doesn't align with specific color types

@iganev
Copy link
Author

iganev commented Apr 28, 2024

Ok I will have a look at how best to implement this when I get a chance.
I am open to suggestions for the naming of what I referred to as DynamicImage::to_color, I have no strong feelings about the approach.

As for ColorType and ExtendedColorType, I noticed that encoders use the extended type. Am I right to assume that ColorType is more intended for use by the end-user and ExtendedColorType is more of an internal thing?

@fintelia
Copy link
Contributor

fintelia commented Apr 28, 2024

Am I right to assume that ColorType is more intended for use by the end-user and ExtendedColorType is more of an internal thing?

Not quite. ColorType is returned by decoders and there is a 1-1 mapping with the variants of DynamicImage. It is designed as a restricted set of color types that are fully supported by the library. ExtendedColorType has a much lower bar for adding variants and allows pixel formats that aren't an integer number of bytes or are otherwise awkward to work with.

The idea is that end-users might get any possible ColorType when they decode an image, but don't have to worry about anything beyond that. Whereas if an end-user wants to pass an image to an encoder, they can pick from the broader set of ExtendedColorType's (subject to support by the underlying encoder)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants