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

Support TrueType hinting, 4th try #654

Merged
merged 47 commits into from Mar 17, 2023
Merged

Support TrueType hinting, 4th try #654

merged 47 commits into from Mar 17, 2023

Conversation

jenskutilek
Copy link
Collaborator

Continued from #577

@moyogo
Copy link
Collaborator

moyogo commented Aug 31, 2022

@anthrotype What do you recommend for InstructionCompiler in the end? Mixin, functions, merge into OutlineTTFCompiler?

@jenskutilek
Copy link
Collaborator Author

Thank you @anthrotype! I will check take a look as soon as possible, hopefully this afternoon.

Yes, it does support variable fonts. In my usage scenario, all master UFOs contain identical fpgm, prep, and glyf programs. The cvts are usually different in each master, of course. fontTools.varLib will build a cvar from the different cvts, and copy the fpgm, prep and glyf programs when that's the case.

It should also be enough if the default master contains the fpgm, prep, and glyf programs, but I need to check again.

@anthrotype
Copy link
Member

I see, thanks for confirming. If we don't have one already, we should add a test that compiles a VF with hinting and confirms that varLib generates the cvar deltas

@jenskutilek
Copy link
Collaborator Author

It should also be enough if the default master contains the fpgm, prep, and glyf programs, but I need to check again.

Confirmed.

I've run the latest version on a couple of fonts, and the resulting fonts are identical. Haven't looked at the code yet.

c.flags &= ~USE_MY_METRICS
else:
c.flags &= ~USE_MY_METRICS
lib_contains_use_my_metrics_key |= TRUETYPE_METRICS_KEY in component_lib
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
lib_contains_use_my_metrics_key |= TRUETYPE_METRICS_KEY in component_lib
if TRUETYPE_METRICS_KEY in component_lib:
lib_contains_use_my_metrics_key = True

I realize this is equivalent, but I had to stop and think when I saw this line 😬

jenskutilek and others added 2 commits March 13, 2023 14:25
Co-authored-by: Nikolaus Waxweiler <nikolaus.waxweiler@daltonmaag.com>
@jenskutilek
Copy link
Collaborator Author

jenskutilek commented Mar 13, 2023

I see, thanks for confirming. If we don't have one already, we should add a test that compiles a VF with hinting and confirms that varLib generates the cvar deltas

It's tested in fontTools.varLib when building from TTF masters (fonttools/fonttools#1069). Or do you mean we should a test here for building from UFO?

@jenskutilek
Copy link
Collaborator Author

@anthrotype I've had a look at your changes. LGTM.

@anthrotype
Copy link
Member

yeah we could/should add a test here as well as a sort of integration test for variable+hinted builds

@jenskutilek
Copy link
Collaborator Author

jenskutilek commented Mar 13, 2023

I've added TrueType data to the integration test. I stumbled over fonttools/fonttools#3036 while preparing the expected ttx data, but I fixed it for now by converting the input to fromAssembly() to a list.

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.

LGTM! Sorry it took me so long to get this in. Many thanks to @jenskutilek for not giving up :)

@anthrotype anthrotype merged commit 0f123a7 into main Mar 17, 2023
7 checks passed
@khaledhosny khaledhosny deleted the truetype-compiler-4 branch August 18, 2023 20:26
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

5 participants