Constant-driven MTFontManager API + caller migration#210
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors font loading in MTFontManager by removing the three per-font convenience methods (latinModernFontWithSize:, xitsFontWithSize:, and termesFontWithSize:) in favor of a generic fontWithName:size: accessor that uses newly introduced public font name constants. It also adds five new OpenType MATH fonts, updates the documentation, and adapts the example projects and tests. The review feedback highlights that MTFontManager.fontManager() should be used instead of instantiating new MTFontManager objects in Swift code and documentation to ensure caching is not bypassed. Additionally, hardcoded string literals in ViewController.m should be replaced with the new public font name constants.
| func font(size: CGFloat) -> MTFont? { | ||
| MTFontManager().font(withName: fontName, size: size) | ||
| } |
There was a problem hiding this comment.
Using MTFontManager() creates a new instance of MTFontManager every time font(size:) is called. This bypasses the singleton instance and defeats the caching mechanism of nameToFontMap, leading to redundant font loading and parsing overhead. Use the shared fontManager() singleton instead.
| func font(size: CGFloat) -> MTFont? { | |
| MTFontManager().font(withName: fontName, size: size) | |
| } | |
| func font(size: CGFloat) -> MTFont? { | |
| MTFontManager.fontManager().font(withName: fontName, size: size) | |
| } |
References
- Use the shared singleton instance
MTFontManager.fontManager()instead of creating a newMTFontManagerinstance to leverage the built-in caching mechanism and avoid performance degradation from re-parsing font files.
There was a problem hiding this comment.
Fixed in 73e3fc4. Switched to MTFontManager.fontManager() so the shared nameToFontMap cache is reused instead of re-parsing the font on every font(size:) call.
There was a problem hiding this comment.
Correction to the previous reply: MTFontManager.fontManager() does not compile in Swift. Because the Obj-C + (instancetype) fontManager is a factory method whose name matches the class, Swift's Clang importer collapses it into init() and marks fontManager() as unavailable ('fontManager()' has been replaced by 'init()'). So MTFontManager() was actually the only spelling Swift offered — and it resolved to -init, producing a fresh, cache-bypassing instance. The original report was correct on both counts.
Properly fixed in 1956418: fontManager is now a class property (@property (class, readonly, strong) MTFontManager *fontManager;) so Swift imports it as MTFontManager.fontManager (no parens, no init() collapse), and init/new are NS_UNAVAILABLE so the bypass can't be written at all. This call site now uses MTFontManager.fontManager.font(withName:size:).
Verified: swift test (265 tests), the iOS Obj-C example, and the SwiftMathExample app all build.
|
|
||
| ```swift | ||
| label.font = MTFontManager().termesFont(withSize: 20) | ||
| label.font = MTFontManager().font(withName: MTFontNameTermes, size: 20) |
There was a problem hiding this comment.
In Swift, calling MTFontManager() instantiates a new font manager every time, bypassing the caching mechanism of the singleton. It is highly recommended to use the shared fontManager() instance instead to ensure optimal performance.
| label.font = MTFontManager().font(withName: MTFontNameTermes, size: 20) | |
| label.font = MTFontManager.fontManager().font(withName: MTFontNameTermes, size: 20) |
References
- Use the shared singleton instance
MTFontManager.fontManager()instead of creating a newMTFontManagerinstance to leverage the built-in caching mechanism and avoid performance degradation from re-parsing font files.
There was a problem hiding this comment.
Fixed in 73e3fc4. Updated the Swift snippet to MTFontManager.fontManager() (the Obj-C snippet below already used the singleton).
There was a problem hiding this comment.
Correction to the previous reply: the MTFontManager.fontManager() snippet does not compile in Swift — the Obj-C +fontManager factory method is imported as init(), making fontManager() unavailable. Fixed properly in 1956418 by exposing the singleton as a class property; the README Swift snippet now reads MTFontManager.fontManager.font(withName:size:) (no parens), and init/new are NS_UNAVAILABLE so a non-singleton instance can't be constructed.
| static NSString *const kFontKeys[] = { | ||
| @"latinmodern-math", @"texgyretermes-math", @"xits-math", | ||
| }; |
There was a problem hiding this comment.
Since this PR introduces public constants for the font names (MTFontNameLatinModern, MTFontNameTermes, MTFontNameXITS), we should use them here instead of hardcoded string literals to prevent potential mismatch bugs and improve maintainability. Note that because these are extern constants, we should remove the static keyword from the array definition to avoid compiler errors regarding non-constant initializers.
NSString *const kFontKeys[] = {
MTFontNameLatinModern, MTFontNameTermes, MTFontNameXITS,
};There was a problem hiding this comment.
Fixed in 73e3fc4. kFontKeys now uses MTFontNameLatinModern/MTFontNameTermes/MTFontNameXITS, and dropped static since extern const NSString* aren't compile-time-constant initializers. Verified the iOS example target still builds.
6e6fb29 to
8b36f91
Compare
Address PR #210 review: - MathLabel.swift / README: call MTFontManager.fontManager() instead of MTFontManager(), so the shared nameToFontMap cache is reused rather than re-parsing the font file on every font(size:) call. - iOS example: key the picker off the public MTFontName* constants instead of hardcoded loader-key literals. Drops `static` since extern const NSString* are not compile-time-constant array initializers. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
MTFontManager() in Swift bypassed the singleton: the ObjC factory method +fontManager was imported as init(), so MTFontManager() called -init and produced a fresh, cache-bypassing instance, while the explicit fontManager() spelling was marked unavailable. Expose the shared instance as a class property instead of a +fontManager factory method so Swift imports it as MTFontManager.fontManager (no parens, no init() collapse), and mark init/new NS_UNAVAILABLE so the bypass can't be written. The getter now builds the instance via dispatch_once + a private -initPrivate. ObjC callers using [MTFontManager fontManager] are unaffected. Update the Swift callers (MathLabel, README, Swift API tests) to the property form and assert singleton identity. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Goal
Replace the three per-font convenience methods with public
MTFontName*constants + the genericfontWithName:size:(keepingdefaultFont), and migrate every in-repo caller. Breaking public API change.Plan:
docs/plans/2026-05-30-add-math-fonts.md(items 9–14)LLD:
docs/lld/2026-05-30-add-math-fonts.mdStack
Commits
[item 9]Add public MTFontName* font-name constants[item 10]Remove per-font convenience methods; route defaultFont via constant[item 11]Migrate iOS example to fontWithName: + applyFontWithName:[item 12]Migrate SwiftMathExample MathFont to fontWithName: + constants[item 13]Update README usage snippet to fontWithName: + constants[item 14]Note breaking font-API change and new fonts in CHANGELOGBreaking change
latinModernFontWithSize:,xitsFontWithSize:,termesFontWithSize:removed. Callers migrate tofontWithName:size:+MTFontName*constants.defaultFontis unchanged.Testing
265 tests, 0 failures (
swift test). iOS example builds (xcodebuild BUILD SUCCEEDED). SwiftMathExample builds against the new enum.