Messed up character width for some fonts

[lukepistrol]

For some fonts the character width is a bit messed up (i.e. too wide).

SwiftTerm:

Apple's Terminal (iTerm looks the same):

The font used in this example is FiraCode but this happens with a couple of other fonts (Source Code Pro for Powerline, Fira Mono for Powerline) too which look totally normal in other Terminal emulators.

[migueldeicaza]

This is odd, how did you get this setup?

[Hukeqing]

This is odd, how did you get this setup?

This is the issue I raised at this address(https://github.com/CodeEditApp/CodeEdit/issues/1084). @lukepistrol helped me to re-raise it here. You can check the configuration information of the original issue. If you need other help information, you can contact me

[migueldeicaza]

Folks, I could appreciate a complete bug report, something I can reproduce on my end.

Otherwise, I spend hours tracking down third party issues. I am using those fonts and do not have that problem on iOS.

[scrapp08]

Adding onto this, nerd fonts do not seem to render correctly.

What I'm getting:

What is expected:

By the looks of if, the font isn't even the right one.

[jwells89]

I’m seeing this issue with a few fonts as well.

Screenshot taken in my own project, but it’s reproducible in the MacTerminal sample project by:

1. Installing the attached version of the free Cascadia Code font
2. In viewDidLoad() in ViewController.swift, inserting terminal.font = NSFont(name: "Cascadia Code", size: 13)!
3. Running the app

I’m running macOS 14.3.1 (23D60) on an Apple Silicon machine. Let me know if there are any details I’ve missed, and if it would be helpful I can try to reproduce this in a clean VM and an Intel mac to help rule out possibilities.

CascadiaCode.zip

[austincondiff]

I am still seeing this even when I am not using a nerd font. Here I am using SF Mono.

Fonts that are spaced out

• SF Mono (if installed from Apple's developer website - system monospace font design works)
• IBM Plex
• Cascadia Code
• Fira Code
• Range Mono

[migueldeicaza]

Mhm, interesting that this is happening on Mac, as I am using this on iOS just fine.

Thanks for the additional detail.

[austincondiff]

For what it's worth this has been a problem with the integrated terminal in VS Code in the past as you can see in this issue.

Xterm (used by VS Code) has had this issue as well.

[nervenes]

Any update for this?

[austincondiff]

@migueldeicaza any thoughts as to why this is happening? This continues to be a recurring issue that keeps getting brought up by our contributors/users.

[migueldeicaza]

I have not had a chance to look into this, I am sadly juggling a lot of things this summer.

[thecoolwinter]

I'm taking a look into this, it appears to be rooted in how SwiftTerm calculates the cell size for a font on macOS here.

To reproduce, the SF Mono font needs to be installed manually. I'm not sure yet why this causes the problem. SF Mono works fine when not installed manually but returns a much wider width otherwise.

[migueldeicaza]

Oh very good catch - it seems like this would return the largest possible width of any of the characters on the set, which explains the problem, as these fonts likely contain glyphs from languages that use two cells (like Chinese characters, or Emoji).

So probably we need something like the "W" hack that is used for iOS, an option is:

https://developer.apple.com/documentation/appkit/nsfont/2887171-getadvancements?changes=_3

And we would pass all the ascii characters to it, and then get the maximum value out of those - for other characters, there is already special handling in the code for it.

[migueldeicaza]

Could folks try this patch?

diff --git a/Sources/SwiftTerm/Apple/AppleTerminalView.swift b/Sources/SwiftTerm/Apple/AppleTerminalView.swift
index 35ce9ce..db1c404 100644
--- a/Sources/SwiftTerm/Apple/AppleTerminalView.swift
+++ b/Sources/SwiftTerm/Apple/AppleTerminalView.swift
@@ -146,7 +146,8 @@ extension TerminalView {
         let lineLeading = CTFontGetLeading (fontSet.normal)
         let cellHeight = ceil(lineAscent + lineDescent + lineLeading)
         #if os(macOS)
-        let cellWidth = fontSet.normal.maximumAdvancement.width
+        let glyph = font.glyph(withName: "W")
+        cellWidth = fontSet.normal.advancement(forGlyph: glyph)
         #else
         let fontAttributes = [NSAttributedString.Key.font: fontSet.normal]
         let cellWidth = "W".size(withAttributes: fontAttributes).width

[thecoolwinter]

That patch fixes the issue. I did also implement the method you mentioned before with grabbing all ASCII characters and getting the max width. Looks like this:

diff --git a/Sources/SwiftTerm/Apple/AppleTerminalView.swift b/Sources/SwiftTerm/Apple/AppleTerminalView.swift
index 35ce9ce..0eb0900 100644
--- a/Sources/SwiftTerm/Apple/AppleTerminalView.swift
+++ b/Sources/SwiftTerm/Apple/AppleTerminalView.swift
@@ -146,7 +146,16 @@ extension TerminalView {
         let lineLeading = CTFontGetLeading (fontSet.normal)
         let cellHeight = ceil(lineAscent + lineDescent + lineLeading)
         #if os(macOS)
-        let cellWidth = fontSet.normal.maximumAdvancement.width
+        // Grab all non-control ascii characters and get the max width.
+        var sizes = UnsafeMutablePointer<NSSize>.allocate(capacity: 95)
+        let ctFont = (font as CTFont)
+        var glyphs = (32..<127).map { CTFontGetGlyphWithName(ctFont, String(Unicode.Scalar($0)) as CFString) }
+        withUnsafePointer(to: glyphs[0]) { glyphsPtr in
+            fontSet.normal.getAdvancements(NSSizeArray(sizes), forCGGlyphs: glyphsPtr, count: 95)
+        }
+        let cellWidth = (0..<95).reduce(into: 0) { partialResult, idx in
+            partialResult = max(partialResult, sizes[idx].width)
+        }
         #else
         let fontAttributes = [NSAttributedString.Key.font: fontSet.normal]
         let cellWidth = "W".size(withAttributes: fontAttributes).width

I found it didn't make a difference for the calculated with on a few different fonts I tried. It is much slower than grabbing a single glyph (about 0.21 in a rough benchmark vs too small to benchmark).

Bechmark code

I wanted to make sure this wouldn't affect performance. ```zsh % swiftc main.swift -o a -Onone && ./a 0.21186870669999877 ms ``` ```swift import Foundation import AppKit @_optimize(none) public func blackHole(_: some Any) {} var times: [TimeInterval] = [] for _ in 0..<10_000 { let font = NSFont.monospacedSystemFont(ofSize: 12, weight: .regular) var info = mach_timebase_info() guard mach_timebase_info(&info) == KERN_SUCCESS else { exit(1) } let start = mach_absolute_time() let sizes = UnsafeMutablePointer.allocate(capacity: 95) let ctFont = (font as CTFont) let glyphs = (32..<127).map { CTFontGetGlyphWithName(ctFont, String(Unicode.Scalar($0)) as CFString) } withUnsafePointer(to: glyphs[0]) { glyphsPtr in font.getAdvancements(NSSizeArray(sizes), forCGGlyphs: glyphsPtr, count: 95) } let cellWidth = (0..<95).reduce(into: 0) { partialResult, idx in partialResult = max(partialResult, sizes[idx].width) } blackHole(cellWidth) let end = mach_absolute_time() let elapsed = end - start let nanos = elapsed * UInt64(info.numer) / UInt64(info.denom) times.append(TimeInterval(nanos) / TimeInterval(NSEC_PER_MSEC)) } print(times.reduce(0, +) / Double(times.count), "ms") ```

I'd be happy to make either of these into a PR. Do you have a preference for either?

[migueldeicaza]

Let us go with my version then, to avoid the startup penalty.

It might be nice to keep your code in a comment, in case we need it one day.

Thank you!

[thecoolwinter]

Opened that up, was out of town for a few days sorry for the delay!

[migueldeicaza]

Closing