search

googlefonts / glyphsLib

A bridge from Glyphs source files (.glyphs) to UFOs
Apache License 2.0
178 stars 51 forks source link

[anchor_propagation] fix issue adjusting mark component anchor #975

Closed anthrotype closed 4 months ago

anthrotype commented 5 months ago

In anchor_propagation.py, we have code (added by @yanone in https://github.com/googlefonts/glyphsLib/pull/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

anthrotype commented 5 months ago

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

anthrotype commented 4 months ago

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

anthrotype commented 4 months ago

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

anthrotype commented 4 months ago

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