Frame owner S4: terminals ready-or-keep, bounds and pixels in one commit (behind motion.frameOwner.terminals, off)

Merged
LA
2d ago
+1303-20

Summary

FRAME OWNER REFACTOR stage S4: terminals ready-or-keep. In every Core Animation commit, a terminal's bounds and its pixels change together, or both keep their last size. The films (P2) showed the problem: a host-sized Ghostty surface took its new bounds at once, but the frame drawn at that size came 2-4 refreshes later. So those commits showed the old frame in the new bounds, and two terminals landed 2 refreshes apart. The earlier fix, a waiting present (G1), cost 4-9 ms of main-thread time per drag frame.

This change is behind the Debug switch motion.frameOwner.terminals (TerminalDragTunables.framesOwnedByCommit). It is off by default, so one build can film old and new.

With the switch on:

  • TerminalHostView places its surface through TerminalSurfaceView.place(_:anchor:). A new size goes to Ghostty with ghostty_surface_set_size, and the grid report works as before. The view keeps its old size and old frame on the anchor edge of the wanted frame; the host clips it and paints the terminal background beside it. Only one size waits at a time, and the newest size goes next.
  • TerminalFramePresenter (one per window) runs a pass just before Core Animation's commit, while a request waits. The pass is a main run-loop observer at order 1,999,999 that never wakes the run loop. For each waiting terminal it calls ghostty_surface_host_frame(present: true):
    • A drawn frame becomes the layer's contents, and the view and its layer take the new bounds in the same commit.
    • A frame that is not drawn yet keeps both the old bounds and the old frame.
    • The main thread never waits.
  • Requests made in the same refresh form a group and present together for at most 2 refreshes. A request kept for more than 6 refreshes takes its bounds anyway, and this is counted.
  • One refresh after a commit that changed a terminal's shown size, the presenter calls ghostty_surface_host_frame_committed, so the terminal's echo goes to the screen off the main thread again.
  • The callers do not change. Springs ask for the rest size once (restAt). A drag keeps its start size until release. A live window resize stays live.
  • A new display scale (the window moved between a 1x and a 2x display) is the same request: Ghostty gets the new scale and pixel size at once, and the layer keeps the contentsScale of the frame it shows until the presenter shows the frame drawn at the new scale, in the same lock span. The landing's commit is reported like a resize's. (Codex review of b9bafbda1e0.)
  • The lock span holds only the present, the view and layer geometry and leaf work. The copy-mode cursor box (which reads Ghostty under the terminal's lock) follows after the span.

With the switch off, the frame is set at once, as before, and Ghostty is never put in the embedder mode.

Reduce Motion (refactor owner, ROOT-CAUSE.md 6b, STATUS 9359): in a key resize, New Column or Close column, the action's turn lands the layout at once. With the switch on, a terminal whose size changes in that turn does not resize on main: its new size goes to Ghostty, and the surface keeps its old frame (ready-or-keep) until the frame at the new size is drawn. New case: aReduceMotionKeyResizeNeverHoldsTheMainThreadForTheTerminals. It checks the key turn from the press to its commit (median of 6 presses at most 8.33 ms; a sample whose commit boundary was missed is dropped), checks that both terminals' frames come through the presenter on each press with no fallback, checks that no Ghostty display pass or draw runs on main, and checks ready-or-keep in every turn.

Out of scope: creating a NEW terminal in the action's turn (New Column) belongs to TERMINAL CREATE COST, not to S4.

The pin moves in two commits. Each commit moves the URL, the checksum, the ghostty-next gitlink, the notices and docs/ghostty-fork.md together:

  • da28dbcc796: 635c6c12a = the pin 34e73ba8b plus fork PR #69 and fork PR manaflow-ai/ghostty-next#70. #70 adds ghostty_surface_set_host_present, ghostty_surface_host_frame and ghostty_surface_host_frame_committed. It had four independent reviews and four red/green pairs on the fork.
  • b9bafbda1e0: GhosttyNextKit xcframework-7d43bb68b05e1bad433267fe28cd7205041fc4f5-apple-v6 = 635c6c12a plus fork PR manaflow-ai/ghostty-next#71. #71 fixes a deadlock in #70's embedder mode, which the switch-on films found (P0, STATUS 9732). The GPU completion thread committed its Core Animation transaction while it held [CATransaction lock], and main's commit waited for that lock, so the app stopped at the first terminal resize. Now every off-main present does begin, lock, checks, unlock, commit. The default mode already used that order, so nothing changes with the switch off. #71: red run 41694823112, green run 41694827381, Codex SHIP and Claude SHIP at its head 3163f4b73.

The refactor owner decided (STATUS 9071) that #32997 does not hold this pin. #32997 pins dfd07de4a, which this pin already contains.

S2 (#33725) landed as b7aa4eca8d0. The branch merges feat at 0c3f269e809 (with #34198 and the settings-strings autofix, the fixes for the base reds, and S3 #34170).

Testing

Films: the OFF films and the OS smoke ran at c4d64b519d5 (build ada386be, the old pin). The refactor owner films the switch round, OFF against ON, and runs the typing pairs at this head (15c88481831), because the new pin changes the Ghostty binary of every terminal. The films do not exercise a display scale change (the kit does not move a window between a 1x and a 2x display): aNewDisplayScaleLandsWithTheFrameDrawnAtIt is its evidence.

Film verdict (refactor owner pixel-perfect): build 645ad29d = scratch d16d72be16e (30df24f6b9d + #34198; the same S4 code as 47281ea9fb2), switch round ab-s4d-sw-22 on cmux22, 120 Hz 2x, judge v7. 20 of 20 films recorded, motion.frameOwner.terminals OFF then ON in one app, x1 and Reduce Motion, for col drag, row drag, single keys, New Column, close column. ON never hangs: every ON film recorded, and every step after it (the next film, the after-check's real HID Ctrl-C and echo through app, daemon and frame) passed. OFF equals base on the terminal layer (single keys: terminal 27 frames, max 115 px, the same as FEAT DOGFOOD BUILD e0e34bd6; row drag 117 frames at 20 px vs 104 at 20 px). Dropped refreshes OFF/ON per film: 4/3, 1/2, 2/1, 4/6, 1/0. Typing (cmux18, p50/p95/p99/max ms): OFF 3.27/9.07/13.80/20.0 and 3.44/9.87/16.55/41.3; ON 3.30/9.09/12.04/19.0: PASS.

Known with the switch ON, both for the ON PR (the switch stays off here): a row divider drag keeps the old frame longer than OFF lags (content edge off in 156 of 667 frames, max 40 px, vs OFF 117 of 676, max 20 px); and the S3+S4 edge interaction below.

The regression tests are in a new file, TerminalSurfaceCommitSizeTests (CmuxNextAppTests). It uses real MANUAL_MIRROR Ghostty surfaces. After Core Animation's commit, each main run-loop turn samples the layer's bounds in pixels and the IOSurface it shows. The waits use commits and the presenter's own state, never sleeps.

  • aResizeCommitNeverCarriesALayerSizeOtherThanItsIOSurface: 6 drag steps. No turn may end with the layer bounds and the shown frame at different sizes.
  • aReadyFrameIsPresentedInTheCommitThatChangesItsBounds: the first turn with the new bounds shows the frame drawn at that size, and host_fresh_presents grows.
  • aTerminalHiddenWhileASizeWaitsKeepsItsNewestSize: a terminal hidden while a size waits keeps the newest size its host gives it.
  • aNewDisplayScaleLandsWithTheFrameDrawnAtIt: the window moves to the other scale (1x and 2x, through the test seam backingScaleOverride, nil in the app). No turn ends with the layer's bounds x contentsScale and its shown frame at different sizes; the frame comes through the presenter with no fallback; then output goes to the screen off the main thread again within 2 s.
  • aReduceMotionKeyResizeNeverHoldsTheMainThreadForTheTerminals: two terminals, Reduce Motion, 6 presses through PaneResize.press. It checks:
    • the median key turn (from the press to the end of that turn's commit) is at most 8.33 ms;
    • each press presents both terminals' frames, with no fallback;
    • no Ghostty display pass or draw runs on main;
    • every turn is ready-or-keep.

Command: cmux-ci run --class light --script scripts/ci/package-test-lane.sh --arg=suite --arg=Packages/macOS/CmuxNext --arg=TerminalSurfaceCommitSizeTests --arg=DividerDragTerminalSteadyTests,SplitTerminalResizeCountTests,TerminalPaneMotionFinalGridTests,KeyboardResizeContentHoldTests,TerminalHiddenResizeTests,TerminalRevealFrameTests

Red runs:

  • Step 6bb3681d43d848ba158c8f07 at the test commit a6355ecfc5f, run on scratch c226655abe7 (with #33653's one-line fix for the #33578 tip red): 2 of 2 tests fail. 21 turns have "layer 520x300 frame 600x300", and host_fresh_presents stays 0 -> 0.
  • Step c3ade22b26f147130afae2f3 runs the key-turn case with the switch OFF, which is today's behavior: every key turn has "layer 450x600 frame 500x600".
  • Step 32fbe5cb86aebe682c92fbb9 runs the hidden case on the source before the review fixes: the hidden terminal stays at 520 pt, though its host gave it 460.
  • Step 029f257c618059259402eb6e at c4d64b519d5 shows the test race the receipt review found: a press's presents counted one press late. 71b9d2fafca fixes the test.

Green: step a906340dc1ad6f98c89d3f80 at 71b9d2fafca, step 72c6951240c5c3f760413a5e at b9bafbda1e0 (the new pin) step f47ab901aef3cf622f57bb6b at 30df24f6b9d and step 13073b52c8029cdcf2c28977 at the head 47281ea9fb2: all 7 suites pass (TerminalSurfaceCommitSizeTests and 6 terminal resize suites).

The display scale case (Codex review of b9bafbda1e0, BLOCK): red at the test commit 1cc1424477e, step 29b9c75c4d2d07c7a5d79ddf: only aNewDisplayScaleLandsWithTheFrameDrawnAtIt fails, with turns of "layer 1200x600 frame 600x300" and no present through the presenter. Green at the fix 30df24f6b9d: step f47ab901aef3cf622f57bb6b.

The deadlock with the switch on (P0): TerminalFrameOwnerMainThreadTests (display workers only: cmux-ci run --display --label gui-session-v1 --class light --script scripts/ci/package-test-lane.sh --arg=suite --arg=Packages/macOS/CmuxNext --arg=TerminalFrameOwnerMainThreadTests). A real surface in a window on screen; the switch goes on after the first frame; three resizes, each with 2 s of output every 4 ms; a layer reads the terminal layer under [CATransaction lock] in every main commit, as the film recorder does. A sampler pings main every 250 ms; with no answer in 3 s it samples the process and ends it with exit code 70.

  • Red, old pin 635c6c12a: step 2d6513308bb18bf5dff69553 (scratch 73ad54e8a79 = 71b9d2fafca + the test). Main hung in CA::Transaction::flush > commit > commit_transaction; the Metal completion thread hung in Ghostty's present, CA::Transaction::pop > commit > commit_transaction.
  • Green, new pin: step 0b61836c1fb3b5e4fcbaa678 at b9bafbda1e0 (6.1 s, no stall) and step df7d7c86adc1556f1f55b5ec at 30df24f6b9d. That second run took 55 s and its sampler fired once: main stopped for more than 3 s, then came back. The old test passed it quietly, so 4b529209769 makes the case fail on any main stall over 3 s after the switch goes on, and print its phase timings. The stricter case at 30df24f6b9d passed in 6.1 s with no stall (step 0fa5064b4395cd050c0b504f: each resize 2.0 s with 33-36k commits, longest commit wait 24.5 ms, presenter fallbacks 0).
  • At the head 47281ea9fb2: step ea99af46f70d0b028e2b1780.

Observed / Not run

  • Known with the switch ON (Codex at 15c88481831, for the films): S3's key-resize hold (#34170) gives the surface a frame on the hold's still edges (trailing and bottom too), but while a new size waits, S4 keeps the old frame on the wanted frame's leading edge, and on its top edge unless surfaceHold says bottom. So a kept frame can sit on the other edge for the 1-2 refreshes until its new frame lands. The switch is off by default; the fix (carry the hold's edges into place) comes before the ON PR.

  • Observed: the fleet red and green steps above; the fork red and green runs (manaflow-ai/ghostty-next#70 and #71); prepush-gate.py passes; Codex SHIP at 30df24f6b9d, 15c88481831 and the head 47281ea9fb2 (comments on this PR).

  • Not run by me:

    • the 120 Hz switch round, OFF against ON at this head (the refactor owner: drags, keys, keys under Reduce Motion, New Column, Close column);
    • the typing pairs, OFF and ON, at this head (the refactor owner);
    • bench-terminal-resize.py light and STRESS (two OFF runs were cancelled by the fleet; the bench is evidence for the ON PR, not for this OFF landing);
    • macOS 14 and 15 at this head. At ada386be the os-matrix smoke failed the same way as the base 8e149f2e (base findings, STATUS 9647).

  • No capture: the switch is off by default, so the default look does not change. The films are the evidence for the switch.

Changelog

none