From 7ce02a6161f02db60af0a1ba434bfbccfe6b34f8 Mon Sep 17 00:00:00 2001 From: Forketyfork Date: Fri, 11 Sep 2026 15:47:03 +0200 Subject: [PATCH 1/3] fix(ui): retain button labels and use readable text colors Issue: Fix dark Cancel labels and button text flickering during caret-driven redraws, and address the same texture-lifetime problem in related UI buttons. Solution: Use readable theme foreground colors for outlined labels and retain cached SDL text textures across frames. Invalidate caches when fonts, labels, or colors change so dynamic modal text remains correct without same-frame texture destruction. --- docs/ARCHITECTURE.md | 2 +- src/main.zig | 1 + src/ui/components/button.zig | 96 +++++++++++++++-- src/ui/components/diff_overlay.zig | 140 ++++++++++++++++--------- src/ui/components/quit_confirm.zig | 8 +- src/ui/components/reader_overlay.zig | 17 +-- src/ui/components/worktree_overlay.zig | 89 +++++++++------- 7 files changed, 250 insertions(+), 103 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index cef62eb9..30cec944 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -109,7 +109,7 @@ These patterns are mandatory for all new code. They are derived from the archite 1. **UI components use the vtable interface and communicate via UiAction queue.** Never mutate application state directly from a UI component. Push a `UiAction` to the queue; the main loop drains it after all component updates complete. (See ADR-003.) -2. **Render invalidation uses epoch comparison.** When terminal content changes, increment `render_epoch` on the `SessionState`. The renderer checks whether `presented_epoch` no longer matches `render_epoch` to know whether a session needs to be redrawn this frame, and cached session textures refresh when their stored epoch, overlay composition, or grid/full render mode no longer matches the requested render. This applies to both grid tiles and the steady-state full-screen terminal view. A dirty session is a request for a frame: `app/frame_schedule.zig` may defer output-only demand to its 30 FPS cadence, while interactive demand renders immediately. There is no periodic re-render fallback. Never force a full re-render. (See ADR-004.) The dirty check only counts sessions visible in the current view mode (`app_state.sessionVisibleInMode`): in Full view, background sessions keep producing output but are never presented, so counting them would keep the app compositing and presenting full-window frames at the maximum rate for pixels nobody sees. Two related frame-loop rules: rendering is suppressed entirely while the window is occluded (`shouldRenderFrame` in `app/runtime.zig`) because macOS stops handing out `CAMetalLayer` drawables for covered windows and each render attempt would block the main thread — including PTY draining — for the full ~1s `nextDrawable` timeout; and static UI textures must never be created and destroyed within a single frame (see `ui/components/glyph_badge.zig`) because destroying a texture queued for rendering forces SDL's Metal backend to flush its command queue and acquire a drawable mid-frame. +2. **Render invalidation uses epoch comparison.** When terminal content changes, increment `render_epoch` on the `SessionState`. The renderer checks whether `presented_epoch` no longer matches `render_epoch` to know whether a session needs to be redrawn this frame, and cached session textures refresh when their stored epoch, overlay composition, or grid/full render mode no longer matches the requested render. This applies to both grid tiles and the steady-state full-screen terminal view. A dirty session is a request for a frame: `app/frame_schedule.zig` may defer output-only demand to its 30 FPS cadence, while interactive demand renders immediately. There is no periodic re-render fallback. Never force a full re-render. (See ADR-004.) The dirty check only counts sessions visible in the current view mode (`app_state.sessionVisibleInMode`): in Full view, background sessions keep producing output but are never presented, so counting them would keep the app compositing and presenting full-window frames at the maximum rate for pixels nobody sees. Two related frame-loop rules: rendering is suppressed entirely while the window is occluded (`shouldRenderFrame` in `app/runtime.zig`) because macOS stops handing out `CAMetalLayer` drawables for covered windows and each render attempt would block the main thread — including PTY draining — for the full ~1s `nextDrawable` timeout; and static UI textures must never be created and destroyed within a single frame (see `ui/components/glyph_badge.zig` and `ui/components/button.zig`) because destroying a texture queued for rendering forces SDL's Metal backend to flush its command queue and acquire a drawable mid-frame. Retained button and modal-label caches invalidate when their font, text, or color changes; default outlined button labels use the theme foreground so they remain readable against the selection fill. 3. **Blocking I/O goes on a background thread with a thread-safe queue.** The frame loop must never block. Any new external I/O source must follow the notification/control socket pattern: background thread + queue + main-loop drain. (See ADR-009.) diff --git a/src/main.zig b/src/main.zig index 096c5645..92126fd0 100644 --- a/src/main.zig +++ b/src/main.zig @@ -66,6 +66,7 @@ test { _ = @import("session/state.zig"); _ = @import("shell.zig"); _ = @import("ui/components/cwd_bar.zig"); + _ = @import("ui/components/button.zig"); _ = @import("ui/components/diff_comment_layout.zig"); _ = @import("ui/components/diff_overlay.zig"); _ = @import("ui/components/dropdown_menu.zig"); diff --git a/src/ui/components/button.zig b/src/ui/components/button.zig index 274cb544..0c9499ac 100644 --- a/src/ui/components/button.zig +++ b/src/ui/components/button.zig @@ -1,7 +1,11 @@ +const std = @import("std"); const c = @import("../../c.zig"); const geom = @import("../../geom.zig"); const primitives = @import("../../gfx/primitives.zig"); const dpi = @import("../../dpi.zig"); +const colors = @import("../../colors.zig"); + +const log = std.log.scoped(.ui_button); pub const ButtonVariant = enum { default, @@ -9,6 +13,49 @@ pub const ButtonVariant = enum { danger, }; +/// A button label texture that survives until the next frame. +/// +/// SDL's Metal renderer may queue texture draws, so destroying a label after +/// rendering it in the same frame can make the label disappear or force a +/// synchronous command-buffer flush. The cache is invalidated by the font, +/// label, or color changing. +pub const ButtonTexture = struct { + tex: ?*c.SDL_Texture = null, + w: c_int = 0, + h: c_int = 0, + font: ?*c.TTF_Font = null, + label: []const u8 = &.{}, + color: c.SDL_Color = .{ .r = 0, .g = 0, .b = 0, .a = 0 }, + + pub fn deinit(self: *ButtonTexture) void { + if (self.tex) |tex| c.SDL_DestroyTexture(tex); + self.* = .{}; + } + + pub fn ensure( + self: *ButtonTexture, + renderer: *c.SDL_Renderer, + font: *c.TTF_Font, + label: []const u8, + color: c.SDL_Color, + ) !void { + if (self.tex != null and self.font == font and std.mem.eql(u8, self.label, label) and colorsEqual(self.color, color)) { + return; + } + + const next = try makeTextTexture(renderer, font, label, color); + if (self.tex) |old| c.SDL_DestroyTexture(old); + self.* = .{ + .tex = next.tex, + .w = next.w, + .h = next.h, + .font = font, + .label = label, + .color = color, + }; + } +}; + pub fn renderButton( renderer: *c.SDL_Renderer, font: *c.TTF_Font, @@ -17,6 +64,7 @@ pub fn renderButton( variant: ButtonVariant, theme: *const @import("../../colors.zig").Theme, ui_scale: f32, + texture: *ButtonTexture, hovered: bool, ) void { const rect_int = geom.Rect{ @@ -62,24 +110,31 @@ pub fn renderButton( primitives.fillRoundedRect(renderer, rect_int, fill_radius); } - const text_color = switch (variant) { - .default => theme.accent, - .primary => theme.background, - .danger => theme.foreground, + const text_color = labelColor(variant, theme); + texture.ensure(renderer, font, label, text_color) catch |err| { + log.warn("failed to cache {s} button label: {}", .{ label, err }); + return; }; - const tex = makeTextTexture(renderer, font, label, text_color) catch return; - defer c.SDL_DestroyTexture(tex.tex); + const tex = texture.tex orelse return; - const text_x = rect.x + (rect.w - @as(f32, @floatFromInt(tex.w))) / 2.0; - const text_y = rect.y + (rect.h - @as(f32, @floatFromInt(tex.h))) / 2.0; - _ = c.SDL_RenderTexture(renderer, tex.tex, null, &c.SDL_FRect{ + const text_x = rect.x + (rect.w - @as(f32, @floatFromInt(texture.w))) / 2.0; + const text_y = rect.y + (rect.h - @as(f32, @floatFromInt(texture.h))) / 2.0; + _ = c.SDL_RenderTexture(renderer, tex, null, &c.SDL_FRect{ .x = text_x, .y = text_y, - .w = @floatFromInt(tex.w), - .h = @floatFromInt(tex.h), + .w = @floatFromInt(texture.w), + .h = @floatFromInt(texture.h), }); } +pub fn labelColor(variant: ButtonVariant, theme: *const colors.Theme) c.SDL_Color { + return switch (variant) { + .default => theme.foreground, + .primary => theme.background, + .danger => theme.foreground, + }; +} + const TextTex = struct { tex: *c.SDL_Texture, w: c_int, @@ -109,3 +164,22 @@ fn makeTextTexture( .h = @intFromFloat(h), }; } + +fn colorsEqual(a: c.SDL_Color, b: c.SDL_Color) bool { + return a.r == b.r and a.g == b.g and a.b == b.b and a.a == b.a; +} + +test "button label colors contrast with their fills" { + const palette_color = c.SDL_Color{ .r = 30, .g = 40, .b = 50, .a = 255 }; + const theme = colors.Theme{ + .background = .{ .r = 1, .g = 2, .b = 3, .a = 255 }, + .foreground = .{ .r = 220, .g = 221, .b = 222, .a = 255 }, + .selection = .{ .r = 10, .g = 11, .b = 12, .a = 255 }, + .accent = .{ .r = 90, .g = 160, .b = 230, .a = 255 }, + .palette = [_]c.SDL_Color{palette_color} ** 16, + }; + + try std.testing.expectEqual(theme.foreground, labelColor(.default, &theme)); + try std.testing.expectEqual(theme.background, labelColor(.primary, &theme)); + try std.testing.expectEqual(theme.foreground, labelColor(.danger, &theme)); +} diff --git a/src/ui/components/diff_overlay.zig b/src/ui/components/diff_overlay.zig index 2fde9d34..d4e1dfa3 100644 --- a/src/ui/components/diff_overlay.zig +++ b/src/ui/components/diff_overlay.zig @@ -12,6 +12,7 @@ const session_state = @import("../../session/state.zig"); const scrollbar = @import("scrollbar.zig"); const comment_layout = @import("diff_comment_layout.zig"); const dropdown_menu = @import("dropdown_menu.zig"); +const button = @import("button.zig"); const text_render = @import("../text_render.zig"); const text_edit = @import("../text_edit.zig"); @@ -147,6 +148,9 @@ pub const DiffOverlayComponent = struct { delete_hovered_comment: ?usize = null, comment_submit_hovered: bool = false, comment_cancel_hovered: bool = false, + comment_submit_button: button.ButtonTexture = .{}, + comment_cancel_button: button.ButtonTexture = .{}, + send_button: button.ButtonTexture = .{}, wrap_cols: usize = 0, @@ -3104,15 +3108,18 @@ pub const DiffOverlayComponent = struct { _ = c.SDL_SetRenderDrawColor(renderer, 255, 255, 255, 25); primitives.fillRoundedRect(renderer, .{ .x = submit_x, .y = btn_y, .w = btn_w, .h = btn_h }, btn_radius); } - const submit_tex = self.makeTextTexture(renderer, fonts.regular, "Submit", .{ .r = 255, .g = 255, .b = 255, .a = 255 }) catch return; - defer c.SDL_DestroyTexture(submit_tex.tex); - _ = c.SDL_SetTextureAlphaMod(submit_tex.tex, @intFromFloat(255.0 * alpha)); - _ = c.SDL_RenderTexture(renderer, submit_tex.tex, null, &c.SDL_FRect{ - .x = @floatFromInt(submit_x + @divFloor(btn_w - submit_tex.w, 2)), - .y = @floatFromInt(btn_y + @divFloor(btn_h - submit_tex.h, 2)), - .w = @floatFromInt(submit_tex.w), - .h = @floatFromInt(submit_tex.h), - }); + renderCachedButtonLabel( + &self.comment_submit_button, + renderer, + fonts.regular, + "Submit", + .{ .r = 255, .g = 255, .b = 255, .a = 255 }, + alpha, + submit_x, + btn_y, + btn_w, + btn_h, + ); // Cancel button const cancel_x = submit_x + btn_w + dpi.scale(6, host.ui_scale); @@ -3125,15 +3132,18 @@ pub const DiffOverlayComponent = struct { _ = c.SDL_SetRenderDrawColor(renderer, 255, 255, 255, 25); primitives.fillRoundedRect(renderer, .{ .x = cancel_x, .y = btn_y, .w = btn_w, .h = btn_h }, btn_radius); } - const cancel_tex = self.makeTextTexture(renderer, fonts.regular, "Cancel", host.theme.foreground) catch return; - defer c.SDL_DestroyTexture(cancel_tex.tex); - _ = c.SDL_SetTextureAlphaMod(cancel_tex.tex, @intFromFloat(255.0 * alpha)); - _ = c.SDL_RenderTexture(renderer, cancel_tex.tex, null, &c.SDL_FRect{ - .x = @floatFromInt(cancel_x + @divFloor(btn_w - cancel_tex.w, 2)), - .y = @floatFromInt(btn_y + @divFloor(btn_h - cancel_tex.h, 2)), - .w = @floatFromInt(cancel_tex.w), - .h = @floatFromInt(cancel_tex.h), - }); + renderCachedButtonLabel( + &self.comment_cancel_button, + renderer, + fonts.regular, + "Cancel", + host.theme.foreground, + alpha, + cancel_x, + btn_y, + btn_w, + btn_h, + ); } fn renderEditingCommentAnimated(self: *DiffOverlayComponent, host: *const types.UiHost, renderer: *c.SDL_Renderer, assets: *types.UiAssets, rect: geom.Rect, y_pos: c_int, progress: f32, is_closing: bool) void { @@ -3218,16 +3228,18 @@ pub const DiffOverlayComponent = struct { const submit_x = rect.x + rect.w - scaled_padding - btn_w * 2 - dpi.scale(12, host.ui_scale); _ = c.SDL_SetRenderDrawColor(renderer, 40, 167, 69, @intFromFloat(220.0 * alpha)); primitives.fillRoundedRect(renderer, .{ .x = submit_x, .y = btn_y, .w = btn_w, .h = btn_h }, dpi.scale(4, host.ui_scale)); - if (self.makeTextTexture(renderer, fonts.regular, "Submit", .{ .r = 255, .g = 255, .b = 255, .a = 255 })) |submit_tex| { - defer c.SDL_DestroyTexture(submit_tex.tex); - _ = c.SDL_SetTextureAlphaMod(submit_tex.tex, @intFromFloat(255.0 * alpha)); - _ = c.SDL_RenderTexture(renderer, submit_tex.tex, null, &c.SDL_FRect{ - .x = @floatFromInt(submit_x + @divFloor(btn_w - submit_tex.w, 2)), - .y = @floatFromInt(btn_y + @divFloor(btn_h - submit_tex.h, 2)), - .w = @floatFromInt(submit_tex.w), - .h = @floatFromInt(submit_tex.h), - }); - } else |_| {} + renderCachedButtonLabel( + &self.comment_submit_button, + renderer, + fonts.regular, + "Submit", + .{ .r = 255, .g = 255, .b = 255, .a = 255 }, + alpha, + submit_x, + btn_y, + btn_w, + btn_h, + ); const cancel_x = submit_x + btn_w + dpi.scale(6, host.ui_scale); const fg = host.theme.foreground; @@ -3235,16 +3247,18 @@ pub const DiffOverlayComponent = struct { primitives.fillRoundedRect(renderer, .{ .x = cancel_x, .y = btn_y, .w = btn_w, .h = btn_h }, dpi.scale(4, host.ui_scale)); _ = c.SDL_SetRenderDrawColor(renderer, fg.r, fg.g, fg.b, @intFromFloat(80.0 * alpha)); primitives.drawRoundedBorder(renderer, .{ .x = cancel_x, .y = btn_y, .w = btn_w, .h = btn_h }, dpi.scale(4, host.ui_scale)); - if (self.makeTextTexture(renderer, fonts.regular, "Cancel", host.theme.foreground)) |cancel_tex| { - defer c.SDL_DestroyTexture(cancel_tex.tex); - _ = c.SDL_SetTextureAlphaMod(cancel_tex.tex, @intFromFloat(255.0 * alpha)); - _ = c.SDL_RenderTexture(renderer, cancel_tex.tex, null, &c.SDL_FRect{ - .x = @floatFromInt(cancel_x + @divFloor(btn_w - cancel_tex.w, 2)), - .y = @floatFromInt(btn_y + @divFloor(btn_h - cancel_tex.h, 2)), - .w = @floatFromInt(cancel_tex.w), - .h = @floatFromInt(cancel_tex.h), - }); - } else |_| {} + renderCachedButtonLabel( + &self.comment_cancel_button, + renderer, + fonts.regular, + "Cancel", + host.theme.foreground, + alpha, + cancel_x, + btn_y, + btn_w, + btn_h, + ); _ = c.SDL_SetRenderClipRect(renderer, if (had_clip) &prev_clip else null); } @@ -3457,6 +3471,32 @@ pub const DiffOverlayComponent = struct { } } + fn renderCachedButtonLabel( + cache: *button.ButtonTexture, + renderer: *c.SDL_Renderer, + font: *c.TTF_Font, + label: []const u8, + color: c.SDL_Color, + alpha: f32, + x: c_int, + y: c_int, + w: c_int, + h: c_int, + ) void { + cache.ensure(renderer, font, label, color) catch |err| { + log.warn("failed to cache diff button label {s}: {}", .{ label, err }); + return; + }; + const tex = cache.tex orelse return; + _ = c.SDL_SetTextureAlphaMod(tex, @intFromFloat(255.0 * alpha)); + _ = c.SDL_RenderTexture(renderer, tex, null, &c.SDL_FRect{ + .x = @floatFromInt(x + @divFloor(w - cache.w, 2)), + .y = @floatFromInt(y + @divFloor(h - cache.h, 2)), + .w = @floatFromInt(cache.w), + .h = @floatFromInt(cache.h), + }); + } + fn renderSendButton(self: *DiffOverlayComponent, host: *const types.UiHost, renderer: *c.SDL_Renderer, assets: *types.UiAssets, overlay_rect: geom.Rect) void { if (!self.hasUnsentComments()) return; @@ -3472,15 +3512,18 @@ pub const DiffOverlayComponent = struct { const font_cache = assets.font_cache orelse return; const scaled_font_size = dpi.scale(font_size, host.ui_scale); const fonts = font_cache.get(scaled_font_size) catch return; - const tex = self.makeTextTexture(renderer, fonts.regular, "Send to agent", .{ .r = 255, .g = 255, .b = 255, .a = 255 }) catch return; - defer c.SDL_DestroyTexture(tex.tex); - _ = c.SDL_SetTextureAlphaMod(tex.tex, @intFromFloat(255.0 * alpha)); - _ = c.SDL_RenderTexture(renderer, tex.tex, null, &c.SDL_FRect{ - .x = @floatFromInt(btn.x + @divFloor(btn.w - tex.w, 2)), - .y = @floatFromInt(btn.y + @divFloor(btn.h - tex.h, 2)), - .w = @floatFromInt(tex.w), - .h = @floatFromInt(tex.h), - }); + renderCachedButtonLabel( + &self.send_button, + renderer, + fonts.regular, + "Send to agent", + .{ .r = 255, .g = 255, .b = 255, .a = 255 }, + alpha, + btn.x, + btn.y, + btn.w, + btn.h, + ); } fn renderAgentDropdown(self: *DiffOverlayComponent, host: *const types.UiHost, renderer: *c.SDL_Renderer, assets: *types.UiAssets, overlay_rect: geom.Rect) void { @@ -3505,6 +3548,9 @@ pub const DiffOverlayComponent = struct { fn destroy(self: *DiffOverlayComponent, renderer: *c.SDL_Renderer) void { _ = renderer; + self.comment_submit_button.deinit(); + self.comment_cancel_button.deinit(); + self.send_button.deinit(); self.scrollbar_state.deinit(); self.clearContent(); self.agent_dropdown.deinit(); diff --git a/src/ui/components/quit_confirm.zig b/src/ui/components/quit_confirm.zig index 5e6b2883..4d96e2d6 100644 --- a/src/ui/components/quit_confirm.zig +++ b/src/ui/components/quit_confirm.zig @@ -22,6 +22,8 @@ pub const QuitConfirmComponent = struct { escape_pressed: bool = false, cancel_hovered: bool = false, quit_hovered: bool = false, + cancel_button: button.ButtonTexture = .{}, + quit_button: button.ButtonTexture = .{}, title_tex: ?*c.SDL_Texture = null, title_w: c_int = 0, @@ -58,6 +60,8 @@ pub const QuitConfirmComponent = struct { } pub fn destroy(self: *QuitConfirmComponent, renderer: *c.SDL_Renderer) void { + self.cancel_button.deinit(); + self.quit_button.deinit(); if (self.title_tex) |tex| c.SDL_DestroyTexture(tex); if (self.message_tex) |tex| c.SDL_DestroyTexture(tex); self.allocator.destroy(self); @@ -233,7 +237,7 @@ pub const QuitConfirmComponent = struct { .w = @floatFromInt(buttons.cancel.w), .h = @floatFromInt(buttons.cancel.h), }; - button.renderButton(renderer, font, cancel_rect, "Cancel", .default, theme, ui_scale, self.cancel_hovered); + button.renderButton(renderer, font, cancel_rect, "Cancel", .default, theme, ui_scale, &self.cancel_button, self.cancel_hovered); const quit_rect = c.SDL_FRect{ .x = @floatFromInt(buttons.quit.x), @@ -241,7 +245,7 @@ pub const QuitConfirmComponent = struct { .w = @floatFromInt(buttons.quit.w), .h = @floatFromInt(buttons.quit.h), }; - button.renderButton(renderer, font, quit_rect, "Quit", .danger, theme, ui_scale, self.quit_hovered); + button.renderButton(renderer, font, quit_rect, "Quit", .danger, theme, ui_scale, &self.quit_button, self.quit_hovered); } fn modalRect(self: *QuitConfirmComponent, host: *const types.UiHost) geom.Rect { diff --git a/src/ui/components/reader_overlay.zig b/src/ui/components/reader_overlay.zig index f8b45982..c52aeb3c 100644 --- a/src/ui/components/reader_overlay.zig +++ b/src/ui/components/reader_overlay.zig @@ -15,6 +15,7 @@ const markdown_renderer = @import("markdown_renderer.zig"); const scrollbar = @import("scrollbar.zig"); const search_utils = @import("search_utils.zig"); const text_edit = @import("../text_edit.zig"); +const button = @import("button.zig"); const log = std.log.scoped(.reader_overlay); const SessionState = session_state.SessionState; @@ -80,6 +81,7 @@ pub const ReaderOverlayComponent = struct { link_hits: std.ArrayList(LinkHit) = .empty, hovered_link: ?usize = null, jump_button_hovered: bool = false, + jump_button_label: button.ButtonTexture = .{}, arrow_cursor: ?*c.SDL_Cursor = null, pointer_cursor: ?*c.SDL_Cursor = null, @@ -145,6 +147,7 @@ pub const ReaderOverlayComponent = struct { fn destroy(self: *ReaderOverlayComponent, renderer: *c.SDL_Renderer) void { _ = renderer; + self.jump_button_label.deinit(); self.clearContent(); self.blocks.deinit(self.allocator); self.lines.deinit(self.allocator); @@ -1588,13 +1591,13 @@ pub const ReaderOverlayComponent = struct { } const fonts = try font_cache.get(dpi.scale(13, host.ui_scale)); - const label_tex = try makeTextTexture(self.allocator, renderer, fonts.bold orelse fonts.regular, "Jump to bottom", host.theme.background); - defer c.SDL_DestroyTexture(label_tex.tex); - _ = c.SDL_RenderTexture(renderer, label_tex.tex, null, &c.SDL_FRect{ - .x = @floatFromInt(rect.x + @divFloor(rect.w - label_tex.w, 2)), - .y = @floatFromInt(rect.y + @divFloor(rect.h - label_tex.h, 2)), - .w = @floatFromInt(label_tex.w), - .h = @floatFromInt(label_tex.h), + try self.jump_button_label.ensure(renderer, fonts.bold orelse fonts.regular, "Jump to bottom", host.theme.background); + const label_tex = self.jump_button_label.tex orelse return; + _ = c.SDL_RenderTexture(renderer, label_tex, null, &c.SDL_FRect{ + .x = @floatFromInt(rect.x + @divFloor(rect.w - self.jump_button_label.w, 2)), + .y = @floatFromInt(rect.y + @divFloor(rect.h - self.jump_button_label.h, 2)), + .w = @floatFromInt(self.jump_button_label.w), + .h = @floatFromInt(self.jump_button_label.h), }); } diff --git a/src/ui/components/worktree_overlay.zig b/src/ui/components/worktree_overlay.zig index a9c19d2c..6a45440a 100644 --- a/src/ui/components/worktree_overlay.zig +++ b/src/ui/components/worktree_overlay.zig @@ -49,6 +49,13 @@ pub const WorktreeOverlayComponent = struct { flow_animation_start_ms: i64 = 0, modal_confirm_hovered: bool = false, modal_cancel_hovered: bool = false, + modal_confirm_button: button.ButtonTexture = .{}, + modal_remove_button: button.ButtonTexture = .{}, + modal_cancel_button: button.ButtonTexture = .{}, + modal_create_title: button.ButtonTexture = .{}, + modal_remove_title: button.ButtonTexture = .{}, + modal_input_text: button.ButtonTexture = .{}, + modal_error_text: button.ButtonTexture = .{}, const create_name_max_len: usize = 64; const button_size_small: c_int = 40; @@ -119,6 +126,13 @@ pub const WorktreeOverlayComponent = struct { fn deinit(self_ptr: *anyopaque, _: *c.SDL_Renderer) void { const self: *WorktreeOverlayComponent = @ptrCast(@alignCast(self_ptr)); self.badge.deinit(); + self.modal_confirm_button.deinit(); + self.modal_remove_button.deinit(); + self.modal_cancel_button.deinit(); + self.modal_create_title.deinit(); + self.modal_remove_title.deinit(); + self.modal_input_text.deinit(); + self.modal_error_text.deinit(); self.destroyCache(); self.clearWorktrees(); self.clearCreateInput(); @@ -1033,6 +1047,8 @@ pub const WorktreeOverlayComponent = struct { } fn clearCreateInput(self: *WorktreeOverlayComponent) void { + self.modal_input_text.deinit(); + self.modal_error_text.deinit(); self.create_input.clear(); if (self.create_error) |err| { self.allocator.free(err); @@ -1064,6 +1080,7 @@ pub const WorktreeOverlayComponent = struct { } fn setCreateError(self: *WorktreeOverlayComponent, msg: []const u8) void { + self.modal_error_text.deinit(); if (self.create_error) |err| self.allocator.free(err); self.create_error = self.allocator.dupe(u8, msg) catch |err| blk: { log.warn("failed to allocate create error message: {}", .{err}); @@ -1079,6 +1096,7 @@ pub const WorktreeOverlayComponent = struct { } fn appendCreateText(self: *WorktreeOverlayComponent, text: []const u8, now_ms: i64) void { + self.modal_input_text.deinit(); _ = self.create_input.insert(self.allocator, text, now_ms); } @@ -1109,7 +1127,11 @@ pub const WorktreeOverlayComponent = struct { self.clearCreateInput(); return true; }, - else => return self.create_input.handleKey(self.allocator, key, mod, host.now_ms).consumed, + else => { + const result = self.create_input.handleKey(self.allocator, key, mod, host.now_ms); + if (result.text_changed) self.modal_input_text.deinit(); + return result.consumed; + }, } } @@ -1217,19 +1239,18 @@ pub const WorktreeOverlayComponent = struct { primitives.drawRoundedBorder(renderer, modal_rect, modal_radius); const title_color = c.SDL_Color{ .r = theme.foreground.r, .g = theme.foreground.g, .b = theme.foreground.b, .a = 255 }; - const title_tex = makeTextTexture(renderer, title_fonts.regular, "Create worktree", title_color) catch |err| blk: { + self.modal_create_title.ensure(renderer, title_fonts.regular, "Create worktree", title_color) catch |err| { log.warn("failed to create title texture: {}", .{err}); - break :blk null; + return; }; - if (title_tex) |tex| { - defer c.SDL_DestroyTexture(tex.tex); - const title_x = layout.modal.x + (layout.modal.w - @as(f32, @floatFromInt(tex.w))) / 2.0; + if (self.modal_create_title.tex) |texture| { + const title_x = layout.modal.x + (layout.modal.w - @as(f32, @floatFromInt(self.modal_create_title.w))) / 2.0; const title_y = layout.modal.y + @as(f32, @floatFromInt(dpi.scale(10, host.ui_scale))); - _ = c.SDL_RenderTexture(renderer, tex.tex, null, &c.SDL_FRect{ + _ = c.SDL_RenderTexture(renderer, texture, null, &c.SDL_FRect{ .x = title_x, .y = title_y, - .w = @floatFromInt(tex.w), - .h = @floatFromInt(tex.h), + .w = @floatFromInt(self.modal_create_title.w), + .h = @floatFromInt(self.modal_create_title.h), }); } @@ -1249,17 +1270,16 @@ pub const WorktreeOverlayComponent = struct { const placeholder = self.create_input.isEmpty(); const input_text = if (placeholder) "name" else self.create_input.text(); const input_color = if (placeholder) input_style.placeholder else input_style.text; - const input_tex = makeTextTexture(renderer, entry_fonts.regular, input_text, input_color) catch |err| blk: { + self.modal_input_text.ensure(renderer, entry_fonts.regular, input_text, input_color) catch |err| { log.warn("failed to create input texture: {}", .{err}); - break :blk null; + return; }; const input_pad: f32 = @floatFromInt(dpi.scale(8, host.ui_scale)); var text_width: f32 = 0; var text_height: f32 = 0; - if (input_tex) |tex| { - defer c.SDL_DestroyTexture(tex.tex); - text_width = @floatFromInt(tex.w); - text_height = @floatFromInt(tex.h); + if (self.modal_input_text.tex) |texture| { + text_width = @floatFromInt(self.modal_input_text.w); + text_height = @floatFromInt(self.modal_input_text.h); if (self.create_input.select_all and !placeholder) { const sel_bg = theme.accent; _ = c.SDL_SetRenderDrawColor(renderer, sel_bg.r, sel_bg.g, sel_bg.b, 110); @@ -1270,7 +1290,7 @@ pub const WorktreeOverlayComponent = struct { .h = text_height, }); } - _ = c.SDL_RenderTexture(renderer, tex.tex, null, &c.SDL_FRect{ + _ = c.SDL_RenderTexture(renderer, texture, null, &c.SDL_FRect{ .x = layout.input.x + input_pad, .y = layout.input.y + input_pad, .w = text_width, @@ -1287,24 +1307,24 @@ pub const WorktreeOverlayComponent = struct { } // Buttons - button.renderButton(renderer, entry_fonts.regular, layout.confirm, "Confirm", .primary, theme, host.ui_scale, self.modal_confirm_hovered); - button.renderButton(renderer, entry_fonts.regular, layout.cancel, "Cancel", .default, theme, host.ui_scale, self.modal_cancel_hovered); + button.renderButton(renderer, entry_fonts.regular, layout.confirm, "Confirm", .primary, theme, host.ui_scale, &self.modal_confirm_button, self.modal_confirm_hovered); + button.renderButton(renderer, entry_fonts.regular, layout.cancel, "Cancel", .default, theme, host.ui_scale, &self.modal_cancel_button, self.modal_cancel_hovered); // Error message if (self.create_error) |err| { - const err_tex = makeTextTexture(renderer, entry_fonts.regular, err, c.SDL_Color{ .r = 255, .g = 99, .b = 99, .a = 255 }) catch |tex_err| blk: { + const err_color = c.SDL_Color{ .r = 255, .g = 99, .b = 99, .a = 255 }; + self.modal_error_text.ensure(renderer, entry_fonts.regular, err, err_color) catch |tex_err| { log.warn("operation failed: {}", .{tex_err}); - break :blk null; + return; }; - if (err_tex) |tex| { - defer c.SDL_DestroyTexture(tex.tex); + if (self.modal_error_text.tex) |texture| { const err_x = layout.input.x; const err_y = layout.input.y + layout.input.h + @as(f32, @floatFromInt(dpi.scale(8, host.ui_scale))); - _ = c.SDL_RenderTexture(renderer, tex.tex, null, &c.SDL_FRect{ + _ = c.SDL_RenderTexture(renderer, texture, null, &c.SDL_FRect{ .x = err_x, .y = err_y, - .w = @floatFromInt(tex.w), - .h = @floatFromInt(tex.h), + .w = @floatFromInt(self.modal_error_text.w), + .h = @floatFromInt(self.modal_error_text.h), }); } } @@ -1352,19 +1372,18 @@ pub const WorktreeOverlayComponent = struct { primitives.drawRoundedBorder(renderer, delete_modal_rect, modal_radius); const title_color = c.SDL_Color{ .r = theme.foreground.r, .g = theme.foreground.g, .b = theme.foreground.b, .a = 255 }; - const title_tex = makeTextTexture(renderer, title_fonts.regular, "Remove worktree", title_color) catch |err| blk: { + self.modal_remove_title.ensure(renderer, title_fonts.regular, "Remove worktree", title_color) catch |err| { log.warn("failed to create title texture: {}", .{err}); - break :blk null; + return; }; - if (title_tex) |tex| { - defer c.SDL_DestroyTexture(tex.tex); - const title_x = layout.modal.x + (layout.modal.w - @as(f32, @floatFromInt(tex.w))) / 2.0; + if (self.modal_remove_title.tex) |texture| { + const title_x = layout.modal.x + (layout.modal.w - @as(f32, @floatFromInt(self.modal_remove_title.w))) / 2.0; const title_y = layout.modal.y + @as(f32, @floatFromInt(dpi.scale(10, host.ui_scale))); - _ = c.SDL_RenderTexture(renderer, tex.tex, null, &c.SDL_FRect{ + _ = c.SDL_RenderTexture(renderer, texture, null, &c.SDL_FRect{ .x = title_x, .y = title_y, - .w = @floatFromInt(tex.w), - .h = @floatFromInt(tex.h), + .w = @floatFromInt(self.modal_remove_title.w), + .h = @floatFromInt(self.modal_remove_title.h), }); } @@ -1379,8 +1398,8 @@ pub const WorktreeOverlayComponent = struct { } } - button.renderButton(renderer, entry_fonts.regular, layout.confirm, "Remove", .danger, theme, host.ui_scale, self.modal_confirm_hovered); - button.renderButton(renderer, entry_fonts.regular, layout.cancel, "Cancel", .default, theme, host.ui_scale, self.modal_cancel_hovered); + button.renderButton(renderer, entry_fonts.regular, layout.confirm, "Remove", .danger, theme, host.ui_scale, &self.modal_remove_button, self.modal_confirm_hovered); + button.renderButton(renderer, entry_fonts.regular, layout.cancel, "Cancel", .default, theme, host.ui_scale, &self.modal_cancel_button, self.modal_cancel_hovered); } fn entryCount(self: *WorktreeOverlayComponent) usize { From 7df5e84ffdf9482f605430fb281e9f697e686fb6 Mon Sep 17 00:00:00 2001 From: Forketyfork Date: Fri, 11 Sep 2026 16:17:43 +0200 Subject: [PATCH 2/3] fix(worktree): cache wrapped removal path text Address review comment 3989851456: retain the remove-modal path textures across frames. Cache wrapped path lines by their source text, font identity and generation, color, and available width, and release them when the removal target changes or the modal closes. --- src/ui/components/worktree_overlay.zig | 153 +++++++++++++++++++++---- 1 file changed, 129 insertions(+), 24 deletions(-) diff --git a/src/ui/components/worktree_overlay.zig b/src/ui/components/worktree_overlay.zig index 6a45440a..3f7ca93a 100644 --- a/src/ui/components/worktree_overlay.zig +++ b/src/ui/components/worktree_overlay.zig @@ -56,6 +56,7 @@ pub const WorktreeOverlayComponent = struct { modal_remove_title: button.ButtonTexture = .{}, modal_input_text: button.ButtonTexture = .{}, modal_error_text: button.ButtonTexture = .{}, + remove_path_cache: WrappedPathCache = .{}, const create_name_max_len: usize = 64; const button_size_small: c_int = 40; @@ -88,6 +89,23 @@ pub const WorktreeOverlayComponent = struct { h: c_int, }; + const WrappedPathCache = struct { + source: ?[]u8 = null, + lines: std.ArrayList(TextTex) = .empty, + font: ?*c.TTF_Font = null, + font_size: c_int = 0, + font_generation: u64 = 0, + color: c.SDL_Color = .{}, + max_width: c_int = 0, + + fn deinit(self: *WrappedPathCache, allocator: std.mem.Allocator) void { + for (self.lines.items) |line| c.SDL_DestroyTexture(line.tex); + self.lines.deinit(allocator); + if (self.source) |source| allocator.free(source); + self.* = .{}; + } + }; + const EntryTex = struct { hotkey: TextTex, path: TextTex, @@ -133,6 +151,7 @@ pub const WorktreeOverlayComponent = struct { self.modal_remove_title.deinit(); self.modal_input_text.deinit(); self.modal_error_text.deinit(); + self.remove_path_cache.deinit(self.allocator); self.destroyCache(); self.clearWorktrees(); self.clearCreateInput(); @@ -906,30 +925,69 @@ pub const WorktreeOverlayComponent = struct { }; } - fn renderWrappedPath( + fn ensureWrappedPathCache( + self: *WorktreeOverlayComponent, renderer: *c.SDL_Renderer, font: *c.TTF_Font, + font_size: c_int, + font_generation: u64, text: []const u8, color: c.SDL_Color, - modal_x: f32, - start_y: f32, - modal_w: f32, - max_w: c_int, - row_height: c_int, - ) void { + max_width: c_int, + ) !void { + const cache = &self.remove_path_cache; + if (cache.source != null and + cache.font == font and + cache.font_size == font_size and + cache.font_generation == font_generation and + colorsEqual(cache.color, color) and + cache.max_width == max_width and + std.mem.eql(u8, cache.source.?, text)) + { + return; + } + + const source = try self.allocator.dupe(u8, text); + errdefer self.allocator.free(source); + + var lines: std.ArrayList(TextTex) = .empty; + errdefer { + for (lines.items) |line| c.SDL_DestroyTexture(line.tex); + lines.deinit(self.allocator); + } + + try appendWrappedPathTextures(self.allocator, renderer, font, source, color, max_width, &lines); + + cache.deinit(self.allocator); + cache.* = .{ + .source = source, + .lines = lines, + .font = font, + .font_size = font_size, + .font_generation = font_generation, + .color = color, + .max_width = max_width, + }; + } + + fn appendWrappedPathTextures( + allocator: std.mem.Allocator, + renderer: *c.SDL_Renderer, + font: *c.TTF_Font, + text: []const u8, + color: c.SDL_Color, + max_width: c_int, + lines: *std.ArrayList(TextTex), + ) !void { var full_w: c_int = 0; var full_h: c_int = 0; _ = c.TTF_GetStringSize(font, text.ptr, text.len, &full_w, &full_h); - if (full_w <= max_w) { - const tex = makeTextTexture(renderer, font, text, color) catch return; - defer c.SDL_DestroyTexture(tex.tex); - const x = modal_x + (modal_w - @as(f32, @floatFromInt(tex.w))) / 2.0; - _ = c.SDL_RenderTexture(renderer, tex.tex, null, &c.SDL_FRect{ .x = x, .y = start_y, .w = @floatFromInt(tex.w), .h = @floatFromInt(tex.h) }); + if (full_w <= max_width) { + try appendTextTexture(allocator, renderer, font, text, color, lines); return; } - var y = start_y; var line_start: usize = 0; var last_slash: usize = 0; @@ -941,13 +999,9 @@ pub const WorktreeOverlayComponent = struct { var seg_h: c_int = 0; _ = c.TTF_GetStringSize(font, segment.ptr, segment.len, &seg_w, &seg_h); - if (seg_w > max_w and last_slash > line_start) { + if (seg_w > max_width and last_slash > line_start) { const line = text[line_start .. last_slash + 1]; - const tex = makeTextTexture(renderer, font, line, color) catch return; - defer c.SDL_DestroyTexture(tex.tex); - const x = modal_x + (modal_w - @as(f32, @floatFromInt(tex.w))) / 2.0; - _ = c.SDL_RenderTexture(renderer, tex.tex, null, &c.SDL_FRect{ .x = x, .y = y, .w = @floatFromInt(tex.w), .h = @floatFromInt(tex.h) }); - y += @floatFromInt(row_height); + try appendTextTexture(allocator, renderer, font, line, color, lines); line_start = last_slash + 1; last_slash = line_start; } @@ -955,13 +1009,50 @@ pub const WorktreeOverlayComponent = struct { if (line_start < text.len) { const line = text[line_start..]; - const tex = makeTextTexture(renderer, font, line, color) catch return; - defer c.SDL_DestroyTexture(tex.tex); - const x = modal_x + (modal_w - @as(f32, @floatFromInt(tex.w))) / 2.0; - _ = c.SDL_RenderTexture(renderer, tex.tex, null, &c.SDL_FRect{ .x = x, .y = y, .w = @floatFromInt(tex.w), .h = @floatFromInt(tex.h) }); + try appendTextTexture(allocator, renderer, font, line, color, lines); } } + fn appendTextTexture( + allocator: std.mem.Allocator, + renderer: *c.SDL_Renderer, + font: *c.TTF_Font, + text: []const u8, + color: c.SDL_Color, + lines: *std.ArrayList(TextTex), + ) !void { + const texture = try makeTextTexture(renderer, font, text, color); + lines.append(allocator, texture) catch |err| { + c.SDL_DestroyTexture(texture.tex); + return err; + }; + } + + fn renderWrappedPath( + cache: *const WrappedPathCache, + renderer: *c.SDL_Renderer, + modal_x: f32, + start_y: f32, + modal_w: f32, + row_height: c_int, + ) void { + var y = start_y; + for (cache.lines.items) |line| { + const x = modal_x + (modal_w - @as(f32, @floatFromInt(line.w))) / 2.0; + _ = c.SDL_RenderTexture(renderer, line.tex, null, &c.SDL_FRect{ + .x = x, + .y = y, + .w = @floatFromInt(line.w), + .h = @floatFromInt(line.h), + }); + y += @floatFromInt(row_height); + } + } + + fn colorsEqual(a: c.SDL_Color, b: c.SDL_Color) bool { + return a.r == b.r and a.g == b.g and a.b == b.b and a.a == b.a; + } + fn destroyEntryTextures(entries: []EntryTex) void { for (entries) |entry| { c.SDL_DestroyTexture(entry.hotkey.tex); @@ -1036,6 +1127,7 @@ pub const WorktreeOverlayComponent = struct { const worktree = self.worktrees.items[wt_idx]; self.confirming_removal = true; self.pending_removal_index = wt_idx; + self.remove_path_cache.deinit(self.allocator); if (self.pending_removal_path) |old_path| { self.allocator.free(old_path); } @@ -1073,6 +1165,7 @@ pub const WorktreeOverlayComponent = struct { fn clearPendingRemoval(self: *WorktreeOverlayComponent) void { self.confirming_removal = false; self.pending_removal_index = null; + self.remove_path_cache.deinit(self.allocator); if (self.pending_removal_path) |path| { self.allocator.free(path); self.pending_removal_path = null; @@ -1394,7 +1487,19 @@ pub const WorktreeOverlayComponent = struct { const message_color = c.SDL_Color{ .r = theme.foreground.r, .g = theme.foreground.g, .b = theme.foreground.b, .a = 200 }; const max_w: c_int = @as(c_int, @intFromFloat(layout.modal.w)) - 2 * dpi.scale(modal_padding, host.ui_scale); const scaled_lh: c_int = dpi.scale(line_height, host.ui_scale); - renderWrappedPath(renderer, entry_fonts.regular, worktree.display, message_color, layout.modal.x, message_y, layout.modal.w, max_w, scaled_lh); + self.ensureWrappedPathCache( + renderer, + entry_fonts.regular, + cache.entry_font_size, + cache.font_generation, + worktree.display, + message_color, + max_w, + ) catch |err| { + log.warn("failed to cache removal path: {}", .{err}); + return; + }; + self.renderWrappedPath(self.remove_path_cache, renderer, layout.modal.x, message_y, layout.modal.w, scaled_lh); } } From e55bfc66a23b27d7a36fd38d9ed9e74c0563241b Mon Sep 17 00:00:00 2001 From: Forketyfork Date: Fri, 11 Sep 2026 16:28:57 +0200 Subject: [PATCH 3/3] fix(worktree): call cached path renderer correctly Issue: The worktree removal-modal cache refactor left a stale method-style call that made the macOS build fail during compilation.\nSolution: Invoke the static cached-path renderer with the cache pointer it expects, preserving the existing texture cache while restoring type-correct compilation. --- src/ui/components/worktree_overlay.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ui/components/worktree_overlay.zig b/src/ui/components/worktree_overlay.zig index 3f7ca93a..4cf31d70 100644 --- a/src/ui/components/worktree_overlay.zig +++ b/src/ui/components/worktree_overlay.zig @@ -1499,7 +1499,7 @@ pub const WorktreeOverlayComponent = struct { log.warn("failed to cache removal path: {}", .{err}); return; }; - self.renderWrappedPath(self.remove_path_cache, renderer, layout.modal.x, message_y, layout.modal.w, scaled_lh); + renderWrappedPath(&self.remove_path_cache, renderer, layout.modal.x, message_y, layout.modal.w, scaled_lh); } }