From 5cb6ebe34d4df575836e92ca2447359062497f57 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 4 Nov 2022 22:32:06 -0700 Subject: [PATCH] Actually, we'll manage selection and viewports on the windowing thread --- src/Window.zig | 67 +++++++++++++++++++++++++++--------------- src/termio/Exec.zig | 18 ------------ src/termio/Thread.zig | 2 -- src/termio/message.zig | 6 ---- 4 files changed, 43 insertions(+), 50 deletions(-) diff --git a/src/Window.zig b/src/Window.zig index a9aafa5ca..aad2f9116 100644 --- a/src/Window.zig +++ b/src/Window.zig @@ -810,17 +810,25 @@ fn charCallback(window: glfw.Window, codepoint: u21) void { return; } - // Anytime a char is created, we have to clear the selection if there is one. - _ = win.io_thread.mailbox.push(.{ - .clear_selection = {}, - }, .{ .forever = {} }); + // Critical area + { + win.renderer_state.mutex.lock(); + defer win.renderer_state.mutex.unlock(); - // Scroll to the bottom - _ = win.io_thread.mailbox.push(.{ - .scroll_viewport = .{ .bottom = {} }, - }, .{ .forever = {} }); + // Clear the selction if we have one. + if (win.terminal.selection != null) { + win.terminal.selection = null; + win.queueRender() catch |err| + log.err("error scheduling render in charCallback err={}", .{err}); + } - // Write the char to the pty + // We want to scroll to the bottom + // TODO: detect if we're at the bottom to avoid the render call here. + win.terminal.scrollViewport(.{ .bottom = {} }) catch |err| + log.err("error scrolling viewport err={}", .{err}); + } + + // Ask our IO thread to write the data var data: termio.message.IO.SmallWriteArray = undefined; data[0] = @intCast(u8, codepoint); _ = win.io_thread.mailbox.push(.{ @@ -835,16 +843,8 @@ fn charCallback(window: glfw.Window, codepoint: u21) void { // TODO: the stuff below goes away with IO thread - if (win.terminal.selection != null) { - win.terminal.selection = null; - win.queueRender() catch |err| - log.err("error scheduling render in charCallback err={}", .{err}); - } - // We want to scroll to the bottom // TODO: detect if we're at the bottom to avoid the render call here. - win.terminal.scrollViewport(.{ .bottom = {} }) catch |err| - log.err("error scrolling viewport err={}", .{err}); win.queueRender() catch |err| log.err("error scheduling render in charCallback err={}", .{err}); @@ -959,8 +959,13 @@ fn keyCallback( }, .copy_to_clipboard => { - if (win.terminal.selection) |sel| { - var buf = win.terminal.screen.selectionString(win.alloc, sel) catch |err| { + // We can read from the renderer state without holding + // the lock because only we will write to this field. + if (win.renderer_state.terminal.selection) |sel| { + var buf = win.renderer_state.terminal.screen.selectionString( + win.alloc, + sel, + ) catch |err| { log.err("error reading selection string err={}", .{err}); return; }; @@ -1129,12 +1134,17 @@ fn scrollCallback(window: glfw.Window, xoff: f64, yoff: f64) void { const sign: isize = if (yoff > 0) -1 else 1; const delta: isize = sign * @max(@divFloor(win.grid_size.rows, 15), 1); log.info("scroll: delta={}", .{delta}); - win.terminal.scrollViewport(.{ .delta = delta }) catch |err| - log.err("error scrolling viewport err={}", .{err}); - // Schedule render since scrolling usually does something. - // TODO(perf): we can only schedule render if we know scrolling - // did something + // Modify our viewport, this requires a lock since it affects rendering + { + win.renderer_state.mutex.lock(); + defer win.renderer_state.mutex.unlock(); + + win.terminal.scrollViewport(.{ .delta = delta }) catch |err| + log.err("error scrolling viewport err={}", .{err}); + } + + // TODO: drop after IO thread win.queueRender() catch unreachable; } @@ -1389,6 +1399,9 @@ fn mouseButtonCallback( // Selection is always cleared if (win.terminal.selection != null) { + // We only need the lock to write since only we'll ever write. + win.renderer_state.mutex.lock(); + defer win.renderer_state.mutex.unlock(); win.terminal.selection = null; win.queueRender() catch |err| log.err("error scheduling render in mouseButtinCallback err={}", .{err}); @@ -1462,6 +1475,12 @@ fn cursorPosCallback( // often. // TODO: unit test this, this logic sucks + // At some point below we likely write selection state so we just grab + // a lock. This is not optimal efficiency but its not a common operation + // so its okay. + win.renderer_state.mutex.lock(); + defer win.renderer_state.mutex.unlock(); + // If we were selecting, and we switched directions, then we restart // calculations because it forces us to reconsider if the first cell is // selected. diff --git a/src/termio/Exec.zig b/src/termio/Exec.zig index 47675b16b..dfbedfb63 100644 --- a/src/termio/Exec.zig +++ b/src/termio/Exec.zig @@ -199,24 +199,6 @@ pub fn resize( } } -pub fn clearSelection(self: *Exec) !void { - // We don't need a lock to read because nothing else can possibly write - // as we're looking at this. - if (self.terminal.selection != null) { - // We need to lock so we can write because other things might be reading. - self.renderer_state.mutex.lock(); - defer self.renderer_state.mutex.unlock(); - self.terminal.selection = null; - } -} - -pub fn scrollViewport(self: *Exec, scroll: terminal.Terminal.ScrollViewport) !void { - self.renderer_state.mutex.lock(); - defer self.renderer_state.mutex.unlock(); - - try self.terminal.scrollViewport(scroll); -} - pub inline fn queueWrite(self: *Exec, data: []const u8) !void { try self.data.?.queueWrite(data); } diff --git a/src/termio/Thread.zig b/src/termio/Thread.zig index c05f3dbb8..0338617f6 100644 --- a/src/termio/Thread.zig +++ b/src/termio/Thread.zig @@ -162,8 +162,6 @@ fn drainMailbox(self: *Thread) !void { log.debug("mailbox message={}", .{message}); switch (message) { .resize => |v| try self.impl.resize(v.grid_size, v.screen_size), - .clear_selection => try self.impl.clearSelection(), - .scroll_viewport => |v| try self.impl.scrollViewport(v), .small_write => |v| try self.impl.queueWrite(v.data[0..v.len]), } } diff --git a/src/termio/message.zig b/src/termio/message.zig index b80ae2e4f..ba3d7be3b 100644 --- a/src/termio/message.zig +++ b/src/termio/message.zig @@ -12,12 +12,6 @@ pub const IO = union(enum) { screen_size: renderer.ScreenSize, }, - /// Clear the selection - clear_selection: void, - - /// Scroll the viewport - scroll_viewport: terminal.Terminal.ScrollViewport, - /// Write where the data fits in the union. small_write: struct { data: [22]u8,