From a1ed79bd2c014be49a85e2100ce374b86c8839e8 Mon Sep 17 00:00:00 2001 From: Christian Duerr Date: Sat, 5 Oct 2024 11:39:06 +0200 Subject: [PATCH] Fix highlight invalidation on grid scroll This fixes an issue where hints highlighted by vi or mouse cursor would produce an underline on the incorrect line since the highlights only store the initial match boundaries without accounting for new content scrolling the terminal. To accurately invalidate the hint highlights, we use existing damage information of the current frame. The existing logic to damage hints for the next frame to account for removal has been changed, since the hints would otherwise be cleared immediately. Instead we now mark the terminal as fully damaged for the upcoming frame whenever the hints are cleared. Closes #7737. --- CHANGELOG.md | 1 + alacritty/src/display/damage.rs | 9 ++ alacritty/src/display/mod.rs | 175 +++++++++++++++------------- alacritty_terminal/src/term/cell.rs | 2 - 4 files changed, 106 insertions(+), 81 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 659a098e..2026893c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,7 @@ Notable changes to the `alacritty_terminal` crate are documented in its - Fullwidth semantic escape characters - Windows app icon now displays properly in old alt+tab on Windows - Alacritty not being properly activated with startup notify +- Invalid URL highlights after terminal scrolling ## 0.13.2 diff --git a/alacritty/src/display/damage.rs b/alacritty/src/display/damage.rs index 450643b7..8efe0133 100644 --- a/alacritty/src/display/damage.rs +++ b/alacritty/src/display/damage.rs @@ -189,6 +189,15 @@ impl FrameDamage { self.lines.push(LineDamageBounds::undamaged(line, num_cols)); } } + + /// Check if a range is damaged. + #[inline] + pub fn intersects(&self, start: Point, end: Point) -> bool { + self.full + || self.lines[start.line].left <= start.column + || self.lines[end.line].right >= end.column + || (start.line + 1..end.line).any(|line| self.lines[line].is_damaged()) + } } /// Convert viewport `y` coordinate to [`Rect`] damage coordinate. diff --git a/alacritty/src/display/mod.rs b/alacritty/src/display/mod.rs index 7809c824..51553288 100644 --- a/alacritty/src/display/mod.rs +++ b/alacritty/src/display/mod.rs @@ -29,8 +29,7 @@ use alacritty_terminal::index::{Column, Direction, Line, Point}; use alacritty_terminal::selection::Selection; use alacritty_terminal::term::cell::Flags; use alacritty_terminal::term::{ - self, point_to_viewport, LineDamageBounds, Term, TermDamage, TermMode, MIN_COLUMNS, - MIN_SCREEN_LINES, + self, LineDamageBounds, Term, TermDamage, TermMode, MIN_COLUMNS, MIN_SCREEN_LINES, }; use alacritty_terminal::vte::ansi::{CursorShape, NamedColor}; @@ -746,39 +745,37 @@ impl Display { let vi_cursor_point = if vi_mode { Some(terminal.vi_mode_cursor.point) } else { None }; // Add damage from the terminal. - if self.collect_damage() { - match terminal.damage() { - TermDamage::Full => self.damage_tracker.frame().mark_fully_damaged(), - TermDamage::Partial(damaged_lines) => { - for damage in damaged_lines { - self.damage_tracker.frame().damage_line(damage); - } - }, - } - terminal.reset_damage(); + match terminal.damage() { + TermDamage::Full => self.damage_tracker.frame().mark_fully_damaged(), + TermDamage::Partial(damaged_lines) => { + for damage in damaged_lines { + self.damage_tracker.frame().damage_line(damage); + } + }, } + terminal.reset_damage(); // Drop terminal as early as possible to free lock. drop(terminal); + // Invalidate highlighted hints if grid has changed. + self.validate_hints(display_offset); + // Add damage from alacritty's UI elements overlapping terminal. - if self.collect_damage() { - let requires_full_damage = self.visual_bell.intensity() != 0. - || self.hint_state.active() - || search_state.regex().is_some(); - if requires_full_damage { - self.damage_tracker.frame().mark_fully_damaged(); - self.damage_tracker.next_frame().mark_fully_damaged(); - } - - let vi_cursor_viewport_point = - vi_cursor_point.and_then(|cursor| point_to_viewport(display_offset, cursor)); - - self.damage_tracker.damage_vi_cursor(vi_cursor_viewport_point); - self.damage_tracker.damage_selection(selection_range, display_offset); + let requires_full_damage = self.visual_bell.intensity() != 0. + || self.hint_state.active() + || search_state.regex().is_some(); + if requires_full_damage { + self.damage_tracker.frame().mark_fully_damaged(); + self.damage_tracker.next_frame().mark_fully_damaged(); } + let vi_cursor_viewport_point = + vi_cursor_point.and_then(|cursor| term::point_to_viewport(display_offset, cursor)); + self.damage_tracker.damage_vi_cursor(vi_cursor_viewport_point); + self.damage_tracker.damage_selection(selection_range, display_offset); + // Make sure this window's OpenGL context is active. self.make_current(); @@ -802,36 +799,27 @@ impl Display { let vi_highlighted_hint = &self.vi_highlighted_hint; let damage_tracker = &mut self.damage_tracker; - self.renderer.draw_cells( - &size_info, - glyph_cache, - grid_cells.into_iter().map(|mut cell| { - // Underline hints hovered by mouse or vi mode cursor. + let cells = grid_cells.into_iter().map(|mut cell| { + // Underline hints hovered by mouse or vi mode cursor. + if has_highlighted_hint { let point = term::viewport_to_point(display_offset, cell.point); + let hyperlink = cell.extra.as_ref().and_then(|extra| extra.hyperlink.as_ref()); - if has_highlighted_hint { - let hyperlink = - cell.extra.as_ref().and_then(|extra| extra.hyperlink.as_ref()); - if highlighted_hint - .as_ref() - .map_or(false, |hint| hint.should_highlight(point, hyperlink)) - || vi_highlighted_hint - .as_ref() - .map_or(false, |hint| hint.should_highlight(point, hyperlink)) - { - cell.flags.insert(Flags::UNDERLINE); - // Damage hints for the current and next frames. - damage_tracker.frame().damage_point(cell.point); - damage_tracker.next_frame().damage_point(cell.point); - } + let should_highlight = |hint: &Option| { + hint.as_ref().map_or(false, |hint| hint.should_highlight(point, hyperlink)) + }; + if should_highlight(highlighted_hint) || should_highlight(vi_highlighted_hint) { + damage_tracker.frame().damage_point(cell.point); + cell.flags.insert(Flags::UNDERLINE); } + } - // Update underline/strikeout. - lines.update(&cell); + // Update underline/strikeout. + lines.update(&cell); - cell - }), - ); + cell + }); + self.renderer.draw_cells(&size_info, glyph_cache, cells); } let mut rects = lines.rects(&metrics, &size_info); @@ -1025,10 +1013,17 @@ impl Display { let mut dirty = vi_highlighted_hint != self.vi_highlighted_hint; self.vi_highlighted_hint = vi_highlighted_hint; + // Force full redraw if the vi mode highlight was cleared. + if dirty && self.vi_highlighted_hint.is_none() { + self.damage_tracker.frame().mark_fully_damaged(); + } + // Abort if mouse highlighting conditions are not met. if !mouse.inside_text_area || !term.selection.as_ref().map_or(true, Selection::is_empty) { - dirty |= self.highlighted_hint.is_some(); - self.highlighted_hint = None; + if self.highlighted_hint.take().is_some() { + self.damage_tracker.frame().mark_fully_damaged(); + dirty = true; + } return dirty; } @@ -1052,9 +1047,15 @@ impl Display { } } - dirty |= self.highlighted_hint != highlighted_hint; + let mouse_highlight_dirty = self.highlighted_hint != highlighted_hint; + dirty |= mouse_highlight_dirty; self.highlighted_hint = highlighted_hint; + // Force full redraw if the mouse cursor highlight was cleared. + if mouse_highlight_dirty && self.highlighted_hint.is_none() { + self.damage_tracker.frame().mark_fully_damaged(); + } + dirty } @@ -1113,7 +1114,7 @@ impl Display { ); // Damage preedit inside the terminal viewport. - if self.collect_damage() && point.line < self.size_info.screen_lines() { + if point.line < self.size_info.screen_lines() { let damage = LineDamageBounds::new(start.line, 0, num_cols); self.damage_tracker.frame().damage_line(damage); self.damage_tracker.next_frame().damage_line(damage); @@ -1224,13 +1225,11 @@ impl Display { let bg = config.colors.footer_bar_background(); for (uri, point) in uris.into_iter().zip(uri_lines) { // Damage the uri preview. - if self.collect_damage() { - let damage = LineDamageBounds::new(point.line, point.column.0, num_cols); - self.damage_tracker.frame().damage_line(damage); + let damage = LineDamageBounds::new(point.line, point.column.0, num_cols); + self.damage_tracker.frame().damage_line(damage); - // Damage the uri preview for the next frame as well. - self.damage_tracker.next_frame().damage_line(damage); - } + // Damage the uri preview for the next frame as well. + self.damage_tracker.next_frame().damage_line(damage); self.renderer.draw_string(point, fg, bg, uri, &self.size_info, &mut self.glyph_cache); } @@ -1270,12 +1269,10 @@ impl Display { let fg = config.colors.primary.background; let bg = config.colors.normal.red; - if self.collect_damage() { - let damage = LineDamageBounds::new(point.line, point.column.0, timing.len()); - self.damage_tracker.frame().damage_line(damage); - // Damage the render timer for the next frame. - self.damage_tracker.next_frame().damage_line(damage); - } + // Damage render timer for current and next frame. + let damage = LineDamageBounds::new(point.line, point.column.0, timing.len()); + self.damage_tracker.frame().damage_line(damage); + self.damage_tracker.next_frame().damage_line(damage); let glyph_cache = &mut self.glyph_cache; self.renderer.draw_string(point, fg, bg, timing.chars(), &self.size_info, glyph_cache); @@ -1295,12 +1292,10 @@ impl Display { let column = Column(self.size_info.columns().saturating_sub(text.len())); let point = Point::new(0, column); - if self.collect_damage() { - let damage = LineDamageBounds::new(point.line, point.column.0, columns - 1); - self.damage_tracker.frame().damage_line(damage); - // Damage it on the next frame in case it goes away. - self.damage_tracker.next_frame().damage_line(damage); - } + // Damage the line indicator for current and next frame. + let damage = LineDamageBounds::new(point.line, point.column.0, columns - 1); + self.damage_tracker.frame().damage_line(damage); + self.damage_tracker.next_frame().damage_line(damage); let colors = &config.colors; let fg = colors.line_indicator.foreground.unwrap_or(colors.primary.background); @@ -1313,12 +1308,6 @@ impl Display { } } - /// Returns `true` if damage information should be collected, `false` otherwise. - #[inline] - fn collect_damage(&self) -> bool { - matches!(self.raw_window_handle, RawWindowHandle::Wayland(_)) || self.damage_tracker.debug - } - /// Highlight damaged rects. /// /// This function is for debug purposes only. @@ -1334,6 +1323,34 @@ impl Display { } } + /// Check whether a hint highlight needs to be cleared. + fn validate_hints(&mut self, display_offset: usize) { + let frame = self.damage_tracker.frame(); + for (hint, reset_mouse) in + [(&mut self.highlighted_hint, true), (&mut self.vi_highlighted_hint, false)] + { + let (start, end) = match hint { + Some(hint) => (*hint.bounds().start(), *hint.bounds().end()), + None => return, + }; + + // Convert hint bounds to viewport coordinates. + let start = term::point_to_viewport(display_offset, start).unwrap_or_default(); + let end = term::point_to_viewport(display_offset, end).unwrap_or_else(|| { + Point::new(self.size_info.screen_lines() - 1, self.size_info.last_column()) + }); + + // Clear invalidated hints. + if frame.intersects(start, end) { + if reset_mouse { + self.window.set_mouse_cursor(CursorIcon::Default); + } + frame.mark_fully_damaged(); + *hint = None; + } + } + } + /// Request a new frame for a window on Wayland. fn request_frame(&mut self, scheduler: &mut Scheduler) { // Mark that we've used a frame. diff --git a/alacritty_terminal/src/term/cell.rs b/alacritty_terminal/src/term/cell.rs index 81dc1e3a..73077644 100644 --- a/alacritty_terminal/src/term/cell.rs +++ b/alacritty_terminal/src/term/cell.rs @@ -124,9 +124,7 @@ impl ResetDiscriminant for Cell { #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] pub struct CellExtra { zerowidth: Vec, - underline_color: Option, - hyperlink: Option, }