Fix rounding errors with font scaling

crates/epaint/CHANGELOG.md

@@ -7,6 +7,8 @@ All notable changes to the epaint crate will be documented in this file.
 * Don't render `\r` (Carriage Return) ([#2452](https://github.com/emilk/egui/pull/2452)).
 * Fix bug in `Mesh::split_to_u16` ([#2459](https://github.com/emilk/egui/pull/2459)).
 * Improve rendering of very thin rectangles.
+* Fix rounding errors with font scaling ([#2490](https://github.com/emilk/egui/pull/2490)):
+  * Added opt-in feature `proportional_font_scaling` to maintain text bounding box sizes when display DPI is changed.
 
 
 ## 0.20.0 - 2022-12-08

crates/epaint/Cargo.toml

@@ -49,6 +49,9 @@ deadlock_detection = ["dep:backtrace"]
 ## If you plan on specifying your own fonts you may disable this feature.
 default_fonts = []
 
+## Enabling this feature will allow fixed-width fonts to scale correctly for any display DPI.
+proportional_font_scaling = []
+
 ## Enable additional checks if debug assertions are enabled (debug builds).
 extra_debug_asserts = [
   "emath/extra_debug_asserts",

crates/epaint/src/text/font.rs

@@ -61,7 +61,7 @@ pub struct FontImpl {
     name: String,
     ab_glyph_font: ab_glyph::FontArc,
     /// Maximum character height
-    scale_in_pixels: u32,
+    scale_in_points: f32,
     height_in_points: f32,
     // move each character by this much (hack)
     y_offset: f32,
@@ -76,13 +76,13 @@ impl FontImpl {
         pixels_per_point: f32,
         name: String,
         ab_glyph_font: ab_glyph::FontArc,
-        scale_in_pixels: u32,
+        scale_in_points: f32,
         y_offset_points: f32,
     ) -> FontImpl {
-        assert!(scale_in_pixels > 0);
+        assert!(scale_in_points > 0.0);
         assert!(pixels_per_point > 0.0);
 
-        let height_in_points = scale_in_pixels as f32 / pixels_per_point;
+        let height_in_points = scale_in_points;
 
         // TODO(emilk): use these font metrics?
         // use ab_glyph::ScaleFont as _;
@@ -92,12 +92,12 @@ impl FontImpl {
         // dbg!(scaled.line_gap());
 
         // Round to closest pixel:
-        let y_offset = (y_offset_points * pixels_per_point).round() / pixels_per_point;
+        let y_offset = y_offset_points.round();
 
         Self {
             name,
             ab_glyph_font,
-            scale_in_pixels,
+            scale_in_points,
             height_in_points,
             y_offset,
             pixels_per_point,
@@ -194,7 +194,7 @@ impl FontImpl {
                 &mut self.atlas.lock(),
                 &self.ab_glyph_font,
                 glyph_id,
-                self.scale_in_pixels as f32,
+                self.scale_in_points,
                 self.y_offset,
                 self.pixels_per_point,
             );
@@ -212,9 +212,8 @@ impl FontImpl {
     ) -> f32 {
         use ab_glyph::{Font as _, ScaleFont};
         self.ab_glyph_font
-            .as_scaled(self.scale_in_pixels as f32)
+            .as_scaled(self.scale_in_points)
             .kern(last_glyph_id, glyph_id)
-            / self.pixels_per_point
     }
 
     /// Height of one row of text. In points
@@ -430,13 +429,14 @@ fn allocate_glyph(
     atlas: &mut TextureAtlas,
     font: &ab_glyph::FontArc,
     glyph_id: ab_glyph::GlyphId,
-    scale_in_pixels: f32,
+    scale_in_points: f32,
     y_offset: f32,
     pixels_per_point: f32,
 ) -> GlyphInfo {
     assert!(glyph_id.0 != 0);
     use ab_glyph::{Font as _, ScaleFont};
 
+    let scale_in_pixels = scale_in_points * pixels_per_point;
     let glyph =
         glyph_id.with_scale_and_position(scale_in_pixels, ab_glyph::Point { x: 0.0, y: 0.0 });
 
@@ -471,8 +471,7 @@ fn allocate_glyph(
     });
     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;
+    let advance_width_in_points = font.as_scaled(scale_in_points).h_advance(glyph_id);
 
     GlyphInfo {
         id: glyph_id,

crates/epaint/src/text/fonts.rs

@@ -748,8 +748,6 @@ impl FontImplCache {
             .unwrap_or_else(|| panic!("No font data found for {:?}", font_name))
             .clone();
 
-        let scale_in_pixels = self.pixels_per_point * scale_in_points;
-
         // Scale the font properly (see https://github.com/emilk/egui/issues/2068).
         let units_per_em = ab_glyph_font.units_per_em().unwrap_or_else(|| {
             panic!(
@@ -758,29 +756,26 @@ impl FontImplCache {
             )
         });
         let font_scaling = ab_glyph_font.height_unscaled() / units_per_em;
-        let scale_in_pixels = scale_in_pixels * font_scaling;
+        let scale_in_points = scale_in_points * font_scaling;
 
         // Tweak the scale as the user desired:
-        let scale_in_pixels = scale_in_pixels * tweak.scale;
+        let scale_in_points = scale_in_points * 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 scale_in_points = scale_in_points.round();
 
-        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;
+        let y_offset_points = scale_in_points * tweak.y_offset_factor + tweak.y_offset;
 
         self.cache
-            .entry((scale_in_pixels, font_name.to_owned()))
+            .entry((scale_in_points 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,
+                    scale_in_points,
                     y_offset_points,
                 ))
             })

crates/epaint/src/text/text_layout.rs

@@ -129,7 +129,26 @@ fn layout_section(
             });
 
             paragraph.cursor_x += glyph_info.advance_width;
-            paragraph.cursor_x = font.round_to_pixel(paragraph.cursor_x);
+
+            // This opt-in feature flag makes fixed width fonts scale proportionally to their
+            // surroundings regardless of the `pixels_per_point` setting. I.e., the text bounding
+            // box will be proportionally identical even when the window is moved between displays
+            // with varying DPIs.
+            //
+            // By default this feature is off, and text layout will nudge glyph positions to align
+            // with the pixel grid to retain the appearance of decent spacing between characters.
+            //
+            // See https://github.com/emilk/egui/pull/2490 for additional details.
+            #[cfg(feature = "proportional_font_scaling")]
+            {
+                paragraph.cursor_x = paragraph.cursor_x.round();
+            }
+
+            #[cfg(not(feature = "proportional_font_scaling"))]
+            {
+                paragraph.cursor_x = font.round_to_pixel(paragraph.cursor_x);
+            }
+
             last_glyph_id = Some(glyph_info.id);
         }
     }
@@ -819,6 +838,36 @@ fn test_zero_max_width() {
     assert_eq!(galley.rows.len(), 1);
 }
 
+#[test]
+fn test_font_scaling() {
+    fn measure_max_glyph_size(pixels_per_point: f32) -> Vec2 {
+        let mut fonts = FontsImpl::new(pixels_per_point, 1024, super::FontDefinitions::default());
+        let layout_job = LayoutJob::single_section(
+            "Hello, world!".into(),
+            super::TextFormat {
+                font_id: super::FontId::monospace(14.0),
+                ..Default::default()
+            },
+        );
+        let galley = super::layout(&mut fonts, layout_job.into());
+
+        galley
+            .rows
+            .iter()
+            .flat_map(|row| row.glyphs.iter().map(|g| g.size))
+            .fold(Vec2::ZERO, |acc, size| acc.max(size))
+    }
+
+    let expected_size = Vec2::new(8.275167, 16.0);
+
+    assert_eq!(measure_max_glyph_size(1.0), expected_size);
+    assert_eq!(measure_max_glyph_size(1.15), expected_size);
+    assert_eq!(measure_max_glyph_size(1.5), expected_size);
+    assert_eq!(measure_max_glyph_size(2.0), expected_size);
+    assert_eq!(measure_max_glyph_size(3.0), expected_size);
+    assert_eq!(measure_max_glyph_size(3.333), expected_size);
+}
+
 #[test]
 fn test_cjk() {
     let mut fonts = FontsImpl::new(1.0, 1024, super::FontDefinitions::default());