diff --git a/crates/epaint/CHANGELOG.md b/crates/epaint/CHANGELOG.md index c3e6ea7fbf7..1ca4e8f9774 100644 --- a/crates/epaint/CHANGELOG.md +++ b/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 diff --git a/crates/epaint/Cargo.toml b/crates/epaint/Cargo.toml index 8f2f91650ae..252ba6607e4 100644 --- a/crates/epaint/Cargo.toml +++ b/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", diff --git a/crates/epaint/src/text/font.rs b/crates/epaint/src/text/font.rs index e97687f0572..172b2c26011 100644 --- a/crates/epaint/src/text/font.rs +++ b/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, diff --git a/crates/epaint/src/text/fonts.rs b/crates/epaint/src/text/fonts.rs index 4857213c21f..563cc536bb6 100644 --- a/crates/epaint/src/text/fonts.rs +++ b/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, )) }) diff --git a/crates/epaint/src/text/text_layout.rs b/crates/epaint/src/text/text_layout.rs index bbf677d1795..feef3ab9461 100644 --- a/crates/epaint/src/text/text_layout.rs +++ b/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());