From d213c1a939710a26672e4711bbb1337610f32a37 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 20 Nov 2022 20:05:07 -0800 Subject: [PATCH] fix selection regression caused by screen copy optimization --- src/Window.zig | 4 ++-- src/main.zig | 1 - src/renderer/Metal.zig | 9 +++++++- src/renderer/OpenGL.zig | 9 +++++++- src/terminal/Selection.zig | 43 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 61 insertions(+), 5 deletions(-) diff --git a/src/Window.zig b/src/Window.zig index d0a467128..1609f5419 100644 --- a/src/Window.zig +++ b/src/Window.zig @@ -1650,13 +1650,13 @@ fn posToViewport(self: Window, xpos: f64, ypos: f64) terminal.point.Viewport { // Can be off the screen if the user drags it out, so max // it out on our available columns - break :x @min(x, self.io.terminal.cols - 1); + break :x @min(x, self.grid_size.columns - 1); }, .y = if (ypos < 0) 0 else y: { const cell_height = @floatCast(f64, self.cell_size.height); const y = @floatToInt(usize, ypos / cell_height); - break :y @min(y, self.io.terminal.rows - 1); + break :y @min(y, self.grid_size.rows - 1); }, }; } diff --git a/src/main.zig b/src/main.zig index 2b245baa8..00d198f7b 100644 --- a/src/main.zig +++ b/src/main.zig @@ -203,7 +203,6 @@ test { _ = @import("TempDir.zig"); _ = @import("font/main.zig"); _ = @import("renderer.zig"); - _ = @import("terminal/Terminal.zig"); _ = @import("termio.zig"); _ = @import("input.zig"); diff --git a/src/renderer/Metal.zig b/src/renderer/Metal.zig index 21b68726c..c094d8ecb 100644 --- a/src/renderer/Metal.zig +++ b/src/renderer/Metal.zig @@ -458,10 +458,17 @@ pub fn render( ); errdefer screen_copy.deinit(); + // Convert our selection to viewport points because we copy only + // the viewport above. + const selection: ?terminal.Selection = if (state.terminal.selection) |sel| + sel.toViewport(&state.terminal.screen) + else + null; + break :critical .{ .bg = self.background, .devmode = if (state.devmode) |dm| dm.visible else false, - .selection = state.terminal.selection, + .selection = selection, .screen = screen_copy, .draw_cursor = self.cursor_visible and state.terminal.screen.viewportIsBottom(), }; diff --git a/src/renderer/OpenGL.zig b/src/renderer/OpenGL.zig index d36670917..1302979ef 100644 --- a/src/renderer/OpenGL.zig +++ b/src/renderer/OpenGL.zig @@ -593,11 +593,18 @@ pub fn render( ); errdefer screen_copy.deinit(); + // Convert our selection to viewport points because we copy only + // the viewport above. + const selection: ?terminal.Selection = if (state.terminal.selection) |sel| + sel.toViewport(&state.terminal.screen) + else + null; + break :critical .{ .gl_bg = self.background, .devmode_data = devmode_data, .active_screen = state.terminal.active_screen, - .selection = state.terminal.selection, + .selection = selection, .screen = screen_copy, .draw_cursor = self.cursor_visible and state.terminal.screen.viewportIsBottom(), }; diff --git a/src/terminal/Selection.zig b/src/terminal/Selection.zig index 8edf90215..973e1ab67 100644 --- a/src/terminal/Selection.zig +++ b/src/terminal/Selection.zig @@ -4,6 +4,7 @@ const Selection = @This(); const std = @import("std"); const point = @import("point.zig"); +const Screen = @import("Screen.zig"); const ScreenPoint = point.ScreenPoint; /// Start and end of the selection. There is no guarantee that @@ -13,6 +14,25 @@ const ScreenPoint = point.ScreenPoint; start: ScreenPoint, end: ScreenPoint, +/// Converts a selection screen points to viewport points (still typed +/// as ScreenPoints) if the selection is present within the viewport +/// of the screen. +pub fn toViewport(self: Selection, screen: *const Screen) ?Selection { + const top = (point.Viewport{ .x = 0, .y = 0 }).toScreen(screen); + const bot = (point.Viewport{ .x = 0, .y = screen.rows - 1 }).toScreen(screen); + + // If our selection isn't within the viewport, do nothing. + if (!self.within(top, bot)) return null; + + // Convert + const start = self.start.toViewport(screen); + const end = self.end.toViewport(screen); + return Selection{ + .start = .{ .x = start.x, .y = start.y }, + .end = .{ .x = end.x, .y = end.y }, + }; +} + /// Returns true if the selection contains the given point. /// /// This recalculates top left and bottom right each call. If you have @@ -40,6 +60,15 @@ pub fn contains(self: Selection, p: ScreenPoint) bool { return p.y > tl.y and p.y < br.y; } +/// Returns true if the selection contains any of the points between +/// (and including) the start and end. The x values are ignored this is +/// just a section match +pub fn within(self: Selection, start: ScreenPoint, end: ScreenPoint) bool { + const tl = self.topLeft(); + const br = self.bottomRight(); + return tl.y >= start.y and br.y <= end.y; +} + /// Returns true if the selection contains the row of the given point, /// regardless of the x value. pub fn containsRow(self: Selection, p: ScreenPoint) bool { @@ -116,3 +145,17 @@ test "Selection: contains" { try testing.expect(!sel.contains(.{ .x = 12, .y = 1 })); } } + +test "Selection: within" { + const testing = std.testing; + { + const sel: Selection = .{ + .start = .{ .x = 5, .y = 1 }, + .end = .{ .x = 3, .y = 2 }, + }; + + try testing.expect(sel.within(.{ .x = 6, .y = 0 }, .{ .x = 6, .y = 3 })); + try testing.expect(sel.within(.{ .x = 3, .y = 1 }, .{ .x = 6, .y = 3 })); + try testing.expect(sel.within(.{ .x = 3, .y = 0 }, .{ .x = 6, .y = 2 })); + } +}