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

[anchor_propagation] fix issue adjusting mark component anchor #975

Merged
merged 3 commits into from
Feb 2, 2024

Conversation

anthrotype
Copy link
Member

@anthrotype anthrotype commented Jan 30, 2024

In anchor_propagation.py, we have code (added by @yanone in #617) to adjust propagated anchor position when a mark component is anchored to a specific named anchor as specified in the ComponentInfo lib key (this can be used to specify which of the seveal ligature anchors the given component is supposed to be attached to e.g. top_1, top_2 etc).

The code indiscriminately applies the adjustment inside a for loop, without checking the anchor names match the expected component anchor.

I modified one of the AnchorPropagation.glyphs test file by changing the order of anchors in a mark glyph used as component in order to trigger a failure (anchor order is itself meaningless, it only happened to work before because the last anchor was the right one).
I will follow up with a fix

In anchor_propagation.py, we have code to adjust propagated anchor position when a mark component is anchored to a specific named anchor as specified in the ComponentInfo lib key.
The code indiscriminately applies the adjustment in a for loop, without checking the anchor names match the expected component anchor.
I modified one of the AnchorPropagation.glyphs test file by changing the order of anchors in a mark glyph used as component in order to trigger a failure. Will follow up with a fix
@anthrotype anthrotype changed the title [anchor_propagation] fix issue [anchor_propagation] fix issue adjusting mark component anchor Jan 30, 2024
@anthrotype
Copy link
Member Author

I'm not sure about the regression tests failing, if it's a good thing or not in this particular case.

@anthrotype
Copy link
Member Author

I've inspected the regresson test and I've convinced myself that these were bugs and this fixes them

@anthrotype
Copy link
Member Author

@simoncozens does this change look sane to you? I'm inclined to merge

@anthrotype
Copy link
Member Author

I think this is correct so I'm gonna merge, but feel free to file any issues that may arise

@anthrotype anthrotype merged commit 90f10c4 into main Feb 2, 2024
9 of 10 checks passed
@khaledhosny khaledhosny deleted the fix-adjust-mark-anchor branch February 11, 2024 23:09
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.

1 participant