Preserve original anchor names through propagation#1890
Merged
anthrotype merged 2 commits intomainfrom Feb 13, 2026
Merged
Conversation
Reproduces the ComforterBrush ffi/ffl issue: non-standard caret anchor names like "caret_1_" are lost during propagation because AnchorKind::to_name() maps both "caret_1" and "caret_1_" to "caret_1". This test currently FAILS, demonstrating the bug.
Member
Author
|
https://github.com/googlefonts/fontc/actions/runs/21998353880/job/63564211580?pr=1890#step:5:246 |
rsheeter
reviewed
Feb 13, 2026
rsheeter
approved these changes
Feb 13, 2026
Add `original_name: SmolStr` field to `Anchor` struct to preserve the original source name. Use `anchor.original_name` instead of `anchor.kind.to_name()` during propagation, avoiding the lossy round-trip where non-standard caret names like "caret_1_" were mapped to "caret_1" via AnchorKind. This fixes the ComforterBrush regression where ffi/ffl ligatures lost one of their two caret positions.
616368e to
e8d4585
Compare
This file contains hidden or 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
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.
After #1832 (propagate anchors in IR), fonts with non-standard caret anchor naming lost caret positions. For example, ComforterBrush-Pro.glyphs defines two caret anchors on "ffi":
caret_1at x=242caret_1_at x=588AnchorKind::new("caret_1_")parses the suffix "1_", failsparse::<usize>(), and falls back toCaret(1), i.e. the same variant ascaret_1. Whento_name()method converts both back to strings during anchor propagation, both become "caret_1" and one overwrites the other in the IndexMap.Before #1832 this didn't matter because
fontbeconsumedVec<Anchor>directly without round-tripping through names. After the merge, propagation serializes anchors viaAnchorKind::to_name()athttps://github.com/googlefonts/fontc/blob/ade0a9bc/fontir/src/propagate_anchors.rs#L87
... introducing the lossy conversion.
(fontmake/ufo2ft doesn't have this problem because it never parses caret indices, it just checks
name.startswith("caret_")and collects positions.. apparently nobody cares about caret indices except me)The fix is to add
original_name: SmolStrfield tofontir::ir::Anchorto preserve the original source name and use that instead of anchor.kind.to_name() when feeding anchors into propagation.The existing
to_name()is kept as a public method but documented as lossy.The first commit adds the regression test that asserts both caret_1 and caret_1_ survive propagation (deliberately fails without the fix). It is followed by the actual fix.