improve fallback fonts alignment (#2724)

* use font metrics in layout

* properly center scaled fonts

* adjust docs

* fix raised text

* fix easymark viewer small text alignment
caused by variable row heights

Author: lictex

crates/egui_demo_lib/src/easy_mark/easy_mark_viewer.rs

@@ -38,11 +38,26 @@ pub fn item_ui(ui: &mut Ui, item: easy_mark::Item<'_>) {
         }
 
         easy_mark::Item::Text(style, text) => {
-            ui.label(rich_text_from_style(text, &style));
+            let label = rich_text_from_style(text, &style);
+            if style.small && !style.raised {
+                ui.with_layout(Layout::left_to_right(Align::BOTTOM), |ui| {
+                    ui.set_min_height(row_height);
+                    ui.label(label);
+                });
+            } else {
+                ui.label(label);
+            }
         }
         easy_mark::Item::Hyperlink(style, text, url) => {
             let label = rich_text_from_style(text, &style);
-            ui.add(Hyperlink::from_label_and_url(label, url));
+            if style.small && !style.raised {
+                ui.with_layout(Layout::left_to_right(Align::BOTTOM), |ui| {
+                    ui.set_height(row_height);
+                    ui.add(Hyperlink::from_label_and_url(label, url));
+                });
+            } else {
+                ui.add(Hyperlink::from_label_and_url(label, url));
+            }
         }
 
         easy_mark::Item::Separator => {

crates/epaint/src/text/font.rs

@@ -1,5 +1,6 @@
 use crate::{
     mutex::{Mutex, RwLock},
+    text::FontTweak,
     TextureAtlas,
 };
 use emath::{vec2, Vec2};
@@ -42,6 +43,17 @@ pub struct GlyphInfo {
     /// Unit: points.
     pub advance_width: f32,
 
+    /// `ascent` value from the font metrics.
+    /// this is the distance from the top to the baseline.
+    ///
+    /// Unit: points.
+    pub ascent: f32,
+
+    /// row height computed from the font metrics.
+    ///
+    /// Unit: points.
+    pub row_height: f32,
+
     /// Texture coordinates.
     pub uv_rect: UvRect,
 }
@@ -52,6 +64,8 @@ impl Default for GlyphInfo {
         Self {
             id: ab_glyph::GlyphId(0),
             advance_width: 0.0,
+            ascent: 0.0,
+            row_height: 0.0,
             uv_rect: Default::default(),
         }
     }
@@ -69,6 +83,7 @@ pub struct FontImpl {
     height_in_points: f32,
     // move each character by this much (hack)
     y_offset: f32,
+    ascent: f32,
     pixels_per_point: f32,
     glyph_info_cache: RwLock<ahash::HashMap<char, GlyphInfo>>, // TODO(emilk): standard Mutex
     atlas: Arc<Mutex<TextureAtlas>>,
@@ -80,20 +95,37 @@ impl FontImpl {
         pixels_per_point: f32,
         name: String,
         ab_glyph_font: ab_glyph::FontArc,
-        scale_in_pixels: u32,
-        y_offset_points: f32,
+        scale_in_pixels: f32,
+        tweak: FontTweak,
     ) -> FontImpl {
-        assert!(scale_in_pixels > 0);
+        assert!(scale_in_pixels > 0.0);
         assert!(pixels_per_point > 0.0);
 
-        let height_in_points = scale_in_pixels as f32 / pixels_per_point;
+        use ab_glyph::*;
+        let scaled = ab_glyph_font.as_scaled(scale_in_pixels);
+        let ascent = scaled.ascent() / pixels_per_point;
+        let descent = scaled.descent() / pixels_per_point;
+        let line_gap = scaled.line_gap() / pixels_per_point;
+
+        // Tweak the scale as the user desired
+        let scale_in_pixels = scale_in_pixels * tweak.scale;
+
+        let baseline_offset = {
+            let scale_in_points = scale_in_pixels / pixels_per_point;
+            scale_in_points * tweak.baseline_offset_factor
+        };
 
-        // TODO(emilk): use these font metrics?
-        // use ab_glyph::ScaleFont as _;
-        // let scaled = ab_glyph_font.as_scaled(scale_in_pixels as f32);
-        // dbg!(scaled.ascent());
-        // dbg!(scaled.descent());
-        // dbg!(scaled.line_gap());
+        let y_offset_points = {
+            let scale_in_points = scale_in_pixels / pixels_per_point;
+            scale_in_points * tweak.y_offset_factor
+        } + tweak.y_offset;
+
+        // center scaled glyphs properly
+        let y_offset_points = y_offset_points + (tweak.scale - 1.0) * 0.5 * (ascent + descent);
+
+        // Round to an even number of physical pixels to get even kerning.
+        // See https://github.com/emilk/egui/issues/382
+        let scale_in_pixels = scale_in_pixels.round() as u32;
 
         // Round to closest pixel:
         let y_offset = (y_offset_points * pixels_per_point).round() / pixels_per_point;
@@ -102,8 +134,9 @@ impl FontImpl {
             name,
             ab_glyph_font,
             scale_in_pixels,
-            height_in_points,
+            height_in_points: ascent - descent + line_gap,
             y_offset,
+            ascent: ascent + baseline_offset,
             pixels_per_point,
             glyph_info_cache: Default::default(),
             atlas,
@@ -194,15 +227,7 @@ impl FontImpl {
         if glyph_id.0 == 0 {
             None // unsupported character
         } else {
-            let glyph_info = allocate_glyph(
-                &mut self.atlas.lock(),
-                &self.ab_glyph_font,
-                glyph_id,
-                self.scale_in_pixels as f32,
-                self.y_offset,
-                self.pixels_per_point,
-            );
-
+            let glyph_info = self.allocate_glyph(glyph_id);
             self.glyph_info_cache.write().insert(c, glyph_info);
             Some(glyph_info)
         }
@@ -231,6 +256,62 @@ impl FontImpl {
     pub fn pixels_per_point(&self) -> f32 {
         self.pixels_per_point
     }
+
+    fn allocate_glyph(&self, glyph_id: ab_glyph::GlyphId) -> GlyphInfo {
+        assert!(glyph_id.0 != 0);
+        use ab_glyph::{Font as _, ScaleFont};
+
+        let glyph = glyph_id.with_scale_and_position(
+            self.scale_in_pixels as f32,
+            ab_glyph::Point { x: 0.0, y: 0.0 },
+        );
+
+        let uv_rect = self.ab_glyph_font.outline_glyph(glyph).map(|glyph| {
+            let bb = glyph.px_bounds();
+            let glyph_width = bb.width() as usize;
+            let glyph_height = bb.height() as usize;
+            if glyph_width == 0 || glyph_height == 0 {
+                UvRect::default()
+            } else {
+                let atlas = &mut self.atlas.lock();
+                let (glyph_pos, image) = atlas.allocate((glyph_width, glyph_height));
+                glyph.draw(|x, y, v| {
+                    if v > 0.0 {
+                        let px = glyph_pos.0 + x as usize;
+                        let py = glyph_pos.1 + y as usize;
+                        image[(px, py)] = v;
+                    }
+                });
+
+                let offset_in_pixels = vec2(bb.min.x, bb.min.y);
+                let offset = offset_in_pixels / self.pixels_per_point + self.y_offset * Vec2::Y;
+                UvRect {
+                    offset,
+                    size: vec2(glyph_width as f32, glyph_height as f32) / self.pixels_per_point,
+                    min: [glyph_pos.0 as u16, glyph_pos.1 as u16],
+                    max: [
+                        (glyph_pos.0 + glyph_width) as u16,
+                        (glyph_pos.1 + glyph_height) as u16,
+                    ],
+                }
+            }
+        });
+        let uv_rect = uv_rect.unwrap_or_default();
+
+        let advance_width_in_points = self
+            .ab_glyph_font
+            .as_scaled(self.scale_in_pixels as f32)
+            .h_advance(glyph_id)
+            / self.pixels_per_point;
+
+        GlyphInfo {
+            id: glyph_id,
+            advance_width: advance_width_in_points,
+            ascent: self.ascent,
+            row_height: self.row_height(),
+            uv_rect,
+        }
+    }
 }
 
 type FontIndex = usize;
@@ -429,58 +510,3 @@ fn invisible_char(c: char) -> bool {
             | '\u{FEFF}' // ZERO WIDTH NO-BREAK SPACE
     )
 }
-
-fn allocate_glyph(
-    atlas: &mut TextureAtlas,
-    font: &ab_glyph::FontArc,
-    glyph_id: ab_glyph::GlyphId,
-    scale_in_pixels: f32,
-    y_offset: f32,
-    pixels_per_point: f32,
-) -> GlyphInfo {
-    assert!(glyph_id.0 != 0);
-    use ab_glyph::{Font as _, ScaleFont};
-
-    let glyph =
-        glyph_id.with_scale_and_position(scale_in_pixels, ab_glyph::Point { x: 0.0, y: 0.0 });
-
-    let uv_rect = font.outline_glyph(glyph).map(|glyph| {
-        let bb = glyph.px_bounds();
-        let glyph_width = bb.width() as usize;
-        let glyph_height = bb.height() as usize;
-        if glyph_width == 0 || glyph_height == 0 {
-            UvRect::default()
-        } else {
-            let (glyph_pos, image) = atlas.allocate((glyph_width, glyph_height));
-            glyph.draw(|x, y, v| {
-                if v > 0.0 {
-                    let px = glyph_pos.0 + x as usize;
-                    let py = glyph_pos.1 + y as usize;
-                    image[(px, py)] = v;
-                }
-            });
-
-            let offset_in_pixels = vec2(bb.min.x, scale_in_pixels + bb.min.y);
-            let offset = offset_in_pixels / pixels_per_point + y_offset * Vec2::Y;
-            UvRect {
-                offset,
-                size: vec2(glyph_width as f32, glyph_height as f32) / pixels_per_point,
-                min: [glyph_pos.0 as u16, glyph_pos.1 as u16],
-                max: [
-                    (glyph_pos.0 + glyph_width) as u16,
-                    (glyph_pos.1 + glyph_height) as u16,
-                ],
-            }
-        }
-    });
-    let uv_rect = uv_rect.unwrap_or_default();
-
-    let advance_width_in_points =
-        font.as_scaled(scale_in_pixels).h_advance(glyph_id) / pixels_per_point;
-
-    GlyphInfo {
-        id: glyph_id,
-        advance_width: advance_width_in_points,
-        uv_rect,
-    }
-}

crates/epaint/src/text/fonts.rs

@@ -153,31 +153,42 @@ impl FontData {
 #[derive(Copy, Clone, Debug, PartialEq)]
 #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
 pub struct FontTweak {
-    /// Scale the font by this much.
+    /// Scale the font's glyphs by this much.
+    /// this is only a visual effect and does not affect the text layout.
     ///
     /// Default: `1.0` (no scaling).
     pub scale: f32,
 
-    /// Shift font downwards by this fraction of the font size (in points).
+    /// Shift font's glyphs downwards by this fraction of the font size (in points).
+    /// this is only a visual effect and does not affect the text layout.
     ///
     /// A positive value shifts the text downwards.
     /// A negative value shifts it upwards.
     ///
     /// Example value: `-0.2`.
     pub y_offset_factor: f32,
 
-    /// Shift font downwards by this amount of logical points.
+    /// Shift font's glyphs downwards by this amount of logical points.
+    /// this is only a visual effect and does not affect the text layout.
     ///
     /// Example value: `2.0`.
     pub y_offset: f32,
+
+    /// When using this font's metrics to layout a row,
+    /// shift the entire row downwards by this fraction of the font size (in points).
+    ///
+    /// A positive value shifts the text downwards.
+    /// A negative value shifts it upwards.
+    pub baseline_offset_factor: f32,
 }
 
 impl Default for FontTweak {
     fn default() -> Self {
         Self {
             scale: 1.0,
-            y_offset_factor: -0.2, // makes the default fonts look more centered in buttons and such
+            y_offset_factor: 0.0,
             y_offset: 0.0,
+            baseline_offset_factor: -0.0333, // makes the default fonts look more centered in buttons and such
         }
     }
 }
@@ -272,9 +283,8 @@ impl Default for FontDefinitions {
             "NotoEmoji-Regular".to_owned(),
             FontData::from_static(include_bytes!("../../fonts/NotoEmoji-Regular.ttf")).tweak(
                 FontTweak {
-                    scale: 0.81,           // make it smaller
-                    y_offset_factor: -0.2, // move it up
-                    y_offset: 0.0,
+                    scale: 0.81, // make it smaller
+                    ..Default::default()
                 },
             ),
         );
@@ -284,9 +294,12 @@ impl Default for FontDefinitions {
             "emoji-icon-font".to_owned(),
             FontData::from_static(include_bytes!("../../fonts/emoji-icon-font.ttf")).tweak(
                 FontTweak {
-                    scale: 0.88,           // make it smaller
-                    y_offset_factor: 0.07, // move it down slightly
-                    y_offset: 0.0,
+                    scale: 0.88, // make it smaller
+
+                    // probably not correct, but this does make texts look better (#2724 for details)
+                    y_offset_factor: 0.11, // move glyphs down to better align with common fonts
+                    baseline_offset_factor: -0.11, // ...now the entire row is a bit down so shift it back
+                    ..Default::default()
                 },
             ),
         );
@@ -760,28 +773,19 @@ impl FontImplCache {
         let font_scaling = ab_glyph_font.height_unscaled() / units_per_em;
         let scale_in_pixels = scale_in_pixels * font_scaling;
 
-        // Tweak the scale as the user desired:
-        let scale_in_pixels = scale_in_pixels * tweak.scale;
-
-        // Round to an even number of physical pixels to get even kerning.
-        // See https://github.com/emilk/egui/issues/382
-        let scale_in_pixels = scale_in_pixels.round() as u32;
-
-        let y_offset_points = {
-            let scale_in_points = scale_in_pixels as f32 / self.pixels_per_point;
-            scale_in_points * tweak.y_offset_factor
-        } + tweak.y_offset;
-
         self.cache
-            .entry((scale_in_pixels, font_name.to_owned()))
+            .entry((
+                (scale_in_pixels * tweak.scale).round() as u32,
+                font_name.to_owned(),
+            ))
             .or_insert_with(|| {
                 Arc::new(FontImpl::new(
                     self.atlas.clone(),
                     self.pixels_per_point,
                     font_name.to_owned(),
                     ab_glyph_font,
                     scale_in_pixels,
-                    y_offset_points,
+                    tweak,
                 ))
             })
             .clone()