Skip to content

renderer + embedded: the embedder presents a new host size with its bounds (set_host_present, host_frame) - #70

Merged
lawrencecchen merged 8 commits into
release/cmux-9ce9b7d0-e2from
fix/host-present-embedder
Nov 3, 2026
Merged

lawrencecchen merged 8 commits into
release/cmux-9ce9b7d0-e2from
fix/host-present-embedder

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • cmux-next (frame owner refactor, stage S4 "terminals ready-or-keep") needs this rule for every terminal in every refresh: the view's bounds and the layer's pixels change together, or neither changes. With the renderer showing new host sizes itself, a new-size frame goes to main by dispatch_async, and setSurfaceCallback takes any frame of the host size whatever the view's bounds. In cmux's 120 Hz films the new-size frame landed in the old bounds 2-4 refreshes after the geometry, and two terminals landed 2 refreshes apart. The waiting present (ghostty_surface_present_host_frame, cmux G1) fixed the order but cost main 4-9 ms per drag frame.
  • New, opt-in, macOS host-sized surfaces only (the default does not change; cmux does not call it until its own PR):
    • ghostty_surface_set_host_present(surface, GHOSTTY_HOST_PRESENT_EMBEDDER): a new host size asks for no display pass (a first frame still does), and a display pass never waits. The renderer still draws each new size at once with cells rebuilt for it and keeps that frame (Metal.fresh, now with its present sequence). setSurfaceCallback refuses a frame of another size than the shown one, unless the layer's bounds already have its size. Frames at the shown size still go off main. Back to GHOSTTY_HOST_PRESENT_RENDERER, a frame drawn for the host size shows at once.
    • ghostty_surface_host_frame(surface, present) returns ghostty_host_frame_e: NONE, HIDDEN, SHOWN, READY, PRESENTED, PENDING or HELD. With present, a READY frame becomes the layer's contents in the caller's Core Animation transaction (IOSurfaceLayer.setSurfacePresented), ordered by the present sequence under the CA lock: a frame that reaches main later never replaces it. It never waits and never draws. It is counted in host_present_calls, host_fresh_presents and host_late_presents.
  • publishFresh keeps only the newest frame by present sequence. A host size already on screen disarms publishing, so nothing publishes per frame at rest.
  • Base: release/cmux-9ce9b7d0-e2 (cmux's pin 34e73ba8b + input: one compiled link regex per pattern for the whole process #69), so cmux can pin it without the v1.4.0 / Zig 0.17 merge on main. A port to main is a follow-up.
  • Chromium does the same at a resize: the browser compositor shows a new size only with a frame the renderer submitted for it, and keeps the old surface until then.

Testing

Changelog

none

…ew-size frame first (red)

cmux-next (frame owner refactor S4) changes a terminal view's bounds only in
the Core Animation transaction that shows the frame drawn at the new size, or
keeps both the old bounds and the old frame. Today a host-sized surface shows a
new-size frame itself: setHostDrawableSize asks for a display pass, and the
renderer sends the frame to main, where setSurfaceCallback takes any frame of
the host size, whatever the view's bounds. The frame then lands in the old
bounds in a later main-thread turn.

These tests run on macOS with a real IOSurfaceLayer and real IOSurfaces (no
GPU). Seams with no effect yet: Metal.setEmbedderPresents and
IOSurfaceLayer.setEmbedderPresents (the mode), Metal.hostFrame (the
embedder's query and present; returns .none) and apprt.HostFrame. With the
embedder presenting sizes:
- a new host size asks for no display pass,
- the renderer does not show a frame of a new size; it keeps it as the fresh
  frame,
- a main-thread present of a new size keeps the shown frame,
- hostFrame reports READY, PRESENTED (the frame is the layer's contents in
  the caller's transaction), SHOWN, PENDING (counted late only when it
  presents), HELD and HIDDEN,
- a frame that reaches the main thread late never replaces the frame the
  embedder showed (present sequence).
They fail until the renderer leaves new sizes to the embedder. Controls pass
now and must stay green: the default mode shows a new size as before and asks
for a display pass, a first frame presents itself, a frame at the shown size
still goes off main, a new-size frame that fits bounds the embedder already
set is shown, and going back to the renderer shows a frame drawn for the host
size.
…ounds (ghostty_surface_set_host_present, ghostty_surface_host_frame)

cmux-next (frame owner refactor S4, ready-or-keep): for every terminal in
every refresh, the view's bounds and the layer's pixels change together, or
neither changes. With the renderer showing new sizes itself, a new-size frame
went to main by dispatch_async and setSurfaceCallback took any frame of the
host size, so it landed in the old bounds 2-4 refreshes after the geometry
(cmux films P2). The waiting present (G1) cost main 4-9 ms per drag frame.

Opt-in, macOS host-sized surfaces only; the default does not change.

- ghostty_surface_set_host_present(surface, GHOSTTY_HOST_PRESENT_EMBEDDER):
  setHostDrawableSize asks for no display pass (a first frame still does);
  a display pass never waits (displayWhileEmbedderPresents); the renderer
  still draws each new size at once with cells rebuilt for it and keeps that
  frame (Metal.fresh, now with its present sequence), but setSurfaceCallback
  refuses a frame of another size than the shown one unless the layer's
  bounds already have its size. Frames at the shown size still go off main.
  Back to GHOSTTY_HOST_PRESENT_RENDERER, a frame drawn for the host size is
  shown at once.
- ghostty_surface_host_frame(surface, present) -> ghostty_host_frame_e:
  NONE, HIDDEN, SHOWN, READY, PRESENTED, PENDING or HELD. With present, a
  READY frame becomes the layer's contents in the caller's transaction
  (IOSurfaceLayer.setSurfacePresented), ordered by the present sequence under
  the Core Animation lock, so a frame that reaches main later never replaces
  it. Never waits, never draws. Counted in host_present_calls,
  host_fresh_presents and host_late_presents.
- publishFresh keeps only the newest frame by present sequence; a host size
  already on screen disarms publishing (no per-frame publish at rest).
- ghostty.h parity tests for both enums (lib/enum.zig checkGhosttyHEnum).

Chromium does the same for a resize: the browser compositor shows the new
size only with a frame the renderer submitted for it, and keeps the old
surface until then (fallback surface).
@coderabbitai

coderabbitai Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

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: 832dcfea-4a64-4fdd-a162-811ea57152e4

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

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.

…mes, refused presents, the old present calls (red)

Review of the embedder host present (62df0ad) found three gaps. These
tests fail on 62df0ad:
- an off-main frame at the presented size commits in its own transaction
  while the embedder's present (and the view's new bounds) is not committed
  yet, so a commit can carry the new pixels before the new bounds;
- a present refused because the layer shows a newer frame leaves publishing
  disarmed, so no later frame of the host size can be presented;
- ghostty_surface_present_host_frame(_ready) in the embedder mode still
  presents with setSurfaceSync (no present sequence), so a late frame
  replaces it.
…ead of an embedder present, a refused present re-arms, the old present calls follow it

- IOSurfaceLayer.setSurfaceOffMain: while the layer shows the embedder's
  present (embedder_seq, set by setSurfacePresented), a completed frame does
  not commit in its own transaction, which could reach the screen before the
  embedder's transaction with the view's new bounds. It goes through main
  (setSurface), after that commit; the next frames go off main again.
- Metal.presentFresh: a present refused because the layer shows a newer frame
  re-arms publishing, so the next frame of the host size can be presented.
- Metal.presentHostSizedWait (ghostty_surface_present_host_frame and _ready)
  in the embedder mode is hostFrame(true): no wait, ordered by the present
  sequence.
The default mode does not change.
…ps frames on main until the embedder committed (red)

Second review of the embedder present (5a9907c): the off-main gate opened
again at the first main-thread present after the embedder's. CFRunLoop runs
main-queue blocks before the before-waiting observers, so a frame queued by
the renderer can show in main's open transaction before the embedder's
commit; the next frame then went off main and its own transaction could
reach the screen ahead of the view's new bounds. The same after a frame
shown because the bounds already fit.

Seam with no effect yet: IOSurfaceLayer.setEmbedderCommitted (the embedder's
'my commit is done'). The existing gate test now reports the commit before
it expects frames off main again.
…surface_host_frame_committed)

