Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gui: improve mouse text selection #1805

Merged
merged 4 commits into from
Apr 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions term/src/input.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ pub struct MouseEvent {
pub kind: MouseEventKind,
pub x: usize,
pub y: VisibleRowIndex,
pub x_pixel_offset: usize,
pub y_pixel_offset: usize,
pub x_pixel_offset: isize,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have time for a full review right now, but this caught my eye: why is the type changing? I'm pretty sure that this can never legitimately be negative. @AutumnMeowMeow: can you confirm?

Copy link
Contributor Author

@davidrios davidrios Apr 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is for them to be able to have negative values, to encode information about the mouse leaving the area. For instance, y = 0 means that the cell is the top one, and y pixel offset = -10 means the mouse is 10 pixels above that cell. They'll only have negative values when x/y are = 0.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Sorry if incoherent, watching Matrix 4 while drunk... 🍷🔫🔫🔫👭🔫🔫🔫🔫🔫🔫🔫🔫🔫👭🔫🔫🔫🔫🔫🔫🔫🔫🔫💞💞)

The terminal should not be reporting negative values (they are required to be positive in CSI sequences), but at one point xterm itself was reporting negative when the mouse was captured (button down) and dragged above the top-left corner of the text area -- when using SGR-Pixel mode. I didn't report that as a bug, someone else might have. Let's see if I can find that thread.... here: contour-terminal/contour#574 (comment) Of course j4james is the real expert. :-)

Within wezterm, the offset values could work the same whether positive or negative values.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that the negative offset is only used internally/where it makes sense, in other places I've clipped it to positive only values, like here:

(event.x * (self.pixel_width / width)) + event.x_pixel_offset.max(0) as usize + 1,

Copy link
Contributor Author

@davidrios davidrios Apr 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the topic of those offsets, they have an unexpected (to me) behavior, so I'm not sure if it's a bug or could have weird side effects. These are the values reported:

wezterm_cell_reporting

The blue cells are what is reported in the x property, because of the rounding here:

// Round the x coordinate so that we're a bit more forgiving of
// the horizontal position when selecting cells
x.round()
, which is done so mouse selection feel more natural, yet the x_pixel_offset is calculated based on the real cell, so the reported cell, offset are not in sync.

I'm thinking maybe it would be better to always report the real cell and offset, and let the code that's using them do the relevant processing.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AutumnMeowMeow: what do you think about these positioning offsets; are they actually what you want to see in the report sent to your app?

pub y_pixel_offset: isize,
pub button: MouseButton,
pub modifiers: KeyModifiers,
}
Expand All @@ -46,8 +46,8 @@ pub struct MouseEvent {
pub struct ClickPosition {
pub column: usize,
pub row: i64,
pub x_pixel_offset: usize,
pub y_pixel_offset: usize,
pub x_pixel_offset: isize,
pub y_pixel_offset: isize,
}

/// This is a little helper that keeps track of the "click streak",
Expand Down
26 changes: 18 additions & 8 deletions term/src/terminalstate/mouse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,10 @@ impl TerminalState {
self.writer,
"\x1b[<{};{};{}M",
button,
(event.x * (self.pixel_width / width)) + event.x_pixel_offset + 1,
(event.y as usize * (self.pixel_height / height)) + event.y_pixel_offset + 1
(event.x * (self.pixel_width / width)) + event.x_pixel_offset.max(0) as usize + 1,
(event.y as usize * (self.pixel_height / height))
+ event.y_pixel_offset.max(0) as usize
+ 1
)?;
self.writer.flush()?;
} else if self.mouse_tracking || self.button_event_mouse || self.any_event_mouse {
Expand Down Expand Up @@ -123,8 +125,10 @@ impl TerminalState {
self.writer,
"\x1b[<{};{};{}M",
button,
(event.x * (self.pixel_width / width)) + event.x_pixel_offset + 1,
(event.y as usize * (self.pixel_height / height)) + event.y_pixel_offset + 1
(event.x * (self.pixel_width / width)) + event.x_pixel_offset.max(0) as usize + 1,
(event.y as usize * (self.pixel_height / height))
+ event.y_pixel_offset.max(0) as usize
+ 1
)?;
self.writer.flush()?;
} else {
Expand Down Expand Up @@ -162,9 +166,11 @@ impl TerminalState {
self.writer,
"\x1b[<{};{};{}m",
release_button,
(event.x * (self.pixel_width / width)) + event.x_pixel_offset + 1,
(event.x * (self.pixel_width / width))
+ event.x_pixel_offset.max(0) as usize
+ 1,
(event.y as usize * (self.pixel_height / height))
+ event.y_pixel_offset
+ event.y_pixel_offset.max(0) as usize
+ 1
)?;
self.writer.flush()?;
Expand Down Expand Up @@ -227,8 +233,12 @@ impl TerminalState {
self.writer,
"\x1b[<{};{};{}M",
button,
(event.x * (self.pixel_width / width)) + event.x_pixel_offset + 1,
(event.y as usize * (self.pixel_height / height)) + event.y_pixel_offset + 1
(event.x * (self.pixel_width / width))
+ event.x_pixel_offset.max(0) as usize
+ 1,
(event.y as usize * (self.pixel_height / height))
+ event.y_pixel_offset.max(0) as usize
+ 1
)?;
self.writer.flush()?;
} else {
Expand Down
2 changes: 1 addition & 1 deletion wezterm-gui/src/overlay/copy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ impl CopyRenderable {
self.window
.notify(TermWindowNotif::Apply(Box::new(move |term_window| {
let mut selection = term_window.selection(pane_id);
selection.start = Some(start);
selection.origin = Some(start);
selection.range = Some(range);
window.invalidate();
})));
Expand Down
4 changes: 2 additions & 2 deletions wezterm-gui/src/overlay/quickselect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -685,7 +685,7 @@ impl QuickSelectRenderable {
self.window
.notify(TermWindowNotif::Apply(Box::new(move |term_window| {
let mut selection = term_window.selection(pane_id);
selection.start.take();
selection.origin.take();
selection.range.take();
})));
}
Expand All @@ -705,7 +705,7 @@ impl QuickSelectRenderable {
x: result.start_x,
y: result.start_y,
};
selection.start = Some(start);
selection.origin = Some(start);
selection.range = Some(SelectionRange {
start,
end: SelectionCoordinate {
Expand Down
4 changes: 2 additions & 2 deletions wezterm-gui/src/overlay/search.rs
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,7 @@ impl SearchRenderable {
self.window
.notify(TermWindowNotif::Apply(Box::new(move |term_window| {
let mut selection = term_window.selection(pane_id);
selection.start.take();
selection.origin.take();
selection.range.take();
})));
}
Expand All @@ -507,7 +507,7 @@ impl SearchRenderable {
x: result.start_x,
y: result.start_y,
};
selection.start = Some(start);
selection.origin = Some(start);
selection.range = Some(SelectionRange {
start,
end: SelectionCoordinate {
Expand Down
8 changes: 4 additions & 4 deletions wezterm-gui/src/selection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use wezterm_term::{SemanticZone, StableRowIndex};
pub struct Selection {
/// Remembers the starting coordinate of the selection prior to
/// dragging.
pub start: Option<SelectionCoordinate>,
pub origin: Option<SelectionCoordinate>,
/// Holds the not-normalized selection range.
pub range: Option<SelectionRange>,
/// When the selection was made wrt. the pane content
Expand All @@ -25,12 +25,12 @@ impl Selection {
#[allow(dead_code)]
pub fn clear(&mut self) {
self.range = None;
self.start = None;
self.origin = None;
}

pub fn begin(&mut self, start: SelectionCoordinate) {
pub fn begin(&mut self, origin: SelectionCoordinate) {
self.range = None;
self.start = Some(start);
self.origin = Some(origin);
}

#[allow(dead_code)]
Expand Down
25 changes: 13 additions & 12 deletions wezterm-gui/src/termwindow/box_model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::customglyph::{BlockKey, Poly};
use crate::glyphcache::CachedGlyph;
use crate::termwindow::render::rgbcolor_to_window_color;
use crate::termwindow::{
MappedQuads, RenderState, SrgbTexture2d, TermWindowNotif, UIItem, UIItemType,
MappedQuads, MouseCapture, RenderState, SrgbTexture2d, TermWindowNotif, UIItem, UIItemType,
};
use crate::utilsprites::RenderMetrics;
use ::window::{RectF, WindowOps};
Expand Down Expand Up @@ -756,17 +756,18 @@ impl super::TermWindow {
) -> anyhow::Result<()> {
let colors = match &element.hover_colors {
Some(hc) => {
let hovering = match &self.current_mouse_event {
Some(event) => {
let mouse_x = event.coords.x as f32;
let mouse_y = event.coords.y as f32;
mouse_x >= element.bounds.min_x()
&& mouse_x <= element.bounds.max_x()
&& mouse_y >= element.bounds.min_y()
&& mouse_y <= element.bounds.max_y()
}
None => false,
};
let hovering =
match &self.current_mouse_event {
Some(event) => {
let mouse_x = event.coords.x as f32;
let mouse_y = event.coords.y as f32;
mouse_x >= element.bounds.min_x()
&& mouse_x <= element.bounds.max_x()
&& mouse_y >= element.bounds.min_y()
&& mouse_y <= element.bounds.max_y()
}
None => false,
} && matches!(self.current_mouse_capture, None | Some(MouseCapture::UI));
if hovering {
hc
} else {
Expand Down
19 changes: 12 additions & 7 deletions wezterm-gui/src/termwindow/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ use crate::scrollbar::*;
use crate::selection::Selection;
use crate::shapecache::*;
use crate::tabbar::{TabBarItem, TabBarState};
use ::wezterm_term::input::{ClickPosition, MouseButton as TMB};
use crate::termwindow::keyevent::KeyTableState;
use ::wezterm_term::input::MouseButton as TMB;
use ::window::*;
use anyhow::{anyhow, ensure, Context};
use config::keyassignment::{
Expand Down Expand Up @@ -86,6 +86,12 @@ pub fn get_window_class() -> String {
WINDOW_CLASS.lock().unwrap().clone()
}

#[derive(Clone, Debug, PartialEq, Eq)]
pub enum MouseCapture {
UI,
Terminal,
}

/// Type used together with Window::notify to do something in the
/// context of the window-specific event loop
pub enum TermWindowNotif {
Expand Down Expand Up @@ -165,7 +171,7 @@ pub struct PaneState {
pub overlay: Option<Rc<dyn Pane>>,

bell_start: Option<Instant>,
pub mouse_terminal_coords: Option<(usize, StableRowIndex)>,
pub mouse_terminal_coords: Option<(ClickPosition, StableRowIndex)>,
}

/// Data used when synchronously formatting pane and window titles
Expand Down Expand Up @@ -330,6 +336,7 @@ pub struct TermWindow {
window_background: Option<Arc<ImageData>>,

current_mouse_buttons: Vec<MousePress>,
current_mouse_capture: Option<MouseCapture>,

/// Keeps track of double and triple clicks
last_mouse_click: Option<LastMouseClick>,
Expand Down Expand Up @@ -426,6 +433,7 @@ impl TermWindow {
if self.focused.is_none() {
self.last_mouse_click = None;
self.current_mouse_buttons.clear();
self.current_mouse_capture = None;
self.is_click_to_focus_window = false;

for state in self.pane_state.borrow_mut().values_mut() {
Expand Down Expand Up @@ -744,6 +752,7 @@ impl TermWindow {
tab_state: RefCell::new(HashMap::new()),
pane_state: RefCell::new(HashMap::new()),
current_mouse_buttons: vec![],
current_mouse_capture: None,
last_mouse_click: None,
current_highlight: None,
shape_cache: RefCell::new(LruCache::new(
Expand Down Expand Up @@ -1364,7 +1373,7 @@ impl TermWindow {

if clear_selection {
self.selection(pane.pane_id()).range.take();
self.selection(pane.pane_id()).start.take();
self.selection(pane.pane_id()).origin.take();
self.selection(pane.pane_id()).seqno = pane.get_current_seqno();
}
}
Expand Down Expand Up @@ -2455,10 +2464,6 @@ impl TermWindow {
})
}

pub fn selection(&self, pane_id: PaneId) -> RefMut<Selection> {
RefMut::map(self.pane_state(pane_id), |state| &mut state.selection)
}

pub fn get_viewport(&self, pane_id: PaneId) -> Option<StableRowIndex> {
self.pane_state(pane_id).viewport
}
Expand Down
73 changes: 48 additions & 25 deletions wezterm-gui/src/termwindow/mouseevent.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::tabbar::TabBarItem;
use crate::termwindow::keyevent::window_mods_to_termwiz_mods;
use crate::termwindow::{PositionedSplit, ScrollHit, UIItem, UIItemType, TMB};
use crate::termwindow::{MouseCapture, PositionedSplit, ScrollHit, UIItem, UIItemType, TMB};
use ::window::{
MouseButtons as WMB, MouseCursor, MouseEvent, MouseEventKind as WMEK, MousePress, WindowOps,
WindowState,
Expand Down Expand Up @@ -90,21 +90,27 @@ impl super::TermWindow {
}
.trunc() as usize;

let y_pixel_offset = (event
let mut y_pixel_offset = event
.coords
.y
.sub(padding_top as isize)
.sub(first_line_offset)
.max(0)
% self.render_metrics.cell_size.height) as usize;
.sub(first_line_offset);
if y > 0 {
y_pixel_offset = y_pixel_offset.max(0) % self.render_metrics.cell_size.height;
}

let x_pixel_offset = (event.coords.x.sub(padding_left as isize).max(0)
% self.render_metrics.cell_size.width) as usize;
let mut x_pixel_offset = event.coords.x.sub(padding_left as isize);
if x > 0 {
x_pixel_offset = x_pixel_offset.max(0) % self.render_metrics.cell_size.width;
}

self.last_mouse_coords = (x, y);

let mut capture_mouse = false;

match event.kind {
WMEK::Release(ref press) => {
self.current_mouse_capture = None;
self.current_mouse_buttons.retain(|p| p != press);
if press == &MousePress::Left && self.window_drag_position.take().is_some() {
// Completed a window drag
Expand All @@ -117,6 +123,8 @@ impl super::TermWindow {
}

WMEK::Press(ref press) => {
capture_mouse = true;

// Perform click counting
let button = mouse_press_to_tmb(press);

Expand Down Expand Up @@ -166,28 +174,43 @@ impl super::TermWindow {
_ => {}
}

let ui_item = self.resolve_ui_item(&event);
let ui_item = if matches!(self.current_mouse_capture, None | Some(MouseCapture::UI)) {
let ui_item = self.resolve_ui_item(&event);

match (self.last_ui_item.take(), &ui_item) {
(Some(prior), Some(item)) => {
self.leave_ui_item(&prior);
self.enter_ui_item(item);
context.invalidate();
}
(Some(prior), None) => {
self.leave_ui_item(&prior);
context.invalidate();
}
(None, Some(item)) => {
self.enter_ui_item(item);
context.invalidate();
match (self.last_ui_item.take(), &ui_item) {
(Some(prior), Some(item)) => {
self.leave_ui_item(&prior);
self.enter_ui_item(item);
context.invalidate();
}
(Some(prior), None) => {
self.leave_ui_item(&prior);
context.invalidate();
}
(None, Some(item)) => {
self.enter_ui_item(item);
context.invalidate();
}
(None, None) => {}
}
(None, None) => {}
}

ui_item
} else {
None
};

if let Some(item) = ui_item {
if capture_mouse {
self.current_mouse_capture = Some(MouseCapture::UI);
}
self.mouse_event_ui_item(item, pane, y, event, context);
} else {
} else if matches!(
self.current_mouse_capture,
None | Some(MouseCapture::Terminal)
) {
if capture_mouse {
self.current_mouse_capture = Some(MouseCapture::Terminal);
}
self.mouse_event_terminal(
pane,
ClickPosition {
Expand Down Expand Up @@ -593,7 +616,7 @@ impl super::TermWindow {

self.pane_state(pane.pane_id())
.mouse_terminal_coords
.replace((x, stable_row));
.replace((position.clone(), stable_row));

let (top, mut lines) = pane.get_lines_with_hyperlinks_applied(
stable_row..stable_row + 1,
Expand Down
Loading