Retain button labels and use readable text colors

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.)
 

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");

src/ui/components/button.zig

@@ -1,14 +1,61 @@
+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,
     primary,
     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));
+}

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,33 +3228,37 @@ 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;
         _ = c.SDL_SetRenderDrawColor(renderer, fg.r, fg.g, fg.b, @intFromFloat(40.0 * alpha));
         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();

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,15 +237,15 @@ 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),
             .y = @floatFromInt(buttons.quit.y),
             .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 {