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

Patching Berkeley Mono v2 leads to incorrectly spaced chars #1772

Closed
3 tasks done
clementpoiret opened this issue Dec 30, 2024 · 17 comments
Closed
3 tasks done

Patching Berkeley Mono v2 leads to incorrectly spaced chars #1772

clementpoiret opened this issue Dec 30, 2024 · 17 comments
Milestone

Comments

@clementpoiret
Copy link

clementpoiret commented Dec 30, 2024

Requirements

  • I have searched the issues for my issue and found nothing related or helpful
  • I have searched the FAQ for help
  • I have checked the Wiki for help

Experienced Behavior

The arrow ligature is offcentered to the left, and the r in the import word is to far to the right. It happens for many characters.
Even if the font is behind a paywall, the issue should also happen on the free trial version of Berkeley Mono v2. Command used to patch fonts:

sudo docker run --rm \
  -v /home/clementpoiret/.local/share/fonts/TX-02:/in \
  -v /home/clementpoiret/.local/share/fonts/TX02-NerdFont:/out \
  nerdfonts/patcher \
  --progressbars \
  --mono \
  --adjust-line-height \
  -c

Expected Behavior

Same behavior as for other fonts, centered chars and glyphs.

Example Symbols or Text

-> import but it happens for a lot of characters and nearly all ligatures.

Font Used

Berkeley Mono v2

Source of Font File

https://usgraphics.com/products/berkeley-mono

Terminal Emulator (and the title of the terminal window)

ghostty, kitty

Operating System and Version

NixOS Unstable

Screenshots

Unpatched font:
2024-12-30-At-10h39m30s

Patched font:
2024-12-30-At-10h37m13s

@Finii
Copy link
Collaborator

Finii commented Dec 30, 2024 via email

@Finii
Copy link
Collaborator

Finii commented Dec 30, 2024

Ah, I see this person uses the same additional flags, and you just use -c (complete) for easiness 👍

I believe in the demo of TX-02 there are no ligs, as this became an add on buy option? [1]

Anyhow, --mono does mess around with the source font in ways that you probably do not need. It forces the whole font to be some specific kind of monosopaced that ALL terminals recognise. When I see the issues that you describe I guess the reason is that the font is forced into being really monospaced.

So you should propably avoid that.

Please try without --mono, which will also leave you with bigger icons - which most (?) people prefer anyhow.
If you must have small (within one monospace cell) icons, that would need a script change (a very simple one), maybe we should add an option for that. If we tried and found it is a solution for you. In retrospect --mono should have been two options maybe from the start because it does two things. Well, usually you want both. 🤔

The demo font FX-050 (?) has to be obtained via some fake order 🙄 [2]

Anyhow. It would be good if you could run the patcher with --debug 2 and post the first lines where the source font analysis is output.
I'm not sure that it works correctly with the docker image, and anyhow it would be good if you either

  • enter the docker image into a shell and run there interactively
  • or use the FontPatcher.zip without docker on your machine directly with all dependencies installed

In that way I can have a look at the debug output and tweak the font-patcher script to not touch the ligs on patching even with --mono.

[1] https://usgraphics.com/catalog/FX-102
[2] https://usgraphics.com/catalog/FX-050

@Finii
Copy link
Collaborator

Finii commented Dec 30, 2024

Oh, the misaligned letter r ... I have no explanation for that. Are more ordinary letters misaligned?
You say you are on NixOS 🤔 There are issues on Windows that could explain it, but none I'm aware of on Nix 😒
This looks like a Font Family mixup. Maybe try to install only one font file of all the files and try again.
If that solves it we would need to have a look on the naming. You did not specify any --makegroups, what you need can be decided on the complete output of --debug 2 when you patch all fonts in the family that you plan to install in parallel.

@clementpoiret
Copy link
Author

