Repository navigation
Corner component placement - #2223
Merged
Merged
Conversation
cmyr
added this pull request to stack #2224
October 7, 2026 19:38
Port of googlefonts/glyphsLib#1200 and its follow-up, corner-component-more. We assumed every corner was drawn with its first node on +y and its last on +x, and ignored left and right anchors, so corners drawn any other way were misplaced, often badly (Plex Sans KR, Alkatra, Aoboshi One, Iansui, Inconsolata). With this patch we fit corners as Glyphs 3.5 does, quirks included, and match it on all but one of the 177 test cases; real_aoboshi_g needs open corners erased first. Only the first corner on a node is applied, as in Glyphs, and a corner we can't apply, because its path is missing or it sits on an off-curve node or a quadratic, is skipped with a warning rather than failing the whole font, so Aoboshi One loads.
Glyphs erases the open corners in all of a glyph's closed paths before applying corners to it. Where a corner sits on the start of an open corner's line, it ends up on the erased corner; where it sits on the line's end, it's lost along with the node. We only erased open corners later, in fontir. With this patch we erase them first, as glyphsLib's corner-component-more does, and match Glyphs on real_aoboshi_g. Of our corner sources, only Aoboshi One's G ends up different; in Solway, the open corners are just erased earlier.
anthrotype
force-pushed
the
corner-component-placement
branch
from
October 8, 2026 09:40
795e6a0 to
999319a
Compare
anthrotype
reviewed
Oct 8, 2026
Comment on lines
+426
to
+429
| self.nodes.remove(p1.max(p2)); | ||
| self.nodes.remove(p1.min(p2)); | ||
| let idx = point_idx - removed_before; | ||
| self.nodes[idx].node_type = NodeType::Line; |
Member
There was a problem hiding this comment.
This turns Libre Franklin Italic into a build failure. When the corner reaches past the start of the curve we drop both handles and make the node a Line, and that happens per master: in haabkhasian-cy it does in Bold Italic but not in Thin Italic, so the masters no longer have the same point structure and fontbe rejects the glyph ("'haabkhasian-cy.BRACKET.varAlt01' has interpolation-incompatible paths"). It compiles on main, and the target is "identical" in last night's crater.
glyphsLib fixed this in 10303a46 (part of googlefonts/glyphsLib#1203): keep the curve and put the handles on the line at 1/3 and 2/3 (otRound), so the outline is the same and the point structure doesn't change. Could you port that, along with its test_straightened_instroke_keeps_its_handles?
anthrotype
reviewed
Oct 8, 2026
| if corner_hints.is_empty() { | ||
| return Ok(()); | ||
| } | ||
| let corner_hints = erase_open_corners(layer, corner_hints); |
Member
There was a problem hiding this comment.
fontir erases open corners a second time after this; glyphsLib stops that in googlefonts/glyphsLib@bf46fb1e.
anthrotype
approved these changes
Oct 8, 2026
anthrotype
left a comment
anthrotype
left a comment
Member
There was a problem hiding this comment.
LGTM apart from the two above comments
Where a corner's cut runs past the start of a curved instroke, Glyphs makes the instroke a line. When that happens in only some masters, as in Libre Franklin Italic haabkhasian-cy, the masters end up with different numbers of points and the variable build fails. With this patch, we keep the stroke a curve that draws the same line, with its handles at a third and two thirds of the way along, as glyphsLib does (googlefonts/glyphsLib#1203).
Since the previous commits we erase open corners before applying corners, as Glyphs does, but glyphs2fontir still erased them again afterwards. That erases spurs that corners leave, but only in the masters where the spur's neighbours cross enough to count as an open corner, which breaks compatibility (Bellota m, n and p, Playfair Italic Y, among others). Glyphs doesn't erase again once the corners are on. With this patch, a layer records that its open corners were erased, and glyphs2fontir leaves it alone, as glyphsLib does (googlefonts/glyphsLib#1203).
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
2 participants
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.
this is the fontc side of googlefonts/glyphsLib#1203, which means we match Glyphs.app on all of our 177 test cases (two of which are allowed off-by-ones, presumably rounding differences).
This is a huge improvement from the previous implementation, which was matching on ~40 of 177 cases.
For visualization of the previously failing cases, see https://www.cmyr.net/artifacts/corner-components.html.