From da3c4c8cd1b39e1a5f77b1fd8d30828e9675cf62 Mon Sep 17 00:00:00 2001 From: Leon Teichroeb Date: Sat, 12 Oct 2024 13:52:41 +0200 Subject: [PATCH 1/7] Add failing test for #252 --- tests/tests.rs | 58 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 57 insertions(+), 1 deletion(-) diff --git a/tests/tests.rs b/tests/tests.rs index 6f4d83b..25fe61c 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -778,7 +778,7 @@ pub fn rasterize_glyph_with_full_hinting() { RasterizationOptions::Bilevel, ) .unwrap(); - let origin = -raster_rect.origin().to_f32(); + let origin: Vector2F = -raster_rect.origin().to_f32(); let mut canvas = Canvas::new(raster_rect.size(), Format::A8); font.rasterize_glyph( &mut canvas, @@ -808,6 +808,61 @@ pub fn rasterize_glyph_with_full_hinting() { } } +// https://github.com/servo/font-kit/issues/252 +// Panic when targeting Canvas larger than glyph with SubpixelAa option in Freetype. +#[cfg(all( + feature = "source", + any( + not(any(target_os = "macos", target_os = "ios", target_family = "windows")), + feature = "loader-freetype-default" + ) +))] +#[test] +pub fn rasterize_glyph_with_full_hinting_subpixel() { + let font = SystemSource::new() + .select_best_match(&[FamilyName::SansSerif], &Properties::new()) + .unwrap() + .load() + .unwrap(); + let glyph_id = font.glyph_for_char('L').unwrap(); + let size = 32.0; + let raster_rect = font + .raster_bounds( + glyph_id, + size, + Transform2F::default(), + HintingOptions::Full(size), + RasterizationOptions::SubpixelAa, + ) + .unwrap(); + let origin: Vector2F = -raster_rect.origin().to_f32(); + let mut canvas = Canvas::new(raster_rect.size(), Format::Rgb24); + font.rasterize_glyph( + &mut canvas, + glyph_id, + size, + Transform2F::from_translation(origin), + HintingOptions::Full(size), + RasterizationOptions::SubpixelAa, + ) + .unwrap(); + check_L_shape(&canvas); + + // Test with larger canvas + let mut canvas = Canvas::new(Vector2I::new(100, 100), Format::Rgb24); + font.rasterize_glyph( + &mut canvas, + glyph_id, + size, + Transform2F::from_translation(origin), + HintingOptions::Full(size), + RasterizationOptions::SubpixelAa, + ) + .unwrap(); + check_L_shape(&canvas); +} + + #[cfg(all(feature = "source", target_family = "windows"))] #[test] pub fn rasterize_glyph() { @@ -888,6 +943,7 @@ pub fn rasterize_empty_glyph_on_empty_canvas() { .unwrap(); } + #[cfg(feature = "source")] #[test] pub fn font_transform() { From a8231d7f2e9e925bcd29f3fb54988635af0fb2b7 Mon Sep 17 00:00:00 2001 From: Leon Teichroeb Date: Sat, 12 Oct 2024 14:26:26 +0200 Subject: [PATCH 2/7] Change bytes_per_pixel to usize. --- src/canvas.rs | 16 ++++++++-------- src/loaders/core_text.rs | 2 +- src/loaders/directwrite.rs | 3 +-- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/canvas.rs b/src/canvas.rs index 4c08abc..da39ccf 100644 --- a/src/canvas.rs +++ b/src/canvas.rs @@ -56,7 +56,7 @@ impl Canvas { pub fn new(size: Vector2I, format: Format) -> Canvas { Canvas::with_stride( size, - size.x() as usize * format.bytes_per_pixel() as usize, + size.x() as usize * format.bytes_per_pixel(), format, ) } @@ -142,7 +142,7 @@ impl Canvas { let size = dst_rect.size(); - let dest_bytes_per_pixel = self.format.bytes_per_pixel() as usize; + let dest_bytes_per_pixel = self.format.bytes_per_pixel(); let dest_row_stride = size.x() as usize * dest_bytes_per_pixel; let src_row_stride = utils::div_round_up(size.x() as usize, 8); @@ -173,8 +173,8 @@ impl Canvas { src_stride: usize, src_format: Format, ) { - let src_bytes_per_pixel = src_format.bytes_per_pixel() as usize; - let dest_bytes_per_pixel = self.format.bytes_per_pixel() as usize; + let src_bytes_per_pixel = src_format.bytes_per_pixel(); + let dest_bytes_per_pixel = self.format.bytes_per_pixel(); for y in 0..rect.height() { let (dest_row_start, src_row_start) = ( @@ -216,7 +216,7 @@ pub enum Format { impl Format { /// Returns the number of bits per pixel that this image format corresponds to. #[inline] - pub fn bits_per_pixel(self) -> u8 { + pub fn bits_per_pixel(self) -> usize { match self { Format::Rgba32 => 32, Format::Rgb24 => 24, @@ -226,7 +226,7 @@ impl Format { /// Returns the number of color channels per pixel that this image format corresponds to. #[inline] - pub fn components_per_pixel(self) -> u8 { + pub fn components_per_pixel(self) -> usize{ match self { Format::Rgba32 => 4, Format::Rgb24 => 3, @@ -236,13 +236,13 @@ impl Format { /// Returns the number of bits per color channel that this image format contains. #[inline] - pub fn bits_per_component(self) -> u8 { + pub fn bits_per_component(self) -> usize { self.bits_per_pixel() / self.components_per_pixel() } /// Returns the number of bytes per pixel that this image format corresponds to. #[inline] - pub fn bytes_per_pixel(self) -> u8 { + pub fn bytes_per_pixel(self) -> usize { self.bits_per_pixel() / 8 } } diff --git a/src/loaders/core_text.rs b/src/loaders/core_text.rs index 8685321..1b6b5eb 100644 --- a/src/loaders/core_text.rs +++ b/src/loaders/core_text.rs @@ -511,7 +511,7 @@ impl Font { Some(canvas.pixels.as_mut_ptr() as *mut _), canvas.size.x() as usize, canvas.size.y() as usize, - canvas.format.bits_per_component() as usize, + canvas.format.bits_per_component(), canvas.stride, &cg_color_space, cg_image_format, diff --git a/src/loaders/directwrite.rs b/src/loaders/directwrite.rs index b39b7d9..ca6e6d1 100644 --- a/src/loaders/directwrite.rs +++ b/src/loaders/directwrite.rs @@ -532,8 +532,7 @@ impl Font { } else { Format::Rgb24 }; - let texture_bits_per_pixel = texture_format.bits_per_pixel(); - let texture_bytes_per_pixel = texture_bits_per_pixel as usize / 8; + let texture_bytes_per_pixel = texture_format.bytes_per_pixel(); let texture_size = Vector2I::new(texture_width, texture_height); let texture_stride = texture_width as usize * texture_bytes_per_pixel; From 1a765eb6639aaf21ce255378039d1de841489f97 Mon Sep 17 00:00:00 2001 From: Leon Teichroeb Date: Sat, 12 Oct 2024 14:27:31 +0200 Subject: [PATCH 3/7] Add checks to avoid confusing runtime exceptions. --- src/canvas.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/canvas.rs b/src/canvas.rs index da39ccf..6fe32a2 100644 --- a/src/canvas.rs +++ b/src/canvas.rs @@ -85,6 +85,9 @@ impl Canvas { ) } + /// Blits to a rectangle with origin at `dst_point` and size according to `src_size`. + /// If the target area overlaps the boundaries of the canvas, only the drawable region is blitted. + /// `dst_point` and `src_size` are specified in pixels. `src_stride` is specified in bytes. #[allow(dead_code)] pub(crate) fn blit_from( &mut self, @@ -94,6 +97,18 @@ impl Canvas { src_stride: usize, src_format: Format, ) { + assert_eq!( + src_size.x() as usize * src_size.y() as usize, + src_bytes.len() / src_format.bytes_per_pixel(), + "Number of pixels in src_bytes does not match src_size." + ); + // It might be preferable to drop src_stride as a parameter to get rid of this check. + assert_eq!( + src_size.x() as usize * src_format.bytes_per_pixel(), + src_stride, + "src_stride does not match src_size.x()" + ); + let dst_rect = RectI::new(dst_point, src_size); let dst_rect = dst_rect.intersection(RectI::new(Vector2I::default(), self.size)); let dst_rect = match dst_rect { @@ -166,6 +181,9 @@ impl Canvas { } } + /// Blits to area `rect` using the data given in the buffer `src_bytes`. + /// `src_stride` must be specified in bytes. + /// The dimensions of `rect` must be in pixels. fn blit_from_with( &mut self, rect: RectI, From 980bdbaccdb797f487913d2cf92b7ebc89af15af Mon Sep 17 00:00:00 2001 From: Leon Teichroeb Date: Sat, 12 Oct 2024 14:42:50 +0200 Subject: [PATCH 4/7] Fix bitmap_size in FreeType rasterize_glyph for MODE_LCD* --- src/loaders/freetype.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/loaders/freetype.rs b/src/loaders/freetype.rs index a07bbc8..a2ca581 100644 --- a/src/loaders/freetype.rs +++ b/src/loaders/freetype.rs @@ -838,9 +838,9 @@ impl Font { // that mode. let bitmap = &(*(*self.freetype_face).glyph).bitmap; let bitmap_stride = bitmap.pitch as usize; + // bitmap_width is given in bytes. let bitmap_width = bitmap.width; let bitmap_height = bitmap.rows; - let bitmap_size = Vector2I::new(bitmap_width, bitmap_height); let bitmap_buffer = bitmap.buffer as *const i8 as *const u8; let bitmap_length = bitmap_stride * bitmap_height as usize; if bitmap_buffer.is_null() { @@ -858,9 +858,12 @@ impl Font { // FIXME(pcwalton): This function should return a Result instead. match bitmap.pixel_mode as u32 { FT_PIXEL_MODE_GRAY => { + let bitmap_size = Vector2I::new(bitmap_width, bitmap_height); canvas.blit_from(dst_point, buffer, bitmap_size, bitmap_stride, Format::A8); } FT_PIXEL_MODE_LCD | FT_PIXEL_MODE_LCD_V => { + // Three bytes per pixel for Rgb24 format + let bitmap_size = Vector2I::new(bitmap_width / 3, bitmap_height); canvas.blit_from( dst_point, buffer, @@ -870,6 +873,7 @@ impl Font { ); } FT_PIXEL_MODE_MONO => { + let bitmap_size = Vector2I::new(bitmap_width, bitmap_height); canvas.blit_from_bitmap_1bpp(dst_point, buffer, bitmap_size, bitmap_stride); } _ => panic!("Unexpected FreeType pixel mode!"), From 9c4c53778aeed3e174d19b0d7eee131401ca2b8c Mon Sep 17 00:00:00 2001 From: Leon Teichroeb Date: Sat, 12 Oct 2024 14:50:04 +0200 Subject: [PATCH 5/7] Fix buffer checks in blit_from --- src/canvas.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/canvas.rs b/src/canvas.rs index 6fe32a2..72cf45b 100644 --- a/src/canvas.rs +++ b/src/canvas.rs @@ -88,6 +88,7 @@ impl Canvas { /// Blits to a rectangle with origin at `dst_point` and size according to `src_size`. /// If the target area overlaps the boundaries of the canvas, only the drawable region is blitted. /// `dst_point` and `src_size` are specified in pixels. `src_stride` is specified in bytes. + /// `src_stride` must be equal or larger than the actual data length. #[allow(dead_code)] pub(crate) fn blit_from( &mut self, @@ -98,16 +99,15 @@ impl Canvas { src_format: Format, ) { assert_eq!( - src_size.x() as usize * src_size.y() as usize, - src_bytes.len() / src_format.bytes_per_pixel(), - "Number of pixels in src_bytes does not match src_size." + src_stride * src_size.y() as usize, + src_bytes.len(), + "Number of pixels in src_bytes does not match stride and size." ); - // It might be preferable to drop src_stride as a parameter to get rid of this check. - assert_eq!( - src_size.x() as usize * src_format.bytes_per_pixel(), - src_stride, - "src_stride does not match src_size.x()" + assert!( + src_stride >= src_size.x() as usize * src_format.bytes_per_pixel(), + "src_stride must be >= than src_size.x()" ); + let dst_rect = RectI::new(dst_point, src_size); let dst_rect = dst_rect.intersection(RectI::new(Vector2I::default(), self.size)); From a6373dca28b4c0fbe1fa9b7e3361a5ff39049798 Mon Sep 17 00:00:00 2001 From: Leon Teichroeb Date: Sat, 12 Oct 2024 17:16:33 +0200 Subject: [PATCH 6/7] Revert "Change bytes_per_pixel to usize." This reverts commit a8231d7f2e9e925bcd29f3fb54988635af0fb2b7. --- src/canvas.rs | 16 ++++++++-------- src/loaders/core_text.rs | 2 +- src/loaders/directwrite.rs | 3 ++- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/canvas.rs b/src/canvas.rs index 72cf45b..f4ee284 100644 --- a/src/canvas.rs +++ b/src/canvas.rs @@ -56,7 +56,7 @@ impl Canvas { pub fn new(size: Vector2I, format: Format) -> Canvas { Canvas::with_stride( size, - size.x() as usize * format.bytes_per_pixel(), + size.x() as usize * format.bytes_per_pixel() as usize, format, ) } @@ -157,7 +157,7 @@ impl Canvas { let size = dst_rect.size(); - let dest_bytes_per_pixel = self.format.bytes_per_pixel(); + let dest_bytes_per_pixel = self.format.bytes_per_pixel() as usize; let dest_row_stride = size.x() as usize * dest_bytes_per_pixel; let src_row_stride = utils::div_round_up(size.x() as usize, 8); @@ -191,8 +191,8 @@ impl Canvas { src_stride: usize, src_format: Format, ) { - let src_bytes_per_pixel = src_format.bytes_per_pixel(); - let dest_bytes_per_pixel = self.format.bytes_per_pixel(); + let src_bytes_per_pixel = src_format.bytes_per_pixel() as usize; + let dest_bytes_per_pixel = self.format.bytes_per_pixel() as usize; for y in 0..rect.height() { let (dest_row_start, src_row_start) = ( @@ -234,7 +234,7 @@ pub enum Format { impl Format { /// Returns the number of bits per pixel that this image format corresponds to. #[inline] - pub fn bits_per_pixel(self) -> usize { + pub fn bits_per_pixel(self) -> u8 { match self { Format::Rgba32 => 32, Format::Rgb24 => 24, @@ -244,7 +244,7 @@ impl Format { /// Returns the number of color channels per pixel that this image format corresponds to. #[inline] - pub fn components_per_pixel(self) -> usize{ + pub fn components_per_pixel(self) -> u8 { match self { Format::Rgba32 => 4, Format::Rgb24 => 3, @@ -254,13 +254,13 @@ impl Format { /// Returns the number of bits per color channel that this image format contains. #[inline] - pub fn bits_per_component(self) -> usize { + pub fn bits_per_component(self) -> u8 { self.bits_per_pixel() / self.components_per_pixel() } /// Returns the number of bytes per pixel that this image format corresponds to. #[inline] - pub fn bytes_per_pixel(self) -> usize { + pub fn bytes_per_pixel(self) -> u8 { self.bits_per_pixel() / 8 } } diff --git a/src/loaders/core_text.rs b/src/loaders/core_text.rs index 1b6b5eb..8685321 100644 --- a/src/loaders/core_text.rs +++ b/src/loaders/core_text.rs @@ -511,7 +511,7 @@ impl Font { Some(canvas.pixels.as_mut_ptr() as *mut _), canvas.size.x() as usize, canvas.size.y() as usize, - canvas.format.bits_per_component(), + canvas.format.bits_per_component() as usize, canvas.stride, &cg_color_space, cg_image_format, diff --git a/src/loaders/directwrite.rs b/src/loaders/directwrite.rs index ca6e6d1..b39b7d9 100644 --- a/src/loaders/directwrite.rs +++ b/src/loaders/directwrite.rs @@ -532,7 +532,8 @@ impl Font { } else { Format::Rgb24 }; - let texture_bytes_per_pixel = texture_format.bytes_per_pixel(); + let texture_bits_per_pixel = texture_format.bits_per_pixel(); + let texture_bytes_per_pixel = texture_bits_per_pixel as usize / 8; let texture_size = Vector2I::new(texture_width, texture_height); let texture_stride = texture_width as usize * texture_bytes_per_pixel; From 83cb2521c132d64c4a9f90e79527472075d5b9b1 Mon Sep 17 00:00:00 2001 From: Leon Teichroeb Date: Sat, 12 Oct 2024 17:17:43 +0200 Subject: [PATCH 7/7] Fix missing cast due to revert --- src/canvas.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/canvas.rs b/src/canvas.rs index f4ee284..896ebe1 100644 --- a/src/canvas.rs +++ b/src/canvas.rs @@ -104,7 +104,7 @@ impl Canvas { "Number of pixels in src_bytes does not match stride and size." ); assert!( - src_stride >= src_size.x() as usize * src_format.bytes_per_pixel(), + src_stride >= src_size.x() as usize * src_format.bytes_per_pixel() as usize, "src_stride must be >= than src_size.x()" );