markFeatureWriter: Ignore contextual anchors #868
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We don’t support contextual anchors in markFeatureWriter, but glyphsLib
subclasses it and adds support for them. However, since we are unaware
of contextual anchors, we end up with positioning statements with
duplicated mark positions like this:
pos base beh-ar.init
<anchor 91 -316> mark @MC_top
<anchor 91 -226> mark @MC_top;
(one is the regular anchor, and the other is the contextual one). Which
makes no sense (feaLib shouldn’t probably allow the same mark class to
be used multiple times in the same statement).
This PR adds isContextual to NamedAnchor and ignores any such anchors
when writing pose statements. It is already set by glyphsLib’s
MarkFeatureWriter, but otherwise should not affect ufo2ft’s as it always
sets it to False.
|
can't we just bring in the whole contextual mark feature writer? It wouldn't be the first time we add some Glyphs-specific feature to ufo2ft which may benefit the rest of the UFO workflows |
|
I wouldn't mind doing that. The current setup is not making things easy, but it wasn’t me who set it up this way. |
|
@simoncozens do you agree with moving the glyphsLib's custom mark writer to ufo2ft to reduce the complexity? |
|
Sure, it will make things easier now, and also in the future if we convince UFO to support contextual anchors. |
|
OK, I’ll work on that. |
|
Closing in favor of #869. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We don’t support contextual anchors in markFeatureWriter, but glyphsLib subclasses it and adds support for them. However, since we are unaware of contextual anchors, we end up with positioning statements with duplicated mark positions like this:
(one is the regular anchor, and the other is the contextual one). Which makes no sense (feaLib shouldn’t probably allow the same mark class to be used multiple times in the same statement).
This PR adds isContextual to NamedAnchor and ignores any such anchors when writing pose statements. It is already set by glyphsLib’s MarkFeatureWriter, but otherwise should not affect ufo2ft’s as it always sets it to False.