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

@@ -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
 

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<N: Notify + OnResize> Processor<N> {
             },
             GlutinEvent::RedrawRequested(_) => processor.ctx.terminal.dirty = true,
             GlutinEvent::WindowEvent { event, window_id, .. } => {
-                use glutin::event::WindowEvent::*;
                 match event {
-                    CloseRequested => processor.ctx.terminal.exit(),
+                    WindowEvent::CloseRequested => processor.ctx.terminal.exit(),
-                    Resized(size) => {
+                    WindowEvent::Resized(size) => {
                         #[cfg(windows)]
                         {
                             // Minimizing the window sends a Resize event with zero width and
@@ -537,7 +538,7 @@ impl<N: Notify + OnResize> Processor<N> {
                         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<N: Notify + OnResize> Processor<N> {
                             }
                         }
                     },
-                    ReceivedCharacter(c) => processor.received_char(c),
+                    WindowEvent::ReceivedCharacter(c) => processor.received_char(c),
-                    MouseInput { state, button, .. } => {
+                    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<N: Notify + OnResize> Processor<N> {
                         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<N: Notify + OnResize> Processor<N> {
                             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, .. }
+                    WindowEvent::KeyboardInput { is_synthetic: true, .. }
-                    | TouchpadPressure { .. }
+                    | WindowEvent::TouchpadPressure { .. }
-                    | ScaleFactorChanged { .. }
+                    | WindowEvent::ScaleFactorChanged { .. }
-                    | CursorEntered { .. }
+                    | WindowEvent::CursorEntered { .. }
-                    | AxisMotion { .. }
+                    | WindowEvent::AxisMotion { .. }
-                    | HoveredFileCancelled
+                    | WindowEvent::HoveredFileCancelled
-                    | Destroyed
+                    | WindowEvent::Destroyed
-                    | ThemeChanged(_)
+                    | WindowEvent::ThemeChanged(_)
-                    | HoveredFile(_)
+                    | WindowEvent::HoveredFile(_)
-                    | Touch(_)
+                    | WindowEvent::Touch(_)
-                    | Moved(_) => (),
+                    | WindowEvent::Moved(_) => (),
                 }
             },
             GlutinEvent::DeviceEvent { event, .. } => {
-                use glutin::event::DeviceEvent::*;
+                if let DeviceEvent::ModifiersChanged(modifiers) = event {
-                if let ModifiersChanged(modifiers) = event {
                     processor.modifiers_input(modifiers);
                 }
             },
@@ -620,20 +620,17 @@ impl<N: Notify + OnResize> Processor<N> {
     /// Check if an event is irrelevant and can be skipped
     fn skip_event(event: &GlutinEvent<Event>) -> bool {
         match event {
-            GlutinEvent::WindowEvent { event, .. } => {
+            GlutinEvent::WindowEvent { event, .. } => match event {
-                use glutin::event::WindowEvent::*;
+                WindowEvent::KeyboardInput { is_synthetic: true, .. }
-                match event {
+                | WindowEvent::TouchpadPressure { .. }
-                    KeyboardInput { is_synthetic: true, .. }
+                | WindowEvent::CursorEntered { .. }
-                    | TouchpadPressure { .. }
+                | WindowEvent::AxisMotion { .. }
-                    | CursorEntered { .. }
+                | WindowEvent::HoveredFileCancelled
-                    | AxisMotion { .. }
+                | WindowEvent::Destroyed
-                    | HoveredFileCancelled
+                | WindowEvent::HoveredFile(_)
-                    | Destroyed
+                | WindowEvent::Touch(_)
-                    | HoveredFile(_)
+                | WindowEvent::Moved(_) => true,
-                    | Touch(_)
+                _ => false,
-                    | Moved(_) => true,
-                    _ => false,
-                }
             },
             GlutinEvent::Suspended { .. }
             | GlutinEvent::NewEvents { .. }

alacritty_terminal/src/grid/mod.rs

@@ -145,24 +145,21 @@ impl<T: GridCell + PartialEq + Copy> Grid<T> {
         Grid { raw, cols, lines, display_offset: 0, selection: None, max_scroll_limit: scrollback }
     }
 
-    pub fn buffer_to_visible(&self, point: impl Into<Point<usize>>) -> Option<Point<usize>> {
+    /// Clamp a buffer point to the visible region.
-        let mut point = point.into();
+    pub fn clamp_buffer_to_visible(&self, point: Point<usize>) -> Point {
-
+        if point.line < self.display_offset {
-        if point.line < self.display_offset || point.line >= self.display_offset + self.lines.0 {
+            Point::new(self.lines - 1, self.cols - 1)
-            return None;
+        } 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<usize> {
-        Point { line: self.visible_line_to_buffer(point.line), col: point.col }
+        Point { line: self.lines.0 + self.display_offset - point.line.0 - 1, col: point.col }
-    }
-
-    fn visible_line_to_buffer(&self, line: Line) -> usize {
-        self.line_to_offset(line) + self.display_offset
     }
 
     /// Update the size of the scrollback history
@@ -453,17 +450,6 @@ impl<T: GridCell + PartialEq + Copy> Grid<T> {
         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<Line>, positions: Line, template: &T) {
         let num_lines = self.num_lines().0;

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() {

alacritty_terminal/src/index.rs

@@ -73,12 +73,11 @@ impl<L> Point<L> {
 
 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,
+            (Ordering::Equal, Ordering::Equal) => Ordering::Equal,
-            (Equal, ord) | (ord, Equal) => ord,
+            (Ordering::Equal, ord) | (ord, Ordering::Equal) => ord,
-            (Less, _) => Less,
+            (Ordering::Less, _) => Ordering::Less,
-            (Greater, _) => Greater,
+            (Ordering::Greater, _) => Ordering::Greater,
         }
     }
 }

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 mut start = grid.clamp_buffer_to_visible(span.start);
-            let end = term.buffer_to_visible(span.end);
+            let mut end = grid.clamp_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,
-            };
 
             // 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<T> Term<T> {
         self.grid.visible_to_buffer(point)
     }
 
-    pub fn buffer_to_visible(&self, point: impl Into<Point<usize>>) -> Option<Point<usize>> {
-        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

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,
+            Width::Ultracondensed => 50,
-            Extracondensed => 63,
+            Width::Extracondensed => 63,
-            Condensed => 75,
+            Width::Condensed => 75,
-            Semicondensed => 87,
+            Width::Semicondensed => 87,
-            Normal => 100,
+            Width::Normal => 100,
-            Semiexpanded => 113,
+            Width::Semiexpanded => 113,
-            Expanded => 125,
+            Width::Expanded => 125,
-            Extraexpanded => 150,
+            Width::Extraexpanded => 150,
-            Ultraexpanded => 200,
+            Width::Ultraexpanded => 200,
-            Other(value) => value as isize,
+            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",
+            Rgba::Unknown => "unknown",
-            Rgb => "rgb",
+            Rgba::Rgb => "rgb",
-            Bgr => "bgr",
+            Rgba::Bgr => "bgr",
-            Vrgb => "vrgb",
+            Rgba::Vrgb => "vrgb",
-            Vbgr => "vbgr",
+            Rgba::Vbgr => "vbgr",
-            None => "none",
+            Rgba::None => "none",
         })
     }
 }