Fix crash on stretchy glyphs with empty variant lists#206
Conversation
MTFontMathTable -getVariantsForGlyph:inDictionary: returned the font's
variant list verbatim. For assembly-only glyphs — whose OpenType
MathGlyphConstruction has a GlyphAssembly but zero MathGlyphVariantRecords
(e.g. XITS's stretchy arrows U+2190/2192/2194, stored as an empty
h_variants array) — this returned an empty array. Both
-findStretchyVariantGlyph: and -findVariantGlyph:withMaxWidth: then tripped
the "numVariants > 0" assertion, crashing on e.g. \overrightarrow{x} in XITS.
Treat an empty variant list the same as an absent one: fall back to the
glyph itself. This restores the "a glyph is always its own variant"
invariant the callers rely on, so the stretchy path falls through to the
horizontal glyph assembly as intended.
Also fix a missing break in the iOS example's font picker: selecting
"TeX Gyre Termes" fell through into the XITS case, so the app actually
switched to XITS (the font that triggers the crash) — which is why the
crash was reported as happening "when switching to TeX Gyre Termes".
Add a regression test rendering the stretchy arrows in the XITS font.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses an issue with assembly-only glyphs that have zero variant records by checking if variantGlyphs.count == 0 instead of !variantGlyphs. It also fixes missing break statements in a switch-case block in ViewController.m and adds a regression test. The review feedback highlights a redundant glyph lookup and variable shadowing in MTFontMathTable.m, suggesting to directly use the existing glyph parameter instead.
| CGGlyph glyph = [self.font getGlyphWithName:glyphName]; | ||
| [glyphArray addObject:@(glyph)]; |
There was a problem hiding this comment.
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.
| CGGlyph glyph = [self.font getGlyphWithName:glyphName]; | |
| [glyphArray addObject:@(glyph)]; | |
| [glyphArray addObject:@(glyph)]; |
There was a problem hiding this comment.
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.
The empty-variant fix in getVariantsForGlyph:inDictionary: also repairs the
vertical large-delimiter path. XITS encodes the stretchy vertical arrows
(U+2191/2193/2195, used as \uparrow/\downarrow/\updownarrow delimiters) as
assembly-only glyphs: empty v_variants but a populated v_assembly. Unlike the
horizontal path, -findGlyph:withHeight: has no numVariants > 0 assertion — with
an empty list it read glyphs[numVariants - 1] = glyphs[-1], an out-of-bounds
stack read (confirmed by AddressSanitizer as a dynamic-stack-buffer-overflow at
MTTypesetter.m:1516). It did not crash without sanitizers because the bogus
glyph value is discarded (glyphAscent/Descent stay 0, forcing the assembly
path), so the defect was latent.
testStretchyVerticalArrowAssemblyOnlyFont renders \left\uparrow \frac{..}{..}
\right\downarrow (and relatives) in XITS, asserting the boundary falls through
to a vertical glyph assembly that covers the content height within the allowed
delimiter shortfall.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Drop the redundant getGlyphWithName: round-trip in getVariantsForGlyph:inDictionary:. The fallback already receives the glyph as a parameter; re-deriving it from its name added an unnecessary lookup, shadowed the parameter, and would return glyph 0 for nameless glyphs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 participant
Problem
iosMathExamplecrashes with an assertion when switching the font and rendering a stretchy construction such as\overrightarrow{x}:Root cause
Two compounding defects:
Empty variant lists (the crash).
MTFontMathTable -getVariantsForGlyph:inDictionary:returned the font's variant list verbatim. Assembly-only glyphs — whose OpenTypeMathGlyphConstructionhas aGlyphAssemblybut zeroMathGlyphVariantRecords — are stored as an emptyh_variantsarray (e.g. XITS's stretchy arrowsuni2190/2192/2194). The lookup returned that empty array, and both-findStretchyVariantGlyph:and-findVariantGlyph:withMaxWidth:assertnumVariants > 0, a false premise for these glyphs.Wrong font actually selected (why it looked like "TeX Gyre Termes"). The iOS example's font-picker
switchwas missing abreakafter the Termes case, so selecting TeX Gyre Termes fell through intoxitsButtonPressedand the app actually switched to XITS — the font whose arrows trigger the crash. Termes itself names its arrows with real variants and never crashed.The bug is font-data driven (identical
.plistdata and CoreText glyph IDs on macOS and iOS), not platform-specific.Fix
getVariantsForGlyph:inDictionary:— fall back to the glyph itself. This restores the "a glyph is always its own variant" invariant the callers depend on, so the stretchy path falls through to the horizontal glyph assembly as intended.breakstatements in the example's font picker so each font selection is honored.Testing
testStretchyArrowAssemblyOnlyFontrenders the stretchy arrows in the XITS font; it reproduced the exact assertion before the fix and passes after.swift test(263 tests, macOS) and the iOS simulator scheme (xcodebuild ... -only-testing:.../testStretchyArrowAssemblyOnlyFont).🤖 Generated with Claude Code