ryanoasis / nerd-fonts

Iconic font aggregator, collection, & patcher. 3,600+ icons, 50+ patched fonts: Hack, Source Code Pro, more. Glyph collections: Font Awesome, Material Design Icons, Octicons, & more
https://NerdFonts.com
Other
53.79k stars 3.62k forks source link

font-patcher: Improve weight checking #1364

Closed Finii closed 11 months ago

Finii commented 11 months ago

[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)

frantisekhanzlikbl commented 11 months ago

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)").

Finii commented 11 months ago

@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 :unamused: Usually Iosevka has some flags set that should be copied over - this seems to be missing now.

frantisekhanzlikbl commented 11 months ago

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! :slightly_smiling_face:

Finii commented 11 months ago

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 commented 11 months ago

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