Second review of the embedder present (5a9907c): the gate keyed on the
embedder's present sequence opened again at the first main-thread present
after it, which can run before the embedder's commit (a main-queue block),
so the next frame could commit off main ahead of the view's new bounds.

Ghostty cannot see the main thread's commit, so the embedder reports it: in
GHOSTTY_HOST_PRESENT_EMBEDDER every main-thread contents change (a present,
a frame shown on main, a sync present) marks the layer main-pending under
the Core Animation lock, and setSurfaceOffMain refuses while it is set;
ghostty_surface_host_frame_committed clears it. Without the call frames go
through main (correct, slower). A mode change clears it. ghostty.h now says
that the first frame's display pass and ghostty_surface_draw wait as they do
by default, and that READY does not reserve the frame.
Review of 63c1398:
- a frame of the shown size shown on main closed the gate, so terminal echo
  after a resize stayed on main until the next commit report (red);
- the mode started with the gate open, though a change the main thread made
  before it may still be in its open transaction (red);
- a sync present (ghostty_surface_draw) of a new size closes the gate
  (coverage, passes now) and one at the shown size keeps it open (red).
The off-main control test and the bounds-fit test now report the
embedder's commit first, since the mode starts closed.
…ts closed, and an off-main commit holds the CA lock

Third review of the embedder present (63c1398):
- An off-main transaction in GHOSTTY_HOST_PRESENT_EMBEDDER now takes
  [CATransaction lock] before begin and commits under it, and commits
  nothing when a check refuses. It can then never run inside the
  embedder's present pass (a layout or display pass of main's commit, which
  holds the lock), so it never ships a present or a frame main set after its
  checks, for this surface or another one in the window. ghostty.h now says
  the embedder presents and sets bounds under the lock or in such a pass.
