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

Draft: font-patcher: Respect gap values in source font #943

Merged
merged 1 commit into from
Oct 7, 2022

Conversation

Finii
Copy link
Collaborator

@Finii Finii commented Sep 26, 2022

[why]
We just ignore specified gaps in the source fonts (i.e. set them to zero). This reduces the line spacing in the patched font (because the gap is missing).

[how]
Distribute the gap INTO the cell, so that we can work with zero gap (we need that for the powerline glyphs), and still keeping the powerline glyphs centered about the regular glyphs AND keeping the total line spacing.

Idea-by: Tushar Singh tusharvickey1999@gmail.com

Fixes: #850

Reported-by: Joe Bolts

Requirements / Checklist

What does this Pull Request (PR) do?

How should this be manually tested?

Any background context you can provide?

What are the relevant tickets (if any)?

Screenshots (if appropriate or helpful)

See Issue.

[why]
We just ignore specified gaps in the source fonts (i.e. set them to
zero). This reduces the line spacing in the patched font (because the
gap is missing).

[how]
Distribute the gap INTO the cell, so that we can work with zero gap (we
need that for the powerline glyphs), and still keeping the powerline
glyphs centered about the regular glyphs AND keeping the total line
spacing.

Idea-by: Tushar Singh <tusharvickey1999@gmail.com>

Fixes: #850

Reported-by: Joe Bolts
Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
@tusharsnx
Copy link

tusharsnx commented Sep 28, 2022

I tried the script, font looks identical to the original font but and now the text is centred as oppose to top/bottom aligned.

Also I didn't have to use the flag to achieve this, may be flag(--preserve-gap) hasn't been added yet ?

@Finii
Copy link
Collaborator Author

Finii commented Sep 28, 2022

I tried the script, font looks identical to the original font but now the text is centred as oppose to top/bottom aligned.

What does that mean? Of course the glyphs will not touch the top of the 'cell' anymore if we put half of the gap 'above' the glyph.

Also I didn't have to use the flag to achieve this, may be flag(--preserve-gap) hasn't been added yet ?

This is correct. I just threw in the code changes, with no (new) option.

@tusharsnx
Copy link

What does that mean? Of course the glyphs will not touch the top of the 'cell' anymore if we put half of the gap 'above' the glyph.

I'm sorry if I sounded confusing. I meant that it is now correctly aligned with line gaps as expected.

@Finii
Copy link
Collaborator Author

Finii commented Sep 28, 2022

Ok, so I will trudge on with the finer details of the PR, the 'main goal' seems fulfilled. Thank you.

@plodocus
Copy link

Weird, still eats the gaps when I patch it:

Original:
image

Patched:
image

@Finii
Copy link
Collaborator Author

Finii commented Sep 28, 2022

What is your terminal?
But you font-patcher has the gap code from this PR?
Maybe I should put "my" patched font somewhere so we all talk about the same font file.

@plodocus
Copy link

What is your terminal? But you font-patcher has the gap code from this PR? Maybe I should put "my" patched font somewhere so we all talk about the same font file.

That's alacritty. Yes, I checked out the feature branch and verified the code is there. Yes, if you could provide your test font that would be helpful as a reference.

@Finii
Copy link
Collaborator Author

Finii commented Sep 29, 2022

Hmm, when I use the original Comic Code Demo and alacritty 0.11.0-dev (589c1e9) there is no gap respected (i.e. is also eat up):

The assumption is wrong here, alacritty displays the gap ABOVE lines

image

f almost touches ceiling, y does touch bottom

@Finii
Copy link
Collaborator Author

Finii commented Sep 29, 2022

Gap comparison...

Orange numbers is your screenshot image from above.
Yellow numbers is my screenshot, size is the same and number 113 aligned.

Peek 2022-09-29 09-30

Do you have a different alacritty?

Addendum:

I do not have any self patched version installed for the experiment above:

image

@tusharsnx
Copy link

tusharsnx commented Sep 29, 2022

Perhaps alacritty modifies font properties and provides its own 'settings' to customize them?

font:
  # Normal (roman) font face
  normal:
    family: Noto Mono for Powerline
    # The `style` can be specified to pick a specific face.
    #style: Regular

  # Offset is the extra space around each character. `offset.y` can be thought of
  # as modifying the line spacing, and `offset.x` as modifying the letter spacing.
  offset:
    x: 0
    y: 0

source: https://gist.github.com/Cogitri/e8d5c63818443f3c8f13cd7760fe77aa

Take a look at:

  1. Line height looking weird on macOS alacritty/alacritty#189
  2. https://superuser.com/questions/1619887/how-to-add-padding-change-space-between-lines-in-alacritty-terminal

@Finii
Copy link
Collaborator Author

Finii commented Sep 29, 2022

Just compared with kitty, which renders it different but same 😬
Especially it does not 'invent' a bold face when the font does not have one

Peek 2022-09-29 09-51

Shown my alacritty and kitty

@Finii
Copy link
Collaborator Author

Finii commented Sep 29, 2022

Hmm, obviously it IS possible to nudge alacritty into the right direction just with font properties

image

Lets make this more sane.

Edit: Add note at end of 1st sentence

@Finii
Copy link
Collaborator Author

Finii commented Sep 29, 2022

Well, for ME the current code is working with alacritty:

Peek 2022-09-29 10-50
Original and Nerd Fonts version alternatring

The gap is displayed correctly by alacritty. The patching relocates 1/2 of the gap to the top and 1/2 to the bottom.

The thing is just, that alacritty shows the gap above lines, not below. Which is what I would have expected, but that does not matter in fact. (With 'original' the y touches the cell bottom, so the gap is in the top of the cell. Also note that the f is in fact as high as the cell, gap ignored - 'original' shows the (full) gap above the f)

But what you can clearly see, the line spacing is not touched.

Here the intermediate steps...

Original -> Gap removed -> Gap redistributed top/bottom -> adding symbols -> Original -> ...

Peek 2022-09-29 10-58

@Finii
Copy link
Collaborator Author

Finii commented Sep 29, 2022

My alacritty:

image

@Finii Finii merged commit 6d30fee into master Oct 7, 2022
@Finii Finii deleted the feature/preserve-gap branch October 7, 2022 09:56
@Finii Finii mentioned this pull request Jan 10, 2023
3 tasks
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.

Patcher reduces the spacing between lines making fonts difficult to read.
3 participants