Coalesce renderer visibility transitions by azooz2003-bit · Pull Request #100 · manaflow-ai/ghostty

azooz2003-bit · 2026-07-09T23:04:05Z

Backports cmux renderer visibility suspension to the pinned Ghostty lineage and coalesces rapid hide/show mailbox transitions. Hidden terminals retain dirty rows and rebuild once when revealed. Includes Zig coverage for visibility ordering, dirty-row accumulation, and full-redraw dominance. This dependency PR stays unmerged until the combined cmux tagged build is dogfood-approved.

^{Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.}

Summary by cubic

Coalesces renderer visibility changes, suspends hidden-frame rebuilds, and guarantees reveal draws exactly once with a confirmed submission. Adds optional, content‑free renderer activity callbacks and a reliable retry path when the app mailbox is full.

Refactors
- Coalesces hide/show per drain via VisibilityDrainState; hidden wakes skip updateFrame.
- Reveal: rebuild once and force one draw submission before setVisible; retain the prepared frame if the app mailbox is full and retry after drain.
- Introduces a generation‑guarded visibility retry path; app thread tracks App.redraw_retry_requested and surfaces call Surface.appMailboxDrained() to trigger the retry.
- On drain‑error fallback, reconciles renderer‑visible state with thread flags and commits the deferred transition; normal wake render remains the fallback for backend errors.
- Separates updateFrame and drawFrame, returns draw results, and keeps prepared damage until draw commit (forces full redraw next draw if needed).
New Features
- Adds ghostty_renderer_event_cb to ghostty.h; plumbed via ghostty_surface_config_s.renderer_event_cb, embedded Surface.Options, and renderer.Instrumentation.
- Emits begin/end events for updateFrame/drawFrame on the renderer thread when a backend draw is attempted or an app‑thread submission is accepted; callback must be thread‑safe and non‑blocking.

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

Summary by CodeRabbit

New Features
- Added renderer activity instrumentation events and a surface configuration callback to report update/draw begin/end activity.
- Added a retained “redraw retry” mechanism when redraw requests are rejected due to mailbox capacity.
Bug Fixes
- Improved renderer visibility handling by coalescing rapid hide/show mailbox updates into a single final transition, with reliable regain/retry behavior.
- Preserved and correctly restored terminal dirty state across hidden/visible cycles.
- Prevented unintended draw-loop suppression by ensuring damage commit semantics properly retry on failure.
Tests
- Expanded coverage for visibility coalescing, retained regain cancellation/retry, dirty-flag partial vs full behavior, and mailbox-drained redraw retry.

# Conflicts: # src/renderer/Thread.zig

coderabbitai · 2026-07-09T23:04:29Z

📝 Walkthrough

Walkthrough

Thread now coalesces visibility changes, retains failed regain submissions for retry, and integrates renderer instrumentation and draw recovery. App mailbox rejection signals propagate to surfaces, while terminal and renderer tests cover dirty-state and retry behavior.

Changes

Renderer updates

Layer / File(s)	Summary
Add renderer instrumentation wiring `include/ghostty.h`, `src/renderer/instrumentation.zig`, `src/renderer.zig`, `src/apprt/embedded.zig`, `src/renderer/Thread.zig`	Adds renderer event enums, callbacks, surface configuration, public exports, and thread initialization wiring.
Retain rejected redraw requests `src/App.zig`, `src/Surface.zig`, `src/renderer/Thread.zig`	Records rejected redraw messages and notifies renderer threads after mailbox capacity returns.
Coalesce mailbox visibility transitions `src/renderer/Thread.zig`	Coalesces visibility messages, retains regain submissions for retry, and coordinates wake and draw behavior to avoid duplicate rendering.
Preserve retryable draw state `src/renderer/generic.zig`	Clears rebuilt-cell state only after the draw path reaches its explicit success commit.
Validate dirty and selection state `src/terminal/render.zig`, `src/terminal/Terminal.zig`	Tests partial and full dirty results and validates selection activity transitions without fixed numeric assumptions.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant AppMailbox
  participant App
  participant Surface
  participant RendererThread
  participant Renderer
  AppMailbox->>App: Drain messages and consume redraw retry
  App->>Surface: Notify mailbox capacity is available
  Surface->>RendererThread: Trigger retained visibility retry
  RendererThread->>Renderer: Apply coalesced visibility transition
  Renderer-->>RendererThread: Submit or defer regain render