- Only a change of the shown size (a present, a first frame, a new-size
  frame shown on main or by a sync present) closes the gate; a frame of the
  shown size on main keeps echo off main.
- The mode starts with the gate closed (a change main made before it may be
  in its open transaction); leaving it opens the gate.
The default mode does not change.
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (independent review subagent): SHIP at 9f88e48

Fresh independent review at the exact head (the 4th review of this PR; reviews 1-3 at 62df0ad, 5a9907c and 63c1398 each found bugs, and each was fixed with red tests first). Findings at 9f88e48, none blocking:

  • Risk (follow-up): a mode switch can race a completion that already read the mode without the lock. That completion then commits outside the lock. Fix: under the lock, return false whenever the mode is on. cmux sets the mode once, when it creates the surface, before any frame shows, so it does not hit this.
  • Risk (follow-up, docs): ghostty.h offers "present while holding [CATransaction lock]" as a safe call site. That holds only while the lock is held until the commit. A present inside a Core Animation layout or display pass of the commit is safe. cmux uses that call site (a layer display hook inside the commit). Fix: drop or narrow that wording.
  • Same as base, not changed by this PR: the first-frame display pass on main waits for a free frame and the GPU while it holds the CA lock.
  • RENDERER mode: no behavior change against dfd07de. Compile, ABI and enum parity: no finding. Tests: valid; the 3 new tests fail at 63c1398 for the intended reasons.

Runs (fork workflow, scratch branches never merged):

  • review 3 red beab6a1: run 41638046842, 3 of 3 new tests fail
  • green 9f88e48: run 41638050805, ghostty-test 239/239, lib-vt 475/475, xcframework build, no-gettext check and link smoke pass

Earlier pairs:

@lawrencecchen
lawrencecchen merged commit 635c6c1 into release/cmux-9ce9b7d0-e2 Nov 3, 2026
9 checks passed
lawrencecchen added a commit that referenced this pull request Nov 3, 2026
…lds the Core Animation lock

renderer: an off-main present never commits while it holds the Core Animation lock (embedder-mode deadlock from #70)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant