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

builder/sources.py: only write locations along defined axes #739

Merged
merged 1 commit into from
Oct 27, 2021

Conversation

anthrotype
Copy link
Member

similarly to what we already do for instance.location at

designspace_axis_tags = {a.tag for a in self.designspace.axes}
location = {}
for axis_def in get_axis_definitions(self.font):
# Only write locations along defined axes
if axis_def.tag in designspace_axis_tags:
location[axis_def.name] = axis_def.get_design_loc(instance)
ufo_instance.location = location

Otherwise source.locations may contain a spurious third axis called "Custom" which may not alwyas be used (e.g. NotoSans only uses "Weight" and "Width".

We don't normally see this because fontTools.designspaceLib prunes those automatically upon writing the designspace file. But if one attempts to build a VF from the glyphsLib-generated designspace object (not the file path saved on disk), the extra undefined axis prompts an error while attempting to find the default location.

@anthrotype anthrotype requested a review from madig October 27, 2021 08:18
@anthrotype
Copy link
Member Author

trivial change, merging.

@anthrotype anthrotype merged commit 5b15a65 into main Oct 27, 2021
@anthrotype anthrotype deleted the prune-ds-sources-location branch October 27, 2021 09:22
@schriftgestalt
Copy link
Collaborator

Wouldn’t it be better to proper set font.axes on loading the file? Most .glyphs file should have a custom parameter (in glyph2) or the actual axes (for glyphs3).

If there us no data, Glyphs is checks all masters and builds axis ranges (for weight, width and Custom). Only if the range has nonzero length, it adds an axis. All other data (axis coordinates) is setup based on those axes.

@anthrotype
Copy link
Member Author

I believe that's what we already do in glyphsLib.builder.axes. Only axes that do vary are written out to designspace axes. The problem here was that when setting the location of designspace <source> elements, we were always outputting a location along all three predefined axes (weight, width, custom) even when some of them actually did nothing. For designspace <instance> locations we were already pruning those (see above linked code)

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