Possibly related PRs

manaflow-ai/ghostty#80: Related mailbox-draining and visibility-regain behavior in Thread.zig.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
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.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately captures the main change: coalescing renderer visibility transitions.

✨ 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 perf-render-visibility-pr

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 · 2026-07-09T23:05:40Z

Greptile Summary

This PR coalesces renderer visibility work after mailbox drains. The main changes are:

Adds a small visibility drain state helper.
Defers renderer rebuild, draw, and visibility notification until the final drained state.
Keeps hidden render callbacks from consuming dirty terminal state.
Adds Zig tests for coalesced visibility and dirty redraw behavior.

Confidence Score: 4/5

The changed reveal path can show a stale frame when synchronized output is active.

The visibility coalescing path is otherwise localized and preserves message-order flag updates.
Dirty row accumulation is covered by the new tests.
The reveal draw is not forced after an updateFrame skip, so the first visible frame can be stale.

src/renderer/Thread.zig

Important Files Changed

Filename	Overview
src/renderer/Thread.zig	Adds visibility transition coalescing and moves renderer rebuild/draw/setVisible work to the end of mailbox draining.
src/terminal/render.zig	Adds tests covering dirty row accumulation and full redraw precedence before one render update.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Mailbox
    participant Thread
    participant Renderer
    Mailbox->>Thread: drain queued messages
    Thread->>Thread: update visible flag per message
    Thread->>Thread: coalesce final visibility state
    alt final state changed to visible
        Thread->>Renderer: updateFrame()
        Thread->>Renderer: drawFrame(false)
        Thread->>Renderer: setVisible(true)
    else final state changed to hidden
        Thread->>Renderer: setVisible(false)
    end

%%{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 Mailbox
    participant Thread
    participant Renderer
    Mailbox->>Thread: drain queued messages
    Thread->>Thread: update visible flag per message
    Thread->>Thread: coalesce final visibility state
    alt final state changed to visible
        Thread->>Renderer: updateFrame()
        Thread->>Renderer: drawFrame(false)
        Thread->>Renderer: setVisible(true)
    else final state changed to hidden
        Thread->>Renderer: setVisible(false)
    end

_{Reviews (1): Last reviewed commit: "Merge fork main for renderer visibility ..." | Re-trigger Greptile}

greptile-apps · 2026-07-09T23:05:44Z

+                    self.flags.cursor_blink_visible,
+                ) catch |err|
+                    log.warn("error rendering on visibility regain err={}", .{err});
+                self.drawFrame(false);


Synchronized Output Skips Redraw

When a hidden terminal is revealed while synchronized output mode is still enabled, updateFrame can return without rebuilding cells, but this path still calls drawFrame(false). Because that draw is not forced and setVisible(true) runs afterward, the reveal can present the old target instead of a fresh frame until another render-triggering event occurs.

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/terminal/render.zig (1)
1789-1809: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider adding negative assertions for non-dirty rows.

Test 1 asserts dirty[0] and dirty[3] are true but doesn't verify that rows 1, 2, and 4 remain clean. The existing "dirty state" test (line 1784-1785) checks both dirty and clean rows. Adding the same here would strengthen confidence that only the marked rows were rebuilt.
💚 Suggested addition
     try testing.expect(dirty[0]);
     try testing.expect(dirty[3]);
