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

Avar2 #2679

Merged
merged 20 commits into from
Mar 15, 2023
Merged

Avar2 #2679

merged 20 commits into from
Mar 15, 2023

Conversation

behdad
Copy link
Member

@behdad behdad commented Jul 5, 2022

No description provided.

@behdad behdad marked this pull request as draft July 5, 2022 18:50
@anthrotype
Copy link
Member

what's the status of this?

@behdad
Copy link
Member Author

behdad commented Oct 29, 2022

what's the status of this?

Needs rework. I ported avar to otData, which breaks the old API. Need to add a compatibility layer in between. Also to figure out how to expose the avar2 portion.

@behdad behdad force-pushed the avar2 branch 2 times, most recently from 6c02b64 to 4548e43 Compare March 7, 2023 19:14
@behdad
Copy link
Member Author

behdad commented Mar 7, 2023

This PR so far moves avar parsing to otData and keeps previous API. No handcoded API is added for the avar2 part and that part can be accessed through the .table mechanism. Since that involves a variationStore, I feel like this is enough of an abstraction. WDYT @anthrotype?

@behdad behdad marked this pull request as ready for review March 7, 2023 19:15
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.

in COLR table module, we do something similar for handling COLRv1 via otData converters but keep the pre-existing interface to not break existing tools.
https://github.com/fonttools/fonttools/blob/main/Lib/fontTools/ttLib/tables/C_O_L_R_.py

Though I didn't make COLR a subclass of BaseTTXConverter, but called OTTableReader/Writer directly and don't keep a table attribute around for the old version's API, only for the new otData-driven one. The reasoning was to avoid having two duplicate or out-of-sync sources of truth for the data i.e. the low level table and the higher-lever constructs from the old API.

But maybe it's fine like you did here.

Lib/fontTools/ttLib/tables/_a_v_a_r.py Show resolved Hide resolved
Using the otData mechanism with handcoded shim.
We don't use VarIdxMap anymore.
@behdad behdad force-pushed the avar2 branch 3 times, most recently from 657eacb to b873562 Compare March 14, 2023 19:55
@behdad
Copy link
Member Author

behdad commented Mar 14, 2023

I added a test. It doesn't test fromXML or check what toXML produces but tests the rest.

@behdad
Copy link
Member Author

behdad commented Mar 14, 2023

Added fromXML as well.

@behdad behdad merged commit 05872d6 into main Mar 15, 2023
@behdad behdad deleted the avar2 branch March 15, 2023 17:56
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.

None yet

2 participants