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

Fix some monospace fonts spacing being non-monospace #2164

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

alphaqu
Copy link

@alphaqu alphaqu commented Oct 18, 2022

On fonts like Jetbrains Mono the rounding to the closest pixel after every glyph made the spacing between the characters non-monospace. This fixes that issue by rounding the glyph location instead of the cursor x position which affects the future characters in a non-expected way.

@emilk
Copy link
Owner

emilk commented Oct 18, 2022

I tried this out with the default font with pixels_per_point = 1.15.

Before:

old-rounding

After:

new-rounding

The new code produces some bad kerning in many places:
Screen Shot 2022-10-18 at 17 17 37

Screen Shot 2022-10-18 at 17 18 44

Screen Shot 2022-10-18 at 17 18 53

@alphaqu
Copy link
Author

alphaqu commented Oct 18, 2022

A possible fix might be to only apply this fix on monospace fonts as those are the ones which need this fix to be quote on quote "monospaced".

@alphaqu
Copy link
Author

alphaqu commented Oct 18, 2022

Another solution to this problem is to provide the glyph width correctly in the first place. Rendering rounds the glyph_width which makes it be different than the application (for example a cursor drawer). Just rounding the glyph_width on the getter makes the behavior consistent across the codebase.

I originally missjudged my issue. I thought the drawer was messing up the spacings but it was just using a different glyph width to fix the spacing.

@parasyte
Copy link
Contributor

@alphaqu could you try the patch in #2490? I don't have an app using Jetbrains Mono, and I'm not exactly sure what I should be looking for to compare between actual and expected rasterization because there are no screenshots here.

The linked patch may address the issue you had if your pixels_per_point is greater than 1.0. (I.e. on a high DPI display.)

@emilk emilk marked this pull request as draft January 7, 2024 15:34
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 this pull request may close these issues.

3 participants