From 0425344ae33b865618c25d3e9c79f55083dee9b7 Mon Sep 17 00:00:00 2001 From: RunDevelopment Date: Mon, 15 Jun 2026 14:46:07 +0200 Subject: [PATCH 1/5] Add ICO BMP encoder --- src/codecs/ico/encoder.rs | 168 +++++++++++++++++++++++++++++++++++++- 1 file changed, 167 insertions(+), 1 deletion(-) diff --git a/src/codecs/ico/encoder.rs b/src/codecs/ico/encoder.rs index 44e47532d4..4d2a8fa0f6 100644 --- a/src/codecs/ico/encoder.rs +++ b/src/codecs/ico/encoder.rs @@ -3,7 +3,9 @@ use std::borrow::Cow; use std::io::{self, Write}; use crate::codecs::png::PngEncoder; -use crate::error::{EncodingError, ImageError, ImageResult, ParameterError, ParameterErrorKind}; +use crate::error::{ + EncodingError, ImageError, ImageResult, ParameterError, ParameterErrorKind, UnsupportedError, +}; use crate::{ExtendedColorType, ImageEncoder, ImageFormat}; // Enum value indicating an ICO image (as opposed to a CUR image): @@ -12,6 +14,8 @@ const ICO_IMAGE_TYPE: u16 = 1; const ICO_ICONDIR_SIZE: u32 = 6; // The length of an ICO file DIRENTRY structure, in bytes: const ICO_DIRENTRY_SIZE: u32 = 16; +// The length of a BITMAPINFOHEADER structure, in bytes: +const BITMAPINFOHEADER_SIZE: u32 = 40; /// ICO encoder pub struct IcoEncoder { @@ -80,6 +84,121 @@ impl<'a> IcoFrame<'a> { let frame = Self::with_encoded(image_data, width, height, color_type)?; Ok(frame) } + + /// Construct a new `IcoFrame` by encoding `buf` as a BMP + /// + /// The `width` and `height` must be between 1 and 256 (inclusive) + pub fn as_bmp( + buf: &[u8], + width: u32, + height: u32, + color_type: ExtendedColorType, + ) -> ImageResult { + if width == 0 || height == 0 { + return Err(ImageError::Parameter(ParameterError::from_kind( + ParameterErrorKind::Generic(format!( + "the image width and height must be non-zero, but width {width} and height {height} were provided", + )), + ))); + } + // same soft limit as BMP decoder + if width > u16::MAX as u32 || height > u16::MAX as u32 { + return Err(ImageError::Parameter(ParameterError::from_kind( + ParameterErrorKind::Generic("Dimensions too big.".into()), + ))); + } + + let mut image_data: Vec = Vec::new(); + + // https://learn.microsoft.com/en-us/previous-versions/ms997538(v=msdn.10)?redirectedfrom=MSDN + // > Only the following members are used: biSize, biWidth, biHeight, biPlanes, biBitCount, + // > biSizeImage. All other members must be 0. + // https://learn.microsoft.com/en-us/windows/win32/api/wingdi/ns-wingdi-bitmapinfoheader + image_data.write_u32::(BITMAPINFOHEADER_SIZE)?; // biSize + image_data.write_i32::(width as i32)?; // biWidth + image_data.write_i32::(height as i32 * 2)?; // biHeight + image_data.write_u16::(1)?; // biPlanes must be 1 + image_data.write_u16::(32)?; // biBitCount (we only support 32-bit for now) + image_data.write_u32::(0)?; // biCompression must be 0 (BI_RGB) for uncompressed + image_data.write_u32::(0)?; // biSizeImage + image_data.extend_from_slice(&[0; 16]); // remaining unused fields + debug_assert_eq!(image_data.len(), BITMAPINFOHEADER_SIZE as usize); + + match color_type { + ExtendedColorType::Rgba8 => { + let pixels = buf.as_chunks::<4>().0; + // BMP format requires the alpha channel to be stored as BGRA, so we need to convert it from RGBA. + for row in pixels.chunks_exact(width as usize).rev() { + for [r, g, b, a] in row { + image_data.extend_from_slice(&[*b, *g, *r, *a]); + } + } + + // AND mask + // For 32-bit BMP the AND mask is typically ignored, but it should be provided nonetheless. + // We use a threshold of 128 for the alpha channel to determine whether a pixel is transparent or opaque in the AND mask. + for row in pixels.chunks_exact(width as usize).rev() { + let mut mask_byte: u8 = 0; + for (x, a) in row.iter().map(|pixel| pixel[3]).enumerate() { + let pos = x % 8; + let bit_pos = 7 - pos; + mask_byte |= if a < 128 { 1 } else { 0 } << bit_pos; + + if pos == 7 { + image_data.push(mask_byte); + mask_byte = 0; + } + } + if !width.is_multiple_of(8) { + image_data.push(mask_byte); + } + + // Each row of the AND mask must be padded to a multiple of 4 bytes. + let mask_row_len = width.div_ceil(8); + let mask_row_padded = mask_row_len.div_ceil(4) * 4; + let padding = &[0_u8; 4][..(mask_row_padded - mask_row_len) as usize]; + image_data.extend_from_slice(padding); + } + } + ExtendedColorType::Rgb8 => { + let pixels = buf.as_chunks::<3>().0; + // ICO doesn't allow 24-bit RGB, so we have to use the 0RGB 32-bit format. + // This is the same as the 32-bit RGBA format but with the alpha channel set to 0 for all pixels. + for row in pixels.chunks_exact(width as usize).rev() { + for [r, g, b] in row { + image_data.extend_from_slice(&[*b, *g, *r, 0]); + } + } + + // AND mask + // Since there is no transparency, we can set all pixels to 0 (opaque) in the AND mask. + for _ in 0..height { + let chunks_per_row = width.div_ceil(32); + for _ in 0..chunks_per_row { + image_data.extend_from_slice(&[0_u8; 4]); + } + } + } + _ => { + return Err(ImageError::Unsupported( + UnsupportedError::from_format_and_kind( + ImageFormat::Ico.into(), + crate::error::UnsupportedErrorKind::Color(color_type), + ), + )); + } + } + + // Set biSizeImage properly + // This isn't strictly necessary since 0 is allowed for uncompressed images (which ICO requires), + // but we set it nonetheless of compatibility. + let size = image_data.len() as u32 - BITMAPINFOHEADER_SIZE; + image_data[20..24].copy_from_slice(&size.to_le_bytes()); + + // color type has to be Rgba8, because the `bit_per_pixel` field in the ICO directory entry has to be set to 32. + let frame = Self::with_encoded(image_data, width, height, ExtendedColorType::Rgba8)?; + Ok(frame) + } } impl IcoEncoder { @@ -210,6 +329,8 @@ fn write_direntry( #[cfg(test)] mod test { + use crate::{DynamicImage, ImageBuffer, PixelWithColorType, Rgb, Rgba, RgbaImage}; + use super::*; // Test that the encoder allows image where all frames have offsets < 4GiB @@ -235,4 +356,49 @@ mod test { let res = encoder.encode_images(&frames); assert!(res.is_err()); } + + #[test] + fn roundtrip() { + fn encode_bmp_decode>( + image: &ImageBuffer>, + ) -> DynamicImage { + let frame = IcoFrame::as_bmp( + image.subpixels(), + image.width(), + image.height(), + P::COLOR_TYPE, + ) + .unwrap(); + + let mut encoded_data: Vec = Vec::new(); + IcoEncoder::new(&mut encoded_data) + .encode_images(&[frame]) + .unwrap(); + + let mut reader = crate::ImageReaderOptions::with_format( + io::Cursor::new(encoded_data), + ImageFormat::Ico, + ); + reader.set_spec_compliance(crate::SpecCompliance::Strict); + reader.decode().unwrap() + } + + let rgba = RgbaImage::from_fn(32, 32, |x, y| { + Rgba([ + x as u8 * 8, + y as u8 * 8, + 0, + if y < 24 { + 255 + } else { + (x + y * 32 - 256 * 3) as u8 + }, + ]) + }); + let rgb = rgba.convert(); + let round_rgba = encode_bmp_decode(&rgba); + let round_rgb = encode_bmp_decode::>(&rgb); + assert_eq!(round_rgba.into_rgba8(), rgba); + assert_eq!(round_rgb.into_rgba8(), rgb.convert()); + } } From adbeff111de6bb9d92645daebdcc226171453896 Mon Sep 17 00:00:00 2001 From: RunDevelopment Date: Mon, 15 Jun 2026 15:12:05 +0200 Subject: [PATCH 2/5] Address some review comments --- src/codecs/ico/encoder.rs | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/src/codecs/ico/encoder.rs b/src/codecs/ico/encoder.rs index 4d2a8fa0f6..a38c3fdbe1 100644 --- a/src/codecs/ico/encoder.rs +++ b/src/codecs/ico/encoder.rs @@ -108,6 +108,18 @@ impl<'a> IcoFrame<'a> { ))); } + // Check to see that the output buffer is not going to be larger than the 4 GiB that ICO can support. + let output_size = BITMAPINFOHEADER_SIZE as u64 // header + + (width as u64 * height as u64 * 4) // XOR mask (32 bits per pixel) + + (height as u64 * width.div_ceil(32) as u64 * 4); // AND mask (1 bit per pixel, padded to 32 bits per row) + if output_size > u32::MAX as u64 { + return Err(ImageError::Parameter(ParameterError::from_kind( + ParameterErrorKind::Generic(format!( + "the encoded image data must be at most 4 GiB, but the provided image would encode to {output_size} bytes", + )), + ))); + } + let mut image_data: Vec = Vec::new(); // https://learn.microsoft.com/en-us/previous-versions/ms997538(v=msdn.10)?redirectedfrom=MSDN @@ -137,12 +149,17 @@ impl<'a> IcoFrame<'a> { // AND mask // For 32-bit BMP the AND mask is typically ignored, but it should be provided nonetheless. // We use a threshold of 128 for the alpha channel to determine whether a pixel is transparent or opaque in the AND mask. + + // Each row of the AND mask must be padded to a multiple of 4 bytes. + let mask_row_len = width.div_ceil(8); // number of mask bytes written per row + let mask_row_padding = mask_row_len.div_ceil(4) * 4 - mask_row_len; + for row in pixels.chunks_exact(width as usize).rev() { let mut mask_byte: u8 = 0; for (x, a) in row.iter().map(|pixel| pixel[3]).enumerate() { let pos = x % 8; let bit_pos = 7 - pos; - mask_byte |= if a < 128 { 1 } else { 0 } << bit_pos; + mask_byte |= u8::from(a < 128) << bit_pos; if pos == 7 { image_data.push(mask_byte); @@ -153,11 +170,7 @@ impl<'a> IcoFrame<'a> { image_data.push(mask_byte); } - // Each row of the AND mask must be padded to a multiple of 4 bytes. - let mask_row_len = width.div_ceil(8); - let mask_row_padded = mask_row_len.div_ceil(4) * 4; - let padding = &[0_u8; 4][..(mask_row_padded - mask_row_len) as usize]; - image_data.extend_from_slice(padding); + image_data.extend(std::iter::repeat_n(0u8, mask_row_padding as usize)); } } ExtendedColorType::Rgb8 => { @@ -174,9 +187,7 @@ impl<'a> IcoFrame<'a> { // Since there is no transparency, we can set all pixels to 0 (opaque) in the AND mask. for _ in 0..height { let chunks_per_row = width.div_ceil(32); - for _ in 0..chunks_per_row { - image_data.extend_from_slice(&[0_u8; 4]); - } + image_data.extend(std::iter::repeat_n(0u8, 4 * chunks_per_row as usize)); } } _ => { @@ -195,6 +206,9 @@ impl<'a> IcoFrame<'a> { let size = image_data.len() as u32 - BITMAPINFOHEADER_SIZE; image_data[20..24].copy_from_slice(&size.to_le_bytes()); + // verify that the output size is correctly calculated. + debug_assert_eq!(image_data.len() as u64, output_size); + // color type has to be Rgba8, because the `bit_per_pixel` field in the ICO directory entry has to be set to 32. let frame = Self::with_encoded(image_data, width, height, ExtendedColorType::Rgba8)?; Ok(frame) From 79304843b71e91d3c58d8f3792e59f03e0ed96ba Mon Sep 17 00:00:00 2001 From: RunDevelopment Date: Mon, 15 Jun 2026 15:24:56 +0200 Subject: [PATCH 3/5] Simplify AND mask logic --- src/codecs/ico/encoder.rs | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/codecs/ico/encoder.rs b/src/codecs/ico/encoder.rs index dbd9535ba4..93cde60e00 100644 --- a/src/codecs/ico/encoder.rs +++ b/src/codecs/ico/encoder.rs @@ -155,18 +155,13 @@ impl<'a> IcoFrame<'a> { let mask_row_padding = mask_row_len.div_ceil(4) * 4 - mask_row_len; for row in pixels.chunks_exact(width as usize).rev() { - let mut mask_byte: u8 = 0; - for (x, a) in row.iter().map(|pixel| pixel[3]).enumerate() { - let pos = x % 8; - let bit_pos = 7 - pos; - mask_byte |= u8::from(a < 128) << bit_pos; - - if pos == 7 { - image_data.push(mask_byte); - mask_byte = 0; + // AND mask is 1 bpp, so 8 pixels go into 1 mask byte. + for chunks in row.chunks(8) { + let mut mask_byte: u8 = 0; + for (x, a) in chunks.iter().map(|pixel| pixel[3]).enumerate() { + let bit_pos = 7 - (x % 8); + mask_byte |= u8::from(a < 128) << bit_pos; } - } - if !width.is_multiple_of(8) { image_data.push(mask_byte); } From 7a07726f98f963108685db5617aed24261be0b6a Mon Sep 17 00:00:00 2001 From: RunDevelopment Date: Mon, 15 Jun 2026 18:32:41 +0200 Subject: [PATCH 4/5] Outline mask padding logic and add another test --- src/codecs/ico/encoder.rs | 76 +++++++++++++++++++++------------------ 1 file changed, 42 insertions(+), 34 deletions(-) diff --git a/src/codecs/ico/encoder.rs b/src/codecs/ico/encoder.rs index 93cde60e00..ffb6688842 100644 --- a/src/codecs/ico/encoder.rs +++ b/src/codecs/ico/encoder.rs @@ -108,10 +108,13 @@ impl<'a> IcoFrame<'a> { ))); } + let mask_row_bytes = width.div_ceil(8); + let mask_row_bytes_padded = mask_row_bytes.next_multiple_of(4); + // Check to see that the output buffer is not going to be larger than the 4 GiB that ICO can support. let output_size = BITMAPINFOHEADER_SIZE as u64 // header + (width as u64 * height as u64 * 4) // XOR mask (32 bits per pixel) - + (height as u64 * width.div_ceil(32) as u64 * 4); // AND mask (1 bit per pixel, padded to 32 bits per row) + + (height as u64 * mask_row_bytes_padded as u64); // AND mask (1 bit per pixel, padded to 32 bits per row) if output_size > u32::MAX as u64 { return Err(ImageError::Parameter(ParameterError::from_kind( ParameterErrorKind::Generic(format!( @@ -149,11 +152,6 @@ impl<'a> IcoFrame<'a> { // AND mask // For 32-bit BMP the AND mask is typically ignored, but it should be provided nonetheless. // We use a threshold of 128 for the alpha channel to determine whether a pixel is transparent or opaque in the AND mask. - - // Each row of the AND mask must be padded to a multiple of 4 bytes. - let mask_row_len = width.div_ceil(8); // number of mask bytes written per row - let mask_row_padding = mask_row_len.div_ceil(4) * 4 - mask_row_len; - for row in pixels.chunks_exact(width as usize).rev() { // AND mask is 1 bpp, so 8 pixels go into 1 mask byte. for chunks in row.chunks(8) { @@ -165,7 +163,11 @@ impl<'a> IcoFrame<'a> { image_data.push(mask_byte); } - image_data.extend(std::iter::repeat_n(0u8, mask_row_padding as usize)); + // Each row of the AND mask must be padded to a multiple of 4 bytes. + image_data.extend(std::iter::repeat_n( + 0u8, + (mask_row_bytes_padded - mask_row_bytes) as usize, + )); } } ExtendedColorType::Rgb8 => { @@ -180,10 +182,10 @@ impl<'a> IcoFrame<'a> { // AND mask // Since there is no transparency, we can set all pixels to 0 (opaque) in the AND mask. - for _ in 0..height { - let chunks_per_row = width.div_ceil(32); - image_data.extend(std::iter::repeat_n(0u8, 4 * chunks_per_row as usize)); - } + image_data.extend(std::iter::repeat_n( + 0u8, + (height * mask_row_bytes_padded) as usize, + )); } _ => { return Err(ImageError::Unsupported( @@ -366,32 +368,30 @@ mod test { assert!(res.is_err()); } - #[test] - fn roundtrip() { - fn encode_bmp_decode>( - image: &ImageBuffer>, - ) -> DynamicImage { - let frame = IcoFrame::as_bmp( - image.subpixels(), - image.width(), - image.height(), - P::COLOR_TYPE, - ) + fn encode_bmp_decode>( + image: &ImageBuffer>, + ) -> DynamicImage { + let frame = IcoFrame::as_bmp( + image.subpixels(), + image.width(), + image.height(), + P::COLOR_TYPE, + ) + .unwrap(); + + let mut encoded_data: Vec = Vec::new(); + IcoEncoder::new(&mut encoded_data) + .encode_images(&[frame]) .unwrap(); - let mut encoded_data: Vec = Vec::new(); - IcoEncoder::new(&mut encoded_data) - .encode_images(&[frame]) - .unwrap(); - - let mut reader = crate::ImageReaderOptions::with_format( - io::Cursor::new(encoded_data), - ImageFormat::Ico, - ); - reader.set_spec_compliance(crate::SpecCompliance::Strict); - reader.decode().unwrap() - } + let mut reader = + crate::ImageReaderOptions::with_format(io::Cursor::new(encoded_data), ImageFormat::Ico); + reader.set_spec_compliance(crate::SpecCompliance::Strict); + reader.decode().unwrap() + } + #[test] + fn roundtrip() { let rgba = RgbaImage::from_fn(32, 32, |x, y| { Rgba([ x as u8 * 8, @@ -410,4 +410,12 @@ mod test { assert_eq!(round_rgba.into_rgba8(), rgba); assert_eq!(round_rgb.into_rgba8(), rgb.convert()); } + + #[test] + fn roundtrip_awkward_size() { + // test a size that will need lots of padding in the AND mask to make sure the logic works correctly. + let image = RgbaImage::from_pixel(33, 5, Rgba([255, 0, 0, 255])); + let round_rgba = encode_bmp_decode(&image); + assert_eq!(round_rgba.into_rgba8(), image); + } } From 87a42e66a7c74d1a151377a5d0aee6d458d12e38 Mon Sep 17 00:00:00 2001 From: RunDevelopment Date: Tue, 16 Jun 2026 09:54:01 +0200 Subject: [PATCH 5/5] Minor improvements --- src/codecs/ico/encoder.rs | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/src/codecs/ico/encoder.rs b/src/codecs/ico/encoder.rs index ffb6688842..75d11e4fc7 100644 --- a/src/codecs/ico/encoder.rs +++ b/src/codecs/ico/encoder.rs @@ -6,6 +6,7 @@ use crate::codecs::png::PngEncoder; use crate::error::{ EncodingError, ImageError, ImageResult, ParameterError, ParameterErrorKind, UnsupportedError, }; +use crate::utils::vec_try_with_capacity; use crate::{ExtendedColorType, ImageEncoder, ImageFormat}; // Enum value indicating an ICO image (as opposed to a CUR image): @@ -123,7 +124,7 @@ impl<'a> IcoFrame<'a> { ))); } - let mut image_data: Vec = Vec::new(); + let mut image_data: Vec = vec_try_with_capacity(output_size as usize)?; // https://learn.microsoft.com/en-us/previous-versions/ms997538(v=msdn.10)?redirectedfrom=MSDN // > Only the following members are used: biSize, biWidth, biHeight, biPlanes, biBitCount, @@ -135,7 +136,7 @@ impl<'a> IcoFrame<'a> { image_data.write_u16::(1)?; // biPlanes must be 1 image_data.write_u16::(32)?; // biBitCount (we only support 32-bit for now) image_data.write_u32::(0)?; // biCompression must be 0 (BI_RGB) for uncompressed - image_data.write_u32::(0)?; // biSizeImage + image_data.write_u32::(output_size as u32 - BITMAPINFOHEADER_SIZE)?; // biSizeImage image_data.extend_from_slice(&[0; 16]); // remaining unused fields debug_assert_eq!(image_data.len(), BITMAPINFOHEADER_SIZE as usize); @@ -156,9 +157,8 @@ impl<'a> IcoFrame<'a> { // AND mask is 1 bpp, so 8 pixels go into 1 mask byte. for chunks in row.chunks(8) { let mut mask_byte: u8 = 0; - for (x, a) in chunks.iter().map(|pixel| pixel[3]).enumerate() { - let bit_pos = 7 - (x % 8); - mask_byte |= u8::from(a < 128) << bit_pos; + for ([_, _, _, a], bit_pos) in chunks.iter().zip((0..8).rev()) { + mask_byte |= u8::from(*a < 128) << bit_pos; } image_data.push(mask_byte); } @@ -197,14 +197,8 @@ impl<'a> IcoFrame<'a> { } } - // Set biSizeImage properly - // This isn't strictly necessary since 0 is allowed for uncompressed images (which ICO requires), - // but we set it nonetheless of compatibility. - let size = image_data.len() as u32 - BITMAPINFOHEADER_SIZE; - image_data[20..24].copy_from_slice(&size.to_le_bytes()); - - // verify that the output size is correctly calculated. - debug_assert_eq!(image_data.len() as u64, output_size); + // verify that the output size was calculated correctly. + assert_eq!(image_data.len() as u64, output_size); // color type has to be Rgba8, because the `bit_per_pixel` field in the ICO directory entry has to be set to 32. let frame = Self::with_encoded(image_data, width, height, ExtendedColorType::Rgba8)?;