Skip to content

Signal completed renderer frames to embedded hosts - #121

Open
lawrencecchen wants to merge 1 commit into
mainfrom
feat-renderer-frame-completed
Open

Signal completed renderer frames to embedded hosts#121
lawrencecchen wants to merge 1 commit into
mainfrom
feat-renderer-frame-completed

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Jul 17, 2026

Copy link
Copy Markdown

Adds an append-only renderer instrumentation event after a healthy GPU frame completes and is presented. Embedded compositors can publish cross-process IOSurfaces without blocking the renderer thread or racing Metal writes.\n\nVerification: zig build test -Dapp-runtime=none -Demit-macos-app=false -Demit-xcframework=false -Dtest-filter="renderer instrumentation"; universal ReleaseFast GhosttyKit build with -Demit-macos-app=false.


View with Codesmith Autofix with Codesmith
Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.


Summary by cubic

Signals a new renderer event when a GPU frame is completed and presented, so embedded hosts can publish IOSurfaces safely without blocking or racing Metal writes. Adds .frame_completed to the C/zig instrumentation and wires it through the renderer.

  • New Features

    • Added GHOSTTY_RENDERER_EVENT_FRAME_COMPLETED / .frame_completed, emitted only after a healthy frame is presented.
    • Plumbed optional instrumentation into Renderer.Options and the backend to fire from GPU completion callbacks.
    • Kept draw_frame_end semantics unchanged (CPU submission completion).
  • Migration

    • Embedders can listen for GHOSTTY_RENDERER_EVENT_FRAME_COMPLETED to publish cross-process surfaces post-presentation.
    • No action needed if you don’t use instrumentation; defaults remain no-op.

Written for commit 827c20f. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • New Features
    • Added a renderer lifecycle event indicating when a frame has completed writing and presentation.
    • Exposed frame-completion notifications for embedded and compositor integrations.
    • Added renderer instrumentation configuration to support GPU completion callbacks.

@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f58e4f91-5f2a-40bc-ac9d-b1e47e823309

📥 Commits

Reviewing files that changed from the base of the PR and between bb30526 and 827c20f.

📒 Files selected for processing (5)
  • include/ghostty.h
  • src/Surface.zig
  • src/renderer/Options.zig
  • src/renderer/generic.zig
  • src/renderer/instrumentation.zig

📝 Walkthrough

Walkthrough

The renderer instrumentation path now carries a callback configuration from Surface.init into Renderer, adds a public frame-completed event identifier, and emits that event after healthy frame completion.

Changes

Frame completion instrumentation

Layer / File(s) Summary
Instrumentation contract and renderer wiring
include/ghostty.h, src/Surface.zig, src/renderer/Options.zig, src/renderer/generic.zig, src/renderer/instrumentation.zig
Adds the FRAME_COMPLETED/frame_completed event, stores instrumentation in renderer options and state, and passes runtime surface instrumentation during initialization.
Healthy frame completion emission
src/renderer/generic.zig
Emits .frame_completed when frameCompleted reports a healthy frame.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Surface
  participant Renderer
  participant GraphicsBackend
  participant InstrumentationCallback
  Surface->>Renderer: pass instrumentation during initialization
  Renderer->>GraphicsBackend: complete frame
  GraphicsBackend->>Renderer: report healthy frame
  Renderer->>InstrumentationCallback: emit frame_completed
Loading

Possibly related PRs

Suggested reviewers: azooz2003-bit, mitchellh

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: signaling completed renderer frames to embedded hosts.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat-renderer-frame-completed

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

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Jul 17, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds a frame_completed instrumentation event that fires inside the Metal command-buffer completion block (and synchronously after gl.finish on OpenGL), after present() but before releaseFrame(). The ordering guarantee — frame still held in the swap chain when the callback fires — lets embedded compositors publish cross-process IOSurfaces with no risk of racing a Metal write into the same buffer.

  • New event plumbing: frame_completed = 4 is added to both the Zig Event enum and the C ghostty_renderer_event_e enum; the existing checkGhosttyHEnum test validates parity automatically.
  • Renderer init ordering fix: renderer_instrumentation initialization is correctly moved before Renderer.init() in Surface.zig so the renderer can store the callback at construction time while still forwarding the same value to Thread.init.
  • Emit site: frameCompleted() in generic.zig emits the event under the existing health == .healthy guard, keeping it distinct from draw_frame_end (which records CPU submission, not GPU completion).

Confidence Score: 5/5

Safe to merge — the change is narrow, the callback fires at the correct point in the swap-chain lifecycle, and the existing enum-parity test enforces C/Zig alignment.

All five changed files are consistent: the new event is plumbed through Options → renderer struct → frameCompleted(), the emit happens before releaseFrame() so the frame is still held, the C header and Zig enum stay in sync, and the Surface.zig reordering is correct. No functional regressions or missing guard conditions were found.

No files require special attention.

Important Files Changed

Filename Overview
src/renderer/instrumentation.zig Adds frame_completed = 4 event to the Event enum; existing test (checkGhosttyHEnum) validates C/Zig enum parity automatically.
include/ghostty.h Adds GHOSTTY_RENDERER_EVENT_FRAME_COMPLETED = 4 to the C enum; stays in sync with the Zig enum as required by the existing enum-parity test.
src/renderer/Options.zig Adds instrumentation field (defaulting to empty) so Renderer.init can receive and store the callback without a separate post-init call.
src/renderer/generic.zig Stores instrumentation by value in the renderer struct and emits frame_completed at the top of frameCompleted() before releaseFrame(), ensuring the swap-chain frame is still held when the callback fires.
src/Surface.zig Moves renderer_instrumentation initialization before Renderer.init (correct, since init now takes it via Options) while still forwarding the same value to Thread.init.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant RT as Renderer Thread
    participant GPU as GPU / Metal CB
    participant FC as frameCompleted()
    participant IC as Instrumentation Callback
    participant SC as SwapChain

    RT->>GPU: commit command buffer (draw_frame_end fired)
    Note over RT: renderer thread returns immediately (async)

    GPU-->>FC: bufferCompleted block (Metal CB thread)
    FC->>FC: determine health from MTLCommandBufferStatus
    FC->>GPU: api.present() [if healthy]
    FC->>FC: emit frame_completed [if healthy]
    FC->>IC: callback(userdata, FRAME_COMPLETED)
    Note over IC: embedded compositor publishes cross-process IOSurface
    FC->>SC: releaseFrame() — frame permit returned
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant RT as Renderer Thread
    participant GPU as GPU / Metal CB
    participant FC as frameCompleted()
    participant IC as Instrumentation Callback
    participant SC as SwapChain

    RT->>GPU: commit command buffer (draw_frame_end fired)
    Note over RT: renderer thread returns immediately (async)

    GPU-->>FC: bufferCompleted block (Metal CB thread)
    FC->>FC: determine health from MTLCommandBufferStatus
    FC->>GPU: api.present() [if healthy]
    FC->>FC: emit frame_completed [if healthy]
    FC->>IC: callback(userdata, FRAME_COMPLETED)
    Note over IC: embedded compositor publishes cross-process IOSurface
    FC->>SC: releaseFrame() — frame permit returned
Loading

Reviews (1): Last reviewed commit: "renderer: signal completed embedded fram..." | Re-trigger Greptile

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