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

Feature: Process TTC files #783

Merged
merged 6 commits into from
Sep 23, 2022
Merged

Feature: Process TTC files #783

merged 6 commits into from
Sep 23, 2022

Conversation

Finii
Copy link
Collaborator

@Finii Finii commented Feb 9, 2022

Description

[why]
Someone might want to patch a whole lot of fonts that come in a ttc.

[how]
Just open all fonts that the input file contains (1 or more) and create
a single font or collection font file.

The automatic layer detection does not work in all cases for me, so we
need to manually search for the foreground layer (usually '1').

Code inspiration taken from
powerline/fontpatcher#6

[note]
Changed output in the end to the filename (before it was the font name),
so that one can easily copy&paste or open that file.

Reported-by: Lily Ballard lily@ballards.net

Requirements / Checklist

What does this Pull Request (PR) do?

Add possibility to 'batch patch' True Type Collections and store the result again in a TTC.

The first 3 commits just juggle around the existing code without (*cough*) functional changes. These changes are anyhow good as they clean up a bit the what-does-what.

Only the 4th commit finally adds functionality.

It does not use ttcflags=('merge',) as I do not know if old fontforges can do that (probably not), and it seems to do not much.

How should this be manually tested?

Any background context you can provide?

What are the relevant tickets (if any)?

#175
#735
#752 (comment)

Screenshots (if appropriate or helpful)

Patching Iosevka TTC:

image

Needs of course a fontforge built from HEAD because of the now fixed 16 bit bug

@Finii Finii mentioned this pull request Feb 9, 2022
2 tasks
@Finii Finii force-pushed the feature/process-ttc branch from 2e029c4 to 517d4ec Compare February 9, 2022 19:31
@Finii
Copy link
Collaborator Author

Finii commented Feb 9, 2022

Thank god we have the tests! Had the indentation level wrong 😒
Force push to keep the commits clean.

@torarnv
Copy link
Contributor

torarnv commented Apr 9, 2022

Does this depend on any other changes? I pulled down the version from the branch and got:

❯ fontforge -script /tmp/nerd-font.py "/System/Library/Fonts/Menlo.ttc"
Copyright (c) 2000-2022. 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: 20220308
 Based on sources from 2022-03-08 10:16 UTC-D.
Nerd Fonts Patcher v2.1.0 executing

Nerd Fonts: Processing Menlo Regular
The following table(s) in the font have been ignored by FontForge
  Ignoring 'hdmx' horizontal device metrics table
  Ignoring 'meta' metadata table
The glyph named Omega is mapped to U+03A9.
But its name indicates it should be mapped to U+2126.
No configfile given, skipping configfile related actions
The requested file, original-source.otf, does not exist
Traceback (most recent call last):
  File "/tmp/nerd-font.py", line 1046, in <module>
    main()
  File "/tmp/nerd-font.py", line 1011, in main
    patcher.patch(sourceFonts[-1])
  File "/tmp/nerd-font.py", line 88, in patch
    symfont = fontforge.open(self.args.glyphdir + patch['Filename'])
OSError: Open failed

@torarnv
Copy link
Contributor

torarnv commented Apr 9, 2022

Ah, wasn't aware I had to have the glyphs locally. Pulled down via

❯ git clone --depth 1  --filter=blob:none --sparse https://github.com/ryanoasis/nerd-fonts.git /tmp/nerd-fonts
❯ cd /tmp/nerd-fonts
❯ git sparse-checkout set src/glyphs

And now it seems to work:

❯ python3 /tmp/nerd-font.py --glyphdir /tmp/nerd-fonts/src/glyphs/ /System/Library/Fonts/Menlo.ttc
Nerd Fonts Patcher v2.1.0 executing

Nerd Fonts: Processing Menlo Regular
The following table(s) in the font have been ignored by FontForge
  Ignoring 'hdmx' horizontal device metrics table
  Ignoring 'meta' metadata table
