diff --git a/src/codecs/bmp/encoder.rs b/src/codecs/bmp/encoder.rs index 31264a852d..dac056d709 100644 --- a/src/codecs/bmp/encoder.rs +++ b/src/codecs/bmp/encoder.rs @@ -5,7 +5,7 @@ use crate::error::{ EncodingError, ImageError, ImageFormatHint, ImageResult, ParameterError, ParameterErrorKind, UnsupportedError, UnsupportedErrorKind, }; -use crate::{DynamicImage, ExtendedColorType, ImageEncoder, ImageFormat}; +use crate::{ExtendedColorType, ImageEncoder, ImageFormat}; const BITMAPFILEHEADER_SIZE: u32 = 14; const BITMAPINFOHEADER_SIZE: u32 = 40; @@ -366,12 +366,9 @@ impl ImageEncoder for BmpEncoder { self.encode(buf, width, height, color_type) } - fn make_compatible_img( - &self, - _: crate::io::encoder::MethodSealedToImage, - img: &DynamicImage, - ) -> Option { - crate::io::encoder::dynimage_conversion_8bit(img) + fn supported_colors(&self) -> Option<&[ExtendedColorType]> { + use ExtendedColorType::*; + Some(&[Rgb8, Rgba8, L1, L8, La8]) } } diff --git a/src/codecs/jpeg/encoder.rs b/src/codecs/jpeg/encoder.rs index 816fb2b66b..db1b51a84b 100644 --- a/src/codecs/jpeg/encoder.rs +++ b/src/codecs/jpeg/encoder.rs @@ -5,7 +5,7 @@ use std::{error, fmt}; use crate::error::{ EncodingError, ImageError, ImageFormatHint, ImageResult, UnsupportedError, UnsupportedErrorKind, }; -use crate::{ColorType, DynamicImage, ExtendedColorType, ImageEncoder, ImageFormat}; +use crate::{ExtendedColorType, ImageEncoder, ImageFormat}; use jpeg_encoder::Encoder; @@ -293,17 +293,9 @@ impl ImageEncoder for JpegEncoder { Ok(()) } - fn make_compatible_img( - &self, - _: crate::io::encoder::MethodSealedToImage, - img: &DynamicImage, - ) -> Option { - use ColorType::*; - match img.color() { - L8 | Rgb8 => None, - La8 | L16 | La16 => Some(img.to_luma8().into()), - Rgba8 | Rgb16 | Rgb32F | Rgba16 | Rgba32F => Some(img.to_rgb8().into()), - } + fn supported_colors(&self) -> Option<&[ExtendedColorType]> { + use ExtendedColorType::*; + Some(&[Rgb8, L8]) } } diff --git a/src/codecs/png.rs b/src/codecs/png.rs index b7cc944cdf..d31064e9b4 100644 --- a/src/codecs/png.rs +++ b/src/codecs/png.rs @@ -932,17 +932,9 @@ impl ImageEncoder for PngEncoder { Ok(()) } - fn make_compatible_img( - &self, - _: crate::io::encoder::MethodSealedToImage, - img: &DynamicImage, - ) -> Option { - use ColorType::*; - match img.color() { - Rgb32F => Some(img.to_rgb16().into()), - Rgba32F => Some(img.to_rgba16().into()), - L8 | La8 | Rgb8 | Rgba8 | L16 | La16 | Rgb16 | Rgba16 => None, - } + fn supported_colors(&self) -> Option<&[ExtendedColorType]> { + use ExtendedColorType::*; + Some(&[Rgb8, Rgba8, L8, La8, Rgb16, Rgba16, L16, La16]) } } diff --git a/src/codecs/tga/encoder.rs b/src/codecs/tga/encoder.rs index 6a65d164cb..0a033f69ca 100644 --- a/src/codecs/tga/encoder.rs +++ b/src/codecs/tga/encoder.rs @@ -1,6 +1,6 @@ use super::header::Header; use crate::{codecs::tga::header::ImageType, error::EncodingError, utils::vec_try_with_capacity}; -use crate::{DynamicImage, ExtendedColorType, ImageEncoder, ImageError, ImageFormat, ImageResult}; +use crate::{ExtendedColorType, ImageEncoder, ImageError, ImageFormat, ImageResult}; use std::{error, fmt, io::Write}; /// Errors that can occur during encoding and saving of a TGA image. @@ -253,12 +253,9 @@ impl ImageEncoder for TgaEncoder { self.encode(buf, width, height, color_type) } - fn make_compatible_img( - &self, - _: crate::io::encoder::MethodSealedToImage, - img: &DynamicImage, - ) -> Option { - crate::io::encoder::dynimage_conversion_8bit(img) + fn supported_colors(&self) -> Option<&[ExtendedColorType]> { + use ExtendedColorType::*; + Some(&[Rgb8, Rgba8, L8, La8]) } } diff --git a/src/codecs/webp/encoder.rs b/src/codecs/webp/encoder.rs index 70423d8406..e503b06d2f 100644 --- a/src/codecs/webp/encoder.rs +++ b/src/codecs/webp/encoder.rs @@ -3,7 +3,7 @@ use std::io::Write; use crate::error::{EncodingError, UnsupportedError, UnsupportedErrorKind}; -use crate::{DynamicImage, ExtendedColorType, ImageEncoder, ImageError, ImageFormat, ImageResult}; +use crate::{ExtendedColorType, ImageEncoder, ImageError, ImageFormat, ImageResult}; /// WebP Encoder. /// @@ -110,12 +110,9 @@ impl ImageEncoder for WebPEncoder { Ok(()) } - fn make_compatible_img( - &self, - _: crate::io::encoder::MethodSealedToImage, - img: &DynamicImage, - ) -> Option { - crate::io::encoder::dynimage_conversion_8bit(img) + fn supported_colors(&self) -> Option<&[ExtendedColorType]> { + use ExtendedColorType::*; + Some(&[Rgb8, Rgba8, L8, La8]) } } diff --git a/src/images/dynimage.rs b/src/images/dynimage.rs index 469ac2040e..3a0d6a0129 100644 --- a/src/images/dynimage.rs +++ b/src/images/dynimage.rs @@ -1389,7 +1389,7 @@ impl DynamicImage { &self, encoder: Box, ) -> ImageResult<()> { - let converted = encoder.make_compatible_img(crate::io::encoder::MethodSealedToImage, self); + let converted = crate::io::encoder::make_compatible_img(self, encoder.supported_colors()); let img = converted.as_ref().unwrap_or(self); encoder.write_image( diff --git a/src/io/encoder.rs b/src/io/encoder.rs index eb9ae816a4..f2a63c7698 100644 --- a/src/io/encoder.rs +++ b/src/io/encoder.rs @@ -1,26 +1,6 @@ use crate::error::{ImageFormatHint, ImageResult, UnsupportedError, UnsupportedErrorKind}; use crate::{ColorType, DynamicImage, ExtendedColorType}; -/// Nominally public but DO NOT expose this type. -/// -/// To be somewhat sure here's a compile fail test: -/// -/// ```compile_fail -/// use image::MethodSealedToImage; -/// ``` -/// -/// ```compile_fail -/// use image::io::MethodSealedToImage; -/// ``` -/// -/// The same implementation strategy for a partially public trait is used in the standard library, -/// for the different effect of forbidding `Error::type_id` overrides thus making them reliable for -/// their calls through the `dyn` version of the trait. -/// -/// Read more: -#[derive(Clone, Copy)] -pub struct MethodSealedToImage; - /// The trait all encoders implement pub trait ImageEncoder { /// Writes all the bytes in an image to the encoder. @@ -100,17 +80,15 @@ pub trait ImageEncoder { )) } - /// Convert the image to a compatible format for the encoder. This is used by the encoding - /// methods on `DynamicImage`. + /// All color types supported by this encoder. If `None`, supported colors aren't known. + /// + /// Encoders typically only support a select few color types for writing, and supported ones + /// vary from encoder to encoder. This method allows encoders to specify which color types + /// their [`write_image`](Self::write_image) method supports. /// - /// Note that this is method is sealed to the crate and effectively pub(crate) due to the - /// argument type not being nameable. - #[doc(hidden)] - fn make_compatible_img( - &self, - _: MethodSealedToImage, - _input: &DynamicImage, - ) -> Option { + /// This information is currently used by the save and write method on [`DynamicImage`] to + /// perform necessary conversions before encoding. + fn supported_colors(&self) -> Option<&[ExtendedColorType]> { None } } @@ -136,17 +114,159 @@ impl ImageEncoderBoxed for T { } } -/// Implement `dynimage_conversion_sequence` for the common case of supporting only 8-bit colors -/// (with and without alpha). -#[allow(unused)] -pub(crate) fn dynimage_conversion_8bit(img: &DynamicImage) -> Option { - use ColorType::*; - - match img.color() { - Rgb8 | Rgba8 | L8 | La8 => None, - L16 => Some(img.to_luma8().into()), - La16 => Some(img.to_luma_alpha8().into()), - Rgb16 | Rgb32F => Some(img.to_rgb8().into()), - Rgba16 | Rgba32F => Some(img.to_rgba8().into()), +pub(crate) fn make_compatible_img( + img: &DynamicImage, + supported: Option<&[ExtendedColorType]>, +) -> Option { + let color = img.color(); + let to = to_supported_color(color, supported?)?; + if to == color { + // no conversion necessary + return None; + } + + Some(match to { + ColorType::L8 => img.to_luma8().into(), + ColorType::La8 => img.to_luma_alpha8().into(), + ColorType::Rgb8 => img.to_rgb8().into(), + ColorType::Rgba8 => img.to_rgba8().into(), + ColorType::L16 => img.to_luma16().into(), + ColorType::La16 => img.to_luma_alpha16().into(), + ColorType::Rgb16 => img.to_rgb16().into(), + ColorType::Rgba16 => img.to_rgba16().into(), + ColorType::Rgb32F => img.to_rgb32f().into(), + ColorType::Rgba32F => img.to_rgba32f().into(), + }) +} +fn to_supported_color(from: ColorType, supported: &[ExtendedColorType]) -> Option { + supported + .iter() + .filter_map(|c| c.color_type()) + .min_by_key(|&to| { + let mut loss = 0; + + // channel losses are heavily penalized, since a lot of information is lost + // channel gains are penalized, since they are inefficient and don't add any information + const ALPHA_LOST: u16 = 100; + const ALPHA_GAIN: u16 = 3; + const COLOR_LOST: u16 = 200; + const COLOR_GAIN: u16 = 6; + + match (from.has_alpha(), to.has_alpha()) { + (true, false) => loss += ALPHA_LOST, + (false, true) => loss += ALPHA_GAIN, + _ => {} + } + match (from.has_color(), to.has_color()) { + (true, false) => loss += COLOR_LOST, + (false, true) => loss += COLOR_GAIN, + _ => {} + } + + fn get_precision(c: ColorType) -> i16 { + match c { + ColorType::L8 | ColorType::La8 | ColorType::Rgb8 | ColorType::Rgba8 => 0, + ColorType::L16 | ColorType::La16 | ColorType::Rgb16 | ColorType::Rgba16 => 1, + ColorType::Rgb32F | ColorType::Rgba32F => 2, + } + } + + const PRECISION_LOST: u16 = 10; + const PRECISION_GAIN: u16 = 1; + match get_precision(to) - get_precision(from) { + m @ 1.. => loss += PRECISION_LOST * m as u16, + m @ ..=-1 => loss += PRECISION_GAIN * m.unsigned_abs(), + 0 => {} + } + + loss + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_conversion_png() { + let png_supported_colors = &[ + ExtendedColorType::Rgb8, + ExtendedColorType::Rgba8, + ExtendedColorType::L8, + ExtendedColorType::La8, + ExtendedColorType::Rgb16, + ExtendedColorType::Rgba16, + ExtendedColorType::L16, + ExtendedColorType::La16, + ]; + let to = |from| to_supported_color(from, png_supported_colors).unwrap_or(from); + + assert_eq!(to(ColorType::L8), ColorType::L8); + assert_eq!(to(ColorType::La8), ColorType::La8); + assert_eq!(to(ColorType::Rgb8), ColorType::Rgb8); + assert_eq!(to(ColorType::Rgba8), ColorType::Rgba8); + assert_eq!(to(ColorType::L16), ColorType::L16); + assert_eq!(to(ColorType::La16), ColorType::La16); + assert_eq!(to(ColorType::Rgb16), ColorType::Rgb16); + assert_eq!(to(ColorType::Rgba16), ColorType::Rgba16); + assert_eq!(to(ColorType::Rgb32F), ColorType::Rgb16); + assert_eq!(to(ColorType::Rgba32F), ColorType::Rgba16); + } + + #[test] + fn test_conversion_jpeg() { + let jpeg_supported_colors = &[ExtendedColorType::Rgb8, ExtendedColorType::L8]; + let to = |from| to_supported_color(from, jpeg_supported_colors).unwrap_or(from); + + assert_eq!(to(ColorType::L8), ColorType::L8); + assert_eq!(to(ColorType::La8), ColorType::L8); + assert_eq!(to(ColorType::Rgb8), ColorType::Rgb8); + assert_eq!(to(ColorType::Rgba8), ColorType::Rgb8); + assert_eq!(to(ColorType::L16), ColorType::L8); + assert_eq!(to(ColorType::La16), ColorType::L8); + assert_eq!(to(ColorType::Rgb16), ColorType::Rgb8); + assert_eq!(to(ColorType::Rgba16), ColorType::Rgb8); + assert_eq!(to(ColorType::Rgb32F), ColorType::Rgb8); + assert_eq!(to(ColorType::Rgba32F), ColorType::Rgb8); + } + + #[test] + fn test_conversion_bmp() { + let bmp_supported_colors = &[ + ExtendedColorType::Rgb8, + ExtendedColorType::Rgba8, + ExtendedColorType::L1, + ExtendedColorType::L8, + ExtendedColorType::La8, + ]; + let to = |from| to_supported_color(from, bmp_supported_colors).unwrap_or(from); + + assert_eq!(to(ColorType::L8), ColorType::L8); + assert_eq!(to(ColorType::La8), ColorType::La8); + assert_eq!(to(ColorType::Rgb8), ColorType::Rgb8); + assert_eq!(to(ColorType::Rgba8), ColorType::Rgba8); + assert_eq!(to(ColorType::L16), ColorType::L8); + assert_eq!(to(ColorType::La16), ColorType::La8); + assert_eq!(to(ColorType::Rgb16), ColorType::Rgb8); + assert_eq!(to(ColorType::Rgba16), ColorType::Rgba8); + assert_eq!(to(ColorType::Rgb32F), ColorType::Rgb8); + assert_eq!(to(ColorType::Rgba32F), ColorType::Rgba8); + } + + #[test] + fn test_conversion_hdr() { + let hdr_supported_colors = &[ExtendedColorType::Rgb32F]; + let to = |from| to_supported_color(from, hdr_supported_colors).unwrap_or(from); + + assert_eq!(to(ColorType::L8), ColorType::Rgb32F); + assert_eq!(to(ColorType::La8), ColorType::Rgb32F); + assert_eq!(to(ColorType::Rgb8), ColorType::Rgb32F); + assert_eq!(to(ColorType::Rgba8), ColorType::Rgb32F); + assert_eq!(to(ColorType::L16), ColorType::Rgb32F); + assert_eq!(to(ColorType::La16), ColorType::Rgb32F); + assert_eq!(to(ColorType::Rgb16), ColorType::Rgb32F); + assert_eq!(to(ColorType::Rgba16), ColorType::Rgb32F); + assert_eq!(to(ColorType::Rgb32F), ColorType::Rgb32F); + assert_eq!(to(ColorType::Rgba32F), ColorType::Rgb32F); } } diff --git a/tests/encoder_colors.rs b/tests/encoder_colors.rs new file mode 100644 index 0000000000..ff9eba1109 --- /dev/null +++ b/tests/encoder_colors.rs @@ -0,0 +1,155 @@ +use image::{ExtendedColorType, ImageEncoder}; + +const ALL_COLORS: &[ExtendedColorType] = &[ + ExtendedColorType::A8, + ExtendedColorType::L1, + ExtendedColorType::La1, + ExtendedColorType::Rgb1, + ExtendedColorType::Rgba1, + ExtendedColorType::L2, + ExtendedColorType::La2, + ExtendedColorType::Rgb2, + ExtendedColorType::Rgba2, + ExtendedColorType::L4, + ExtendedColorType::La4, + ExtendedColorType::Rgb4, + ExtendedColorType::Rgba4, + ExtendedColorType::Rgb5x1, + ExtendedColorType::L8, + ExtendedColorType::La8, + ExtendedColorType::Rgb8, + ExtendedColorType::Rgba8, + ExtendedColorType::L16, + ExtendedColorType::La16, + ExtendedColorType::Rgb16, + ExtendedColorType::Rgba16, + ExtendedColorType::Bgr8, + ExtendedColorType::Bgra8, + ExtendedColorType::L32F, + ExtendedColorType::La32F, + ExtendedColorType::Rgb32F, + ExtendedColorType::Rgba32F, + ExtendedColorType::Cmyk8, + ExtendedColorType::Cmyk16, + ExtendedColorType::YCbCr8, +]; + +/// Verify that encoders' reported supported colors match the colors they can actually encode. +fn verify_supported_colors(f: impl Fn() -> E) { + let reference = f(); + let Some(supported_colors) = reference.supported_colors() else { + // If the encoder doesn't report supported colors, we can't verify anything + return; + }; + + let width = 8; + let height = 8; + + for &color in ALL_COLORS { + let buf = vec![0_u8; color.buffer_size(width, height) as usize]; + let result = f().write_image(&buf, width, height, color); + let actually_supported = result.is_ok(); + let claim_supported = supported_colors.contains(&color); + + if actually_supported && !claim_supported { + panic!( + "Encoder claimed to not support color {:?} but was able to encode it", + color + ); + } else if !actually_supported && claim_supported { + panic!( + "Encoder claimed to support color {:?} but failed to encode it: {:?}", + color, + result.err() + ); + } + } +} + +fn writer() -> impl std::io::Write + std::io::Seek { + std::io::Cursor::new(Vec::new()) +} + +#[cfg(feature = "avif")] +#[test] +fn test_encoder_avif() { + verify_supported_colors(|| image::codecs::avif::AvifEncoder::new(writer())); +} + +#[cfg(feature = "bmp")] +#[test] +fn test_encoder_bmp() { + verify_supported_colors(|| image::codecs::bmp::BmpEncoder::new(writer())); +} + +#[cfg(feature = "exr")] +#[test] +fn test_encoder_exr() { + verify_supported_colors(|| image::codecs::openexr::OpenExrEncoder::new(writer())); +} + +#[cfg(feature = "ff")] +#[test] +fn test_encoder_farbfeld() { + verify_supported_colors(|| image::codecs::farbfeld::FarbfeldEncoder::new(writer())); +} + +#[cfg(feature = "gif")] +#[test] +fn test_encoder_gif() { + verify_supported_colors(|| image::codecs::gif::GifEncoder::new(writer())); +} + +#[cfg(feature = "hdr")] +#[test] +fn test_encoder_hdr() { + verify_supported_colors(|| image::codecs::hdr::HdrEncoder::new(writer())); +} + +#[cfg(feature = "ico")] +#[test] +fn test_encoder_ico() { + verify_supported_colors(|| image::codecs::ico::IcoEncoder::new(writer())); +} + +#[cfg(feature = "jpeg")] +#[test] +fn test_encoder_jpeg() { + verify_supported_colors(|| image::codecs::jpeg::JpegEncoder::new(writer())); +} + +#[cfg(feature = "png")] +#[test] +fn test_encoder_png() { + verify_supported_colors(|| image::codecs::png::PngEncoder::new(writer())); +} + +#[cfg(feature = "pnm")] +#[test] +fn test_encoder_pnm() { + verify_supported_colors(|| image::codecs::pnm::PnmEncoder::new(writer())); +} + +#[cfg(feature = "qoi")] +#[test] +fn test_encoder_qoi() { + verify_supported_colors(|| image::codecs::qoi::QoiEncoder::new(writer())); +} + +#[cfg(feature = "tga")] +#[test] +fn test_encoder_tga() { + verify_supported_colors(|| image::codecs::tga::TgaEncoder::new(writer())); +} + +#[cfg(feature = "tiff")] +#[test] +fn test_encoder_tiff() { + verify_supported_colors(|| image::codecs::tiff::TiffEncoder::new(writer())); +} + +#[cfg(feature = "webp")] +#[test] +fn test_encoder_webp() { + verify_supported_colors(|| image::codecs::webp::WebPEncoder::new_lossless(writer())); +}