Skip to content

Fix crash on stretchy glyphs with empty variant lists #206

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.

Sign up for GitHub

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

Merged
kostub merged 3 commits into from
May 29, 2026
Merged

Fix crash on stretchy glyphs with empty variant lists #206

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3414c2f
Fix crash on stretchy glyphs with empty variant lists
kostub May 29, 2026
b04e5b1
Add regression test for vertical assembly-only stretchy glyphs
kostub May 29, 2026
120a6e3
Use glyph parameter directly in empty-variant fallback
kostub May 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions iosMath/render/internal/MTFontMathTable.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -427,8 +427,12 @@ - (CGFloat) stretchStackTopShiftUp {
NSString* glyphName = [self.font getGlyphName:glyph];
NSArray* variantGlyphs = (NSArray*) variants[glyphName];
NSMutableArray* glyphArray = [NSMutableArray arrayWithCapacity:variantGlyphs.count];
if (!variantGlyphs) {
// There are no extra variants, so just add the current glyph to it.
if (variantGlyphs.count == 0) {
// No sized variants for this glyph. This covers two cases: the glyph has no
// MathGlyphConstruction entry at all (variantGlyphs == nil), and assembly-only
// glyphs whose construction has a GlyphAssembly but zero variant records (an
// empty array, e.g. XITS's stretchy arrows). In both cases the glyph itself is
// its only variant, so callers can rely on a non-empty result.
CGGlyph glyph = [self.font getGlyphWithName:glyphName];
[glyphArray addObject:@(glyph)];
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The method already receives the glyph as a parameter. Performing a lookup via [self.font getGlyphWithName:glyphName] is redundant and introduces unnecessary overhead (string/dictionary lookups). Additionally, declaring CGGlyph glyph inside this block shadows the method's parameter glyph. We can directly add the parameter glyph to glyphArray.

Suggested change
CGGlyph glyph = [self.font getGlyphWithName:glyphName];
[glyphArray addObject:@(glyph)];
[glyphArray addObject:@(glyph)];

Copy link
Copy Markdown
Owner Author

@kostub kostub May 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Dropped the round-trip and used the glyph parameter directly. It's also subtly more correct: for a glyph with no name, getGlyphName: returns nil and getGlyphWithName:nil would return glyph 0, losing the real glyph — the parameter is always right.

return glyphArray;
Expand Down
2 changes: 2 additions & 0 deletions iosMathExample/example/ViewController.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,9 +350,11 @@ - (void)pickerView:(UIPickerView *)pickerView didSelectRow:(NSInteger)row inComp

case 1:
[self.controller termesButtonPressed:nil];
break;

case 2:
[self.controller xitsButtonPressed:nil];
break;

default:
break;
Expand Down
28 changes: 28 additions & 0 deletions iosMathTests/MTTypesetterTest.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2001,6 +2001,34 @@ - (void)testOverrightarrowNarrow
XCTAssertGreaterThanOrEqual(display.width + 0.01, stack.over.width);
}

// Regression: XITS encodes the stretchy arrows (U+2190/2192/2194) as assembly-only
// glyphs — their OpenType MathGlyphConstruction has a GlyphAssembly but zero variant
// records, so h_variants is an empty list. Typesetting an \overrightarrow with such a
// font must not trip the "numVariants > 0" assertion; it should fall through to the
// horizontal glyph assembly.
- (void)testStretchyArrowAssemblyOnlyFont
{
MTFont* xits = [MTFontManager.fontManager xitsFontWithSize:20];
XCTAssertNotNil(xits);

for (NSString* latex in @[@"\\overrightarrow{x}", @"\\overrightarrow{ABCD}",
@"\\overleftarrow{y}", @"\\overleftrightarrow{ABC}"]) {
MTMathList* list = [MTMathListBuilder buildFromString:latex];
XCTAssertNotNil(list, @"%@", latex);
MTMathListDisplay* display = [MTTypesetter createLineForMathList:list font:xits style:kMTLineStyleDisplay];
XCTAssertNotNil(display, @"%@", latex);
XCTAssertEqual(display.subDisplays.count, 1u, @"%@", latex);

MTDisplay* sub0 = display.subDisplays[0];
XCTAssertTrue([sub0 isKindOfClass:[MTStackDisplay class]], @"%@", latex);
MTStackDisplay* stack = (MTStackDisplay*)sub0;
XCTAssertNotNil(stack.over, @"%@", latex);
XCTAssertNil(stack.under, @"%@", latex);
// The over-row must cover the base width.
XCTAssertGreaterThanOrEqual(stack.over.width + 0.01, stack.base.width, @"%@", latex);
}
}

- (void)testOverrightarrowWide
{
MTMathListDisplay* display = [self displayForLaTeX:@"\\overrightarrow{ABCD}"];
Expand Down
Loading