Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use the largest ascent as the ascent for a font #5126

Closed
wants to merge 2 commits into from

Conversation

emilk
Copy link
Owner

@emilk emilk commented Sep 18, 2024

I think we need both this and some sort of centering (like in #5117) – I'll experiment more. Maybe we should to take the FontImpl ascent and the Font ascent into account when centering 🤔

@emilk emilk added text Problems related to text egui epaint labels Sep 18, 2024
@lictex
Copy link
Contributor

lictex commented Sep 18, 2024

let pixels_per_point = fonts[0].pixels_per_point();
let row_height = fonts[0].row_height();
let mut slf = Self {

#[inline(always)]
pub fn row_height(&self) -> f32 {
self.row_height
}

row height should be taken from the same font as the ascent, otherwise rows could be clipped vertically.
doing this will make all rows higher but text should be naturally centered again then.

imo whether to use metrics from the primary font (font[0]) or the largest font is more like a design choice that, is the fallback font important enough to massively affect the layout process?

probably offtopic:
FontTweak::baseline_offset_factor (used to manually shift rows as a last resort) is getting a bit confusing. maybe it should be moved to Font instead

@emilk
Copy link
Owner Author

emilk commented Sep 19, 2024

I've returned to a mixed approach in #5117 now - it seems to work very well (finally!)

@emilk emilk closed this Sep 26, 2024
@emilk emilk deleted the emilk/better-text-ascent branch September 26, 2024 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
egui epaint text Problems related to text
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve vertical centering of text (e.g. in buttons)
2 participants