From 64a3115648d354731ef08c2de8b2168b9326b7b5 Mon Sep 17 00:00:00 2001 From: Christian Duerr Date: Sat, 7 Mar 2020 22:17:38 +0000 Subject: [PATCH] Fix selection with invisible start and end This resolves an issue with the selection clamping, where no selection would be rendered at all when the start was above the viewport while the end was below it. --- CHANGELOG.md | 1 + alacritty/src/event.rs | 75 +++++++++++++--------------- alacritty_terminal/src/grid/mod.rs | 36 ++++--------- alacritty_terminal/src/grid/tests.rs | 34 +++++++++++++ alacritty_terminal/src/index.rs | 9 ++-- alacritty_terminal/src/term/mod.rs | 27 ++-------- font/src/ft/fc/mod.rs | 34 ++++++------- 7 files changed, 106 insertions(+), 110 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a0ab066..cd56c85 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,6 +61,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Tabstops not breaking across lines - Crash when parsing DCS escape with more than 16 parameters - Ignoring of slow touchpad scrolling +- Selection invisible when starting above viewport and ending below it ### Removed diff --git a/alacritty/src/event.rs b/alacritty/src/event.rs index 5888140..94a340d 100644 --- a/alacritty/src/event.rs +++ b/alacritty/src/event.rs @@ -11,7 +11,9 @@ use std::sync::Arc; use std::time::Instant; use glutin::dpi::PhysicalSize; -use glutin::event::{ElementState, Event as GlutinEvent, ModifiersState, MouseButton, WindowEvent}; +use glutin::event::{ + DeviceEvent, ElementState, Event as GlutinEvent, ModifiersState, MouseButton, WindowEvent, +}; use glutin::event_loop::{ControlFlow, EventLoop, EventLoopProxy, EventLoopWindowTarget}; use glutin::platform::desktop::EventLoopExtDesktop; use log::{debug, info, warn}; @@ -519,10 +521,9 @@ impl Processor { }, GlutinEvent::RedrawRequested(_) => processor.ctx.terminal.dirty = true, GlutinEvent::WindowEvent { event, window_id, .. } => { - use glutin::event::WindowEvent::*; match event { - CloseRequested => processor.ctx.terminal.exit(), - Resized(size) => { + WindowEvent::CloseRequested => processor.ctx.terminal.exit(), + WindowEvent::Resized(size) => { #[cfg(windows)] { // Minimizing the window sends a Resize event with zero width and @@ -537,7 +538,7 @@ impl Processor { processor.ctx.display_update_pending.dimensions = Some(size); processor.ctx.terminal.dirty = true; }, - KeyboardInput { input, is_synthetic: false, .. } => { + WindowEvent::KeyboardInput { input, is_synthetic: false, .. } => { processor.key_input(input); if input.state == ElementState::Pressed { // Hide cursor while typing @@ -546,13 +547,13 @@ impl Processor { } } }, - ReceivedCharacter(c) => processor.received_char(c), - MouseInput { state, button, .. } => { + WindowEvent::ReceivedCharacter(c) => processor.received_char(c), + WindowEvent::MouseInput { state, button, .. } => { processor.ctx.window.set_mouse_visible(true); processor.mouse_input(state, button); processor.ctx.terminal.dirty = true; }, - CursorMoved { position, .. } => { + WindowEvent::CursorMoved { position, .. } => { let (x, y) = position.into(); let x = limit(x, 0, processor.ctx.size_info.width as i32); let y = limit(y, 0, processor.ctx.size_info.height as i32); @@ -560,11 +561,11 @@ impl Processor { processor.ctx.window.set_mouse_visible(true); processor.mouse_moved(x as usize, y as usize); }, - MouseWheel { delta, phase, .. } => { + WindowEvent::MouseWheel { delta, phase, .. } => { processor.ctx.window.set_mouse_visible(true); processor.mouse_wheel_input(delta, phase); }, - Focused(is_focused) => { + WindowEvent::Focused(is_focused) => { if window_id == processor.ctx.window.window_id() { processor.ctx.terminal.is_focused = is_focused; processor.ctx.terminal.dirty = true; @@ -578,33 +579,32 @@ impl Processor { processor.on_focus_change(is_focused); } }, - DroppedFile(path) => { + WindowEvent::DroppedFile(path) => { let path: String = path.to_string_lossy().into(); processor.ctx.write_to_pty(path.into_bytes()); }, - CursorLeft { .. } => { + WindowEvent::CursorLeft { .. } => { processor.ctx.mouse.inside_grid = false; if processor.highlighted_url.is_some() { processor.ctx.terminal.dirty = true; } }, - KeyboardInput { is_synthetic: true, .. } - | TouchpadPressure { .. } - | ScaleFactorChanged { .. } - | CursorEntered { .. } - | AxisMotion { .. } - | HoveredFileCancelled - | Destroyed - | ThemeChanged(_) - | HoveredFile(_) - | Touch(_) - | Moved(_) => (), + WindowEvent::KeyboardInput { is_synthetic: true, .. } + | WindowEvent::TouchpadPressure { .. } + | WindowEvent::ScaleFactorChanged { .. } + | WindowEvent::CursorEntered { .. } + | WindowEvent::AxisMotion { .. } + | WindowEvent::HoveredFileCancelled + | WindowEvent::Destroyed + | WindowEvent::ThemeChanged(_) + | WindowEvent::HoveredFile(_) + | WindowEvent::Touch(_) + | WindowEvent::Moved(_) => (), } }, GlutinEvent::DeviceEvent { event, .. } => { - use glutin::event::DeviceEvent::*; - if let ModifiersChanged(modifiers) = event { + if let DeviceEvent::ModifiersChanged(modifiers) = event { processor.modifiers_input(modifiers); } }, @@ -620,20 +620,17 @@ impl Processor { /// Check if an event is irrelevant and can be skipped fn skip_event(event: &GlutinEvent) -> bool { match event { - GlutinEvent::WindowEvent { event, .. } => { - use glutin::event::WindowEvent::*; - match event { - KeyboardInput { is_synthetic: true, .. } - | TouchpadPressure { .. } - | CursorEntered { .. } - | AxisMotion { .. } - | HoveredFileCancelled - | Destroyed - | HoveredFile(_) - | Touch(_) - | Moved(_) => true, - _ => false, - } + GlutinEvent::WindowEvent { event, .. } => match event { + WindowEvent::KeyboardInput { is_synthetic: true, .. } + | WindowEvent::TouchpadPressure { .. } + | WindowEvent::CursorEntered { .. } + | WindowEvent::AxisMotion { .. } + | WindowEvent::HoveredFileCancelled + | WindowEvent::Destroyed + | WindowEvent::HoveredFile(_) + | WindowEvent::Touch(_) + | WindowEvent::Moved(_) => true, + _ => false, }, GlutinEvent::Suspended { .. } | GlutinEvent::NewEvents { .. } diff --git a/alacritty_terminal/src/grid/mod.rs b/alacritty_terminal/src/grid/mod.rs index febdff6..34d989d 100644 --- a/alacritty_terminal/src/grid/mod.rs +++ b/alacritty_terminal/src/grid/mod.rs @@ -145,24 +145,21 @@ impl Grid { Grid { raw, cols, lines, display_offset: 0, selection: None, max_scroll_limit: scrollback } } - pub fn buffer_to_visible(&self, point: impl Into>) -> Option> { - let mut point = point.into(); - - if point.line < self.display_offset || point.line >= self.display_offset + self.lines.0 { - return None; + /// Clamp a buffer point to the visible region. + pub fn clamp_buffer_to_visible(&self, point: Point) -> Point { + if point.line < self.display_offset { + Point::new(self.lines - 1, self.cols - 1) + } else if point.line >= self.display_offset + self.lines.0 { + Point::new(Line(0), Column(0)) + } else { + // Since edgecases are handled, conversion is identical as visible to buffer + self.visible_to_buffer(point.into()).into() } - - point.line = self.lines.0 + self.display_offset - point.line - 1; - - Some(point) } + /// Convert viewport relative point to global buffer indexing. pub fn visible_to_buffer(&self, point: Point) -> Point { - Point { line: self.visible_line_to_buffer(point.line), col: point.col } - } - - fn visible_line_to_buffer(&self, line: Line) -> usize { - self.line_to_offset(line) + self.display_offset + Point { line: self.lines.0 + self.display_offset - point.line.0 - 1, col: point.col } } /// Update the size of the scrollback history @@ -453,17 +450,6 @@ impl Grid { self.lines = target; } - /// Convert a Line index (active region) to a buffer offset - /// - /// # Panics - /// - /// This method will panic if `Line` is larger than the grid dimensions - pub fn line_to_offset(&self, line: Line) -> usize { - assert!(line < self.num_lines()); - - *(self.num_lines() - line - 1) - } - #[inline] pub fn scroll_down(&mut self, region: &Range, positions: Line, template: &T) { let num_lines = self.num_lines().0; diff --git a/alacritty_terminal/src/grid/tests.rs b/alacritty_terminal/src/grid/tests.rs index e4fdad5..e8f4fb8 100644 --- a/alacritty_terminal/src/grid/tests.rs +++ b/alacritty_terminal/src/grid/tests.rs @@ -37,6 +37,40 @@ impl GridCell for usize { } } +#[test] +fn grid_clamp_buffer_point() { + let mut grid = Grid::new(Line(10), Column(10), 1_000, 0); + grid.display_offset = 5; + + let point = grid.clamp_buffer_to_visible(Point::new(10, Column(3))); + assert_eq!(point, Point::new(Line(4), Column(3))); + + let point = grid.clamp_buffer_to_visible(Point::new(15, Column(3))); + assert_eq!(point, Point::new(Line(0), Column(0))); + + let point = grid.clamp_buffer_to_visible(Point::new(4, Column(3))); + assert_eq!(point, Point::new(Line(9), Column(9))); + + grid.display_offset = 0; + + let point = grid.clamp_buffer_to_visible(Point::new(4, Column(3))); + assert_eq!(point, Point::new(Line(5), Column(3))); +} + +#[test] +fn visible_to_buffer() { + let mut grid = Grid::new(Line(10), Column(10), 1_000, 0); + grid.display_offset = 5; + + let point = grid.visible_to_buffer(Point::new(Line(4), Column(3))); + assert_eq!(point, Point::new(10, Column(3))); + + grid.display_offset = 0; + + let point = grid.visible_to_buffer(Point::new(Line(5), Column(3))); + assert_eq!(point, Point::new(4, Column(3))); +} + // Scroll up moves lines upwards #[test] fn scroll_up() { diff --git a/alacritty_terminal/src/index.rs b/alacritty_terminal/src/index.rs index 51566d8..56d3200 100644 --- a/alacritty_terminal/src/index.rs +++ b/alacritty_terminal/src/index.rs @@ -73,12 +73,11 @@ impl Point { impl Ord for Point { fn cmp(&self, other: &Point) -> Ordering { - use std::cmp::Ordering::*; match (self.line.cmp(&other.line), self.col.cmp(&other.col)) { - (Equal, Equal) => Equal, - (Equal, ord) | (ord, Equal) => ord, - (Less, _) => Less, - (Greater, _) => Greater, + (Ordering::Equal, Ordering::Equal) => Ordering::Equal, + (Ordering::Equal, ord) | (ord, Ordering::Equal) => ord, + (Ordering::Less, _) => Ordering::Less, + (Ordering::Greater, _) => Ordering::Greater, } } } diff --git a/alacritty_terminal/src/term/mod.rs b/alacritty_terminal/src/term/mod.rs index d545d68..660aeb7 100644 --- a/alacritty_terminal/src/term/mod.rs +++ b/alacritty_terminal/src/term/mod.rs @@ -222,9 +222,8 @@ impl<'a, C> RenderableCellsIter<'a, C> { ) -> RenderableCellsIter<'b, C> { let grid = &term.grid; let num_cols = grid.num_cols(); - let num_lines = grid.num_lines(); - let cursor_offset = grid.line_to_offset(term.cursor.point.line); + let cursor_offset = grid.num_lines().0 - term.cursor.point.line.0 - 1; let inner = grid.display_iter(); let selection_range = selection.and_then(|span| { @@ -235,28 +234,14 @@ impl<'a, C> RenderableCellsIter<'a, C> { }; // Get on-screen lines of the selection's locations - let start = term.buffer_to_visible(span.start); - let end = term.buffer_to_visible(span.end); - - // Clamp visible selection to the viewport - let (mut start, mut end) = match (start, end) { - (Some(start), Some(end)) => (start, end), - (Some(start), None) => { - let end = Point::new(num_lines.0 - 1, num_cols - 1); - (start, end) - }, - (None, Some(end)) => { - let start = Point::new(0, Column(0)); - (start, end) - }, - (None, None) => return None, - }; + let mut start = grid.clamp_buffer_to_visible(span.start); + let mut end = grid.clamp_buffer_to_visible(span.end); // Trim start/end with partially visible block selection start.col = max(limit_start, start.col); end.col = min(limit_end, end.col); - Some(SelectionRange::new(start.into(), end.into(), span.is_block)) + Some(SelectionRange::new(start, end, span.is_block)) }); // Load cursor glyph @@ -1085,10 +1070,6 @@ impl Term { self.grid.visible_to_buffer(point) } - pub fn buffer_to_visible(&self, point: impl Into>) -> Option> { - self.grid.buffer_to_visible(point) - } - /// Access to the raw grid data structure /// /// This is a bit of a hack; when the window is closed, the event processor diff --git a/font/src/ft/fc/mod.rs b/font/src/ft/fc/mod.rs index cfcac4a..ecb056f 100644 --- a/font/src/ft/fc/mod.rs +++ b/font/src/ft/fc/mod.rs @@ -155,18 +155,17 @@ pub enum Width { impl Width { fn to_isize(self) -> isize { - use self::Width::*; match self { - Ultracondensed => 50, - Extracondensed => 63, - Condensed => 75, - Semicondensed => 87, - Normal => 100, - Semiexpanded => 113, - Expanded => 125, - Extraexpanded => 150, - Ultraexpanded => 200, - Other(value) => value as isize, + Width::Ultracondensed => 50, + Width::Extracondensed => 63, + Width::Condensed => 75, + Width::Semicondensed => 87, + Width::Normal => 100, + Width::Semiexpanded => 113, + Width::Expanded => 125, + Width::Extraexpanded => 150, + Width::Ultraexpanded => 200, + Width::Other(value) => value as isize, } } } @@ -214,14 +213,13 @@ impl Rgba { impl fmt::Display for Rgba { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - use self::Rgba::*; f.write_str(match *self { - Unknown => "unknown", - Rgb => "rgb", - Bgr => "bgr", - Vrgb => "vrgb", - Vbgr => "vbgr", - None => "none", + Rgba::Unknown => "unknown", + Rgba::Rgb => "rgb", + Rgba::Bgr => "bgr", + Rgba::Vrgb => "vrgb", + Rgba::Vbgr => "vbgr", + Rgba::None => "none", }) } }