Thanks for your replies. I'm not in front of my computer right now, but I'll come back with debug logs asap. In the meantime, I have some more details:

  • As this is a v2, I simply naively tried commands others used to patch Berkeley Mono v1,
  • I tried with and without --mono and --adjust-line-height, same issue,
  • The r is not the only char to be moved, I don't know why but it's the most visible one,
  • it reminds me an issue I had when trying (an earlier version of) MonaSpice, which had similar ligature issues on my side. I didn't patch this one myself,
  • patching the TTF files produces correct fonts, only patching the OTF ones are producing those weird misalignments

@Finii
Copy link
Collaborator

Finii commented Dec 31, 2024

Hmm, the demo font is kind of useless. It just contains these basic glyphs, and no ligatures or any other stuff.

patching the TTF files produces correct fonts, only patching the OTF ones are producing those weird misalignments

Hmm, is there a specific reason that you prefer the otf variant? The ttf is also an OpenType font, nowadays ;) and it has technically more capabilities. If I have a choice here we usually patch (only) the ttf variants.

Screenshot 2024-12-31 at 10 12 02

Just the basic glyphs

Screenshot 2024-12-31 at 10 22 30

While the otf has the blues defined the ttf seems to be not even autohinted

@mrp52
Copy link

mrp52 commented Dec 31, 2024

Just ran into this myself. So far I am unable to find a solution playing around with a whole bunch of options to the patcher docker image. You can see the issue very clearly in the two screenshots of fastfetch, things like the "mor" in my name with the o and r being spaced too far apart and r and m in "arm64" being too close together.

I can confirm patching the ttf font works correctly. Only the otf seems to result in a buggy patched font.

First image is the unpatched (trial) font. Second is patched.

trial patched

@Finii
Copy link
Collaborator

Finii commented Dec 31, 2024

I believe, what destroys the ligs is this

        if self.args.single:
            # Force width to be equal on all glyphs to ensure the font is considered monospaced on Windows.
            # This needs to be done on all characters, as some information seems to be lost from the original font file.
            self.set_sourcefont_glyph_widths()

I will just quickly create a new docker image with an option to suppress that.
But I think that will not solve the 'r' issue. Maybe one step at a time.

Lets see.

Will open a PR now and merge it, maybe we can discuss if this is a viable solution there.

@Finii
Copy link
Collaborator

Finii commented Dec 31, 2024

Wait, what, it is in the trial font?
Testing myself.

@mrp52
Copy link

mrp52 commented Dec 31, 2024

Wait, what, it is in the trial font? Testing myself.

Yes I am using the trial font (wanted to see if I could patch it before buying it as it isn't cheap!). It is a little annoying you have to "sign up" to get the trial download.

@Finii
Copy link
Collaborator

Finii commented Dec 31, 2024

Well, see it. Should be easy to find the reason...

Screenshot 2024-12-31 at 14 29 04

@Finii
Copy link
Collaborator

Finii commented Dec 31, 2024

Hmm, the demo font has this defect on purpose 😬
This is not added by the patcher script.

Screenshot 2024-12-31 at 14 30 57

@Finii
Copy link
Collaborator

Finii commented Dec 31, 2024

Interestingly can not see that in neither Glyphs3 nor FontGoggles 🤔

Screenshot 2024-12-31 at 14 37 36

@Finii
Copy link
Collaborator

Finii commented Dec 31, 2024

The shifted letters like r seem to be a fontforge bug.
Built HEAD from git, and it is still there. The glyphs are all somehow 'right aligned'-ish, with some it is most obvious, with some it is not so much. See image below.

This does not happen when the ttf is opened, just the otf has that.

Also tried to convert the otf to ttx and back to otf, and it is still the same.
While the individual glyphs in ttx look ok, here the vertical bar (where the code is very easy):

        <CharString name="bar">
          262 -86 rmoveto
          76 872 -76 hlineto
          endchar
        </CharString>

It has 262 left hand side bearing, is 76 wide and with a an advance width of 600 the right hand side bearing calculates also to 262 (600 - 262 - 76).
Strangely this is not how fontforge reads it.


image

Opened with self-built from HEAD fontforge

image

Description of 'bar' as decoded by ttx

Screenshot 2024-12-31 at 17 58 08

As opened in Glyphs3

image

As opened in fontforge

@clementpoiret
Copy link
Author

clementpoiret commented Jan 1, 2025

Thanks a lot for your investigations. You're right, I don't particularly need OTF, it was just the default format when downloading the fonts :)

