From 8fe5913f7ee3eb1ea9da1b8fb1803fbe143dc069 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 3 Jul 2026 11:39:31 -0700 Subject: [PATCH] kitty/gfx: fix build on targets without 64-bit atomics The generation counter added in bdc0b6c19 is a process-global std.atomic.Value(u64). The Zig compiler rejects 64-bit atomic operations on targets whose largest supported atomic size is 32 bits (e.g. arm-linux-androideabi), which broke the libghostty-vt Android build. This slipped past other CI targets because they're either 64-bit or compile kitty graphics out entirely (wasm32-freestanding). The counter backing nextGeneration is now comptime-selected: 64-bit targets keep the lock-free atomic counter, while 32-bit targets fall back to a mutex-protected u64. This preserves the process-wide uniqueness and monotonicity guarantees of generation stamps everywhere. The mutex cost is irrelevant since this is a cold path, only invoked on content mutations. --- src/terminal/kitty/graphics_storage.zig | 37 +++++++++++++++++++++---- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/src/terminal/kitty/graphics_storage.zig b/src/terminal/kitty/graphics_storage.zig index 7c1c7f83f..ba82ea402 100644 --- a/src/terminal/kitty/graphics_storage.zig +++ b/src/terminal/kitty/graphics_storage.zig @@ -24,18 +24,45 @@ const log = std.log.scoped(.kitty_gfx); /// terminals. This lets consumers use a generation value alone as a /// cache key without any ambiguity. /// -/// Atomic because separate terminals may mutate their storages from -/// different threads. On single-threaded targets (e.g. wasm without -/// atomics) this lowers to plain operations. -var generation_counter: std.atomic.Value(u64) = .init(0); +/// Thread-safe because separate terminals may mutate their storages +/// from different threads. On single-threaded targets this lowers to +/// plain operations. +var generation_counter: GenerationCounter = .{}; /// Returns the next generation stamp. Stamps are unique and strictly /// monotonically increasing process-wide, starting at 1 (0 is reserved /// to mean "never stamped"). pub fn nextGeneration() u64 { - return generation_counter.fetchAdd(1, .monotonic) + 1; + return generation_counter.next(); } +/// Backing implementation for the generation counter. We use a +/// lock-free atomic counter where we can, but not all targets support +/// 64-bit atomic operations (e.g. 32-bit ARM Android), so we fall back +/// to a mutex-protected counter on those. This is a cold path (only +/// invoked on content mutations) so the mutex cost is irrelevant. +/// +/// The pointer-width check is a conservative proxy for 64-bit atomic +/// support: every 64-bit target supports 64-bit atomics, while 32-bit +/// targets may not (per the compiler's atomic operand validation). +const GenerationCounter = if (@bitSizeOf(usize) >= 64) struct { + value: std.atomic.Value(u64) = .init(0), + + fn next(self: *@This()) u64 { + return self.value.fetchAdd(1, .monotonic) + 1; + } +} else struct { + mutex: std.Thread.Mutex = .{}, + value: u64 = 0, + + fn next(self: *@This()) u64 { + self.mutex.lock(); + defer self.mutex.unlock(); + self.value += 1; + return self.value; + } +}; + /// An image storage is associated with a terminal screen (i.e. main /// screen, alt screen) and contains all the transmitted images and /// placements.