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

why the line-height changed? #15

Closed
shabahengam opened this issue Jul 17, 2020 · 9 comments
Closed

why the line-height changed? #15

shabahengam opened this issue Jul 17, 2020 · 9 comments
Labels

Comments

@shabahengam
Copy link

One of the main reasons that I liked agave was the line-height.It was so compact (but completely legible) and I could have more lines in my terminal (for example in vim) but unfortunately v16 changed the line-height.for example before that I could have 43 line in my terminal but now I just have 37 line and I think 6 line less is a big change.I don't know anything about font designing but I hope the line height can get back to v15.(sorry for my English)
screenshot is from neovim.
left is v21 ... right is v15
https://0x0.st/iv_I.png

@blobject
Copy link
Owner

Hi, thanks a lot for this issue.

I overlooked this while making revision v16, partly because I did not fully understand how line-height gets calculated. (But it is also a bit unsettling that some programs do not easily let you customise line-heights at runtime :-/)

I understand that the visible number of lines is important, especially for engineers and admins, so I will definitely try to change this back to pre-v16.

Unfortunately, this will cause capital letters with top diacritics to look cramped, so I will have to redesign those. Please give me a few days to apply the changes.

@shabahengam
Copy link
Author

Unfortunately, this will cause capital letters with top diacritics to look cramped, so I will have to redesign those.

why? because you changed the line-height for box-drawing?

@blobject
Copy link
Owner

Yes, and actually the problem with cramped diacritics was always present (for instance, #4), and the box-drawing glyphs were drawn to accommodate them.

For some reason, I assumed that the line-height would reflect the height of these box-drawing glyphs. And with v16, I explicitly set the global line-height, again, assuming that this height value wouldn't change anything. Your ticket has shown me that the assumption was wrong, so I will have to shorten the box-drawing glyphs and also lower the diacritics.

Maybe what happened before is that the height of the line would dynamically change to accommodate glyphs appearing on the line, but I haven't tested this hypothesis. I might test it, but I also prefer just explicitly setting everything and making the glyph dimensions consistent and invariant.

@shabahengam
Copy link
Author

thanks for explanation.I think agave can improved more by a few changes.for example I made an issue on JetBrains Mono font(here) and complained about the lowercase f and the maintainer told me that changes make the font more consistent.after that I looked to agave and I can see agave has that problem too.Look at the picture
agave
as you see the f,j,i and t are not in the same "wave".

the below picture is from JetBrains Mono.left one is the old version of the font and the right one is the latest version.
as you see in the latest version f,i,j,t are in the same "wave"
image

PS 1: should I open another issue for that?
PS 2: I'm so sorry if I ask too much.I know this a free project and you don't have time for all this issues but I just like this font and want to make it better and better.of course all the hard work should be done by you :)

@blobject
Copy link
Owner

blobject commented Jul 17, 2020

This is an excellent observation. Hm, I must have made a mistake again, as I intended for the wave to line up. I do like consistency in this case, even if Consolas would do it differently :-p

Yes please make a separatue issue, you can copy and paste your comment or make whatever change you'd like. Just having an exact issue to refer to can be useful.

All issues, questions, comments are good. Give me everything you got!

@blobject
Copy link
Owner

blobject commented Jul 21, 2020

Fixed in https://github.com/agarick/agave/releases/tag/v22

I have reverted the line-height to pre-v16. So 2048 units rather than 2304.

Please check the added source glyphs and the SFD file (via fontforge) for details on how the capitals+diacritics have been handled.

@blobject blobject added the bug label Jul 21, 2020
@blobject
Copy link
Owner

Hi @shabahengam, sorry I should have let you close this.

Has the issue been fixed for you with release version 22?

@shabahengam
Copy link
Author

Yes,v22 fixed the problem.Thank you so much.

@blobject
Copy link
Owner

Cool, this is the way it should be :-) Thanks for using the font!

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

No branches or pull requests

2 participants