Center-align all text vertically - #5117

ah mixing fonts is always that hard.. :(

but this looks like still break things in some rare cases

	master (baseline aligned) sourcehansans is higher than cantarell and it pushes the baseline down, while characters are still aligned properly
	this pr (center aligned) ...misaligned, seems that glyph center of some fonts is just not the visual center

Before, mixing text of different heights (e.g. emojis and latin text) in a label or button would cause the text to jump vertically.

wondering if sth like below could make things better

diff --git a/crates/epaint/src/text/font.rs b/crates/epaint/src/text/font.rs
index 5a119cdc..3135d870 100644
--- a/crates/epaint/src/text/font.rs
+++ b/crates/epaint/src/text/font.rs
@@ -473,6 +473,10 @@ impl Font {
         }
         None
     }
+
+    pub fn ascent(&self) -> f32 {
+        self.fonts.first().map_or(0., |f| f.ascent())
+    }
 }
 
 /// Code points that will always be invisible (zero width).
diff --git a/crates/epaint/src/text/text_layout.rs b/crates/epaint/src/text/text_layout.rs
index 4fa3ba49..4cd22dcb 100644
--- a/crates/epaint/src/text/text_layout.rs
+++ b/crates/epaint/src/text/text_layout.rs
@@ -172,7 +172,7 @@ fn layout_section(
                 chr,
                 pos: pos2(paragraph.cursor_x, f32::NAN),
                 size: vec2(glyph_info.advance_width, line_height),
-                ascent: font_impl.map_or(0.0, |font| font.ascent()), // Failure to find the font here would be weird
+                ascent: font.ascent(),
                 uv_rect: glyph_info.uv_rect,
                 section_index,
             });

this way only metrics from the primary (first) font are used during layout

It doesn't really help. I think the problem is mixing fonts that don't properly center the text in them.

We could center on the visual bounds, but the problem with that would be that text would move vertically when you add new letters to a TextEdit.

I think the code in this PR is still an improvement. You can tweak the problematic fonts on load with FontTweak::y_offset_factor

but the problem with that would be that text would move vertically when you add new letters to a TextEdit.

with a fixed ascent value i don t really get how this move could happen..

did a quick test and the results are:

master	master with diff above
15.mp4	16.mp4

You misunderstand me – your diff does not move the text, but it also does not solve all centering issues. Here is egui with this PR plus you diff, using Cantarell:

Not great.

EDIT: oh wait i meant to apply that diff to master, not this pr, sry

😮 i really got some different results..

+        font_data.insert(
+            "Cantarell".to_owned(),
+            FontData::from_static(include_bytes!("/usr/share/fonts/truetype/Cantarell-VF.otf")),
+        );
         families.insert(
             FontFamily::Proportional,
             vec![
-                "Ubuntu-Light".to_owned(),
+                "Cantarell".to_owned(),
                 "NotoEmoji-Regular".to_owned(),
                 "emoji-icon-font".to_owned(),

and if i remove all the FontTweak on default fonts..

while it doesn t look that good, it renders just like in my web browser (msedge):

which should be somewhat correct

Good point… I'll try out your ascent idea in a separate PR then and hold off on this one for a little bit

This is starting to look good now! This is using default (zero) FontTweak for everything (including emoji fonts):

With default egui fonts:

With Cantarell and NotoSansCJK:

Looks pretty good imho, and without any tweaking!

There is a big downside to using valign = Center though: mixing fonts of different heights (e.g. Heading and Body) looks bad:

I think there should be a solution to this though, which looks something like this:

• Revert to using valign = Bottom
• Always center glyphs within the same Font (and size), regardless of valign
• Only apply valign to the difference in height between the largest Font height the the height of the current glyph

I guess I need to do some more experiments…

EDIT: actually, the old code didn't do a good job of this either, so this is not necessarily a blocker. It would just be nice to be able to fix it all in one go.

Ok, I think this is good now!