These were actually hurting performance lol, except in the places where
I added the `.always_inline` calls- for some reason if these functions
aren't inlined there it really messes up the top region scrolling
benchmark in vtebench and I'm not entirely certain why...
This PR partially addresses #4504 with a one-liner, all feedback is very
welcome.
### AI disclaimer
I used Claude Code to help navigate the various layers of the rendering
stack, to instrument a ton of intermediate now-deleted log statements,
and ultimately to identify and fix this bug. I directed it
conversationally, gave it experiments to run, audited all of its work,
threw most of it out, and finally landed on this extremely small and
simple change that fixes the issue _for me_ but I think for a lot of
other cases as well. The fix ended up being small, so hopefully it's
easy to review and discuss.
Details:
# Fix horizontal glyph spacing on non-Retina displays
On external monitors (1.0x scale, 72 DPI), text had excessive horizontal
spacing between glyphs. The issue was less noticeable on Retina displays
(2.0x scale, 144 DPI) due to higher pixel density, but the proportional
spacing error was identical.
## Before
Background is ghostty 1.2.3 from homebrew, foregreound is iTerm2:
<img width="862" height="938" alt="Screenshot 2025-10-31 at 3 04 47 PM"
src="https://github.com/user-attachments/assets/feff5279-05cc-4008-b2f5-8ea3b4d6d14b"
/>
## After
Background is ghostty tip + this change, foreground is iTerm2:
<img width="659" height="774" alt="Screenshot 2025-10-31 at 3 00 42 PM"
src="https://github.com/user-attachments/assets/702dc7f8-bb46-43ec-8156-f69d003e8a37"
/>
(my iTerm2 has some custom thickening / brightening you can see; that's
not a part of this change)
## Root Cause
The metrics calculation in Metrics.zig used `@ceil()` to round the cell
width from CoreText's glyph measurements, which created a mismatch
between cell width (which determines glyph positions) and the actual
glyph advances.
At 72 DPI with a 12pt font:
- CoreText returns glyph advance: 7.224609375 pixels
- Cell width was ceiled to: 8 pixels
- Gap per character: 0.78 pixels (~10.8% error)
At 144 DPI with the same font:
- CoreText returns glyph advance: 14.44921875 pixels (exactly 2x)
- Cell width was ceiled to: 15 pixels
- Gap per character: 0.55 pixels (~3.8% error)
The error at high DPI is much better than at low DPI, since the absolute
error is always no more than 1px.
## Fix
Changed `@ceil(face_width)` to `@round(face_width)`. This makes cell
width match the glyph advances better, reducing the error to at most
0.5px:
- 72 DPI: round(7.22) = 7
- 144 DPI: round(14.45) = 14
Height continues using `@ceil()` since vertical space can be slightly
larger without visual issues, but... should it? I'm not sure; this is a
good topic for discussion.
This improves the `clearCells` function since it only has to update once
after clearing all of the individual cells, or not at all if the whole
row was cleared since then it knows for sure that it cleared them all.
This also makes it so that the row style flag is properly tracked when
cells are cleared but not the whole row.
I am so sick and tired of people complaining that the build instructions
on the website are wrong when they clearly haven't realized the difference
between Git-based and tarball-based builds, so here's the extra work to
make sure people actually realize that
Bumps [actions/checkout](https://github.com/actions/checkout) from 5.0.0
to 5.0.1.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a
href="https://github.com/actions/checkout/releases">actions/checkout's
releases</a>.</em></p>
<blockquote>
<h2>v5.0.1</h2>
<h2>What's Changed</h2>
<ul>
<li>Port v6 cleanup to v5 by <a
href="https://github.com/ericsciple"><code>@ericsciple</code></a> in <a
href="https://redirect.github.com/actions/checkout/pull/2301">actions/checkout#2301</a></li>
</ul>
<p><strong>Full Changelog</strong>: <a
href="https://github.com/actions/checkout/compare/v5...v5.0.1">https://github.com/actions/checkout/compare/v5...v5.0.1</a></p>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="93cb6efe18"><code>93cb6ef</code></a>
Cleanup actions/checkout@v6 auth style (<a
href="https://redirect.github.com/actions/checkout/issues/2301">#2301</a>)</li>
<li>See full diff in <a
href="08c6903cd8...93cb6efe18">compare
view</a></li>
</ul>
</details>
<br />
[](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)
Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.
[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)
---
<details>
<summary>Dependabot commands and options</summary>
<br />
You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)
</details>
Bumps
[cachix/install-nix-action](https://github.com/cachix/install-nix-action)
from 31.8.3 to 31.8.4.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a
href="https://github.com/cachix/install-nix-action/releases">cachix/install-nix-action's
releases</a>.</em></p>
<blockquote>
<h2>v31.8.4</h2>
<h2>What's Changed</h2>
<ul>
<li>nix: 2.32.3 -> 2.32.4 by <a
href="https://github.com/github-actions"><code>@github-actions</code></a>[bot]
in <a
href="https://redirect.github.com/cachix/install-nix-action/pull/261">cachix/install-nix-action#261</a></li>
</ul>
<p><strong>Full Changelog</strong>: <a
href="https://github.com/cachix/install-nix-action/compare/v31.8.3...v31.8.4">https://github.com/cachix/install-nix-action/compare/v31.8.3...v31.8.4</a></p>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="0b0e072294"><code>0b0e072</code></a>
Merge pull request <a
href="https://redirect.github.com/cachix/install-nix-action/issues/261">#261</a>
from cachix/create-pull-request/patch</li>
<li><a
href="16d2e3294d"><code>16d2e32</code></a>
nix: 2.32.3 -> 2.32.4</li>
<li>See full diff in <a
href="7ec16f2c06...0b0e072294">compare
view</a></li>
</ul>
</details>
<br />
[](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)
Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.
[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)
---
<details>
<summary>Dependabot commands and options</summary>
<br />
You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)
</details>
Currently, the scroller's appearance is the same as the window's.
So, with the light system appearance and the following config:
```
window-theme = system
macos-titlebar-style = native
```
It's hard to see where the scroller is. This pr changes the
scroller’s(ScrollView) appearance to match the surface's background
colour, so it's always easier to find.
> Changing `verticalScroller?.appearance` doesn't seem to work
<img width="601" height="630" alt="image"
src="https://github.com/user-attachments/assets/9dc18439-9dcb-479a-802a-de439b7dc9d8"
/>
This adds a benchmark and some test coverage for a `screen-clone`
benchmark. This benchmarks the screen cloning which is a hot spot for
lock contention for the renderer + IO threads. I wasn't able to
meaningfully speed this up, but still want to commit this benchmark.
### Problem
Custom icon configuration (`macos-icon = custom-style` with
`macos-icon-screen-color`) stopped working, reverting to the default
icon.
### Root cause
The `ColorList.clone()` method only cloned the `colors` array but not
the `colors_c` array. The Swift code reads from `colors_c` via the C API
(`ghostty_config_get`), so when configs were cloned, the C-accessible
color list was empty.
### Why it broke
This bug was introduced in the original implementation in 29929a473 (Dec
2024), but remained dormant until commit f60bdb0fa (Sep 19, 2025), which
moved the icon-setting `switch` statement into `syncAppearance()`. Since
`syncAppearance()` is called with cloned configs from
`ghosttyConfigDidChange()` (which receives cloned configs from
`Ghostty.App.swift:1639`), the icon code now ran with cloned configs
that had empty `colors_c` arrays.
### Fix
Clone both arrays in `ColorList.clone()`:
```zig
.colors = try self.colors.clone(alloc),
.colors_c = try self.colors_c.clone(alloc), // Added
```
### Testing
- Added ColorList.test.clone test case that verifies both colors and
colors_c arrays are properly cloned
- Verified test fails without the fix (expected 3 colors_c items, found
0)
- Verified test passes with the fix
- Confirmed custom icon now persists correctly with both initial config
load and subsequent config change notifications
### Discussion
I opened a discussion to report this
([9616](https://github.com/ghostty-org/ghostty/discussions/9616)) that
this PR will resolve.
> [!NOTE]
> **LLM Usage Disclosure**
> This bug was investigated and debugged with assistance from Claude
Code. The root cause analysis and fix were developed through interactive
debugging.
This could cause a 0-length hyperlink to be present in the screen,
which, in ReleaseFast, causes a lockup as the string alloc tries to
iterate `1..0` to allocate 0 chunks.
I encountered these bugs while trying to benchmark Ghostty for
performance work.
- Tmux control mode parsing would start accessing deallocated memory
after entering the "broken" state, and worse yet, would cause a
double-free once it was supposed to be deinited.
- Despite our best efforts, CoreText can still produce non-monotonic
(non-ltr) runs. Our renderer code relies on monotonic ltr ordering so in
the rare case where this happens we just sort the buffer before
returning it.
- C1 (8-bit) controls can be executed in certain parser states, so we
need to handle them in the stream's `execute` function. Luckily this was
pretty straightforward since all C1 controls are equivalent to `ESC`
followed by `C1 - 0x40`.
- `Terminal.Screen`'s `cursorScrollDown` function could cause memory
corruption because of `eraseRow` moving the cursor's tracked pin to a
different page. In fixing this, I actually reduced the complexity of
that codepath.
- **Bonus!** Added a nice helper function to `Offset.Slice` so that you
can just do `offset_slice.slice()` instead of
`offset_slice.offset.ptr(base)[0..offset_slice.len]`. Much more
readable.
### `vtebench` before/after
<img width="984" height="691" alt="image"
src="https://github.com/user-attachments/assets/ef20dcc5-d611-4763-9107-355d715a6c0b"
/>
Doesn't seem like any of these changes caused a performance regression.
It was previously possible for `eraseRow` to move the cursor pin to a
different page, and then the call to `cursorChangePin` would try to free
the cursor style from that page even though that's not the page it
belongs to, which creates memory corruption in release modes and
integrity violations or assertions in debug mode.
As a bonus, this should actually be faster this way than the old code,
since it avoids needless work that `cursorChangePin` otherwise does.
These can be unambiguously invoked in certain parser states, and as such
we need to handle them. In real world use they are extremely rare, hence
the branch hint. Without this, we get illegal behavior by trying to cast
the value to the 7-bit C0 enum.
