renderer: a frame the embedder presents itself wakes no main-thread present (sync line) by lawrencecchen · Pull Request #76 · manaflow-ai/ghostty-next

lawrencecchen · 2026-11-04T22:07:36Z

Summary

The same change as renderer: a frame the embedder presents itself wakes no main-thread present #75 (into release/cmux-9ce9b7d0-e2), cherry-picked with -x onto release/cmux-sync-20261104 (the upstream-sync line that cmux PR ghostty-next: sync to upstream Ghostty c7f68a7f4 (Zig 0.17.0), fixes the CVDisplayLinkStop freeze cmux#35478 pins). The ghostty-next pin carries the change whichever cmux PR moves it first.
With GHOSTTY_HOST_PRESENT_EMBEDDER, a frame of the host size kept for the embedder's present, which the layer cannot show in its bounds now, goes to no main-queue block (setSurfaceCallback dropped it there). The skip is decided under the Core Animation lock and then fresh.mutex, only while the kept frame is still that frame (Metal.embedderShowsKeptFrame). All other frames still go through main. See renderer: a frame the embedder presents itself wakes no main-thread present #75 for the full description, the kept-frame rule after a host change, and the four Codex rounds.
The cherry-picks applied with no conflict. IOSurfaceLayer.zig is the same file on both lines; Metal.zig, ghostty.h and NEXT.md differ only outside the changed parts.

Testing

Green on this line (Zig 0.17.0; scratch branch scratch/embedder-wake-sync-green only narrows the workflow's test filter to host present: , never merged): https://github.com/manaflow-ai/ghostty-next/actions/runs/41920331766: 109/109 and lib-vt 97/97.
Red and green on the other line, and the reviews: renderer: a frame the embedder presents itself wakes no main-thread present #75.

Observed / Not run

Observed: the run above; zig fmt --check and zig ast-check with Zig 0.17.0.
Not run: the cmux side (see renderer: a frame the embedder presents itself wakes no main-thread present #75).

Changelog

none

^{Open in [code]smith · Need help on this PR? Tag @codesmith-bot with what you need.}

Summary by cubic

Cherry-picks #75 onto release/cmux-sync-20261104 so the ghostty-next pin carries the fix whichever cmux PR moves it first. When GHOSTTY_HOST_PRESENT_EMBEDDER is on, a frame of the host size kept for the embedder's present no longer wakes the main thread: setSurfaceCallback previously dropped it after dispatching to the main queue.

The skip is decided under the Core Animation lock then fresh.mutex, and only while the kept frame is still that frame; a host change drops it, so a skipped frame never shows for a later equal size (A->B->A).
All other frames — first frames, frames at the shown size, frames for bounds the embedder already set, frames of a size the host no longer has — still go through main.

^{Written for commit c7d97b5. Summary will update on new commits.}

…d present (red) cmux FRAME OWNER S4 (held key resize, 7 moving terminals, cmux24 job c33bd85dbb064376b348d0c2): with GHOSTTY_HOST_PRESENT_EMBEDDER, every frame of a new host size went to the main queue (IOSurfaceLayer.setSurface), where setSurfaceCallback dropped it: the embedder shows that frame itself, with its bounds (Metal.hostFrame). One main-thread wake per moving terminal and frame. HostFrameCounters.main_presents counts the frames Metal.present hands to the main thread's present. Tests (host present:): - a frame of a new host size that the embedder presents: no main present (red: 1), and hostFrame(true) presents it; - a frame for bounds the embedder already set: through main, as before; - a frame of a size the host no longer has, in embedder mode: no main present (red: 1); the same frame by default (renderer mode): through main, as before. (cherry picked from commit 051ecec)

… no longer has, wakes no main-thread present With GHOSTTY_HOST_PRESENT_EMBEDDER, a frame that cannot go to the screen off main went to the main queue (IOSurfaceLayer.setSurface), where setSurfaceCallback dropped it in two cases: - a frame of the host size, kept for the embedder's present (publishFresh), while the layer shows another size in bounds of another size: the embedder shows it with its bounds (hostFrame); - a frame of a size the host no longer has (the host size changes in every refresh of a key resize): the layer never shows it, and the renderer draws the new size next. cmux FRAME OWNER S4 measured one main-thread wake per moving terminal and frame in a held key resize (7 terminals: +0.27 ms of main per motion frame, more missed refreshes with the switch on; cmux24 job c33bd85dbb064376b348d0c2). Metal.present now returns for these frames (Metal.embedderTakesFrame); the check of the shown size and the bounds runs under the Core Animation lock (IOSurfaceLayer.embedderShowsFrame, the same test setSurfaceCallback makes). A first frame (nothing shown), a frame at the shown size (it waits for the embedder's commit) and a frame for bounds the embedder already set still go through main. If the embedder sets the bounds after a frame was kept, the frame waits for its present (READY): ghostty.h says to call again until SHOWN, NONE or HIDDEN, and says so for this case now. Renderer mode is unchanged. (cherry picked from commit bc2ba9d)

… main (Codex review FIX at bc2ba9d, finding 1) The early return for such a frame decided with the host size the completion thread read, without ordering against the main thread's host changes, and the frame is kept nowhere. After A -> B -> A (and the embedder's bounds at A) the main thread's present would have shown it; with the renderer's frames held (synchronized output) nothing else would come for up to the hold's end. In cmux's embedder mode one size is in flight at a time, so these frames are rare and the return saved almost nothing. It is gone: only a frame kept for the embedder's present, checked under the Core Animation lock (IOSurfaceLayer.embedderShowsFrame), skips the main-queue block. The test of that case is now a guard (it expects the main present, in both modes). ghostty.h (finding 6): the frame that waits for a present is one whose completion left its present to the embedder; after giving bounds without a present, call with present true when it returns READY. (cherry picked from commit 3d20928)

