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

Glyphs taller than font.ascent are clipped #143

Open
alexheretic opened this issue Dec 15, 2021 · 2 comments
Open

Glyphs taller than font.ascent are clipped #143

alexheretic opened this issue Dec 15, 2021 · 2 comments

Comments

@alexheretic
Copy link
Owner

alexheretic/ab-glyph#50 font CooperHewitt-Book.otf glyphs clip outside of font ascender/descender vbounds. This means the top line (for default layout) glyphs may clip too.

This can be fixed by using the font line_gap as top-padding for the first line, essentially shifting down the glyphs of all fonts with non-zero line_gap values.

I'm unsure if this is the "correct" thing to do though. It's possible the font itself is wrong instead?

@alexheretic
Copy link
Owner Author

alexheretic commented Dec 15, 2021

Looking at this font in gimp, it seems to "fix" the layout on the fly using the render bounds. That's not something glyph-brush-layout can do so easily as we calculate all glyph positions without outlining the glyphs.

cooper-hewitt-book-gimp.mp4

This does also suggest that using line_gap as top-padding isn't correct 😞

alexheretic added a commit that referenced this issue Dec 15, 2021
@alexheretic alexheretic changed the title Use font.line_gap as top-padding for all lines? Glyphs taller than font.ascent are clipped Dec 15, 2021
@alexheretic
Copy link
Owner Author

alexheretic commented Dec 15, 2021

A workaround is to give the sections with this font some extra margin at the top.

However, depending on the to_vertex fn implementation, glyph bound clipping may need to also be disabled. (This is not specified by glyph-brush itself, though the opengl example does clip glyphs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant