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 (some) Glyphs 3 alternate layers #729

Merged
merged 6 commits into from Oct 28, 2021

Conversation

simoncozens
Copy link
Collaborator

This PR partially addresses #715. Notably, it only deals with alternate layers:

  • Which apply to one axis only
  • Which apply only to the first axis
  • Which have a max value or a min value but not both

I believe this is all that can be done without serious reworking of how alternate layers are handled by the builders, in particular the way that we name them on export. (It may be possible to have alternate layers on other axes than the first without too much trouble, but I deliberately want to do this in fairly small steps.)

This may be easier to review commit-by-commit as I have done things in small discrete changes

@anthrotype
Copy link
Member

pardon my ignorance, what are "Glyphs 3 alternate layers"? Is it a new way to encode 'bracket' layers?

@simoncozens
Copy link
Collaborator Author

simoncozens commented Oct 6, 2021

Sort of.

Glyphs 2 bracket layers can specify a "switch point" and be either forwards or reverse - so [300] applies from 300-axis_max on the first axis and ]300] applies from axis_min-300 on the first axis.

Glyphs 3 alternate layers do a lot more than that; they directly specify a min and/or a max on each axis. So in G3 you could have a layer that applies [wght<500] or [500<wght] or even [300<wght<500] - or even [500<wght,12<opsz<16].

The min-max range and multiple-axis cases are going to be very difficult to manage with our current setup; we will need new glyph naming conventions, and will need to tread carefully if we want to prioritise back-conversion from Designspace->glyphs.

@moyogo
Copy link
Collaborator

moyogo commented Oct 6, 2021

@RosaWagner
Copy link

RosaWagner commented Oct 6, 2021

Saira is a good candidate to test this PR (has several simple one-axis bracket layers) and the alternate shape got exported properly :)

@anthrotype yes, brace and bracket layers were quite confusing compare to "intermediate" and "alternate", and translation probably complicated. (sorry didn't see previous answer from Simon and Denis)

@RosaWagner
Copy link

Trying this onto another project and I have this error:
TypeError: '<=' not supported between instances of 'float' and 'str'
It only has one axis, and alternate layer has only a min value, so should match the requirements or this branch

@RosaWagner
Copy link

I rounded coordinates of all glyphs and now it works 🤷‍♀️ sorry for disturbance

@simoncozens
Copy link
Collaborator Author

Good to know it works after rounding, but that’s a bug we should fix - it shouldn’t die with an error.

@anthrotype anthrotype mentioned this pull request Oct 25, 2021
@simoncozens
Copy link
Collaborator Author

@RosaWagner I can't reproduce the error you've found. I've tried making alternate layers with unrounded points in the layers, but it still seems to work. I'm going to merge this for now, but I would like to get it fixed. If you can let me look at the project that's failing, maybe I can boil it down to a simple test case.

@simoncozens simoncozens merged commit bb171b7 into googlefonts:main Oct 28, 2021
@simoncozens simoncozens deleted the alternate-layers branch October 28, 2021 08:49
@RosaWagner
Copy link

RosaWagner commented Oct 28, 2021

Good you merge :) I can't share the sources for this one. But I think it was the translation coordinates of the components that were fractional, not the points themselves. I'll try to reproduce.

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

4 participants