Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6324f6e470
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
This PR transitions the main event loop from a polling-based approach to an event-driven model, reducing idle CPU usage. The IO thread monitors PTY readiness and process exits via xev, posting SDL user events to wake the main loop on-demand. The main loop now waits with SDL_WaitEventTimeout, rendering only when events occur or UI animations/scroll inertia require it.
Changes:
- Introduced dedicated IO thread running xev loop to watch PTY readiness and process exits
- Replaced busy polling with
SDL_WaitEventTimeoutin main loop with variable wait timeouts - Added window position clamping to ensure spawned windows remain on primary display
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| src/session/state.zig | Added IO watchers for PTY read events and process exits; introduced SDL user event posting |
| src/main.zig | Implemented IO thread with xev loop; converted main loop to event-driven with SDL_WaitEventTimeout; added window position validation |
| src/c.zig | Exported SDL functions for event waiting, event pushing, and display bounds retrieval |
| docs/architecture.md | Updated runtime flow documentation to describe event-driven loop and IO thread architecture |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/main.zig
Outdated
| queue_mutex.lock(); | ||
| defer queue_mutex.unlock(); | ||
| try queue.append(std.heap.page_allocator, session); | ||
| async.notify() catch {}; |
There was a problem hiding this comment.
Async notification failures are silently ignored. If the IO thread cannot be woken, sessions may not spawn correctly. Consider logging these errors.
src/main.zig
Outdated
| if (c.SDL_WaitEventTimeout(&first_event, @as(i32, @intCast(wait_ms)))) { | ||
| processed_event = true; | ||
| // Push back so we can process uniformly with the poll loop below. | ||
| _ = c.SDL_PushEvent(&first_event); | ||
| } |
There was a problem hiding this comment.
The first event is retrieved then immediately pushed back onto the queue, adding unnecessary overhead. Consider processing the first event directly here instead of re-queuing it.
1 participant
Summary
Testing