8000 [merge] allow passing in TTFont, change to use argparse by NightFurySL2001 · Pull Request #3621 · fonttools/fonttools · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[merge] allow passing in TTFont, change to use argparse #3621

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[merge] allow passing in TTFont, change to use argparse #3621

wants to merge 2 commits into from

Conversation

NightFurySL2001
Copy link
Contributor

Implements #3614

@NightFurySL2001 NightFurySL2001 changed the title allow passing in TTFont, change to use argparse [merge] allow passing in TTFont, change to use argparse Aug 29, 2024
@behdad
Copy link
Member
behdad commented Aug 29, 2024

I have a vague memory that the merger doesn't take TTFonts, because it has to load fonts multiple times. But I'm probably wrong. I don't have time to review this currently.

@NightFurySL2001
Copy link
Contributor Author

From what I saw the fonts are loaded multiple times to build a new GlyphOrder. Tried to sidestep it by using deepcopy (although I believe it's not required at all).

@behdad
Copy link
Member
behdad commented Aug 30, 2024

From what I saw the fonts are loaded multiple times to build a new GlyphOrder. Tried to sidestep it by using deepcopy (although I believe it's not required at all).

I don't think that would 8000 work. Se want to reload the fonts using the new GlyphOrder.

@NightFurySL2001
Copy link
Contributor Author

Pretty sure the reload happens before setting the new GlyphOrder. No other files call for _openFonts() in merge module.

fonttools/Lib/fontTools/merge/__init__.py

Lines 79 to 91 in 770917d

fonts = self._openFonts(fontfiles)
glyphOrders = [list(font.getGlyphOrder()) for font in fonts]
computeMegaGlyphOrder(self, glyphOrders)
# Take first input file sfntVersion
sfntVersion = fonts[0].sfntVersion
# Reload fonts and set new glyph names on them.
fonts = self._openFonts(fontfiles)
for font, glyphOrder in zip(fonts, glyphOrders):
font.setGlyphOrder(glyphOrder)
if "CFF " in font:
renameCFFCharStrings(self, glyphOrder, font["CFF "])

@behdad
Copy link
Member
behdad commented Aug 30, 2024

The font's GlyphOrder is loaded lazily. After the first _openFonts, the GlyphOrder of the font is used, which means cmap and some other tables are loaded.

We load the font a second time, and set GlyphOrder on it before accessing any tables, so all tables are loaded using the new GlyphOrder...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@NightFurySL2001 @behdad
0