…till this one (Codex review FIX at 3d20928) `published` recorded that publishFresh ran, not that the frame is still kept: a host change after it (A -> B -> A) drops the kept frame, and a skip then left the frame nowhere. The skip is now decided in Metal.embedderShowsKeptFrame, under the Core Animation lock and then fresh.mutex (the order the main thread takes them in hostFrame's lock span, so there is no inversion): the layer cannot show the frame in its bounds now (IOSurfaceLayer.embedderShowsFrameLocked) and the kept fresh frame is this surface, at its present sequence and host size generation. A drop after that decision is a host change, whose new size the renderer draws. A declined publish (a newer frame kept) fails the check too. New test: a kept frame that a host change dropped since its publish still goes through main (embedderShowsKeptFrame false after the host went to 160 and back to 120). (cherry picked from commit 17ccbcf)

…e rule; test the new size's frame replaces it Codex review FIX at 17ccbcf: after the completion skipped the main-thread present of a kept frame, a host change A -> B -> A drops it, and the old queued block could have shown that frame for the second A. The fork's rule for kept frames (setHostDrawableSize) says no frame kept from before a host change is the new size's frame, even at an equal size; the renderer draws a new size at once while visible (Thread: resize sets `resized`, drawsNow), and while it holds its frames the layer keeps its last frame (ready-or-keep). embedderShowsKeptFrame's comment and ghostty.h now say so. New test: after A -> B -> A and the embedder's A bounds without a present, hostFrame(false) is PENDING (the layer keeps its last frame, no stale frame), and the renderer's frame of the second A presents itself through main. (cherry picked from commit cd11dc5)

…kept for a present (Codex review SHIP at cd11dc5, nit 2) Comment only. (cherry picked from commit 5f97f63)

coderabbitai · 2026-11-04T22:07:43Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: defaults
Review profile: CHILL
Plan: Advanced
Run ID: 72cc9ed5-6da2-4430-b757-05d3d69568ef

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

Trigger review

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

lawrencecchen · 2026-11-04T22:15:34Z

Review (independent subagent): SHIP at c7d97b5

I found no correctness defect. This PR is the change of #75 on the sync line. The sync line does not change my analysis of #75 (#75 (comment)).

Scope. I compared the two patch series. I also compared the two lines in each file that my #75 analysis uses. I did not build or run tests. I did not read the cmux side.

1. The patch is the same as #75

git range-diff bd7ff2687..5f97f6389 a6b9048e9..c7d97b598: the 6 commits map one to one. The only change is the (cherry picked from commit ...) line in each commit message.
git diff a6b9048e9...c7d97b598 is equal to git diff bd7ff2687...5f97f6389, apart from the index lines and the hunk line numbers. The merge-base of the head and the base is a6b9048.

2. The sync line does not change the analysis

src/renderer/metal/IOSurfaceLayer.zig and src/renderer/metal/Frame.zig are byte-identical on the two heads.
src/renderer/Metal.zig has only two changes. init uses builtin.target.os.tag. The futex calls use fresh.seq.raw as a u32 (Zig 0.17: Metal.zig:534, 642-647, 736). The new code uses no futex. The functions, the lock order and the line numbers are the same as on renderer: a frame the embedder presents itself wakes no main-thread present #75 until presentHostSizedWait. After it, the lines move by +1 (for example, hostFrame is at Metal.zig:660).
src/renderer/Thread.zig and src/renderer/metal/Target.zig have Zig 0.17 changes only. The resize path is the same (Thread.zig:424, 629).
src/renderer/generic.zig: the display link is new, and there are Kitty-image and Zig 0.17 changes. needs_redraw only adds images_rebuilt (generic.zig:2112-2118). Thus the renderer draws in more cases, never in fewer. These parts are the same: setVisible, the frame hold (generic.zig:1633-1636), cells_fresh and size_generation (generic.zig:2244, 2247), and the frame release after the present (generic.zig:2414). The new display_realized = false is in threadExit (teardown).
src/Surface.zig, src/apprt/embedded.zig, include/ghostty.h and NEXT.md do not change the size and occlusion paths, the host-frame C API, hostCounter or the C counters struct.
I checked the A->B->A redraw in more detail on both lines (Codex nit 3 on renderer: a frame the embedder presents itself wakes no main-thread present #75). After a host change, the renderer always draws a new frame while it is visible and its frames are not held. updateFrame calls rebuildCells on each update (generic.zig:1951), unless frames are held (generic.zig:1633-1636). rebuildCells has no early return and sets cells_rebuilt (generic.zig:3045-3370), so drawFrame passes needs_redraw. syncHostedLayer changes the generation before sizeCallback queues the resize (embedded.zig:1149-1152), so the new frame publishes for the current host size.

3. Evidence

Green run 41920331766 is scratch commit 393c1f6. That commit is c7d97b5 with one workflow change: all 80 removed lines are -Dtest-filter= lines, and one host present: filter replaces them. The log shows "zig": "0.17.0", 109/109 tests and lib-vt 97/97.
There is no red run on this line. The tests and the code are the same as on renderer: a frame the embedder presents itself wakes no main-thread present #75, where red run 41915563142 failed as expected.
Zig 0.16.0 fmt --check and ast-check pass on Metal.zig and IOSurfaceLayer.zig. This line needs Zig 0.17.0, so this check is weaker. The 0.17 CI build compiled the two files.
The PR's own build (run 41922449473) was still running when I wrote this comment.

Findings (not blocking; the same as on #75, with the line numbers of this head)

Test gap. IOSurfaceLayer.zig:288-289 (a frame at the shown size gives false) has no test.
- Why it matters: without this check, a published frame at the shown size, with main_pending set and bounds of another size, is skipped. Then hostFrame releases it as SHOWN (Metal.zig:670-678), and its content does not show until the next frame.
- Fix: copy the test at Metal.zig:1235 without hostFrameCommitted(). Expect main_presents == 1 and contents == drawn.
One rule in two places. IOSurfaceLayer.zig:284-295 repeats the embedder drop test of setSurfaceCallback (IOSurfaceLayer.zig:373-381).
- Why it matters: the skip is safe only while the two tests stay equal.
- Fix: put the test in one function that takes the layer object, and call it from both places.
A second CA lock per new-size frame. Metal.zig:505-509 takes the CA lock again after setSurfaceOffMain released it (IOSurfaceLayer.zig:260).
- Why it matters: the completion thread can wait two times while main holds the lock. This can remove a part of the gain.
- Fix (optional, if the cmux A/B shows these waits): do both tests in one locked section, then unlock, then commit.
A race that was there before this PR. present checks the generation (Metal.zig:461-463) before publishFresh takes fresh.mutex. setHostDrawableSize changes the generation after dropFresh, outside the mutex (Metal.zig:296-297; setShownFromMain does the same at Metal.zig:719-720).
- Why it matters: after A->B->A, a frame of the first A can stay kept and show for the second A. This PR sends that frame through main (Metal.zig:513-514), so it does not make the race larger.
- Fix (separate PR): change the generation in the same fresh.mutex section that clears fresh.surface, and check target.size_generation again in publishFresh under the mutex.

Review (independent subagent): SHIP at c7d97b5

Those PRs ported #75, #77 and #79 onto this branch while the sync was open; a force push here briefly replaced them. Keep both lines so every pushed commit stays reachable.

lawrencecchen added 6 commits November 4, 2026 21:53

ghostty.h: the kept-frame rule after a host change, scoped to frames …

c7d97b5

…kept for a present (Codex review SHIP at cd11dc5, nit 2) Comment only. (cherry picked from commit 5f97f63)

lawrencecchen merged commit a97d604 into release/cmux-sync-20261104 Nov 4, 2026
9 checks passed

lawrencecchen mentioned this pull request Nov 5, 2026

renderer: one render target per gesture (ghostty_surface_set_host_capacity) #82

Open

Navigation Menu

Repository navigation

renderer: a frame the embedder presents itself wakes no main-thread present (sync line) - #76

renderer: a frame the embedder presents itself wakes no main-thread present (sync line)#76
lawrencecchen merged 6 commits into
release/cmux-sync-20261104from
fix/embedder-new-size-no-main-wake-sync

lawrencecchen commented Nov 4, 2026 •

edited by cubic-dev-ai Bot

Loading

Uh oh!

coderabbitai Bot commented Nov 4, 2026

Review skipped

Uh oh!

lawrencecchen commented Nov 4, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

lawrencecchen commented Nov 4, 2026 • edited by cubic-dev-ai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Testing

Observed / Not run

Changelog

Summary by cubic

Uh oh!

coderabbitai Bot commented Nov 4, 2026

Review skipped

Uh oh!

lawrencecchen commented Nov 4, 2026

1. The patch is the same as #75

2. The sync line does not change the analysis

3. Evidence

Findings (not blocking; the same as on #75, with the line numbers of this head)

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

lawrencecchen commented Nov 4, 2026 •

edited by cubic-dev-ai Bot

Loading