googlefonts / ufo2ft

A bridge from UFOs to FontTools objects (and therefore, OTFs and TTFs).
MIT License
151 stars 43 forks source link

Build DS5 #598

Closed belluzj closed 2 years ago

belluzj commented 2 years ago

This PR implements building a DesignSpace version 5 using fontmake / ufo2ft. The main new feature is that a single input designspace can build several variable fonts.

This PR implements logic in ufo2ft to choose which of the source UFOs need to be compiled, based on which variable fonts the user is requesting fontmake to build. Only the needed source fonts are built, then passed over to varLib in order to merge all the requested variable fonts.

This PR needs: https://github.com/fonttools/fonttools/pull/2436

The commit names are useless and I'll squash everything later, sorry about that.

@anthrotype

belluzj commented 2 years ago

@anthrotype I measured the difference between dump/load and the copy.deepcopy methods, the deepcopy is faster:

https://gist.github.com/belluzj/d86a75cc5305424070c19c840b4ced15

$ python time_ds_deepcopy.py
collected 56 test documents
deepcopy_dump_load
13.100488499999999
deepcopy_python
4.283137499999999
deepcopyExceptFonts
4.2913587
madig commented 2 years ago

It seems the font attribute gets lost somewhere.

madig commented 2 years ago

It seems DesignSpaceDocument.deepcopyExceptFonts actually leaves the font attribute None?!

belluzj commented 2 years ago

It seems DesignSpaceDocument.deepcopyExceptFonts actually leaves the font attribute None?!

https://github.com/fonttools/fonttools/blob/main/Lib/fontTools/designspaceLib/__init__.py#L2942-L2943

I think it is at least trying to restore the source.font values? Maybe that goes wrong somehow but I would say the problem is somewhere else. Let's debug together

madig commented 2 years ago

res.font should be source.font.

anthrotype commented 2 years ago

res.font should be source.font.

yep. let's patch and quickly push a new ft

belluzj commented 2 years ago

🤦 Thanks for spotting it, I should have written a unit test for that

madig commented 2 years ago

Todo: use different drawings per glyph and layer so we can see deltas in the TTX dumps. Maybe take from Jany's fontmake PR?

belluzj commented 2 years ago

We've implemented what @anthrotype was requesting but he's off on paternity leave for a few more weeks so in order to keep this moving we'll merge it and make a pre-release so people can test it. If there are any bugs in the pre-release or any requests for further changes in the code we'll fix them before the next actual release.