From 4291ccf2c526331f4db4b5c813f53b1f3dbd829c Mon Sep 17 00:00:00 2001 From: CJ van den Berg Date: Mon, 30 Mar 2026 00:24:12 +0200 Subject: [PATCH] fix(gui): resolve crashes and glyph rendering bugs from M3 smoke test --- src/gui/gpu/builtin.glsl.zig | 4 +- src/gui/gpu/gpu.zig | 46 +++++++------ src/gui/rasterizer/truetype.zig | 7 +- src/gui/wio/app.zig | 114 +++++++++++++++++++++----------- 4 files changed, 105 insertions(+), 66 deletions(-) diff --git a/src/gui/gpu/builtin.glsl.zig b/src/gui/gpu/builtin.glsl.zig index 40509965..61a58294 100644 --- a/src/gui/gpu/builtin.glsl.zig +++ b/src/gui/gpu/builtin.glsl.zig @@ -17,7 +17,7 @@ pub const FsParams = extern struct { }; const vs_src = - \\#version 410 + \\#version 330 core \\void main() { \\ int id = gl_VertexID; \\ float x = 2.0 * (float(id & 1) - 0.5); @@ -27,7 +27,7 @@ const vs_src = ; const fs_src = - \\#version 410 + \\#version 330 core \\uniform int cell_size_x; \\uniform int cell_size_y; \\uniform int col_count; diff --git a/src/gui/gpu/gpu.zig b/src/gui/gpu/gpu.zig index b640c809..40816fea 100644 --- a/src/gui/gpu/gpu.zig +++ b/src/gui/gpu/gpu.zig @@ -12,14 +12,16 @@ const XY = @import("xy").XY; const builtin_shader = @import("builtin.glsl.zig"); pub const Font = Rasterizer.Font; +pub const GlyphKind = Rasterizer.GlyphKind; pub const Cell = gui_cell.Cell; pub const Color = gui_cell.Rgba8; const Rgba8 = gui_cell.Rgba8; const log = std.log.scoped(.gpu); -// Maximum glyph atlas dimension (matching D3D11_REQ_TEXTURE2D_U_OR_V_DIMENSION) -const max_atlas_dim: u16 = 16384; +// Maximum glyph atlas dimension. 4096 is universally supported and gives +// 65536+ glyph slots at typical cell sizes — far more than needed in practice. +const max_atlas_dim: u16 = 4096; fn getAtlasCellCount(cell_size: XY(u16)) XY(u16) { return .{ @@ -121,6 +123,8 @@ pub const WindowState = struct { // Glyph index cache glyph_cache_cell_size: ?XY(u16) = null, glyph_index_cache: ?GlyphIndexCache = null, + // Set when the CPU atlas shadow was updated; cleared after GPU upload. + glyph_atlas_dirty: bool = false, pub fn init() WindowState { std.debug.assert(global.init_called); @@ -269,15 +273,10 @@ pub const WindowState = struct { @memcpy(region_buf[dst_off .. dst_off + glyph_w], staging_buf[src_off .. src_off + glyph_w]); } - // Upload to atlas via a temporary full-atlas image data struct. - // sokol's updateImage uploads the whole mip0 slice. We use a - // trick: create a temporary single-cell-sized image, upload it, - // then… actually, sokol doesn't expose sub-rect uploads. - // - // Workaround: upload the WHOLE atlas with only the new cell - // updated. For M2 this is acceptable; a smarter approach - // (ping-pong or persistent mapped buffer) can be added later. - uploadGlyphAtlasCell(state, atlas_x, atlas_y, glyph_w, glyph_h, region_buf); + // Write into the CPU-side atlas shadow. The GPU upload is + // deferred to paint() so it happens at most once per frame. + blitAtlasCpu(state, atlas_x, atlas_y, glyph_w, glyph_h, region_buf); + state.glyph_atlas_dirty = true; return reserved.index; }, @@ -286,15 +285,13 @@ pub const WindowState = struct { } }; -// Upload one cell's pixels into the glyph atlas. -// Because sokol only supports full-image updates via sg.updateImage, we -// maintain a CPU-side copy of the atlas and re-upload it each time. -// (M2 budget approach — acceptable for the stub rasterizer that always -// produces blank glyphs anyway.) +// CPU-side shadow copy of the glyph atlas (R8, row-major). +// Kept alive for the process lifetime; resized when the atlas image grows. var atlas_cpu: ?[]u8 = null; var atlas_cpu_size: XY(u16) = .{ .x = 0, .y = 0 }; -fn uploadGlyphAtlasCell( +// Blit one glyph cell into the CPU-side atlas shadow. +fn blitAtlasCpu( state: *const WindowState, x: u16, y: u16, @@ -305,7 +302,6 @@ fn uploadGlyphAtlasCell( const asz = state.glyph_image_size; const total: usize = @as(usize, asz.x) * @as(usize, asz.y); - // Resize cpu shadow if needed if (!atlas_cpu_size.eql(asz)) { if (atlas_cpu) |old| std.heap.page_allocator.free(old); atlas_cpu = std.heap.page_allocator.alloc(u8, total) catch |e| oom(e); @@ -313,18 +309,25 @@ fn uploadGlyphAtlasCell( atlas_cpu_size = asz; } - // Blit the cell into the cpu shadow const buf = atlas_cpu.?; for (0..h) |row_i| { const src_off = row_i * w; const dst_off = (@as(usize, y) + row_i) * asz.x + x; @memcpy(buf[dst_off .. dst_off + w], pixels[src_off .. src_off + w]); } +} + +// Upload the CPU shadow to the GPU. Called once per frame if dirty. +// Must be called outside a sokol render pass. +fn flushGlyphAtlas(state: *WindowState) void { + const asz = state.glyph_image_size; + const total: usize = @as(usize, asz.x) * @as(usize, asz.y); + const buf = atlas_cpu orelse return; - // Re-upload the full atlas var img_data: sg.ImageData = .{}; img_data.mip_levels[0] = .{ .ptr = buf.ptr, .size = total }; sg.updateImage(state.glyph_image, img_data); + state.glyph_atlas_dirty = false; } pub fn paint( @@ -373,6 +376,9 @@ pub fn paint( } } + // Upload glyph atlas to GPU if any new glyphs were rasterized this frame. + if (state.glyph_atlas_dirty) flushGlyphAtlas(state); + // Upload cell texture var cell_data: sg.ImageData = .{}; const cell_bytes = std.mem.sliceAsBytes(shader_cells); diff --git a/src/gui/rasterizer/truetype.zig b/src/gui/rasterizer/truetype.zig index d4281e1f..6a2bfb7c 100644 --- a/src/gui/rasterizer/truetype.zig +++ b/src/gui/rasterizer/truetype.zig @@ -88,10 +88,9 @@ pub fn render( if (dims.width == 0 or dims.height == 0) return; - const buf_w: i32 = switch (kind) { - .single => @intCast(font.cell_size.x), - .left, .right => @as(i32, @intCast(font.cell_size.x)) * 2, - }; + // Always use 2*cell_w as the row stride so it matches the staging buffer + // width allocated by generateGlyph (which always allocates 2*cell_w wide). + const buf_w: i32 = @as(i32, @intCast(font.cell_size.x)) * 2; const buf_h: i32 = @intCast(font.cell_size.y); const x_offset: i32 = switch (kind) { diff --git a/src/gui/wio/app.zig b/src/gui/wio/app.zig index f05a9c89..89dc243f 100644 --- a/src/gui/wio/app.zig +++ b/src/gui/wio/app.zig @@ -24,9 +24,9 @@ const log = std.log.scoped(.wio_app); const ScreenSnapshot = struct { cells: []gpu.Cell, + codepoints: []u21, width: u16, height: u16, - font: gpu.Font, }; var gpa: std.heap.GeneralPurposeAllocator(.{}) = .{}; @@ -37,6 +37,10 @@ var tui_pid: thespian.pid = undefined; var font_size_px: u16 = 16; var font_name_buf: [256]u8 = undefined; var font_name_len: usize = 0; +var font_dirty: std.atomic.Value(bool) = .init(true); + +// Current font — written and read only from the wio thread (after gpu.init). +var wio_font: gpu.Font = .{ .cell_size = .{ .x = 8, .y = 16 } }; // ── Public API (called from tui thread) ─────────────────────────────────── @@ -58,28 +62,39 @@ pub fn updateScreen(vx_screen: *const vaxis.Screen) void { const cell_count: usize = @as(usize, vx_screen.width) * @as(usize, vx_screen.height); const new_cells = allocator.alloc(gpu.Cell, cell_count) catch return; - const new_font = getFont(); + const new_codepoints = allocator.alloc(u21, cell_count) catch { + allocator.free(new_cells); + return; + }; - // Convert vaxis cells → gpu.Cell (glyph + colours) - // Glyph indices are filled in on the GPU thread; here we just store 0. - for (vx_screen.buf[0..cell_count], new_cells) |*vc, *gc| { + // Convert vaxis cells → gpu.Cell (colours only; glyph indices filled on GPU thread). + for (vx_screen.buf[0..cell_count], new_cells, new_codepoints) |*vc, *gc, *cp| { gc.* = .{ .glyph_index = 0, .background = colorFromVaxis(vc.style.bg), .foreground = colorFromVaxis(vc.style.fg), }; + // Decode first codepoint from the grapheme cluster. + const g = vc.char.grapheme; + cp.* = if (g.len > 0) blk: { + const seq_len = std.unicode.utf8ByteSequenceLength(g[0]) catch break :blk ' '; + break :blk std.unicode.utf8Decode(g[0..@min(seq_len, g.len)]) catch ' '; + } else ' '; } screen_mutex.lock(); defer screen_mutex.unlock(); // Free the previous snapshot - if (screen_snap) |old| allocator.free(old.cells); + if (screen_snap) |old| { + allocator.free(old.cells); + allocator.free(old.codepoints); + } screen_snap = .{ .cells = new_cells, + .codepoints = new_codepoints, .width = vx_screen.width, .height = vx_screen.height, - .font = new_font, }; screen_pending.store(true, .release); @@ -93,6 +108,7 @@ pub fn requestRender() void { pub fn setFontSize(size_px: f32) void { font_size_px = @intFromFloat(@max(4, size_px)); + font_dirty.store(true, .release); requestRender(); } @@ -105,14 +121,24 @@ pub fn setFontFace(name: []const u8) void { const copy_len = @min(name.len, font_name_buf.len); @memcpy(font_name_buf[0..copy_len], name[0..copy_len]); font_name_len = copy_len; + font_dirty.store(true, .release); requestRender(); } -// ── Internal helpers ────────────────────────────────────────────────────── +// ── Internal helpers (wio thread only) ──────────────────────────────────── -fn getFont() gpu.Font { +// Reload wio_font from current settings. Called only from the wio thread. +fn reloadFont() void { const name = if (font_name_len > 0) font_name_buf[0..font_name_len] else "monospace"; - return gpu.loadFont(name, font_size_px) catch gpu.Font{ .cell_size = .{ .x = 8, .y = 16 } }; + wio_font = gpu.loadFont(name, font_size_px) catch return; +} + +// Check dirty flag and reload if needed. +fn maybeReloadFont(win_size: wio.Size, state: *gpu.WindowState, cell_width: *u16, cell_height: *u16) void { + if (font_dirty.swap(false, .acq_rel)) { + reloadFont(); + sendResize(win_size, state, cell_width, cell_height); + } } fn colorFromVaxis(color: vaxis.Cell.Color) gpu.Color { @@ -174,6 +200,9 @@ fn wioLoop() void { }; defer gpu.deinit(); + // Load the initial font on the wio thread (gpu.init must be done first). + reloadFont(); + var state = gpu.WindowState.init(); defer state.deinit(); @@ -194,6 +223,9 @@ fn wioLoop() void { while (running) { wio.wait(.{}); + // Reload font if settings changed (font_dirty set by TUI thread). + maybeReloadFont(win_size, &state, &cell_width, &cell_height); + while (window.getEvent()) |event| { switch (event) { .close => { @@ -209,11 +241,10 @@ fn wioLoop() void { if (input_translate.mouseButtonId(btn)) |mb_id| { const col: i32 = @intCast(mouse_pos.x); const row: i32 = @intCast(mouse_pos.y); - const font = getFont(); - const col_cell: i32 = @intCast(@divTrunc(col, font.cell_size.x)); - const row_cell: i32 = @intCast(@divTrunc(row, font.cell_size.y)); - const xoff: i32 = @intCast(@mod(col, font.cell_size.x)); - const yoff: i32 = @intCast(@mod(row, font.cell_size.y)); + const col_cell: i32 = @intCast(@divTrunc(col, wio_font.cell_size.x)); + const row_cell: i32 = @intCast(@divTrunc(row, wio_font.cell_size.y)); + const xoff: i32 = @intCast(@mod(col, wio_font.cell_size.x)); + const yoff: i32 = @intCast(@mod(row, wio_font.cell_size.y)); tui_pid.send(.{ "RDR", "B", @as(u8, 1), // press @@ -225,14 +256,14 @@ fn wioLoop() void { }) catch {}; } else { const cp = input_translate.codepointFromButton(btn, mods); - sendKey(1, cp, cp, mods); + if (cp != 0) sendKey(1, cp, cp, mods); } }, .button_repeat => |btn| { const mods = input_translate.Mods.fromButtons(held_buttons); if (input_translate.mouseButtonId(btn) == null) { const cp = input_translate.codepointFromButton(btn, mods); - sendKey(2, cp, cp, mods); + if (cp != 0) sendKey(2, cp, cp, mods); } }, .button_release => |btn| { @@ -241,14 +272,13 @@ fn wioLoop() void { if (input_translate.mouseButtonId(btn)) |mb_id| { const col: i32 = @intCast(mouse_pos.x); const row: i32 = @intCast(mouse_pos.y); - const font = getFont(); - const col_cell: i32 = @intCast(@divTrunc(col, font.cell_size.x)); - const row_cell: i32 = @intCast(@divTrunc(row, font.cell_size.y)); - const xoff: i32 = @intCast(@mod(col, font.cell_size.x)); - const yoff: i32 = @intCast(@mod(row, font.cell_size.y)); + const col_cell: i32 = @intCast(@divTrunc(col, wio_font.cell_size.x)); + const row_cell: i32 = @intCast(@divTrunc(row, wio_font.cell_size.y)); + const xoff: i32 = @intCast(@mod(col, wio_font.cell_size.x)); + const yoff: i32 = @intCast(@mod(row, wio_font.cell_size.y)); tui_pid.send(.{ "RDR", "B", - @as(u8, 0), // release + @as(u8, 3), // release mb_id, col_cell, row_cell, @@ -257,7 +287,7 @@ fn wioLoop() void { }) catch {}; } else { const cp = input_translate.codepointFromButton(btn, mods); - sendKey(3, cp, cp, mods); + if (cp != 0) sendKey(3, cp, cp, mods); } }, .char => |cp| { @@ -266,11 +296,10 @@ fn wioLoop() void { }, .mouse => |pos| { mouse_pos = pos; - const font = getFont(); - const col_cell: i32 = @intCast(@divTrunc(@as(i32, @intCast(pos.x)), font.cell_size.x)); - const row_cell: i32 = @intCast(@divTrunc(@as(i32, @intCast(pos.y)), font.cell_size.y)); - const xoff: i32 = @intCast(@mod(@as(i32, @intCast(pos.x)), font.cell_size.x)); - const yoff: i32 = @intCast(@mod(@as(i32, @intCast(pos.y)), font.cell_size.y)); + const col_cell: i32 = @intCast(@divTrunc(@as(i32, @intCast(pos.x)), wio_font.cell_size.x)); + const row_cell: i32 = @intCast(@divTrunc(@as(i32, @intCast(pos.y)), wio_font.cell_size.y)); + const xoff: i32 = @intCast(@mod(@as(i32, @intCast(pos.x)), wio_font.cell_size.x)); + const yoff: i32 = @intCast(@mod(@as(i32, @intCast(pos.y)), wio_font.cell_size.y)); tui_pid.send(.{ "RDR", "M", col_cell, row_cell, @@ -279,33 +308,39 @@ fn wioLoop() void { }, .scroll_vertical => |dy| { const btn_id: u8 = if (dy < 0) 64 else 65; // up / down scroll - const font = getFont(); - const col_cell: i32 = @intCast(@divTrunc(@as(i32, @intCast(mouse_pos.x)), font.cell_size.x)); - const row_cell: i32 = @intCast(@divTrunc(@as(i32, @intCast(mouse_pos.y)), font.cell_size.y)); + const col_cell: i32 = @intCast(@divTrunc(@as(i32, @intCast(mouse_pos.x)), wio_font.cell_size.x)); + const row_cell: i32 = @intCast(@divTrunc(@as(i32, @intCast(mouse_pos.y)), wio_font.cell_size.y)); tui_pid.send(.{ "RDR", "B", @as(u8, 1), btn_id, col_cell, row_cell, @as(i32, 0), @as(i32, 0) }) catch {}; }, else => {}, } } - // Paint if the tui pushed new screen data + // Paint if the tui pushed new screen data. + // Take ownership of the snap (set screen_snap = null under the mutex) + // so the TUI thread cannot free the backing memory while we use it. if (screen_pending.swap(false, .acq_rel)) { screen_mutex.lock(); const snap = screen_snap; + screen_snap = null; // wio thread now owns this allocation screen_mutex.unlock(); if (snap) |s| { + defer { + allocator.free(s.cells); + allocator.free(s.codepoints); + } + state.size = .{ .x = win_size.width, .y = win_size.height }; - const font = s.font; + const font = wio_font; // Regenerate glyph indices using the GPU state const cells_with_glyphs = allocator.alloc(gpu.Cell, s.cells.len) catch continue; defer allocator.free(cells_with_glyphs); @memcpy(cells_with_glyphs, s.cells); - for (cells_with_glyphs) |*cell| { - // TODO: carry codepoint/width from the vaxis screen snapshot. - cell.glyph_index = state.generateGlyph(font, ' ', .single); + for (cells_with_glyphs, s.codepoints) |*cell, cp| { + cell.glyph_index = state.generateGlyph(font, cp, .single); } gpu.paint( @@ -332,9 +367,8 @@ fn sendResize( cell_width: *u16, cell_height: *u16, ) void { - const font = getFont(); - cell_width.* = @intCast(@divTrunc(sz.width, font.cell_size.x)); - cell_height.* = @intCast(@divTrunc(sz.height, font.cell_size.y)); + cell_width.* = @intCast(@divTrunc(sz.width, wio_font.cell_size.x)); + cell_height.* = @intCast(@divTrunc(sz.height, wio_font.cell_size.y)); state.size = .{ .x = sz.width, .y = sz.height }; tui_pid.send(.{ "RDR", "Resize",