SubpixelAA FreeType rasterize_glyph fix (#254)

* Add failing test for #252

* Change bytes_per_pixel to usize.

* Add checks to avoid confusing runtime exceptions.

* Fix bitmap_size in FreeType rasterize_glyph for MODE_LCD*

* Fix buffer checks in blit_from

* Revert "Change bytes_per_pixel to usize."

This reverts commit a8231d7.

* Fix missing cast due to revert

src/canvas.rs

@@ -85,6 +85,10 @@ 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,
@@ -94,6 +98,17 @@ impl Canvas {
         src_stride: usize,
         src_format: Format,
     ) {
+        assert_eq!(
+            src_stride * src_size.y() as usize,
+            src_bytes.len(),
+            "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() as usize,
+            "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));
         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<B: Blit>(
         &mut self,
         rect: RectI,

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!"),

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() {