From e5133b2ba10085097b69c0ce903f34cf1354d5d5 Mon Sep 17 00:00:00 2001 From: Forketyfork Date: Mon, 26 Oct 2026 13:44:53 +0100 Subject: [PATCH 1/6] fix(ui): repaint after indicators, toasts and overlay animations finish MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue: After Cmd+Enter the hotkey badge's blue circle faded out but the "⌘↵" text stayed on screen until the mouse moved. Other components had the same class of bug: an expired toast, and expand/collapse overlays that finished animating or closed immediately, left their last drawn frame on screen. Solution: The frame loop has no periodic repaint, so a component that stops requesting frames must request one final frame when it leaves a visible state. Expiry and animation completion now flip state in update() and arm the first-frame guard; ExpandingOverlay.settle() reports each finished animation exactly once so every overlay handles it the same way. Glyph text also applies the color's alpha so fading callers actually fade their text. --- CLAUDE.md | 1 + src/font.zig | 4 ++- src/main.zig | 4 +++ src/ui/components/expanding_overlay.zig | 27 +++++++++++++- src/ui/components/fullscreen_overlay.zig | 23 ++++++++++++ src/ui/components/help_overlay.zig | 28 +++++++++++---- src/ui/components/hotkey_indicator.zig | 26 ++++++++++++++ src/ui/components/pr_dropdown.zig | 38 ++++++++++++++------ src/ui/components/recent_folders_overlay.zig | 34 ++++++++++++------ src/ui/components/toast.zig | 27 +++++++++++++- src/ui/components/worktree_overlay.zig | 38 ++++++++++++++------ 11 files changed, 210 insertions(+), 40 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index ea041df9..9cb83cc6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -245,6 +245,7 @@ The `<= len` pattern is only correct when `pos` represents a position *after* pr - Do not store UI state or UI textures in session structs or `app_state.zig`; UI state must live inside UI components or UI-managed assets. - Add new UI features by registering components with `UiRoot`; never bypass `UiAction` for UI→app mutations. - When a UI component moves into a new visible state (modals expanding, toasts appearing, gesture indicators starting), use `src/ui/first_frame_guard.zig`: call `markTransition()` when the state flips and `markDrawn()` at the top of every render; have `wantsFrame` return `guard.wantsFrame()` so the main loop renders immediately even under idle throttling. +- Arm the guard when a component *leaves* a visible state too: an indicator or toast expiring, an expand/collapse settling (`ExpandingOverlay.settle`), an immediate close. Once `wantsFrame` turns false nothing repaints, so the last presented frame — drawn mid-fade or at an intermediate size — stays on screen until unrelated input arrives. A time-based expiry therefore needs an explicit state flip in `update()` that arms the guard; a `wantsFrame` that merely compares `now` against a deadline never gets that final frame. - Restored terminals use `SessionState.ensureSpawnedWithDir` to launch shells already in the target directory; changing cwd of a live shell is not supported—spawn or restart with the desired path. ## Rendering & Unicode Notes diff --git a/src/font.zig b/src/font.zig index cffad830..25ef1056 100644 --- a/src/font.zig +++ b/src/font.zig @@ -689,8 +689,10 @@ pub const Font = struct { /// texture at draw time. Non-emoji glyphs are rasterized in white once and /// tinted per draw via color mod; emoji glyphs are already full-color, so /// their mod is explicitly reset to white to avoid inheriting a stale tint - /// left on the texture object from a prior draw. + /// left on the texture object from a prior draw. Alpha mod is applied to + /// both so fading callers (e.g. the hotkey indicator) fade their text. fn applyColorMod(texture: *c.SDL_Texture, fallback: Fallback, fg_color: c.SDL_Color) void { + _ = c.SDL_SetTextureAlphaMod(texture, fg_color.a); if (fallback == .emoji) { _ = c.SDL_SetTextureColorMod(texture, white.r, white.g, white.b); } else { diff --git a/src/main.zig b/src/main.zig index 6f01a1c9..7285fa4f 100644 --- a/src/main.zig +++ b/src/main.zig @@ -72,7 +72,10 @@ test { _ = @import("ui/components/diff_overlay.zig"); _ = @import("ui/components/dropdown_menu.zig"); _ = @import("ui/components/expanding_overlay.zig"); + _ = @import("ui/components/fullscreen_overlay.zig"); + _ = @import("ui/components/help_overlay.zig"); _ = @import("ui/components/escape_hold.zig"); + _ = @import("ui/components/hotkey_indicator.zig"); _ = @import("ui/components/markdown_parser.zig"); _ = @import("ui/components/markdown_renderer.zig"); _ = @import("ui/components/metrics_overlay.zig"); @@ -89,6 +92,7 @@ test { _ = @import("ui/components/search_utils.zig"); _ = @import("ui/components/selection_agent_overlay.zig"); _ = @import("ui/components/session_interaction.zig"); + _ = @import("ui/components/toast.zig"); _ = @import("ui/components/worktree_overlay.zig"); _ = @import("ui/first_frame_guard.zig"); _ = @import("ui/text_edit.zig"); diff --git a/src/ui/components/expanding_overlay.zig b/src/ui/components/expanding_overlay.zig index 4d258205..2ca524e0 100644 --- a/src/ui/components/expanding_overlay.zig +++ b/src/ui/components/expanding_overlay.zig @@ -81,7 +81,17 @@ pub const ExpandingOverlay = struct { return self.state == .Expanding or self.state == .Collapsing; } - pub fn isComplete(self: *const ExpandingOverlay, now: i64) bool { + /// Moves a finished expand/collapse to `.Open`/`.Closed` and returns the + /// new state, or null while no animation has finished. Callers must request + /// a frame on either result: the last animation frame was drawn at an + /// intermediate size, and nothing else repaints the final one. + pub fn settle(self: *ExpandingOverlay, now: i64) ?State { + if (!self.isAnimating() or !self.isComplete(now)) return null; + self.state = if (self.state == .Expanding) .Open else .Closed; + return self.state; + } + + fn isComplete(self: *const ExpandingOverlay, now: i64) bool { const elapsed = now - self.start_time; return elapsed >= self.duration_ms; } @@ -167,6 +177,21 @@ test "collapsing from the open state starts at full size" { try std.testing.expectEqual(@as(c_int, 40), overlay.currentSize(1200, 1.0)); } +test "settle reports each finished animation exactly once" { + var overlay = ExpandingOverlay.init(0, 20, 40, 400, 200); + try std.testing.expectEqual(@as(?ExpandingOverlay.State, null), overlay.settle(0)); + + overlay.startExpanding(0); + try std.testing.expectEqual(@as(?ExpandingOverlay.State, null), overlay.settle(199)); + try std.testing.expectEqual(@as(?ExpandingOverlay.State, .Open), overlay.settle(200)); + try std.testing.expectEqual(@as(?ExpandingOverlay.State, null), overlay.settle(300)); + + overlay.startCollapsing(1000); + try std.testing.expectEqual(@as(?ExpandingOverlay.State, null), overlay.settle(1199)); + try std.testing.expectEqual(@as(?ExpandingOverlay.State, .Closed), overlay.settle(1200)); + try std.testing.expectEqual(@as(?ExpandingOverlay.State, null), overlay.settle(1300)); +} + test "closeImmediately resets the overlay to its collapsed state" { var overlay = ExpandingOverlay.init(0, 20, 40, 400, 200); overlay.startExpanding(0); diff --git a/src/ui/components/fullscreen_overlay.zig b/src/ui/components/fullscreen_overlay.zig index 1ad227b0..26082956 100644 --- a/src/ui/components/fullscreen_overlay.zig +++ b/src/ui/components/fullscreen_overlay.zig @@ -1,3 +1,4 @@ +const std = @import("std"); const c = @import("../../c.zig"); const geom = @import("../../geom.zig"); const primitives = @import("../../gfx/primitives.zig"); @@ -50,12 +51,15 @@ pub const FullscreenOverlay = struct { self.first_frame.markTransition(); } + /// Each finished animation requests a frame: the last animation frame was + /// drawn at an intermediate scale, and nothing else repaints the final one. pub fn updateAnimation(self: *FullscreenOverlay, now_ms: i64) ?AnimationEvent { const elapsed = now_ms - self.animation_start_ms; switch (self.animation_state) { .opening => { if (elapsed >= animation_duration_ms) { self.animation_state = .open; + self.first_frame.markTransition(); return .became_open; } }, @@ -63,6 +67,7 @@ pub const FullscreenOverlay = struct { if (elapsed >= animation_duration_ms) { self.animation_state = .closed; self.visible = false; + self.first_frame.markTransition(); return .became_closed; } }, @@ -279,3 +284,21 @@ pub const FullscreenOverlay = struct { }); } }; + +test "finished open and close animations each demand a final frame" { + var overlay = FullscreenOverlay{}; + + overlay.show(0); + overlay.first_frame.markDrawn(); + try std.testing.expectEqual(@as(?AnimationEvent, .became_open), overlay.updateAnimation(FullscreenOverlay.animation_duration_ms)); + try std.testing.expect(overlay.wantsFrame()); + overlay.first_frame.markDrawn(); + try std.testing.expect(!overlay.wantsFrame()); + + overlay.hide(1000); + overlay.first_frame.markDrawn(); + try std.testing.expectEqual(@as(?AnimationEvent, .became_closed), overlay.updateAnimation(1000 + FullscreenOverlay.animation_duration_ms)); + try std.testing.expect(overlay.wantsFrame()); + overlay.first_frame.markDrawn(); + try std.testing.expect(!overlay.wantsFrame()); +} diff --git a/src/ui/components/help_overlay.zig b/src/ui/components/help_overlay.zig index 7ffaf243..ce734b3c 100644 --- a/src/ui/components/help_overlay.zig +++ b/src/ui/components/help_overlay.zig @@ -135,13 +135,8 @@ pub const HelpOverlayComponent = struct { fn update(self_ptr: *anyopaque, host: *const types.UiHost, _: *types.UiActionQueue) void { const self: *HelpOverlayComponent = @ptrCast(@alignCast(self_ptr)); - if (self.overlay.isAnimating() and self.overlay.isComplete(host.now_ms)) { - self.overlay.state = switch (self.overlay.state) { - .Expanding => .Open, - .Collapsing => .Closed, - else => self.overlay.state, - }; - if (self.overlay.state == .Open) self.first_frame.markTransition(); + if (self.overlay.settle(host.now_ms) != null) { + self.first_frame.markTransition(); } } @@ -346,3 +341,22 @@ pub const HelpOverlayComponent = struct { .wantsFrame = wantsFrame, }; }; + +test "finished collapse demands a frame to draw the collapsed badge" { + var component = HelpOverlayComponent{ .allocator = std.testing.allocator }; + var host: types.UiHost = undefined; + var actions = types.UiActionQueue.init(std.testing.allocator); + defer actions.deinit(); + + component.overlay.state = .Open; + component.overlay.startCollapsing(0); + component.first_frame.markDrawn(); + + host.now_ms = component.overlay.duration_ms; + HelpOverlayComponent.update(&component, &host, &actions); + try std.testing.expectEqual(ExpandingOverlay.State.Closed, component.overlay.state); + try std.testing.expect(HelpOverlayComponent.wantsFrame(&component, &host)); + + component.first_frame.markDrawn(); + try std.testing.expect(!HelpOverlayComponent.wantsFrame(&component, &host)); +} diff --git a/src/ui/components/hotkey_indicator.zig b/src/ui/components/hotkey_indicator.zig index 6cd2efa4..c6d46188 100644 --- a/src/ui/components/hotkey_indicator.zig +++ b/src/ui/components/hotkey_indicator.zig @@ -63,6 +63,9 @@ pub const HotkeyIndicatorComponent = struct { const elapsed = host.now_ms - self.start_ms; if (elapsed >= display_duration_ms) { self.active = false; + // The last presented frame still shows the indicator; without one + // more frame it stays on screen until something else repaints. + self.first_frame.markTransition(); } } @@ -251,3 +254,26 @@ fn countCodepoints(bytes: []const u8) usize { } return total; } + +test "expired indicator demands one frame to erase its last drawn state" { + var component = HotkeyIndicatorComponent{ .allocator = std.testing.allocator, .font = undefined }; + var host: types.UiHost = undefined; + var actions = types.UiActionQueue.init(std.testing.allocator); + defer actions.deinit(); + + component.show("⌘↵", 0); + component.first_frame.markDrawn(); + + host.now_ms = HotkeyIndicatorComponent.display_duration_ms - 10; + HotkeyIndicatorComponent.update(&component, &host, &actions); + try std.testing.expect(HotkeyIndicatorComponent.wantsFrame(&component, &host)); + component.first_frame.markDrawn(); + + host.now_ms = HotkeyIndicatorComponent.display_duration_ms; + HotkeyIndicatorComponent.update(&component, &host, &actions); + try std.testing.expect(!component.active); + try std.testing.expect(HotkeyIndicatorComponent.wantsFrame(&component, &host)); + + component.first_frame.markDrawn(); + try std.testing.expect(!HotkeyIndicatorComponent.wantsFrame(&component, &host)); +} diff --git a/src/ui/components/pr_dropdown.zig b/src/ui/components/pr_dropdown.zig index bf320eb3..d94c09ac 100644 --- a/src/ui/components/pr_dropdown.zig +++ b/src/ui/components/pr_dropdown.zig @@ -343,17 +343,11 @@ pub const PRDropdownComponent = struct { self.collectFetchResults(); // Advance the expand/collapse animation state machine. - if (self.overlay.isAnimating() and self.overlay.isComplete(host.now_ms)) { - self.overlay.state = switch (self.overlay.state) { - .Expanding => .Open, - .Collapsing => .Closed, - else => self.overlay.state, - }; - if (self.overlay.state == .Open) { - self.first_frame.markTransition(); + if (self.overlay.settle(host.now_ms)) |settled| { + self.first_frame.markTransition(); + if (settled == .Open) { self.flow_animation_start_ms = host.now_ms; - } - if (self.overlay.state == .Closed) { + } else { self.hovered_entry = null; self.flow_animation_start_ms = 0; } @@ -438,6 +432,7 @@ pub const PRDropdownComponent = struct { fn closeOverlayImmediately(self: *PRDropdownComponent) void { self.overlay.closeImmediately(); + self.first_frame.markTransition(); self.search_query.clear(); self.refilter(); self.hovered_entry = null; @@ -842,6 +837,7 @@ test "busy update immediately closes the PR picker" { try testing.expectEqual(ExpandingOverlay.State.Closed, component.overlay.state); try testing.expect(!component.overlay.isAnimating()); + try testing.expect(PRDropdownComponent.wantsFrame(&component, &host)); try testing.expectEqualStrings("", component.search_query.text()); try testing.expectEqual(@as(?usize, null), component.hovered_entry); try testing.expect(component.escape_pressed); @@ -856,3 +852,25 @@ test "busy update immediately closes the PR picker" { PRDropdownComponent.update(&component, &host, &actions); try testing.expectEqual(ExpandingOverlay.State.Closed, component.overlay.state); } + +test "finished collapse demands a frame to draw the collapsed pill" { + var component: PRDropdownComponent = .{ .allocator = testing.allocator, .io = undefined }; + defer component.search_query.deinit(testing.allocator); + + component.is_github_repo = true; + component.overlay.state = .Open; + component.overlay.startCollapsing(0); + component.first_frame.markDrawn(); + + var actions = types.UiActionQueue.init(testing.allocator); + defer actions.deinit(); + var theme = testTheme(); + const host = testHost(component.overlay.duration_ms, false, &theme); + + PRDropdownComponent.update(&component, &host, &actions); + try testing.expectEqual(ExpandingOverlay.State.Closed, component.overlay.state); + try testing.expect(PRDropdownComponent.wantsFrame(&component, &host)); + + component.first_frame.markDrawn(); + try testing.expect(!PRDropdownComponent.wantsFrame(&component, &host)); +} diff --git a/src/ui/components/recent_folders_overlay.zig b/src/ui/components/recent_folders_overlay.zig index 42240caa..27a6caf0 100644 --- a/src/ui/components/recent_folders_overlay.zig +++ b/src/ui/components/recent_folders_overlay.zig @@ -306,6 +306,7 @@ pub const RecentFoldersOverlayComponent = struct { fn closeOverlayImmediately(self: *RecentFoldersOverlayComponent) void { self.overlay.closeImmediately(); + self.first_frame.markTransition(); self.search.clear(); self.refilter(); self.hovered_entry = null; @@ -346,17 +347,11 @@ pub const RecentFoldersOverlayComponent = struct { self.closeOverlayImmediately(); } - if (self.overlay.isAnimating() and self.overlay.isComplete(host.now_ms)) { - self.overlay.state = switch (self.overlay.state) { - .Expanding => .Open, - .Collapsing => .Closed, - else => self.overlay.state, - }; - if (self.overlay.state == .Open) { - self.first_frame.markTransition(); + if (self.overlay.settle(host.now_ms)) |settled| { + self.first_frame.markTransition(); + if (settled == .Open) { self.flow_animation_start_ms = host.now_ms; - } - if (self.overlay.state == .Closed) { + } else { self.hovered_entry = null; self.flow_animation_start_ms = 0; } @@ -1098,6 +1093,7 @@ test "busy update immediately closes the recent folders picker" { try testing.expectEqual(ExpandingOverlay.State.Closed, t.comp.overlay.state); try testing.expect(!t.comp.overlay.isAnimating()); + try testing.expect(RecentFoldersOverlayComponent.wantsFrame(&t.comp, &host)); try testing.expectEqualStrings("", t.comp.search.text()); try testing.expectEqual(@as(?usize, null), t.comp.hovered_entry); try testing.expect(t.comp.escape_pressed); @@ -1111,3 +1107,21 @@ test "busy update immediately closes the recent folders picker" { RecentFoldersOverlayComponent.update(&t.comp, &host, &t.actions); try testing.expectEqual(ExpandingOverlay.State.Closed, t.comp.overlay.state); } + +test "finished collapse demands a frame to draw the collapsed pill" { + var t = TestComponent.init(); + defer t.deinit(); + + addTestFolder(&t); + t.comp.overlay.state = .Open; + t.comp.overlay.startCollapsing(0); + t.comp.first_frame.markDrawn(); + + const host = testHost(t.comp.overlay.duration_ms, &t.theme); + RecentFoldersOverlayComponent.update(&t.comp, &host, &t.actions); + try testing.expectEqual(ExpandingOverlay.State.Closed, t.comp.overlay.state); + try testing.expect(RecentFoldersOverlayComponent.wantsFrame(&t.comp, &host)); + + t.comp.first_frame.markDrawn(); + try testing.expect(!RecentFoldersOverlayComponent.wantsFrame(&t.comp, &host)); +} diff --git a/src/ui/components/toast.zig b/src/ui/components/toast.zig index aaa1149e..e2e1019e 100644 --- a/src/ui/components/toast.zig +++ b/src/ui/components/toast.zig @@ -66,7 +66,14 @@ pub const ToastComponent = struct { return false; } - fn update(_: *anyopaque, _: *const types.UiHost, _: *types.UiActionQueue) void {} + fn update(self_ptr: *anyopaque, host: *const types.UiHost, _: *types.UiActionQueue) void { + const self: *ToastComponent = @ptrCast(@alignCast(self_ptr)); + if (self.active and !self.isVisible(host.now_ms)) { + self.active = false; + // The last presented frame still shows the fading toast. + self.first_frame.markTransition(); + } + } fn wantsFrame(self_ptr: *anyopaque, host: *const types.UiHost) bool { const self: *ToastComponent = @ptrCast(@alignCast(self_ptr)); @@ -249,3 +256,21 @@ pub const ToastComponent = struct { .wantsFrame = wantsFrame, }; }; + +test "expired toast demands one frame to erase its last drawn state" { + var component = ToastComponent{ .allocator = std.testing.allocator }; + var host: types.UiHost = undefined; + var actions = types.UiActionQueue.init(std.testing.allocator); + defer actions.deinit(); + + component.show("done", 0); + component.first_frame.markDrawn(); + + host.now_ms = ToastComponent.notification_duration_ms; + ToastComponent.update(&component, &host, &actions); + try std.testing.expect(!component.active); + try std.testing.expect(ToastComponent.wantsFrame(&component, &host)); + + component.first_frame.markDrawn(); + try std.testing.expect(!ToastComponent.wantsFrame(&component, &host)); +} diff --git a/src/ui/components/worktree_overlay.zig b/src/ui/components/worktree_overlay.zig index 4cf31d70..ad44180e 100644 --- a/src/ui/components/worktree_overlay.zig +++ b/src/ui/components/worktree_overlay.zig @@ -333,17 +333,11 @@ pub const WorktreeOverlayComponent = struct { } } - if (self.overlay.isAnimating() and self.overlay.isComplete(host.now_ms)) { - self.overlay.state = switch (self.overlay.state) { - .Expanding => .Open, - .Collapsing => .Closed, - else => self.overlay.state, - }; - if (self.overlay.state == .Open) { - self.first_frame.markTransition(); + if (self.overlay.settle(host.now_ms)) |settled| { + self.first_frame.markTransition(); + if (settled == .Open) { self.flow_animation_start_ms = host.now_ms; - } - if (self.overlay.state == .Closed) { + } else { self.hovered_entry = null; self.flow_animation_start_ms = 0; } @@ -1150,6 +1144,7 @@ pub const WorktreeOverlayComponent = struct { fn closeOverlayImmediately(self: *WorktreeOverlayComponent) void { self.overlay.closeImmediately(); + self.first_frame.markTransition(); self.destroyCache(); self.hovered_entry = null; self.hovered_remove_btn = null; @@ -1835,6 +1830,7 @@ test "busy transition clears the removal confirmation modal" { try std.testing.expect(component.pending_removal_path == null); try std.testing.expectEqual(ExpandingOverlay.State.Closed, component.overlay.state); try std.testing.expect(!component.overlay.isAnimating()); + try std.testing.expect(WorktreeOverlayComponent.wantsFrame(&component, &host)); try std.testing.expectEqual(@as(i64, 0), component.flow_animation_start_ms); try std.testing.expect(component.escape_pressed); @@ -1850,3 +1846,25 @@ test "busy transition clears the removal confirmation modal" { WorktreeOverlayComponent.update(&component, &host, &actions); try std.testing.expectEqual(ExpandingOverlay.State.Closed, component.overlay.state); } + +test "finished collapse demands a frame to draw the collapsed pill" { + var component: WorktreeOverlayComponent = .{ .allocator = std.testing.allocator, .io = undefined }; + // Busy on both sides skips the cwd refresh, which needs real I/O. + component.focused_busy = true; + component.overlay.state = .Open; + component.overlay.startCollapsing(0); + component.first_frame.markDrawn(); + + var host: types.UiHost = undefined; + host.now_ms = component.overlay.duration_ms; + host.focused_has_foreground_process = true; + var actions = types.UiActionQueue.init(std.testing.allocator); + defer actions.deinit(); + + WorktreeOverlayComponent.update(&component, &host, &actions); + try std.testing.expectEqual(ExpandingOverlay.State.Closed, component.overlay.state); + try std.testing.expect(WorktreeOverlayComponent.wantsFrame(&component, &host)); + + component.first_frame.markDrawn(); + try std.testing.expect(!WorktreeOverlayComponent.wantsFrame(&component, &host)); +} From 1117d5dc88ba41580f239c26ad147ef01e30aeb4 Mon Sep 17 00:00:00 2001 From: Forketyfork Date: Mon, 26 Oct 2026 13:45:13 +0100 Subject: [PATCH 2/6] fix(font): return font sets by value from FontCache.get Issue: The diff overlay (Cmd+D) sometimes opened with its green/red line backgrounds but no text. Solution: FontCache.get returned a pointer into its hash map, and any later lookup of a new size could grow the map and relocate the entry, leaving callers holding a dangling FontSet pointer whose fonts then fail to render. get now returns the FontSet by value; the fonts it holds stay valid until the cache is reset, so copies are safe and callers take the value. --- CLAUDE.md | 1 + docs/ARCHITECTURE.md | 2 +- src/font_cache.zig | 47 +++++++++++++++++--- src/main.zig | 1 + src/ui/components/pr_dropdown_view.zig | 2 +- src/ui/components/reader_overlay.zig | 12 ++--- src/ui/components/recent_folders_overlay.zig | 2 +- src/ui/components/story_overlay.zig | 8 ++-- 8 files changed, 57 insertions(+), 18 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 9cb83cc6..41c8d85f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -258,6 +258,7 @@ The `<= len` pattern is only correct when `pos` represents a position *after* pr - Do not keep TOML parser-owned maps after `result.deinit()`: duplicate keys and values into your own storage before freeing the parser arena, or later iteration will segfault. - `std.mem.span` rejects `[:0]const u8` slices; use `std.mem.sliceTo(ptr, 0)` for `[:0]const u8`, and use `std.mem.span` for `[*c]const u8`/`[*:0]const u8` C pointers. - When copying persisted maps (e.g., `[terminals]`), duplicate both key and value slices; borrowing the parser's backing memory causes use-after-free crashes. +- Pointers from `std.HashMap` `getPtr`/`getOrPut` dangle after any later insert that grows the map. Do not return them from cache-style APIs; return the value (as `FontCache.get` does) or store values behind their own allocation. - Terminal cwd persistence is currently macOS-only; other platforms skip saving/restoring terminals to avoid stale directories until cross-platform cwd tracking is implemented. - xev process watchers keep a pointer to the provided userdata; if you reuse a shared struct for multiple spawns, a late callback can read updated fields and wrongly mark a new session dead. Allocate a per-watcher context, free it on teardown or after the callback, and bump a generation counter on spawn/despawn to ignore stale events. - Restart buttons should only render when a session is both `spawned` and `dead`; broader checks can surface controls for never-spawned slots. diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index aa72586e..a6622a50 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -516,7 +516,7 @@ Rotate: rename active file to architect-.log and continue in new | `wake_pipe.zig` | Non-blocking self-pipe used to wake blocking background-thread polls for PTY registry changes and shutdown | `WakePipe`, `poll_error_backoff_ns` | `posix_util`, std (poll) | | `session/*` (shell, pty, vt_stream, cwd) | Shell spawning, PTY abstraction, VT parsing, working directory detection | `spawn()`, `Pty`, `VtStream.processBytes()`, `getCwd()` | std (posix), ghostty-vt | | `render/renderer.zig` | Scene rendering: terminals, borders, animations, terminal scrollbar painting, first-launch onboarding hint. Exposes the shared focused-border primitive used by the cwd-bar UI component to preserve border z-order after the bar label | `render()`, `RenderCache`, `renderFocusBorder()`, per-session texture management | `font`, `font_cache`, `gfx/*`, `anim/easing`, `app/app_state`, `ui/components/scrollbar`, `c` | -| `font.zig` + `font_cache.zig` | Font rendering, HarfBuzz shaping, glyph LRU cache, shared font cache | `Font`, `openFont()`, `renderGlyph()`, `FontCache`, `getOrCreate()` | `font_paths`, `c` (SDL3_ttf) | +| `font.zig` + `font_cache.zig` | Font rendering, HarfBuzz shaping, glyph LRU cache, shared font cache | `Font`, `openFont()`, `renderGlyph()`, `FontCache`, `get()` (returns a `FontSet` by value) | `font_paths`, `c` (SDL3_ttf) | | `gfx/*` (box_drawing, primitives) | Procedural box-drawing characters (U+2500-U+257F), rounded/thick border helpers, bezier arrow rendering | `renderBoxDrawing()`, `drawRoundedRect()`, `drawThickBorder()`, `fillRoundedRect()`, `renderBezierArrow()` | `c` | | `env.zig`, `clock.zig`, `proc.zig` | Process-environment access, I/O-aware timestamps/sleep, and I/O-aware process execution helpers | `get()`, `now*()`, `sleepNanos()`, `run()`, `spawnDetached()` | std | | `os/open.zig` | Bounded, joinable URL-opening worker owner | `Opener`, `open()`, `deinit()` | `proc`, std | diff --git a/src/font_cache.zig b/src/font_cache.zig index e529fe9f..c939570e 100644 --- a/src/font_cache.zig +++ b/src/font_cache.zig @@ -96,9 +96,13 @@ pub const FontCache = struct { self.generation +%= 1; } - pub fn get(self: *FontCache, size: c_int) Error!*FontSet { + /// Returns the set by value: the map relocates its entries when a new size + /// grows it, so a pointer into it would dangle as soon as the caller fetched + /// another size. The fonts themselves stay valid until `reset` bumps + /// `generation`. + pub fn get(self: *FontCache, size: c_int) Error!FontSet { const regular_path = self.regular_path orelse return error.FontUnavailable; - if (self.fonts.getPtr(size)) |existing| return existing; + if (self.fonts.get(size)) |existing| return existing; const regular = try openRequiredFont(regular_path, size, "regular"); errdefer c.TTF_CloseFont(regular); @@ -143,9 +147,8 @@ pub const FontCache = struct { }; errdefer fonts.close(); - const gop = try self.fonts.getOrPut(size); - gop.value_ptr.* = fonts; - return gop.value_ptr; + try self.fonts.put(size, fonts); + return fonts; } fn releaseFonts(self: *FontCache) void { @@ -203,3 +206,37 @@ pub const FontCache = struct { return std.mem.eql(u8, std.mem.sliceTo(a.?, 0), std.mem.sliceTo(b.?, 0)); } }; + +test "font sets returned by get stay valid after later sizes grow the cache" { + const allocator = std.testing.allocator; + const io = std.testing.io; + + try std.testing.expect(c.TTF_Init()); + defer c.TTF_Quit(); + + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + const tmp_path = try tmp.dir.realPathFileAlloc(io, ".", allocator); + defer allocator.free(tmp_path); + const font_path = try std.fs.path.joinZ(allocator, &.{ tmp_path, "regular.ttf" }); + defer allocator.free(font_path); + { + const file = try std.Io.Dir.createFileAbsolute(io, font_path, .{}); + defer file.close(io); + try file.writeStreamingAll(io, assets.symbols_nerd_font); + } + + var cache = FontCache.init(allocator); + defer cache.deinit(); + cache.setPaths(font_path, null, null, null, null, null, null); + + // Callers routinely fetch a second size (e.g. a title font) while still + // holding the first one; inserting enough sizes forces the map to grow. + const first = try cache.get(10); + var size: c_int = 11; + while (size < 40) : (size += 1) { + _ = try cache.get(size); + } + + try std.testing.expectEqual((try cache.get(10)).regular, first.regular); +} diff --git a/src/main.zig b/src/main.zig index 7285fa4f..014e4380 100644 --- a/src/main.zig +++ b/src/main.zig @@ -53,6 +53,7 @@ test { _ = @import("config.zig"); _ = @import("env.zig"); _ = @import("font.zig"); + _ = @import("font_cache.zig"); _ = @import("gfx/shimmer.zig"); _ = @import("input/mapper.zig"); _ = @import("logging.zig"); diff --git a/src/ui/components/pr_dropdown_view.zig b/src/ui/components/pr_dropdown_view.zig index 86109b09..5ba2d473 100644 --- a/src/ui/components/pr_dropdown_view.zig +++ b/src/ui/components/pr_dropdown_view.zig @@ -392,7 +392,7 @@ pub fn renderOverlay( fn renderLabelHighlights( renderer: *c.SDL_Renderer, host: *const types.UiHost, - entry_fonts: *font_cache_mod.FontSet, + entry_fonts: font_cache_mod.FontSet, label_x: c_int, y_offset: c_int, line_height: c_int, diff --git a/src/ui/components/reader_overlay.zig b/src/ui/components/reader_overlay.zig index c52aeb3c..2d280bfd 100644 --- a/src/ui/components/reader_overlay.zig +++ b/src/ui/components/reader_overlay.zig @@ -1216,7 +1216,7 @@ pub const ReaderOverlayComponent = struct { renderer: *c.SDL_Renderer, host: *const types.UiHost, line: markdown_renderer.RenderLine, - fonts: *FontSet, + fonts: FontSet, runs: []const markdown_renderer.RenderRun, plain_text: []const u8, hovered_href: ?[]const u8, @@ -1321,7 +1321,7 @@ pub const ReaderOverlayComponent = struct { renderer: *c.SDL_Renderer, host: *const types.UiHost, line: markdown_renderer.RenderLine, - fonts: *FontSet, + fonts: FontSet, wrapped_runs: []const WrappedTableRun, x: c_int, y: c_int, @@ -1504,7 +1504,7 @@ pub const ReaderOverlayComponent = struct { lh: c_int, line_idx: usize, line: markdown_renderer.RenderLine, - line_fonts: *FontSet, + line_fonts: FontSet, ) void { const query = std.mem.trim(u8, self.search.text(), " \t"); if (query.len == 0) return; @@ -1541,7 +1541,7 @@ pub const ReaderOverlayComponent = struct { fn byteOffsetToPixelX( line: markdown_renderer.RenderLine, - line_fonts: *FontSet, + line_fonts: FontSet, base_x: c_int, max_text_h: c_int, target_byte: usize, @@ -1640,11 +1640,11 @@ pub const ReaderOverlayComponent = struct { return host.theme.foreground; } - fn chooseFont(fonts: *FontSet, run: markdown_renderer.RenderRun, force_bold: bool) *c.TTF_Font { + fn chooseFont(fonts: FontSet, run: markdown_renderer.RenderRun, force_bold: bool) *c.TTF_Font { return chooseFontForStyle(fonts, run.style, force_bold); } - fn chooseFontForStyle(fonts: *FontSet, style: markdown_parser.InlineStyle, force_bold: bool) *c.TTF_Font { + fn chooseFontForStyle(fonts: FontSet, style: markdown_parser.InlineStyle, force_bold: bool) *c.TTF_Font { const want_bold = force_bold or style == .bold; const want_italic = style == .italic; diff --git a/src/ui/components/recent_folders_overlay.zig b/src/ui/components/recent_folders_overlay.zig index 27a6caf0..5f14923e 100644 --- a/src/ui/components/recent_folders_overlay.zig +++ b/src/ui/components/recent_folders_overlay.zig @@ -537,7 +537,7 @@ pub const RecentFoldersOverlayComponent = struct { _: *RecentFoldersOverlayComponent, renderer: *c.SDL_Renderer, host: *const types.UiHost, - entry_fonts: *font_cache_mod.FontSet, + entry_fonts: font_cache_mod.FontSet, path_x: c_int, y_offset: c_int, lh: c_int, diff --git a/src/ui/components/story_overlay.zig b/src/ui/components/story_overlay.zig index 7296823d..7418fdd8 100644 --- a/src/ui/components/story_overlay.zig +++ b/src/ui/components/story_overlay.zig @@ -824,7 +824,7 @@ pub const StoryOverlayComponent = struct { host: *const types.UiHost, line: markdown_renderer.RenderLine, run: markdown_renderer.RenderRun, - line_fonts: *FontSet, + line_fonts: FontSet, hovered_href: ?[]const u8, x: *c_int, y_pos: c_int, @@ -1011,7 +1011,7 @@ pub const StoryOverlayComponent = struct { lh: c_int, line_idx: usize, line: markdown_renderer.RenderLine, - line_fonts: *FontSet, + line_fonts: FontSet, ) void { const query = std.mem.trim(u8, self.search.text(), " \t"); if (query.len == 0) return; @@ -1054,7 +1054,7 @@ pub const StoryOverlayComponent = struct { fn byteOffsetToPixelX( line: markdown_renderer.RenderLine, - line_fonts: *FontSet, + line_fonts: FontSet, base_x: c_int, max_text_h: c_int, target_byte: usize, @@ -1140,7 +1140,7 @@ pub const StoryOverlayComponent = struct { // --- Utilities --- - fn chooseFontForStyle(fonts: *FontSet, style: markdown_parser.InlineStyle, force_bold: bool) *c.TTF_Font { + fn chooseFontForStyle(fonts: FontSet, style: markdown_parser.InlineStyle, force_bold: bool) *c.TTF_Font { const want_bold = force_bold or style == .bold; const want_italic = style == .italic; From 918720e815c8166bb100fc187be3a1eac76a8740 Mon Sep 17 00:00:00 2001 From: Forketyfork Date: Mon, 26 Oct 2026 13:45:19 +0100 Subject: [PATCH 3/6] perf(diff): build diff line textures only for rows that are drawn Issue: The diff overlay rendered a texture for every line of the diff when it opened, and again after every comment edit, even though only one screen of rows is ever visible at a time. Solution: The cache now holds one optional slot per display row and builds a row's textures the first time that row is drawn. Opening a large diff and invalidating the cache after comment changes now cost one screen of rows instead of the whole diff. A row that fails to build is recorded as empty and logged once rather than retried every frame. --- src/c.zig | 1 + src/ui/components/diff_overlay.zig | 116 +++++++++++++++++++++++++---- 2 files changed, 103 insertions(+), 14 deletions(-) diff --git a/src/c.zig b/src/c.zig index 30ea2a7e..32366501 100644 --- a/src/c.zig +++ b/src/c.zig @@ -17,6 +17,7 @@ pub const SDL_WINDOW_INPUT_FOCUS = c_import.SDL_WINDOW_INPUT_FOCUS; pub const SDL_GetWindowDisplayScale = c_import.SDL_GetWindowDisplayScale; pub const SDL_WINDOW_HIGH_PIXEL_DENSITY = c_import.SDL_WINDOW_HIGH_PIXEL_DENSITY; pub const SDL_CreateRenderer = c_import.SDL_CreateRenderer; +pub const SDL_CreateSoftwareRenderer = c_import.SDL_CreateSoftwareRenderer; pub const SDL_GetRendererName = c_import.SDL_GetRendererName; pub const SDL_DestroyRenderer = c_import.SDL_DestroyRenderer; pub const SDL_SetRenderDrawColor = c_import.SDL_SetRenderDrawColor; diff --git a/src/ui/components/diff_overlay.zig b/src/ui/components/diff_overlay.zig index d4e1dfa3..a45b04d1 100644 --- a/src/ui/components/diff_overlay.zig +++ b/src/ui/components/diff_overlay.zig @@ -118,7 +118,13 @@ const Cache = struct { font_generation: u64, line_height: c_int, title: TextTex, - lines: []LineTexture, + /// Valid until `font_generation` changes, which also invalidates this cache. + mono_font: *c.TTF_Font, + bold_font: *c.TTF_Font, + /// One slot per display row, built the first time the row is drawn: a large + /// diff shows a few dozen rows, and rendering every row up front made it + /// slow to open. + lines: []?LineTexture, }; const GitResult = struct { @@ -1760,21 +1766,13 @@ pub const DiffOverlayComponent = struct { ) catch return null; const line_height_scaled = dpi.scale(line_height, host.ui_scale); - const line_textures = self.allocator.alloc(LineTexture, self.display_rows.items.len) catch { + const line_textures = self.allocator.alloc(?LineTexture, self.display_rows.items.len) catch { c.SDL_DestroyTexture(title_tex.tex); return null; }; - - var idx: usize = 0; - while (idx < self.display_rows.items.len) : (idx += 1) { - line_textures[idx] = self.buildLineTexture(renderer, host, mono_font, bold_font, self.display_rows.items[idx]) catch |err| blk: { - log.warn("failed to build diff line texture: {}", .{err}); - break :blk LineTexture{ .segments = &.{} }; - }; - } + @memset(line_textures, null); const cache = self.allocator.create(Cache) catch { - self.destroyLineTextures(line_textures); c.SDL_DestroyTexture(title_tex.tex); self.allocator.free(line_textures); return null; @@ -1784,6 +1782,8 @@ pub const DiffOverlayComponent = struct { .font_generation = generation, .line_height = line_height_scaled, .title = title_tex, + .mono_font = mono_font, + .bold_font = bold_font, .lines = line_textures, }; self.cache = cache; @@ -2037,8 +2037,35 @@ pub const DiffOverlayComponent = struct { self.cache = null; } - fn destroyLineTextures(self: *DiffOverlayComponent, lines: []LineTexture) void { - for (lines) |line| { + /// A row whose build fails is stored with no segments, so the failure is + /// logged once rather than on every frame the row stays visible. + fn lineTexture(self: *DiffOverlayComponent, renderer: *c.SDL_Renderer, host: *const types.UiHost, cache: *Cache, row_index: usize) LineTexture { + if (cache.lines[row_index]) |built| return built; + const built = self.buildLineTexture(renderer, host, cache.mono_font, cache.bold_font, self.display_rows.items[row_index]) catch |err| blk: { + log.warn("failed to build diff line texture: {}", .{err}); + break :blk LineTexture{ .segments = &.{} }; + }; + cache.lines[row_index] = built; + return built; + } + + fn builtLineCount(self: *const DiffOverlayComponent) usize { + const cache = self.cache orelse return 0; + var count: usize = 0; + for (cache.lines) |line| { + if (line != null) count += 1; + } + return count; + } + + fn lineBuilt(self: *const DiffOverlayComponent, row_index: usize) bool { + const cache = self.cache orelse return false; + return row_index < cache.lines.len and cache.lines[row_index] != null; + } + + fn destroyLineTextures(self: *DiffOverlayComponent, lines: []?LineTexture) void { + for (lines) |maybe_line| { + const line = maybe_line orelse continue; for (line.segments) |segment| { c.SDL_DestroyTexture(segment.tex); } @@ -2187,7 +2214,7 @@ pub const DiffOverlayComponent = struct { } if (row_index >= cache.lines.len) continue; - const line_tex = cache.lines[row_index]; + const line_tex = self.lineTexture(renderer, host, cache, row_index); for (line_tex.segments) |segment| { const tex_alpha: u8 = @intFromFloat(255.0 * alpha); _ = c.SDL_SetTextureAlphaMod(segment.tex, tex_alpha); @@ -3678,3 +3705,64 @@ test "diff overlay text input while the agent dropdown is open does not leak int try std.testing.expectEqual(@as(usize, 0), component.editing.?.input.text().len); try std.testing.expect(component.agent_dropdown.open); } + +test "diff overlay builds line textures only for rows it draws" { + if (@import("builtin").os.tag != .macos) return error.SkipZigTest; + const allocator = std.testing.allocator; + const colors = @import("../../colors.zig"); + const font_cache = @import("../../font_cache.zig"); + + if (!c.TTF_Init()) return error.SkipZigTest; + defer c.TTF_Quit(); + + const surface = c.SDL_CreateSurface(800, 600, c.SDL_PIXELFORMAT_RGBA8888) orelse return error.SkipZigTest; + defer c.SDL_DestroySurface(surface); + const renderer = c.SDL_CreateSoftwareRenderer(surface) orelse return error.SkipZigTest; + defer c.SDL_DestroyRenderer(renderer); + + var fonts = font_cache.FontCache.init(allocator); + defer fonts.deinit(); + fonts.setPaths("/System/Library/Fonts/SFNSMono.ttf", null, null, null, null, null, null); + var assets = types.UiAssets{ .font_cache = &fonts }; + + const base = c.SDL_Color{ .r = 200, .g = 200, .b = 200, .a = 255 }; + const theme = colors.Theme{ + .background = base, + .foreground = base, + .selection = base, + .accent = base, + .palette = [_]c.SDL_Color{base} ** 16, + }; + var host = testUiHost(); + host.theme = &theme; + + var diff: std.ArrayList(u8) = .empty; + defer diff.deinit(allocator); + try diff.appendSlice(allocator, "diff --git a/big.txt b/big.txt\n--- a/big.txt\n+++ b/big.txt\n@@ -0,0 +1,1000 @@\n"); + for (0..1000) |i| { + try diff.print(allocator, "+added line {d}\n", .{i}); + } + + const component = try allocator.create(DiffOverlayComponent); + component.* = .{ + .allocator = allocator, + .io = std.testing.io, + .overlay = .{ .visible = true, .animation_state = .open }, + .agent_dropdown = dropdown_menu.DropdownMenu.init(allocator), + }; + defer component.destroy(renderer); + component.parseDiffOutput(diff.items); + const row_count = component.display_rows.items.len; + try std.testing.expect(row_count > 1000); + + // A 600 px window shows a few dozen rows; building the other ~1000 up + // front is what made large diffs slow to open. + DiffOverlayComponent.renderFn(component, &host, renderer, &assets); + try std.testing.expect(component.lineBuilt(0)); + try std.testing.expect(component.builtLineCount() < 100); + + component.overlay.scroll_offset = component.overlay.max_scroll; + DiffOverlayComponent.renderFn(component, &host, renderer, &assets); + try std.testing.expect(component.lineBuilt(row_count - 1)); + try std.testing.expect(component.builtLineCount() < 200); +} From bccdd0403d38a10c6cfe5891731b284f72181f6a Mon Sep 17 00:00:00 2001 From: Forketyfork Date: Mon, 26 Oct 2026 13:45:27 +0100 Subject: [PATCH 4/6] fix(input): correct stale SDL mouse coordinates on macOS 26+ Issue: At some point the mouse started selecting text about two lines below the pointer, in every session, until Architect restarted; double-clicks hit the wrong word too, while leaving and re-entering fullscreen fixed it. Solution: On macOS 26+, SDL derives mouse motion positions (and the button and wheel positions built from them) from its cached window position. That cache is refreshed only when the window moves, using the main display height at that moment; after a display reconfiguration it can be left off by the difference between the displays' heights. A clearly marked workaround measures the gap between SDL's cached position and the NSWindow's real one and shifts mouse events by it before anything reads them. A test pins the SDL version it was checked against so the next SDL upgrade forces a review. SDL is also bumped to the latest release, 3.4.18, which still has the bug. --- .air/cloud/startup.sh | 4 +- CLAUDE.md | 3 + docs/ARCHITECTURE.md | 1 + docs/development.md | 3 +- flake.nix | 2 +- overlays/{sdl3-3-4-10.nix => sdl3-3-4-18.nix} | 4 +- src/app/runtime.zig | 4 + src/c.zig | 9 + src/main.zig | 1 + src/platform/sdl_mouse_workaround.zig | 269 ++++++++++++++++++ 10 files changed, 294 insertions(+), 6 deletions(-) rename overlays/{sdl3-3-4-10.nix => sdl3-3-4-18.nix} (87%) create mode 100644 src/platform/sdl_mouse_workaround.zig diff --git a/.air/cloud/startup.sh b/.air/cloud/startup.sh index b1f40f8a..e94c8f3d 100755 --- a/.air/cloud/startup.sh +++ b/.air/cloud/startup.sh @@ -1,7 +1,7 @@ #!/usr/bin/env bash # Provision Architect in AIR Cloud without Nix, apt, sudo, or root access. # Zig supplies the C compiler used for the two SDL builds; all other tools are -# portable user-space archives. SDL 3.4.10 is parsed from the project's overlay +# portable user-space archives. SDL 3.4.18 is parsed from the project's overlay # so this remains aligned with the normal development environment. # # This provisions disposable, non-production CI sandboxes rather than release @@ -101,7 +101,7 @@ download_archive_bin just just-1.40.0.tar.gz "https://github.com/casey/just/rele download_archive_bin shellcheck shellcheck-0.10.0.tar.xz "https://github.com/koalaman/shellcheck/releases/download/v0.10.0/shellcheck-v0.10.0.linux.x86_64.tar.xz" 0.10.0 download_archive_bin ruff ruff-0.12.10.tar.gz "https://github.com/astral-sh/ruff/releases/download/0.12.10/ruff-x86_64-unknown-linux-gnu.tar.gz" 0.12.10 -sdl_version="$(sed -n 's/.*version = "\([0-9][0-9.]*\)".*/\1/p' "$project_dir/overlays/sdl3-3-4-10.nix" | head -1)" +sdl_version="$(sed -n 's/.*version = "\([0-9][0-9.]*\)".*/\1/p' "$project_dir/overlays/sdl3-3-4-18.nix" | head -1)" [ -n "$sdl_version" ] || die "could not read SDL3 version from overlay" sdl_ttf_version=3.2.2 build_sdl() { diff --git a/CLAUDE.md b/CLAUDE.md index 41c8d85f..d533666f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -129,6 +129,9 @@ If you are executing in a git worktree, stay within that worktree and do not att ### Never render while the window is occluded - macOS stops compositing fully covered windows and `CAMetalLayer` stops returning drawables; any render attempt then blocks the main thread for the full ~1s `nextDrawable` timeout, stalling input and PTY processing. The frame loop gates rendering on the `SDL_WINDOW_OCCLUDED` window flag (`shouldRenderFrame` in `src/app/runtime.zig`); keep any new render/present paths behind the same gate. +### Mouse coordinates on macOS 26+ +- SDL derives mouse motion, button, and wheel positions from its cached window position, which can go stale after a display reconfiguration and shift every mouse event by the difference between the displays' heights. `src/platform/sdl_mouse_workaround.zig` corrects the events before anything reads them; route new mouse-event consumers through the same event loop so they see corrected coordinates. The workaround is temporary: on every SDL bump, re-check it against SDL's Cocoa sources (the `verified_sdl_version` test fails until you do). + ### Adding New SDL3 Key Codes When adding references to SDL3 key codes (SDLK_*) or other SDL constants, always add them to `src/c.zig` first instead of searching the web for their values. The build system provides the translated SDL declarations through the `c_sdl` module, and SDL constants must be explicitly re-exported from `src/c.zig` to be accessible throughout the codebase. diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index a6622a50..9503d187 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -508,6 +508,7 @@ Rotate: rename active file to architect-.log and continue in new | `app/terminal_history.zig` | Extract focused terminal scrollback + viewport text, strip ANSI escape sequences, convert OSC 133 prompt markers into reader-friendly prompt marker lines, and extract agent session IDs from PTY output for resumption | `extractSessionText()`, `extractTerminalText()`, `stripAnsiAlloc()`, `extractAgentSessionId()`, `buildResumeCommand()` | `session/state`, `ghostty-vt`, std | | `app/*` (app_state, layout, ui_host, grid_nav, grid_layout, input_keys, input_text, terminal_actions, worktree) | Application logic decomposed by concern: state enums, grid sizing, UI snapshot building, navigation, input encoding, clipboard and submitted-paste construction, worktree commands (with configurable external directory and post-create init) | `ViewMode`, `AnimationState`, `SessionStatus`, `buildUiHost()`, `applyTerminalResize()`, `encodeKey()`, `pasteText()`, `buildSubmittedPaste()`, `clearTerminal()`, `resolveWorktreeDir()` | `geom`, `anim/easing`, `ui/types`, `ui/session_view_state`, `colors`, `input/mapper`, `session/state`, `c` | | `platform/sdl.zig` | SDL3 initialization, window management, HiDPI | `init()`, `createWindow()`, `createRenderer()` | `c` | +| `platform/sdl_mouse_workaround.zig` | Temporary workaround for SDL on macOS 26+: corrects mouse event coordinates when SDL's cached window position goes stale after a display reconfiguration. Remove once SDL fixes it upstream | `MousePositionFix`, `applyOffset()`, `verified_sdl_version` | `c`, AppKit/CoreGraphics via the Objective-C runtime | | `input/mapper.zig` | SDL key adaptation, explicit compatibility bindings, shortcut detection; delegates protocol encoding to ghostty-vt | `encodeKeyWithMod()`, `KeyEncodingOptions`, shortcut helpers | `c`, `ghostty-vt` | | `c.zig` | SDL3 and SDL3_ttf FFI re-exports from the build-system `c_sdl` module | `SDLK_*`, `SDL_*`, `TTF_*` re-exports | `c_sdl`, SDL3 system libraries | | `session/state.zig` | Terminal session lifecycle: PTY, ghostty-vt, process watcher, foreground agent detection, graceful agent teardown at quit, and main-thread ring-buffer consumption | `SessionState`, `AgentKind`, `init()`, `despawn()`, `deinit()`, `ensureSpawnedWithDir()`, `processOutput()`, `render_epoch`, `pending_write`, `detectForegroundAgent()`, `sendTermToForegroundPgrp()` | `shell`, `pty`, `pty_reader`, `vt_stream`, `cwd`, `font`, xev | diff --git a/docs/development.md b/docs/development.md index 554e0ffb..eff49ad1 100644 --- a/docs/development.md +++ b/docs/development.md @@ -71,7 +71,8 @@ zig build run -- --log-dir .tmp/architect-debug-logs - **ghostty-vt** is fetched as a pinned tarball via the Zig package manager (`build.zig.zon`). - **Zwanzig v0.15.1** is pinned as a Zig build dependency and runs as a host-targeted `ReleaseFast` build tool through `zig build lint`. Architect passes its requested target architecture and operating system to Zwanzig for target-aware analysis. -- **SDL3** and **SDL3_ttf** are provided by Nix. SDL3 is pinned to 3.4.10 via `overlays/sdl3-3-4-10.nix` with binaries cached in the public `forketyfork` Cachix to avoid rebuilds. +- **SDL3** and **SDL3_ttf** are provided by Nix. SDL3 is pinned to 3.4.18 via `overlays/sdl3-3-4-18.nix` with binaries cached in the public `forketyfork` Cachix to avoid rebuilds. + When bumping SDL, re-check the workaround in `src/platform/sdl_mouse_workaround.zig` against the new SDL's Cocoa sources; its `verified_sdl_version` test fails until you either delete the workaround or bump the constant. SDL3 and the platform C APIs are translated at build time with Zig's built-in `addTranslateC` steps using the small header shims under `src/c/`. When SDL is diff --git a/flake.nix b/flake.nix index a8bc777b..574a3212 100644 --- a/flake.nix +++ b/flake.nix @@ -23,7 +23,7 @@ pkgs = import nixpkgs { inherit system; overlays = [ - (import ./overlays/sdl3-3-4-10.nix) + (import ./overlays/sdl3-3-4-18.nix) ]; config = { allowUnfree = true; diff --git a/overlays/sdl3-3-4-10.nix b/overlays/sdl3-3-4-18.nix similarity index 87% rename from overlays/sdl3-3-4-10.nix rename to overlays/sdl3-3-4-18.nix index 126b29fb..e4f72e1f 100644 --- a/overlays/sdl3-3-4-10.nix +++ b/overlays/sdl3-3-4-18.nix @@ -1,12 +1,12 @@ final: prev: { sdl3 = prev.sdl3.overrideAttrs (old: rec { - version = "3.4.10"; + version = "3.4.18"; src = prev.fetchFromGitHub { owner = "libsdl-org"; repo = "SDL"; rev = "release-${version}"; - hash = "sha256-6Dph2eLiJUmpQzPWe8EuY5LrWhrFwde2f2dwfgCcWNw="; + hash = "sha256-3dP4FJ5OvZI6UPykJpE3hTH6kdW+Nn7gUSf3yiPCobo="; }; # Drop nixpkgs' Linux-only zenity substitutions that no longer match SDL 3.4.x. diff --git a/src/app/runtime.zig b/src/app/runtime.zig index d160f7be..cac28dbc 100644 --- a/src/app/runtime.zig +++ b/src/app/runtime.zig @@ -26,6 +26,7 @@ const session_state = @import("../session/state.zig"); const view_state = @import("../ui/session_view_state.zig"); const platform = @import("../platform/sdl.zig"); const macos_input = @import("../platform/macos_input_source.zig"); +const sdl_mouse_workaround = @import("../platform/sdl_mouse_workaround.zig"); const input = @import("../input/mapper.zig"); const renderer_mod = @import("../render/renderer.zig"); const pty_mod = @import("../pty.zig"); @@ -1606,6 +1607,8 @@ pub fn run(allocator: std.mem.Allocator, io: std.Io, log_dir_override: ?[]const }; } + var mouse_position_fix = sdl_mouse_workaround.MousePositionFix.init(sdl.window); + const renderer = sdl.renderer; var font_size: c_int = persistence.font_size; @@ -1917,6 +1920,7 @@ pub fn run(allocator: std.mem.Allocator, io: std.Io, log_dir_override: ?[]const c.SDL_EVENT_KEY_DOWN, c.SDL_EVENT_KEY_UP, c.SDL_EVENT_TEXT_INPUT, c.SDL_EVENT_MOUSE_BUTTON_DOWN, c.SDL_EVENT_MOUSE_BUTTON_UP, c.SDL_EVENT_MOUSE_WHEEL, c.SDL_EVENT_MOUSE_MOTION => last_input_ns = frame_start_ns, else => {}, } + mouse_position_fix.correct(sdl.window, &event); var scaled_event = layout.scaleEventToRender(&event, scale_x, scale_y); if (builtin.os.tag == .macos and scaled_event.type == c.SDL_EVENT_KEY_DOWN) { const key = scaled_event.key.key; diff --git a/src/c.zig b/src/c.zig index 32366501..f4645ce9 100644 --- a/src/c.zig +++ b/src/c.zig @@ -5,12 +5,21 @@ const c_import = @import("c_sdl"); pub const SDL_Init = c_import.SDL_Init; pub const SDL_Quit = c_import.SDL_Quit; +pub const SDL_InitSubSystem = c_import.SDL_InitSubSystem; +pub const SDL_QuitSubSystem = c_import.SDL_QuitSubSystem; pub const SDL_EnableScreenSaver = c_import.SDL_EnableScreenSaver; pub const SDL_CreateWindow = c_import.SDL_CreateWindow; pub const SDL_DestroyWindow = c_import.SDL_DestroyWindow; pub const SDL_SetWindowPosition = c_import.SDL_SetWindowPosition; pub const SDL_GetWindowSizeInPixels = c_import.SDL_GetWindowSizeInPixels; pub const SDL_GetWindowSize = c_import.SDL_GetWindowSize; +pub const SDL_GetWindowPosition = c_import.SDL_GetWindowPosition; +pub const SDL_GetWindowProperties = c_import.SDL_GetWindowProperties; +pub const SDL_GetPointerProperty = c_import.SDL_GetPointerProperty; +pub const SDL_PROP_WINDOW_COCOA_WINDOW_POINTER = c_import.SDL_PROP_WINDOW_COCOA_WINDOW_POINTER; +pub const SDL_SyncWindow = c_import.SDL_SyncWindow; +pub const SDL_WINDOW_HIDDEN = c_import.SDL_WINDOW_HIDDEN; +pub const SDL_GetVersion = c_import.SDL_GetVersion; pub const SDL_GetWindowFlags = c_import.SDL_GetWindowFlags; pub const SDL_WINDOW_OCCLUDED = c_import.SDL_WINDOW_OCCLUDED; pub const SDL_WINDOW_INPUT_FOCUS = c_import.SDL_WINDOW_INPUT_FOCUS; diff --git a/src/main.zig b/src/main.zig index 014e4380..47c746fd 100644 --- a/src/main.zig +++ b/src/main.zig @@ -60,6 +60,7 @@ test { _ = @import("metrics.zig"); _ = @import("pty.zig"); _ = @import("platform/sdl.zig"); + _ = @import("platform/sdl_mouse_workaround.zig"); _ = @import("os/open.zig"); _ = @import("proc.zig"); _ = @import("render/renderer.zig"); diff --git a/src/platform/sdl_mouse_workaround.zig b/src/platform/sdl_mouse_workaround.zig new file mode 100644 index 00000000..0820a165 --- /dev/null +++ b/src/platform/sdl_mouse_workaround.zig @@ -0,0 +1,269 @@ +// zwanzig-disable: identifier-style +//! WORKAROUND for an SDL bug on macOS 26+. Re-check on every SDL upgrade: +//! the `verified_sdl_version` test fails until someone does. +//! +//! On macOS 26 and later, SDL computes every mouse motion position as the +//! global mouse position minus its cached window position (`mouseMoved:` in +//! src/video/cocoa/SDL_cocoawindow.m), and button and wheel events reuse that +//! position. The cached position is only refreshed by `windowDidMove:`, which +//! converts Cocoa coordinates using the main display's height at that moment. +//! When the displays reconfigure (sleep/wake, docking, a monitor reconnecting) +//! the main display can briefly change, and a move notification taken then +//! stores a position that is off by the difference between the two displays' +//! heights. SDL refreshes its main display height afterwards but never the +//! window position, so every mouse event lands that far from the pointer until +//! the window moves again (e.g. by leaving and re-entering fullscreen). +//! +//! The fix here measures how far SDL's cached window position is from the real +//! one, read from the NSWindow the same way `windowDidMove:` does, and shifts +//! the coordinates of mouse events by that amount. +//! +//! Remove this file once SDL recomputes window positions after the main display +//! changes (`Cocoa_DisplayReconfigurationCallback` in SDL_cocoamodes.m), or once +//! `mouseMoved:` stops deriving positions from the cached window position. +const std = @import("std"); +const builtin = @import("builtin"); +const c = @import("../c.zig"); + +const log = std.log.scoped(.sdl_mouse_workaround); + +const is_macos = builtin.os.tag == .macos; + +/// The SDL release whose Cocoa sources this workaround was checked against, +/// in `SDL_GetVersion()` encoding. +pub const verified_sdl_version: c_int = 3_004_018; + +/// How far real window-relative mouse coordinates are from the ones SDL +/// reports, in window points. +pub const Offset = struct { + x: f32, + y: f32, + + pub const zero: Offset = .{ .x = 0, .y = 0 }; + + /// SDL subtracts its cached window origin from the global mouse position, + /// so the correction is the cached origin minus the real one. + pub fn between(sdl_origin: Point, real_origin: Point) Offset { + return .{ .x = sdl_origin.x - real_origin.x, .y = sdl_origin.y - real_origin.y }; + } + + pub fn isZero(self: Offset) bool { + return self.x == 0 and self.y == 0; + } +}; + +pub const Point = struct { x: f32, y: f32 }; + +/// Shifts the window-relative coordinates of the mouse events SDL derives from +/// its cached window position. Drop events use window-local coordinates and are +/// left alone. +pub fn applyOffset(event: *c.SDL_Event, offset: Offset) void { + switch (event.type) { + c.SDL_EVENT_MOUSE_MOTION => { + event.motion.x += offset.x; + event.motion.y += offset.y; + }, + c.SDL_EVENT_MOUSE_BUTTON_DOWN, c.SDL_EVENT_MOUSE_BUTTON_UP => { + event.button.x += offset.x; + event.button.y += offset.y; + }, + c.SDL_EVENT_MOUSE_WHEEL => { + event.wheel.mouse_x += offset.x; + event.wheel.mouse_y += offset.y; + }, + else => {}, + } +} + +fn isAffectedEvent(event_type: u32) bool { + return switch (event_type) { + c.SDL_EVENT_MOUSE_MOTION, c.SDL_EVENT_MOUSE_BUTTON_DOWN, c.SDL_EVENT_MOUSE_BUTTON_UP, c.SDL_EVENT_MOUSE_WHEEL => true, + else => false, + }; +} + +/// Hand-written AppKit/CoreGraphics bindings, for the same reasons given in +/// `macos_input_source.zig`. +const appkit = if (is_macos) struct { + pub const NSRect = extern struct { x: f64, y: f64, w: f64, h: f64 }; + pub const NSOperatingSystemVersion = extern struct { major: isize, minor: isize, patch: isize }; + + pub extern fn objc_getClass(name: [*:0]const u8) callconv(.c) ?*anyopaque; + pub extern fn sel_registerName(name: [*:0]const u8) callconv(.c) ?*anyopaque; + pub extern fn objc_msgSend() callconv(.c) void; + // x86_64 returns structs larger than 16 bytes through a hidden pointer, + // which shifts the receiver and selector, so it needs the _stret entry + // point. arm64 has no such variant. + extern fn objc_msgSend_stret() callconv(.c) void; + pub const msg_send_rect_entry = if (builtin.cpu.arch == .x86_64) &objc_msgSend_stret else &objc_msgSend; + + pub extern fn CGMainDisplayID() callconv(.c) u32; + pub extern fn CGDisplayPixelsHigh(display: u32) callconv(.c) usize; + + pub const MsgSendId = *const fn (?*anyopaque, ?*anyopaque) callconv(.c) ?*anyopaque; + pub const MsgSendRect = *const fn (?*anyopaque, ?*anyopaque) callconv(.c) NSRect; + pub const MsgSendRectRect = *const fn (?*anyopaque, ?*anyopaque, NSRect) callconv(.c) NSRect; + pub const MsgSendVersionBool = *const fn (?*anyopaque, ?*anyopaque, NSOperatingSystemVersion) callconv(.c) u8; +} else struct {}; + +pub const MousePositionFix = if (is_macos) struct { + nswindow: ?*anyopaque = null, + sel_frame: ?*anyopaque = null, + sel_content_rect: ?*anyopaque = null, + logged_offset: Offset = Offset.zero, + + /// Inactive below macOS 26, where SDL takes mouse positions from the + /// event's window-local location and the cached position does not matter. + pub fn init(window: *c.SDL_Window) MousePositionFix { + if (!sdlUsesCachedWindowPosition()) return .{}; + return initForWindow(window); + } + + fn initForWindow(window: *c.SDL_Window) MousePositionFix { + const props = c.SDL_GetWindowProperties(window); + const nswindow = c.SDL_GetPointerProperty(props, c.SDL_PROP_WINDOW_COCOA_WINDOW_POINTER, null) orelse { + log.warn("no NSWindow for the SDL window: {s}; mouse positions stay uncorrected", .{c.SDL_GetError()}); + return .{}; + }; + const sel_frame = appkit.sel_registerName("frame"); + const sel_content_rect = appkit.sel_registerName("contentRectForFrameRect:"); + if (sel_frame == null or sel_content_rect == null) { + log.warn("failed to register NSWindow selectors; mouse positions stay uncorrected", .{}); + return .{}; + } + return .{ .nswindow = nswindow, .sel_frame = sel_frame, .sel_content_rect = sel_content_rect }; + } + + pub fn correct(self: *MousePositionFix, window: *c.SDL_Window, event: *c.SDL_Event) void { + if (self.nswindow == null or !isAffectedEvent(event.type)) return; + const offset = self.currentOffset(window) orelse return; + if (offset.x != self.logged_offset.x or offset.y != self.logged_offset.y) { + log.info("SDL window position is off by ({d}, {d}) points; correcting mouse coordinates", .{ offset.x, offset.y }); + self.logged_offset = offset; + } + applyOffset(event, offset); + } + + fn currentOffset(self: *const MousePositionFix, window: *c.SDL_Window) ?Offset { + var sdl_x: c_int = 0; + var sdl_y: c_int = 0; + if (!c.SDL_GetWindowPosition(window, &sdl_x, &sdl_y)) { + log.warn("SDL_GetWindowPosition failed: {s}", .{c.SDL_GetError()}); + return null; + } + const real = self.realContentOrigin(); + return Offset.between( + .{ .x = @floatFromInt(sdl_x), .y = @floatFromInt(sdl_y) }, + real, + ); + } + + /// The content area's top-left corner in SDL's global coordinates, + /// computed like `windowDidMove:` but with the current main display height. + fn realContentOrigin(self: *const MousePositionFix) Point { + const msg_send_rect = @as(appkit.MsgSendRect, @ptrCast(appkit.msg_send_rect_entry)); + const msg_send_rect_rect = @as(appkit.MsgSendRectRect, @ptrCast(appkit.msg_send_rect_entry)); + const frame = msg_send_rect(self.nswindow, self.sel_frame); + const content = msg_send_rect_rect(self.nswindow, self.sel_content_rect, frame); + const main_height: f64 = @floatFromInt(appkit.CGDisplayPixelsHigh(appkit.CGMainDisplayID())); + return .{ + .x = @floatCast(content.x), + .y = @floatCast(main_height - content.y - content.h), + }; + } + + /// Mirrors SDL's `@available(macOS 26.0, *)` guard in `mouseMoved:`. + fn sdlUsesCachedWindowPosition() bool { + const class = appkit.objc_getClass("NSProcessInfo") orelse return false; + const sel_process_info = appkit.sel_registerName("processInfo") orelse return false; + const sel_at_least = appkit.sel_registerName("isOperatingSystemAtLeastVersion:") orelse return false; + const msg_send_id = @as(appkit.MsgSendId, @ptrCast(&appkit.objc_msgSend)); + const process_info = msg_send_id(class, sel_process_info) orelse return false; + const msg_send_version = @as(appkit.MsgSendVersionBool, @ptrCast(&appkit.objc_msgSend)); + return msg_send_version(process_info, sel_at_least, .{ .major = 26, .minor = 0, .patch = 0 }) != 0; + } +} else struct { + pub fn init(window: *c.SDL_Window) MousePositionFix { + _ = window; + return .{}; + } + + pub fn correct(self: *MousePositionFix, window: *c.SDL_Window, event: *c.SDL_Event) void { + _ = self; + _ = window; + _ = event; + } +}; + +test "verified_sdl_version matches the linked SDL" { + // Fails on every SDL upgrade on purpose: re-read the Cocoa sources named at + // the top of this file, then delete the workaround or bump the constant. + try std.testing.expectEqual(verified_sdl_version, c.SDL_GetVersion()); +} + +test "offset is SDL's cached window origin minus the real one" { + // The stale state seen in practice: a 1080 pt display above a 1117 pt main + // display, with SDL holding the position computed while the 1080 pt one was main. + const offset = Offset.between(.{ .x = -94, .y = -1117 }, .{ .x = -94, .y = -1080 }); + try std.testing.expectEqual(@as(f32, 0), offset.x); + try std.testing.expectEqual(@as(f32, -37), offset.y); + + // SDL reported global (-94 + 500, -1080 + 300) - (-94, -1117) = (500, 337) + // for a pointer that is 300 pt below the window's real top edge. + var motion: c.SDL_Event = std.mem.zeroes(c.SDL_Event); + motion.type = c.SDL_EVENT_MOUSE_MOTION; + motion.motion.x = 500; + motion.motion.y = 337; + applyOffset(&motion, offset); + try std.testing.expectEqual(@as(f32, 500), motion.motion.x); + try std.testing.expectEqual(@as(f32, 300), motion.motion.y); +} + +test "applyOffset corrects button and wheel positions but not drops" { + const offset: Offset = .{ .x = 3, .y = -37 }; + + var button: c.SDL_Event = std.mem.zeroes(c.SDL_Event); + button.type = c.SDL_EVENT_MOUSE_BUTTON_DOWN; + button.button.x = 10; + button.button.y = 100; + applyOffset(&button, offset); + try std.testing.expectEqual(@as(f32, 13), button.button.x); + try std.testing.expectEqual(@as(f32, 63), button.button.y); + + var wheel: c.SDL_Event = std.mem.zeroes(c.SDL_Event); + wheel.type = c.SDL_EVENT_MOUSE_WHEEL; + wheel.wheel.mouse_x = 10; + wheel.wheel.mouse_y = 100; + wheel.wheel.y = 1; + applyOffset(&wheel, offset); + try std.testing.expectEqual(@as(f32, 13), wheel.wheel.mouse_x); + try std.testing.expectEqual(@as(f32, 63), wheel.wheel.mouse_y); + try std.testing.expectEqual(@as(f32, 1), wheel.wheel.y); + + var drop: c.SDL_Event = std.mem.zeroes(c.SDL_Event); + drop.type = c.SDL_EVENT_DROP_POSITION; + drop.drop.x = 10; + drop.drop.y = 100; + applyOffset(&drop, offset); + try std.testing.expectEqual(@as(f32, 10), drop.drop.x); + try std.testing.expectEqual(@as(f32, 100), drop.drop.y); +} + +test "real content origin matches SDL's window position while SDL is in sync" { + if (!is_macos) return error.SkipZigTest; + // Keep the test process from showing up in the Dock or taking focus. + _ = c.SDL_SetHint("SDL_MAC_BACKGROUND_APP", "1"); + if (!c.SDL_InitSubSystem(c.SDL_INIT_VIDEO)) return error.SkipZigTest; + defer c.SDL_QuitSubSystem(c.SDL_INIT_VIDEO); + + const window = c.SDL_CreateWindow("mouse workaround test", 320, 200, c.SDL_WINDOW_HIDDEN) orelse + return error.SkipZigTest; + defer c.SDL_DestroyWindow(window); + try std.testing.expect(c.SDL_SetWindowPosition(window, 120, 140)); + _ = c.SDL_SyncWindow(window); + + var fix = MousePositionFix.initForWindow(window); + try std.testing.expect(fix.nswindow != null); + const offset = fix.currentOffset(window) orelse return error.TestUnexpectedResult; + try std.testing.expectEqual(Offset.zero, offset); +} From 4193adacc2eb415e3041843f13156cc52d1e5b03 Mon Sep 17 00:00:00 2001 From: Forketyfork Date: Mon, 26 Oct 2026 13:57:44 +0100 Subject: [PATCH 5/6] fix(build): add an rpath for SDL3_ttf on non-macOS targets Issue: The Linux CI test binary failed to start with "error while loading shared libraries: libSDL3_ttf.so.0" once a test that runs on Linux started calling SDL3_ttf. Solution: sdl3.pc passes an rpath for SDL3, but sdl3-ttf.pc passes none, so an ELF binary linked against SDL3_ttf from the Nix store could not find it at runtime. The app had the same latent problem on Linux; tests only exposed it now. build.zig adds the SDL3_ttf library directory as an rpath on non-macOS targets; Mach-O binaries record absolute install names instead. --- build.zig | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/build.zig b/build.zig index 5413c11f..297b6fec 100644 --- a/build.zig +++ b/build.zig @@ -225,6 +225,13 @@ fn addSdlPaths( translate_c.addIncludePath(include_path); const lib_path = b.fmt("{s}/../lib", .{sdl3_ttf_include}); exe_mod.addLibraryPath(.{ .cwd_relative = lib_path }); + // sdl3.pc carries an rpath for SDL3, but sdl3-ttf.pc carries none, so an + // ELF binary linked against SDL3_ttf outside the default loader paths + // (e.g. in the Nix store) fails to start. Mach-O records each dylib's + // absolute install name instead and needs no rpath. + if (exe_mod.resolved_target.?.result.os.tag != .macos) { + exe_mod.addRPath(.{ .cwd_relative = lib_path }); + } } if (framework_path) |path| { const framework_lazy_path: std.Build.LazyPath = .{ .cwd_relative = path }; From 7f3f71b4514c05b1605b5e616e82068371ee4936 Mon Sep 17 00:00:00 2001 From: Forketyfork Date: Mon, 26 Oct 2026 14:55:16 +0100 Subject: [PATCH 6/6] fix(input): re-sync SDL's stale window position instead of shifting events Issue: Review on #415 pointed out that correcting mouse coordinates after SDL queues them is wrong in two ways: SDL already delivers correct window-local positions for focus-gain motion and background clicks, which the correction then shifted; and SDL drops or clamps motion against the stale origin before queuing, which no later correction can recover. Solution: The workaround now repairs the cause. When SDL's cached window position differs from the NSWindow's real one, it posts NSWindowDidMoveNotification so SDL's own windowDidMove: recomputes the position with the current main display height. The check runs before each event pump and after display and window-moved events, so the input SDL generates afterwards starts from a correct origin on every code path. --- CLAUDE.md | 2 +- docs/ARCHITECTURE.md | 2 +- src/app/runtime.zig | 5 +- src/c.zig | 6 + src/platform/sdl_mouse_workaround.zig | 266 +++++++++++++++----------- 5 files changed, 164 insertions(+), 117 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index d533666f..929d4db5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -130,7 +130,7 @@ If you are executing in a git worktree, stay within that worktree and do not att - macOS stops compositing fully covered windows and `CAMetalLayer` stops returning drawables; any render attempt then blocks the main thread for the full ~1s `nextDrawable` timeout, stalling input and PTY processing. The frame loop gates rendering on the `SDL_WINDOW_OCCLUDED` window flag (`shouldRenderFrame` in `src/app/runtime.zig`); keep any new render/present paths behind the same gate. ### Mouse coordinates on macOS 26+ -- SDL derives mouse motion, button, and wheel positions from its cached window position, which can go stale after a display reconfiguration and shift every mouse event by the difference between the displays' heights. `src/platform/sdl_mouse_workaround.zig` corrects the events before anything reads them; route new mouse-event consumers through the same event loop so they see corrected coordinates. The workaround is temporary: on every SDL bump, re-check it against SDL's Cocoa sources (the `verified_sdl_version` test fails until you do). +- SDL derives mouse motion, button, and wheel positions from its cached window position, which can go stale after a display reconfiguration and shift every mouse event by the difference between the displays' heights. `src/platform/sdl_mouse_workaround.zig` detects the stale cache and makes SDL recompute it (via its own `windowDidMove:`) before SDL generates more input events. Fix the cache rather than shifting event coordinates afterwards: some SDL paths already deliver correct window-local positions, and SDL drops or clamps events against the cached position before queuing them. The workaround is temporary: on every SDL bump, re-check it against SDL's Cocoa sources (the `verified_sdl_version` test fails until you do). ### Adding New SDL3 Key Codes When adding references to SDL3 key codes (SDLK_*) or other SDL constants, always add them to `src/c.zig` first instead of searching the web for their values. The build system provides the translated SDL declarations through the `c_sdl` module, and SDL constants must be explicitly re-exported from `src/c.zig` to be accessible throughout the codebase. diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 9503d187..d1e32399 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -508,7 +508,7 @@ Rotate: rename active file to architect-.log and continue in new | `app/terminal_history.zig` | Extract focused terminal scrollback + viewport text, strip ANSI escape sequences, convert OSC 133 prompt markers into reader-friendly prompt marker lines, and extract agent session IDs from PTY output for resumption | `extractSessionText()`, `extractTerminalText()`, `stripAnsiAlloc()`, `extractAgentSessionId()`, `buildResumeCommand()` | `session/state`, `ghostty-vt`, std | | `app/*` (app_state, layout, ui_host, grid_nav, grid_layout, input_keys, input_text, terminal_actions, worktree) | Application logic decomposed by concern: state enums, grid sizing, UI snapshot building, navigation, input encoding, clipboard and submitted-paste construction, worktree commands (with configurable external directory and post-create init) | `ViewMode`, `AnimationState`, `SessionStatus`, `buildUiHost()`, `applyTerminalResize()`, `encodeKey()`, `pasteText()`, `buildSubmittedPaste()`, `clearTerminal()`, `resolveWorktreeDir()` | `geom`, `anim/easing`, `ui/types`, `ui/session_view_state`, `colors`, `input/mapper`, `session/state`, `c` | | `platform/sdl.zig` | SDL3 initialization, window management, HiDPI | `init()`, `createWindow()`, `createRenderer()` | `c` | -| `platform/sdl_mouse_workaround.zig` | Temporary workaround for SDL on macOS 26+: corrects mouse event coordinates when SDL's cached window position goes stale after a display reconfiguration. Remove once SDL fixes it upstream | `MousePositionFix`, `applyOffset()`, `verified_sdl_version` | `c`, AppKit/CoreGraphics via the Objective-C runtime | +| `platform/sdl_mouse_workaround.zig` | Temporary workaround for SDL on macOS 26+: re-syncs SDL's cached window position when it goes stale after a display reconfiguration, so mouse events land under the pointer. Remove once SDL fixes it upstream | `WindowPositionRepair`, `mayStaleWindowPosition()`, `verified_sdl_version` | `c`, AppKit/CoreGraphics via the Objective-C runtime | | `input/mapper.zig` | SDL key adaptation, explicit compatibility bindings, shortcut detection; delegates protocol encoding to ghostty-vt | `encodeKeyWithMod()`, `KeyEncodingOptions`, shortcut helpers | `c`, `ghostty-vt` | | `c.zig` | SDL3 and SDL3_ttf FFI re-exports from the build-system `c_sdl` module | `SDLK_*`, `SDL_*`, `TTF_*` re-exports | `c_sdl`, SDL3 system libraries | | `session/state.zig` | Terminal session lifecycle: PTY, ghostty-vt, process watcher, foreground agent detection, graceful agent teardown at quit, and main-thread ring-buffer consumption | `SessionState`, `AgentKind`, `init()`, `despawn()`, `deinit()`, `ensureSpawnedWithDir()`, `processOutput()`, `render_epoch`, `pending_write`, `detectForegroundAgent()`, `sendTermToForegroundPgrp()` | `shell`, `pty`, `pty_reader`, `vt_stream`, `cwd`, `font`, xev | diff --git a/src/app/runtime.zig b/src/app/runtime.zig index cac28dbc..94636a94 100644 --- a/src/app/runtime.zig +++ b/src/app/runtime.zig @@ -1607,7 +1607,7 @@ pub fn run(allocator: std.mem.Allocator, io: std.Io, log_dir_override: ?[]const }; } - var mouse_position_fix = sdl_mouse_workaround.MousePositionFix.init(sdl.window); + const window_position_repair = sdl_mouse_workaround.WindowPositionRepair.init(sdl.window); const renderer = sdl.renderer; @@ -1881,6 +1881,7 @@ pub fn run(allocator: std.mem.Allocator, io: std.Io, log_dir_override: ?[]const var next_wait_timeout_ms: ?c_int = null; var first_frame = true; while (running) { + window_position_repair.check(sdl.window); var next_event = waitForNextFrame(next_wait_timeout_ms); const frame_start_ns: i128 = clock.nowNanos(io); const now = clock.nowMillis(io); @@ -1920,7 +1921,7 @@ pub fn run(allocator: std.mem.Allocator, io: std.Io, log_dir_override: ?[]const c.SDL_EVENT_KEY_DOWN, c.SDL_EVENT_KEY_UP, c.SDL_EVENT_TEXT_INPUT, c.SDL_EVENT_MOUSE_BUTTON_DOWN, c.SDL_EVENT_MOUSE_BUTTON_UP, c.SDL_EVENT_MOUSE_WHEEL, c.SDL_EVENT_MOUSE_MOTION => last_input_ns = frame_start_ns, else => {}, } - mouse_position_fix.correct(sdl.window, &event); + if (sdl_mouse_workaround.mayStaleWindowPosition(event.type)) window_position_repair.check(sdl.window); var scaled_event = layout.scaleEventToRender(&event, scale_x, scale_y); if (builtin.os.tag == .macos and scaled_event.type == c.SDL_EVENT_KEY_DOWN) { const key = scaled_event.key.key; diff --git a/src/c.zig b/src/c.zig index f4645ce9..23667256 100644 --- a/src/c.zig +++ b/src/c.zig @@ -95,6 +95,12 @@ pub const SDL_EVENT_MOUSE_MOTION = c_import.SDL_EVENT_MOUSE_MOTION; pub const SDL_EVENT_MOUSE_WHEEL = c_import.SDL_EVENT_MOUSE_WHEEL; pub const SDL_EVENT_WINDOW_RESIZED = c_import.SDL_EVENT_WINDOW_RESIZED; pub const SDL_EVENT_WINDOW_MOVED = c_import.SDL_EVENT_WINDOW_MOVED; +pub const SDL_EVENT_WINDOW_DISPLAY_CHANGED = c_import.SDL_EVENT_WINDOW_DISPLAY_CHANGED; +pub const SDL_EVENT_DISPLAY_FIRST = c_import.SDL_EVENT_DISPLAY_FIRST; +pub const SDL_EVENT_DISPLAY_LAST = c_import.SDL_EVENT_DISPLAY_LAST; +pub const SDL_EVENT_DISPLAY_ADDED = c_import.SDL_EVENT_DISPLAY_ADDED; +pub const SDL_EVENT_DISPLAY_REMOVED = c_import.SDL_EVENT_DISPLAY_REMOVED; +pub const SDL_EVENT_DISPLAY_MOVED = c_import.SDL_EVENT_DISPLAY_MOVED; pub const SDL_EVENT_WINDOW_FOCUS_GAINED = c_import.SDL_EVENT_WINDOW_FOCUS_GAINED; pub const SDL_EVENT_WINDOW_FOCUS_LOST = c_import.SDL_EVENT_WINDOW_FOCUS_LOST; pub const SDL_EVENT_WINDOW_CLOSE_REQUESTED = c_import.SDL_EVENT_WINDOW_CLOSE_REQUESTED; diff --git a/src/platform/sdl_mouse_workaround.zig b/src/platform/sdl_mouse_workaround.zig index 0820a165..61b911e1 100644 --- a/src/platform/sdl_mouse_workaround.zig +++ b/src/platform/sdl_mouse_workaround.zig @@ -14,9 +14,13 @@ //! window position, so every mouse event lands that far from the pointer until //! the window moves again (e.g. by leaving and re-entering fullscreen). //! -//! The fix here measures how far SDL's cached window position is from the real -//! one, read from the NSWindow the same way `windowDidMove:` does, and shifts -//! the coordinates of mouse events by that amount. +//! The fix here compares SDL's cached window position with the real one, read +//! from the NSWindow the same way `windowDidMove:` does, and when they differ +//! posts `NSWindowDidMoveNotification` so SDL's own `windowDidMove:` recomputes +//! the position with the current main display height. Repairing the cache, +//! rather than shifting event coordinates afterwards, keeps every SDL path +//! consistent: positions SDL already takes from window-local coordinates, and +//! the focus and clamping checks SDL runs before it queues an event. //! //! Remove this file once SDL recomputes window positions after the main display //! changes (`Cocoa_DisplayReconfigurationCallback` in SDL_cocoamodes.m), or once @@ -33,16 +37,13 @@ const is_macos = builtin.os.tag == .macos; /// in `SDL_GetVersion()` encoding. pub const verified_sdl_version: c_int = 3_004_018; -/// How far real window-relative mouse coordinates are from the ones SDL -/// reports, in window points. +/// How far SDL's cached window origin is from the real one, in window points. pub const Offset = struct { x: f32, y: f32, pub const zero: Offset = .{ .x = 0, .y = 0 }; - /// SDL subtracts its cached window origin from the global mouse position, - /// so the correction is the cached origin minus the real one. pub fn between(sdl_origin: Point, real_origin: Point) Offset { return .{ .x = sdl_origin.x - real_origin.x, .y = sdl_origin.y - real_origin.y }; } @@ -54,37 +55,18 @@ pub const Offset = struct { pub const Point = struct { x: f32, y: f32 }; -/// Shifts the window-relative coordinates of the mouse events SDL derives from -/// its cached window position. Drop events use window-local coordinates and are -/// left alone. -pub fn applyOffset(event: *c.SDL_Event, offset: Offset) void { - switch (event.type) { - c.SDL_EVENT_MOUSE_MOTION => { - event.motion.x += offset.x; - event.motion.y += offset.y; - }, - c.SDL_EVENT_MOUSE_BUTTON_DOWN, c.SDL_EVENT_MOUSE_BUTTON_UP => { - event.button.x += offset.x; - event.button.y += offset.y; - }, - c.SDL_EVENT_MOUSE_WHEEL => { - event.wheel.mouse_x += offset.x; - event.wheel.mouse_y += offset.y; - }, - else => {}, - } -} - -fn isAffectedEvent(event_type: u32) bool { - return switch (event_type) { - c.SDL_EVENT_MOUSE_MOTION, c.SDL_EVENT_MOUSE_BUTTON_DOWN, c.SDL_EVENT_MOUSE_BUTTON_UP, c.SDL_EVENT_MOUSE_WHEEL => true, - else => false, - }; +/// Events after which SDL's cached window position may have gone stale: +/// display reconfigurations and the window moves they cause. +pub fn mayStaleWindowPosition(event_type: u32) bool { + return (event_type >= c.SDL_EVENT_DISPLAY_FIRST and event_type <= c.SDL_EVENT_DISPLAY_LAST) or + event_type == c.SDL_EVENT_WINDOW_MOVED or + event_type == c.SDL_EVENT_WINDOW_DISPLAY_CHANGED; } /// Hand-written AppKit/CoreGraphics bindings, for the same reasons given in /// `macos_input_source.zig`. const appkit = if (is_macos) struct { + pub const NSPoint = extern struct { x: f64, y: f64 }; pub const NSRect = extern struct { x: f64, y: f64, w: f64, h: f64 }; pub const NSOperatingSystemVersion = extern struct { major: isize, minor: isize, patch: isize }; @@ -97,70 +79,106 @@ const appkit = if (is_macos) struct { extern fn objc_msgSend_stret() callconv(.c) void; pub const msg_send_rect_entry = if (builtin.cpu.arch == .x86_64) &objc_msgSend_stret else &objc_msgSend; + pub extern const NSWindowDidMoveNotification: ?*anyopaque; + pub extern fn CGMainDisplayID() callconv(.c) u32; pub extern fn CGDisplayPixelsHigh(display: u32) callconv(.c) usize; pub const MsgSendId = *const fn (?*anyopaque, ?*anyopaque) callconv(.c) ?*anyopaque; + pub const MsgSendPost = *const fn (?*anyopaque, ?*anyopaque, ?*anyopaque, ?*anyopaque) callconv(.c) void; pub const MsgSendRect = *const fn (?*anyopaque, ?*anyopaque) callconv(.c) NSRect; pub const MsgSendRectRect = *const fn (?*anyopaque, ?*anyopaque, NSRect) callconv(.c) NSRect; pub const MsgSendVersionBool = *const fn (?*anyopaque, ?*anyopaque, NSOperatingSystemVersion) callconv(.c) u8; + + pub fn sel(name: [*:0]const u8) ?*anyopaque { + return sel_registerName(name); + } + + pub fn msgSendId(receiver: ?*anyopaque, selector: ?*anyopaque) ?*anyopaque { + return @as(MsgSendId, @ptrCast(&objc_msgSend))(receiver, selector); + } } else struct {}; -pub const MousePositionFix = if (is_macos) struct { +pub const WindowPositionRepair = if (is_macos) struct { nswindow: ?*anyopaque = null, sel_frame: ?*anyopaque = null, sel_content_rect: ?*anyopaque = null, - logged_offset: Offset = Offset.zero, + sel_post: ?*anyopaque = null, + notification_center: ?*anyopaque = null, /// Inactive below macOS 26, where SDL takes mouse positions from the /// event's window-local location and the cached position does not matter. - pub fn init(window: *c.SDL_Window) MousePositionFix { + pub fn init(window: *c.SDL_Window) WindowPositionRepair { if (!sdlUsesCachedWindowPosition()) return .{}; return initForWindow(window); } - fn initForWindow(window: *c.SDL_Window) MousePositionFix { + fn initForWindow(window: *c.SDL_Window) WindowPositionRepair { const props = c.SDL_GetWindowProperties(window); const nswindow = c.SDL_GetPointerProperty(props, c.SDL_PROP_WINDOW_COCOA_WINDOW_POINTER, null) orelse { - log.warn("no NSWindow for the SDL window: {s}; mouse positions stay uncorrected", .{c.SDL_GetError()}); + log.warn("no NSWindow for the SDL window: {s}; stale window positions stay unrepaired", .{c.SDL_GetError()}); return .{}; }; - const sel_frame = appkit.sel_registerName("frame"); - const sel_content_rect = appkit.sel_registerName("contentRectForFrameRect:"); - if (sel_frame == null or sel_content_rect == null) { - log.warn("failed to register NSWindow selectors; mouse positions stay uncorrected", .{}); + const center_class = appkit.objc_getClass("NSNotificationCenter"); + const sel_default_center = appkit.sel("defaultCenter"); + const sel_frame = appkit.sel("frame"); + const sel_content_rect = appkit.sel("contentRectForFrameRect:"); + const sel_post = appkit.sel("postNotificationName:object:"); + if (center_class == null or sel_default_center == null or sel_frame == null or sel_content_rect == null or sel_post == null) { + log.warn("failed to look up AppKit classes or selectors; stale window positions stay unrepaired", .{}); return .{}; } - return .{ .nswindow = nswindow, .sel_frame = sel_frame, .sel_content_rect = sel_content_rect }; + const center = appkit.msgSendId(center_class, sel_default_center) orelse { + log.warn("no default NSNotificationCenter; stale window positions stay unrepaired", .{}); + return .{}; + }; + return .{ + .nswindow = nswindow, + .sel_frame = sel_frame, + .sel_content_rect = sel_content_rect, + .sel_post = sel_post, + .notification_center = center, + }; } - pub fn correct(self: *MousePositionFix, window: *c.SDL_Window, event: *c.SDL_Event) void { - if (self.nswindow == null or !isAffectedEvent(event.type)) return; - const offset = self.currentOffset(window) orelse return; - if (offset.x != self.logged_offset.x or offset.y != self.logged_offset.y) { - log.info("SDL window position is off by ({d}, {d}) points; correcting mouse coordinates", .{ offset.x, offset.y }); - self.logged_offset = offset; + /// Re-syncs SDL's cached window position when it differs from the real one. + /// Call it before SDL pumps new input, so the events it generates afterwards + /// start from a correct origin. + pub fn check(self: *const WindowPositionRepair, window: *c.SDL_Window) void { + if (self.nswindow == null) return; + const stale = self.currentOffset(window) orelse return; + if (stale.isZero()) return; + + // SDL listens for this notification as the window's delegate (or as an + // explicit observer) and recomputes window->x/y in `windowDidMove:`. + @as(appkit.MsgSendPost, @ptrCast(&appkit.objc_msgSend))(self.notification_center, self.sel_post, appkit.NSWindowDidMoveNotification, self.nswindow); + + const remaining = self.currentOffset(window) orelse return; + if (remaining.isZero()) { + log.info("SDL window position was off by ({d}, {d}) points; re-synced it", .{ stale.x, stale.y }); + } else { + // Expected while a fullscreen transition runs: SDL ignores moves + // until the transition ends and then recomputes the position itself. + log.debug("SDL window position is off by ({d}, {d}) points and did not re-sync", .{ remaining.x, remaining.y }); } - applyOffset(event, offset); } - fn currentOffset(self: *const MousePositionFix, window: *c.SDL_Window) ?Offset { + fn currentOffset(self: *const WindowPositionRepair, window: *c.SDL_Window) ?Offset { var sdl_x: c_int = 0; var sdl_y: c_int = 0; if (!c.SDL_GetWindowPosition(window, &sdl_x, &sdl_y)) { log.warn("SDL_GetWindowPosition failed: {s}", .{c.SDL_GetError()}); return null; } - const real = self.realContentOrigin(); return Offset.between( .{ .x = @floatFromInt(sdl_x), .y = @floatFromInt(sdl_y) }, - real, + self.realContentOrigin(), ); } /// The content area's top-left corner in SDL's global coordinates, /// computed like `windowDidMove:` but with the current main display height. - fn realContentOrigin(self: *const MousePositionFix) Point { + fn realContentOrigin(self: *const WindowPositionRepair) Point { const msg_send_rect = @as(appkit.MsgSendRect, @ptrCast(appkit.msg_send_rect_entry)); const msg_send_rect_rect = @as(appkit.MsgSendRectRect, @ptrCast(appkit.msg_send_rect_entry)); const frame = msg_send_rect(self.nswindow, self.sel_frame); @@ -175,23 +193,21 @@ pub const MousePositionFix = if (is_macos) struct { /// Mirrors SDL's `@available(macOS 26.0, *)` guard in `mouseMoved:`. fn sdlUsesCachedWindowPosition() bool { const class = appkit.objc_getClass("NSProcessInfo") orelse return false; - const sel_process_info = appkit.sel_registerName("processInfo") orelse return false; - const sel_at_least = appkit.sel_registerName("isOperatingSystemAtLeastVersion:") orelse return false; - const msg_send_id = @as(appkit.MsgSendId, @ptrCast(&appkit.objc_msgSend)); - const process_info = msg_send_id(class, sel_process_info) orelse return false; + const sel_process_info = appkit.sel("processInfo") orelse return false; + const sel_at_least = appkit.sel("isOperatingSystemAtLeastVersion:") orelse return false; + const process_info = appkit.msgSendId(class, sel_process_info) orelse return false; const msg_send_version = @as(appkit.MsgSendVersionBool, @ptrCast(&appkit.objc_msgSend)); return msg_send_version(process_info, sel_at_least, .{ .major = 26, .minor = 0, .patch = 0 }) != 0; } } else struct { - pub fn init(window: *c.SDL_Window) MousePositionFix { + pub fn init(window: *c.SDL_Window) WindowPositionRepair { _ = window; return .{}; } - pub fn correct(self: *MousePositionFix, window: *c.SDL_Window, event: *c.SDL_Event) void { + pub fn check(self: *const WindowPositionRepair, window: *c.SDL_Window) void { _ = self; _ = window; - _ = event; } }; @@ -207,63 +223,87 @@ test "offset is SDL's cached window origin minus the real one" { const offset = Offset.between(.{ .x = -94, .y = -1117 }, .{ .x = -94, .y = -1080 }); try std.testing.expectEqual(@as(f32, 0), offset.x); try std.testing.expectEqual(@as(f32, -37), offset.y); - - // SDL reported global (-94 + 500, -1080 + 300) - (-94, -1117) = (500, 337) - // for a pointer that is 300 pt below the window's real top edge. - var motion: c.SDL_Event = std.mem.zeroes(c.SDL_Event); - motion.type = c.SDL_EVENT_MOUSE_MOTION; - motion.motion.x = 500; - motion.motion.y = 337; - applyOffset(&motion, offset); - try std.testing.expectEqual(@as(f32, 500), motion.motion.x); - try std.testing.expectEqual(@as(f32, 300), motion.motion.y); + try std.testing.expect(!offset.isZero()); } -test "applyOffset corrects button and wheel positions but not drops" { - const offset: Offset = .{ .x = 3, .y = -37 }; - - var button: c.SDL_Event = std.mem.zeroes(c.SDL_Event); - button.type = c.SDL_EVENT_MOUSE_BUTTON_DOWN; - button.button.x = 10; - button.button.y = 100; - applyOffset(&button, offset); - try std.testing.expectEqual(@as(f32, 13), button.button.x); - try std.testing.expectEqual(@as(f32, 63), button.button.y); - - var wheel: c.SDL_Event = std.mem.zeroes(c.SDL_Event); - wheel.type = c.SDL_EVENT_MOUSE_WHEEL; - wheel.wheel.mouse_x = 10; - wheel.wheel.mouse_y = 100; - wheel.wheel.y = 1; - applyOffset(&wheel, offset); - try std.testing.expectEqual(@as(f32, 13), wheel.wheel.mouse_x); - try std.testing.expectEqual(@as(f32, 63), wheel.wheel.mouse_y); - try std.testing.expectEqual(@as(f32, 1), wheel.wheel.y); - - var drop: c.SDL_Event = std.mem.zeroes(c.SDL_Event); - drop.type = c.SDL_EVENT_DROP_POSITION; - drop.drop.x = 10; - drop.drop.y = 100; - applyOffset(&drop, offset); - try std.testing.expectEqual(@as(f32, 10), drop.drop.x); - try std.testing.expectEqual(@as(f32, 100), drop.drop.y); +test "display reconfigurations and window moves trigger a position check" { + try std.testing.expect(mayStaleWindowPosition(c.SDL_EVENT_DISPLAY_ADDED)); + try std.testing.expect(mayStaleWindowPosition(c.SDL_EVENT_DISPLAY_REMOVED)); + try std.testing.expect(mayStaleWindowPosition(c.SDL_EVENT_DISPLAY_MOVED)); + try std.testing.expect(mayStaleWindowPosition(c.SDL_EVENT_WINDOW_MOVED)); + try std.testing.expect(mayStaleWindowPosition(c.SDL_EVENT_WINDOW_DISPLAY_CHANGED)); + try std.testing.expect(!mayStaleWindowPosition(c.SDL_EVENT_MOUSE_MOTION)); + try std.testing.expect(!mayStaleWindowPosition(c.SDL_EVENT_WINDOW_RESIZED)); } +/// A hidden SDL window for tests, with SDL video initialized in the background +/// so the test process neither shows up in the Dock nor takes focus. +const TestWindow = if (is_macos) struct { + window: *c.SDL_Window, + + fn create() !TestWindow { + _ = c.SDL_SetHint("SDL_MAC_BACKGROUND_APP", "1"); + if (!c.SDL_InitSubSystem(c.SDL_INIT_VIDEO)) return error.SkipZigTest; + errdefer c.SDL_QuitSubSystem(c.SDL_INIT_VIDEO); + const window = c.SDL_CreateWindow("mouse workaround test", 320, 200, c.SDL_WINDOW_HIDDEN) orelse + return error.SkipZigTest; + try std.testing.expect(c.SDL_SetWindowPosition(window, 120, 140)); + _ = c.SDL_SyncWindow(window); + return .{ .window = window }; + } + + fn destroy(self: TestWindow) void { + c.SDL_DestroyWindow(self.window); + c.SDL_QuitSubSystem(c.SDL_INIT_VIDEO); + } + + /// Moves the NSWindow while SDL's delegate is detached, so SDL never sees + /// the move: the same stale cache a mis-converted move notification leaves. + fn moveBehindSdlsBack(self: TestWindow, repair: *const WindowPositionRepair, dx: f64, dy: f64) !void { + const sel_delegate = appkit.sel("delegate"); + const sel_set_delegate = appkit.sel("setDelegate:"); + const sel_set_frame_origin = appkit.sel("setFrameOrigin:"); + const delegate = appkit.msgSendId(repair.nswindow, sel_delegate) orelse return error.TestUnexpectedResult; + + const MsgSendSetId = *const fn (?*anyopaque, ?*anyopaque, ?*anyopaque) callconv(.c) void; + const MsgSendSetPoint = *const fn (?*anyopaque, ?*anyopaque, appkit.NSPoint) callconv(.c) void; + const set_delegate = @as(MsgSendSetId, @ptrCast(&appkit.objc_msgSend)); + const set_origin = @as(MsgSendSetPoint, @ptrCast(&appkit.objc_msgSend)); + const frame = @as(appkit.MsgSendRect, @ptrCast(appkit.msg_send_rect_entry))(repair.nswindow, repair.sel_frame); + + set_delegate(repair.nswindow, sel_set_delegate, null); + set_origin(repair.nswindow, sel_set_frame_origin, .{ .x = frame.x + dx, .y = frame.y + dy }); + set_delegate(repair.nswindow, sel_set_delegate, delegate); + _ = self; + } +} else struct {}; + test "real content origin matches SDL's window position while SDL is in sync" { if (!is_macos) return error.SkipZigTest; - // Keep the test process from showing up in the Dock or taking focus. - _ = c.SDL_SetHint("SDL_MAC_BACKGROUND_APP", "1"); - if (!c.SDL_InitSubSystem(c.SDL_INIT_VIDEO)) return error.SkipZigTest; - defer c.SDL_QuitSubSystem(c.SDL_INIT_VIDEO); - - const window = c.SDL_CreateWindow("mouse workaround test", 320, 200, c.SDL_WINDOW_HIDDEN) orelse - return error.SkipZigTest; - defer c.SDL_DestroyWindow(window); - try std.testing.expect(c.SDL_SetWindowPosition(window, 120, 140)); - _ = c.SDL_SyncWindow(window); - - var fix = MousePositionFix.initForWindow(window); - try std.testing.expect(fix.nswindow != null); - const offset = fix.currentOffset(window) orelse return error.TestUnexpectedResult; + const test_window = try TestWindow.create(); + defer test_window.destroy(); + + const repair = WindowPositionRepair.initForWindow(test_window.window); + try std.testing.expect(repair.nswindow != null); + const offset = repair.currentOffset(test_window.window) orelse return error.TestUnexpectedResult; try std.testing.expectEqual(Offset.zero, offset); } + +test "check re-syncs a window position SDL missed" { + if (!is_macos) return error.SkipZigTest; + const test_window = try TestWindow.create(); + defer test_window.destroy(); + + const repair = WindowPositionRepair.initForWindow(test_window.window); + try std.testing.expect(repair.nswindow != null); + + // Cocoa's y axis points up, so moving the frame up by 37 pt leaves SDL's + // cached top edge 37 pt below the real one. + try test_window.moveBehindSdlsBack(&repair, 0, 37); + const stale = repair.currentOffset(test_window.window) orelse return error.TestUnexpectedResult; + try std.testing.expectEqual(Offset{ .x = 0, .y = 37 }, stale); + + repair.check(test_window.window); + const after = repair.currentOffset(test_window.window) orelse return error.TestUnexpectedResult; + try std.testing.expectEqual(Offset.zero, after); +}