Skip to content

[merge] handle cmap format=14 subtable #3830

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

Merged
merged 7 commits into from
May 19, 2025

Conversation

JLHwung
Copy link
Contributor

@JLHwung JLHwung commented May 15, 2025

In this PR we add cmap format=14 table support to the merge tool.

For duplicate UVS mapping entries, the tool prefer the previous font, aligned with how we deal with cmap duplicates.

@JLHwung JLHwung force-pushed the merge-cmap-format-14 branch from f2c41f2 to 71de136 Compare May 15, 2025 13:29
Comment on lines 218 to 219
mapId = id(table.uvsDict if table.isUVS() else table.cmap)
offset = seen.get(mapId)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this change? The PR title and description only mention the merge tool but this seems to be changing the cmap table compile method itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. Previously I thought cmap was not defined in the format=14 subtable, it turns out we have defined a dummy cmap property being an empty dictionary.

uvsSubTable = buildCmapSubTable({}, 14, 0, 5)

I will revise the merger logic and hopefully we don't have to change the cmap compile method.

Copy link
Member

@anthrotype anthrotype left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the changes to the merge.py and corresponding tests look good to me. But see my other question about _c_m_a_p.py

@anthrotype anthrotype requested a review from behdad May 19, 2025 10:54
@anthrotype anthrotype merged commit 63b7535 into fonttools:main May 19, 2025
11 checks passed
@JLHwung JLHwung deleted the merge-cmap-format-14 branch May 19, 2025 16:28
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

Successfully merging this pull request may close these issues.

2 participants