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

font-patcher: Improve weight checking #1364

Merged
merged 3 commits into from
Oct 7, 2023
Merged

font-patcher: Improve weight checking #1364

merged 3 commits into from
Oct 7, 2023

Conversation

Finii
Copy link
Collaborator

@Finii Finii commented Sep 30, 2023

[why]
When the font does not have a PSweight string the font-patcher bugs.

[how]
Rewrite the code to be more robust against unexpected weight values. Also make detected problems non-fatal.

Reported-by: František Hanzlík frantisek_hanzlik@protonmail.com

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)

[why]
When the font does not have a PSweight string the font-patcher bugs.

[how]
Rewrite the code to be more robust against unexpected weight values.
Also make detected problems non-fatal.

Reported-by: František Hanzlík <frantisek_hanzlik@protonmail.com>
Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
@Finii Finii added the Bug fix label Sep 30, 2023
@frantisekhanzlikbl
Copy link
Contributor

frantisekhanzlikbl commented Sep 30, 2023

just gave this a try. it successfully patches my font that fails without the patch, albeit with a warning. if you were curious, here is some info for the file:

original font:

$ otfinfo --info iosevka-polarised-term-extendedextralight.ttf
Family:              iosevka polarised term XLt Ex
Subfamily:           Regular
Full name:           iosevka polarised term Extralight Extended
PostScript name:     iosevka-polarised-term-Extralight-Extended
Preferred family:    iosevka polarised term
Preferred subfamily: Extralight Extended

output font:

$ otfinfo --info IosevkaPolarisedTermNerdFont-ExtraLightExtended.ttf
Family:              Iosevka Polarised Term NerdFont ExtraLight Extended
Subfamily:           Regular
Full name:           Iosevka Polarised Term NerdFont ExtraLight Extended
PostScript name:     IosevkaPolarisedTermNerdFont-ExtraLightExtended
Preferred family:    Iosevka Polarised Term NerdFont
Preferred subfamily: ExtraLight Extended

the build log:

$ fontforge -script font-patcher iosevka-polarised-term-extendedextralight.ttf --name 'Iosevka Polarised Term NerdFont Extralight Extended' --careful --outputdir /tmp/tmp.uyXGxjzqfN
Copyright (c) 2000-2023. See AUTHORS for Contributors.
 License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
 with many parts BSD <http://fontforge.org/license.html>. Please read LICENSE.
 Version: 20230101
 Based on sources from 2023-01-01 00:00 UTC-D.
Core python package 'pkg_resources' not found: Cannot discover plugins
Nerd Fonts Patcher v3.0.2-68 (4.5.3) (ff 20230101)
The following table(s) in the font have been ignored by FontForge
  Ignoring 'DSIG' digital signature table
Adding 180 Glyphs from Seti-UI + Custom Set
╢████████████████████████████████████████╟ 100%
Adding 6 Glyphs from Heavy Angle Brackets Set
╢████████████████████████████████████████╟ 100%
Adding 198 Glyphs from Devicons Set
╢████████████████████████████████████████╟ 100%
Done with Patch Sets, generating font...
WARNING: Can not parse PS-weight: ['ExtraLight']
WARNING: Possible problem with the weight metadata detected, check with --debug
ERROR: ====-< Family (ID 1)      too long (51 > 31): Iosevka Polarised Term NerdFont ExtraLight Extended
ERROR: Can not handle font flags (PermissionError(13, 'Permission denied'))
   Iosevka Polarised Term NerdFont ExtraLight Extended
   \===> '/tmp/tmp.uyXGxjzqfN/IosevkaPolarisedTermNerdFont-ExtraLightExtended.ttf'

Honestly, I'm not even sure that this is an issue on the NF side, as it might be the --name I provided (which is a result of "Iosevka Polarised Term NerdFont $(fc-scan --format "%{style}" "$fontFile" | cut --delimiter ',' --fields 1)").

[why]
When the weight check fails for some input the reason is not shown
correctly (i.e. not the string that actually failed).

[how]
Display exactly the failed string in the warning.

Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
[why]
Some PS weights have a dash in the weight, like 'Extra-Light' in
Iosevka. The parser can not parse it because it expects 'ExtraLight'.

[how]
Filter out all '-' and ' ' from the PS weight string before actually
parsing the string.

Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
@Finii
Copy link
Collaborator Author

Finii commented Oct 7, 2023

@Finii Thanks for the valuable feedback!

WARNING: Can not parse PS-weight: ['ExtraLight']
WARNING: Possible problem with the weight metadata detected, check with --debug

This is fixed with the commit name_parser: Fix weight_string_to_number() I just added to this PR.

ERROR: ====-< Family (ID 1) too long (51 > 31)

The name is just too long (for some old OSs or applications). If the length is working for you there is nothing to do. If you prepare the font for others you might want to get the length down. This is what all the different modes of --makegroups are for. For example 4 uses NF instead of Nerd Font, etc.

ERROR: Can not handle font flags (PermissionError(13, 'Permission denied'))

Not sure about this one. Somehow the font file can not be opened - the error message is not very specific. It tries to copy over some font flags from the unpatched font into the already created patched font (i.e. after Fontforge did create the new font file) - directly file to file, because Fontforge can not handle the flags 😒
Usually Iosevka has some flags set that should be copied over - this seems to be missing now.

@Finii Finii merged commit 01569ca into master Oct 7, 2023
5 checks passed
@Finii Finii deleted the bugfix/weight-checker branch October 7, 2023 11:02
@frantisekhanzlikbl
Copy link
Contributor

Oh, I probably should've clarified that I saw the two errors even without this PR. The missing flags don't seem to be causing problems, so I just ignore those for now.

The only interesting part was the parsing warning.

Thanks for your work on this! 🙂

@Finii
Copy link
Collaborator Author

Finii commented Oct 7, 2023

Well, Iosevka sets the lowest-recommended-ppem to zero:

image

Fontforge can only produce font files that have the value 8.
We correct that afterwards - see 'Tweaking' message.

This means that the small rendering of the patched font might/will be different than the unpatched one. Small means like less then 10 point or so.

Whatever ;-) If you are happy I am happy.

@frantisekhanzlikbl
Copy link
Contributor

Ah, that's unfortunate, but I don't often use <10pt font sizes, so it's fine for my use case :-)
Thanks!

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

Successfully merging this pull request may close these issues.

2 participants