+    try testing.expect(!dirty[1]);
+    try testing.expect(!dirty[2]);
+    try testing.expect(!dirty[4]);
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/terminal/render.zig` around lines 1789 - 1809, Add negative assertions to
the “dirty rows accumulate before one render update” test, verifying that dirty
flags for rows 1, 2, and 4 remain false after the update while rows 0 and 3 are
true.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/renderer/Thread.zig`:
- Around line 627-643: drainMailbox currently rebuilds the frame on visibility
regain, while wakeupCallback then renders it again. Make drainMailbox return a
flag indicating successful regain rendering, have wakeupCallback skip
renderCallback when that flag is set, and add an integration assertion verifying
exactly one update and draw call.

---

Nitpick comments:
In `@src/terminal/render.zig`:
- Around line 1789-1809: Add negative assertions to the “dirty rows accumulate
before one render update” test, verifying that dirty flags for rows 1, 2, and 4
remain false after the update while rows 0 and 3 are true.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8454cf21-dced-4415-86d4-a6e992d0243f

📥 Commits

Reviewing files that changed from the base of the PR and between a410ad7 and 54f80a5.

📒 Files selected for processing (2)

src/renderer/Thread.zig
src/terminal/render.zig

…y-pr # Conflicts: # src/renderer/Thread.zig

azooz2003-bit added 2 commits July 9, 2026 15:36

renderer: suspend hidden frame rebuilds

ae395c5

Merge fork main for renderer visibility backport

54f80a5

# Conflicts: # src/renderer/Thread.zig

greptile-apps Bot reviewed Jul 9, 2026

View reviewed changes

coderabbitai Bot reviewed Jul 9, 2026

View reviewed changes

Comment thread src/renderer/Thread.zig Outdated

azooz2003-bit added 17 commits July 9, 2026 16:12

test: use pinned PageList row access

96b82f4

Merge pinned PageList test compatibility

51c2b82

renderer: avoid duplicate visibility regain render

2f6bb16

Merge exact-once reveal rendering fix

4c814d5

renderer: expose content-free activity events

d4bdeb7

Merge renderer activity instrumentation

a413452

Merge remote-tracking branch 'origin/main' into perf-render-visibilit…

ddb63e6

…y-pr # Conflicts: # src/renderer/Thread.zig

test: cover reveal draw submission failures

228cdec

renderer: require confirmed reveal draw submission

3d134e9

test: require forced visibility regain draw

97d8b86

renderer: force visibility regain submission

530dea5

test: align renderer failure trace semantics

bbf43bb

test: treat selection activity as opaque

804fcec

test: prove reveal survives repeated submission failure

0e2335c

renderer: retain reveals across app queue backpressure

0b1a10f

test: prove draw failures retain prepared damage

1903d99

renderer: retain prepared damage after draw errors

5ba4701

azooz2003-bit mentioned this pull request Jul 10, 2026

Reduce typing latency in large busy workspaces manaflow-ai/cmux#7863

Merged

azooz2003-bit added 5 commits July 10, 2026 17:47

test: expose visibility drift after mailbox errors

a6e1be3

renderer: reconcile visibility after drain errors

de4e06a

test: retain reveal after drain errors

cbc1b20

renderer: retain recovered visibility transitions

5917ad6

Merge VT screen tail export with renderer performance fixes

c66c787

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Coalesce renderer visibility transitions - #100

Coalesce renderer visibility transitions#100
azooz2003-bit wants to merge 24 commits into
mainfrom
perf-render-visibility-pr

azooz2003-bit commented Jul 9, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 9, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

greptile-apps Bot commented Jul 9, 2026

Uh oh!

greptile-apps Bot Jul 9, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

azooz2003-bit commented Jul 9, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by cubic

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

greptile-apps Bot commented Jul 9, 2026

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Sequence Diagram

Uh oh!

greptile-apps Bot Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

azooz2003-bit commented Jul 9, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 9, 2026 •

edited

Loading