The glyph named Omega is mapped to U+03A9.
But its name indicates it should be mapped to U+2126.
No configfile given, skipping configfile related actions
/tmp/nerd-fonts/src/glyphs/
original-source.otf
Adding 56 Glyphs from Seti-UI + Custom Set
╢████████████████████████████████████████╟ 100%
/tmp/nerd-fonts/src/glyphs/
devicons.ttf
Adding 198 Glyphs from Devicons Set
╢████████████████████████████████████████╟ 100%

Nerd Fonts: Processing Menlo Bold
The following table(s) in the font have been ignored by FontForge
  Ignoring 'hdmx' horizontal device metrics table
  Ignoring 'meta' metadata table
The glyph named Omega is mapped to U+03A9.
But its name indicates it should be mapped to U+2126.
No configfile given, skipping configfile related actions
/tmp/nerd-fonts/src/glyphs/
original-source.otf
Adding 56 Glyphs from Seti-UI + Custom Set
╢████████████████████████████████████████╟ 100%
/tmp/nerd-fonts/src/glyphs/
devicons.ttf
Adding 198 Glyphs from Devicons Set
╢████████████████████████████████████████╟ 100%

Nerd Fonts: Processing Menlo Italic
The following table(s) in the font have been ignored by FontForge
  Ignoring 'hdmx' horizontal device metrics table
  Ignoring 'meta' metadata table
The glyph named Omega is mapped to U+03A9.
But its name indicates it should be mapped to U+2126.
No configfile given, skipping configfile related actions
/tmp/nerd-fonts/src/glyphs/
original-source.otf
Adding 56 Glyphs from Seti-UI + Custom Set
╢████████████████████████████████████████╟ 100%
/tmp/nerd-fonts/src/glyphs/
devicons.ttf
Adding 198 Glyphs from Devicons Set
╢████████████████████████████████████████╟ 100%

Nerd Fonts: Processing Menlo Bold Italic
The following table(s) in the font have been ignored by FontForge
  Ignoring 'hdmx' horizontal device metrics table
  Ignoring 'meta' metadata table
The glyph named Omega is mapped to U+03A9.
But its name indicates it should be mapped to U+2126.
No configfile given, skipping configfile related actions
/tmp/nerd-fonts/src/glyphs/
original-source.otf
Adding 56 Glyphs from Seti-UI + Custom Set
╢████████████████████████████████████████╟ 100%
/tmp/nerd-fonts/src/glyphs/
devicons.ttf
Adding 198 Glyphs from Devicons Set
╢████████████████████████████████████████╟ 100%

Done with Patch Sets, generating font...

Generated: 'Menlo Nerd Font.ttc'

@Finii
Copy link
Collaborator Author

Finii commented Apr 9, 2022

Ah, wasn't aware I had to have the glyphs locally

Yes, that comes sometimes unexpected. Will be fixed by #741, if it will be pulled.

❯ git clone --depth 1 --filter=blob:none --sparse https://github.com/ryanoasis/nerd-fonts.git /tmp/nerd-fonts
❯ cd /tmp/nerd-fonts
❯ git sparse-checkout set src/glyphs

Obviously you selected the branch afterwards?

And now it seems to work:
...
Generated: 'Menlo Nerd Font.ttc'

🎉 Great! Nice to see someone finding the work useful :-)
And good to see that it works with the direkt python3 invocation, must admit that I always just call/test it
with fontforge --script :->

@torarnv
Copy link
Contributor

torarnv commented Apr 10, 2022

Yes, that comes sometimes unexpected. Will be fixed by #741, if it will be pulled.

Nice!

Obviously you selected the branch afterwards?

Actually I didn't, the clone already used master, which seemed correct. Is it not?

And now it seems to work:
...
Generated: 'Menlo Nerd Font.ttc'

The only slight weirdness is this, but I don't think that's due to this patch:

image

🎉 Great! Nice to see someone finding the work useful :-) And good to see that it works with the direkt python3 invocation, must admit that I always just call/test it with fontforge --script :->

Thanks for doing these fixes! 👍🏻