Here are the debug 2 logs when converting one of the OTF files, if this can help you 👍
logs.txt

EDIT: And happy new year!

@Finii
Copy link
Collaborator

Finii commented Jan 1, 2025

The log looks excellent 🤔 Thanks for sharing.

I would propose to

  • Ignore the otf problem for now (I fear that is a fontforge bug, and that will take a day or two to find)
  • Check if the ligature problem can be solved by the new option from the PR
    • Let me check if the PR does not change anything unwanted
    • And then pull, a new docker image will be crated
    • You can check the real font

Is that a good plan?

Happy new year to you also 🎉 😃, thank you!

$ grep -ve '\(^ *$\)\|^Updating\|^Adding' logs.txt
Running with options:
 --mono --debug 2 --careful --makegroups 2 --complete --has-no-italic --no-progressbars
Parallelism 0
Nerd Fonts Patcher v3.3.0 (4.16.2) (ff 20230101)
DEBUG: Naming mode 2
DEBUG: Monospace check: Panose says "monospaced"; glyph-width-mono True
INFO: Font vertical metrics slightly off (0.1%)
DEBUG: Font has negative right side bearing in extended glyphs
DEBUG: Final font cell dimensions 600 w x 1201 h (with icon cell 853 h)
DEBUG: 57/160 box drawing glyphs will be replaced
Done with Patch Sets, generating font...
DEBUG: Weight approximations: OS2/PS/Name: 400/400/400 (from 400/'Book'/'Regular')
DEBUG: =====> Family (ID 1)      ok       (19 <=31): TX02 Nerd Font Mono
DEBUG: =====> SubFamily (ID 2)   ok       ( 7 <=31): Regular
DEBUG: =====> Fullname (ID 4)    ok       (19 <=63): TX02 Nerd Font Mono
DEBUG: =====> PSN (ID 6)         ok       (11 <=63): TX02NFM-Reg
DEBUG: Tweaking 1/1
DEBUG: Changing flags from 0xB to 0x3
DEBUG: Changing lowestRecPPEM from 8 to 6
   TX02 Nerd Font Mono
   \===> '/out/TX02NerdFontMono-Regular.otf'

@clementpoiret
Copy link
Author

Everything looks good to me!
And you are right, ignoring OTF issues seems the way to go 👍

Finii added a commit that referenced this issue Jan 8, 2025
…uching existing glyphs

[why]
This can help if you want monospaced icons but not-force the other
glyphs to be monospaced (which we do to make the whole font
monospace-detectable which was a major issue in the beginning, esp with
Windows).

[how]
Add option --single-width-glyphs that makes the added glyphs single
width (like --mono), but leaves preexisting glyphs untouched.

Fixes: #1772

Signed-off-by: Fini Jastrow <[email protected]>
@Finii Finii closed this as completed in 2bda061 Jan 8, 2025
@Finii
Copy link
Collaborator

Finii commented Jan 8, 2025

Please fetch a new docker font-patcher image.

The version tag should be 4.18.0 or later.

https://hub.docker.com/r/nerdfonts/patcher/tags

Then patch with these options:

 sudo docker run --rm \
   -v /home/clementpoiret/.local/share/fonts/TX-02:/in \
   -v /home/clementpoiret/.local/share/fonts/TX02-NerdFont:/out \
   nerdfonts/patcher \
   --progressbars \
-  --mono \
+  --single-width-glyphs \
   --adjust-line-height \
   -c

I.e. with the new single-width-glyphs that is a softer form of mono: It does not touch existing glyphs.
Maybe together with --careful which will prevent destroying preexisting glyphs by (not) overpatching.

Edit: Well, and of course please use the ttf font, as I did not find time to dive (again 🙄) into font-forge for the otf problem.

@Finii Finii mentioned this issue Jan 14, 2025
4 tasks
@Finii Finii added this to the v3.4.0 milestone Jan 14, 2025
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

No branches or pull requests

3 participants