@torarnv
Copy link
Contributor

torarnv commented Apr 10, 2022

#413 is the warning it seems

@Finii Finii mentioned this pull request Aug 29, 2022
3 tasks
@Finii Finii force-pushed the feature/process-ttc branch from 517d4ec to 8e62a57 Compare September 14, 2022 15:25
@Finii
Copy link
Collaborator Author

Finii commented Sep 14, 2022

Rebase on master. Hell, what a job. That diverged a lot...
But it should work now (again), have only superficially tested yet.
Anyone who will join the testing is welcome! 😍

What I did not try to adapt is the PPEM stuff, that is not fixed in the resultant ttc fonts:

But the en passant Windows Compat stuff works.

$ ../fontforge/build/bin/fontforge font-patcher ~/Downloads/iosevkas/iosevka-regular.ttc --also-windows
Copyright (c) 2000-2022. 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: 20220308
 Based on sources from 2022-07-08 17:51 UTC-ML-D-GDK3.
 Based on source from git with hash: 3253fff885ff4d8ca4e36c76282106ed24dd517a
Nerd Fonts Patcher v2.2.2 (3.0.6) executing

Nerd Fonts: Processing Iosevka
[...]
Done with Patch Sets, generating font...

Can not handle font flags (Exception('No HEAD table found'))

Generated: 18 fonts in 'Iosevka Nerd Font.ttc'
Can not handle font flags (Exception('No HEAD table found'))

Generated: 18 fonts in 'Iosevka NF.ttc'

[why]
Parsing the command line arguments has nothing to do with the actual
patching.
If we want to patch more than one font we need to separate the partching
from unrelated work.

[how]
Just do the argument processing in main() and hand the information over
to the patcher object.

[note]
No functional change. Lines copied over (almost *) 1:1.

(*) Exceptions:
self.sym_font_args is now only a local variable, as it is only used in
this one function.
The startup message is shown on ... startup (i.e. main()).

Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
[why]
The extension handling is a bit out-of-place and could be handled by the
arguments handling, which simplifies the code.
Somes goes for other argument validity checks.

[how]
Put argument checks into setup_arguments().

Dropping self.extensions in favour of self.args.extensions.

No functional change.

Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
[why]
These operations are also not strictly patching.
When we want to handle more than one font we need to have this outside
the patch() function.

[how]
Put opening and exporting (previously in __init__() and patch() into
main() outside the patcher object.

No functional change (except the sourceFont is now closed :->)

Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
[why]
Someone might want to patch a whole lot of fonts that come in a ttc.

[how]
Just open all fonts that the input file contains (1 or more) and create
a single font or collection font file.

The automatic layer detection does not work in all cases for me, so we
need to manually search for the foreground layer (usually '1').

Code inspiration taken from
powerline/fontpatcher#6

[note]
Changed output in the end to the filename (before it was the font name),
so that one can easily copy&paste or open that file.

Reported-by: Lily Ballard <lily@ballards.net>
Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
[why]
With the new TTC feature we might increase the minor number ;-)
This is not just a bugfix.

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

Finii commented Sep 22, 2022

Note to self:

Just noticed that the (maybe more important?) PPEM fix #761 does not work with .ttc #783, of course, and that will also consume lots of time. Sigh.

[why]
The font flags and PPEM fix does not work with font collection files,
because it does not know how to handle them. It assumes a ttf or otf
font with the specified table structure.

The fix (for single font files) has been introduced with commit
  40138be  font-patcher: Handle lowestRecPPEM

[how]
Check if the file is of type 'ttcf', and if so fast forward to the given
single font index into the collection.

This can be rather slow...

Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
@Finii Finii force-pushed the feature/process-ttc branch from a5fc03f to 7c5c838 Compare September 23, 2022 08:54
@Finii Finii merged commit 23ae931 into master Sep 23, 2022
@Finii Finii deleted the feature/process-ttc branch September 23, 2022 09:18
LNKLEO pushed a commit to LNKLEO/Nerd that referenced this pull request Nov 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants