From 764597851c4f9bd88e31ccf581c38713e49fe59a Mon Sep 17 00:00:00 2001 From: Wez Furlong Date: Sat, 15 Jun 2019 21:33:10 -0700 Subject: [PATCH] allow collapsing mouse events in the mux protocol Repeated moves or wheel events are collapsed so that we don't clog up the queue. The queue size doesn't matter as much as the latency of processing a large queue. For fast or repeated moves the queue can grow rather quickly, and with what is currently ~25-50 ms round trip per entry for a remote session, that is a poor UX. --- src/frontend/glium/window.rs | 32 +++---- src/frontend/xwindows/xwin.rs | 4 +- src/main.rs | 2 +- src/server/client.rs | 7 +- src/server/domain.rs | 13 ++- src/server/tab.rs | 172 ++++++++++++++++++++++++++++------ term/src/input.rs | 4 +- term/src/terminalstate.rs | 12 +-- 8 files changed, 179 insertions(+), 67 deletions(-) diff --git a/src/frontend/glium/window.rs b/src/frontend/glium/window.rs index 9ebd42e4c..831fae164 100644 --- a/src/frontend/glium/window.rs +++ b/src/frontend/glium/window.rs @@ -380,19 +380,19 @@ impl GliumTerminalWindow { // We currently only care about vertical scrolling so the code // below will return early if all we have is horizontal scroll // components. - let (button, times) = match delta { + let button = match delta { glutin::MouseScrollDelta::LineDelta(_, lines) if lines > 0.0 => { - (MouseButton::WheelUp, lines.abs().ceil() as usize) + MouseButton::WheelUp(lines.abs().ceil() as usize) } glutin::MouseScrollDelta::LineDelta(_, lines) if lines < 0.0 => { - (MouseButton::WheelDown, lines.abs().ceil() as usize) + MouseButton::WheelDown(lines.abs().ceil() as usize) } glutin::MouseScrollDelta::PixelDelta(position) => { let lines = position.y / self.cell_height as f64; if lines > 0.0 { - (MouseButton::WheelUp, lines.abs().ceil() as usize) + MouseButton::WheelUp(lines.abs().ceil() as usize) } else if lines < 0.0 { - (MouseButton::WheelDown, lines.abs().ceil() as usize) + MouseButton::WheelDown(lines.abs().ceil() as usize) } else { return Ok(()); } @@ -405,18 +405,16 @@ impl GliumTerminalWindow { Some(tab) => tab, None => return Ok(()), }; - for _ in 0..times { - tab.mouse_event( - term::MouseEvent { - kind: MouseEventKind::Press, - button, - x: (self.last_mouse_coords.x as usize / self.cell_width) as usize, - y: (self.last_mouse_coords.y as usize / self.cell_height) as i64, - modifiers: Self::decode_modifiers(modifiers), - }, - &mut TabHost::new(&mut *tab.writer(), &mut self.host), - )?; - } + tab.mouse_event( + term::MouseEvent { + kind: MouseEventKind::Press, + button, + x: (self.last_mouse_coords.x as usize / self.cell_width) as usize, + y: (self.last_mouse_coords.y as usize / self.cell_height) as i64, + modifiers: Self::decode_modifiers(modifiers), + }, + &mut TabHost::new(&mut *tab.writer(), &mut self.host), + )?; self.paint_if_needed()?; Ok(()) diff --git a/src/frontend/xwindows/xwin.rs b/src/frontend/xwindows/xwin.rs index a1684512d..409b25612 100644 --- a/src/frontend/xwindows/xwin.rs +++ b/src/frontend/xwindows/xwin.rs @@ -236,8 +236,8 @@ impl X11TerminalWindow { 1 => MouseButton::Left, 2 => MouseButton::Middle, 3 => MouseButton::Right, - 4 => MouseButton::WheelUp, - 5 => MouseButton::WheelDown, + 4 => MouseButton::WheelUp(1), + 5 => MouseButton::WheelDown(1), _ => { error!("button {} is not implemented", button_press.detail()); return Ok(()); diff --git a/src/main.rs b/src/main.rs index 84880ce61..618f13406 100644 --- a/src/main.rs +++ b/src/main.rs @@ -210,7 +210,7 @@ fn main() -> Result<(), Error> { run_terminal_gui(config, &start) } SubCommand::Cli(cli) => { - let mut client = Client::new_unix_domain(&config)?; + let client = Client::new_unix_domain(&config)?; match cli.sub { CliSubCommand::List => { let cols = vec![ diff --git a/src/server/client.rs b/src/server/client.rs index 87b05cd55..e73e31f29 100644 --- a/src/server/client.rs +++ b/src/server/client.rs @@ -23,13 +23,14 @@ enum ReaderMessage { SendPdu { pdu: Pdu, promise: Promise }, } +#[derive(Clone)] pub struct Client { sender: Sender, } macro_rules! rpc { ($method_name:ident, $request_type:ident, $response_type:ident) => { - pub fn $method_name(&mut self, pdu: $request_type) -> Future<$response_type> { + pub fn $method_name(&self, pdu: $request_type) -> Future<$response_type> { self.send_pdu(Pdu::$request_type(pdu)).then(|result| { match result { Ok(Pdu::$response_type(res)) => Ok(res), @@ -44,7 +45,7 @@ macro_rules! rpc { // in the case where the struct is empty and present only for the purpose // of typing the request. ($method_name:ident, $request_type:ident=(), $response_type:ident) => { - pub fn $method_name(&mut self) -> Future<$response_type> { + pub fn $method_name(&self) -> Future<$response_type> { self.send_pdu(Pdu::$request_type($request_type{})).then(|result| { match result { Ok(Pdu::$response_type(res)) => Ok(res), @@ -173,7 +174,7 @@ impl Client { Ok(Self::new(stream)) } - pub fn send_pdu(&mut self, pdu: Pdu) -> Future { + pub fn send_pdu(&self, pdu: Pdu) -> Future { let mut promise = Promise::new(); let future = promise.get_future().expect("future already taken!?"); match self.sender.send(ReaderMessage::SendPdu { pdu, promise }) { diff --git a/src/server/domain.rs b/src/server/domain.rs index df7f2b225..7cdcd34a4 100644 --- a/src/server/domain.rs +++ b/src/server/domain.rs @@ -14,7 +14,7 @@ use std::rc::Rc; use std::sync::{Arc, Mutex}; pub struct ClientInner { - pub client: Mutex, + pub client: Client, pub local_domain_id: DomainId, pub remote_domain_id: DomainId, remote_to_local_window: Mutex>, @@ -64,7 +64,7 @@ impl ClientInner { // this a bit rigorously. let remote_domain_id = 0; Self { - client: Mutex::new(client), + client, local_domain_id, remote_domain_id, remote_to_local_window: Mutex::new(HashMap::new()), @@ -91,9 +91,9 @@ impl Domain for ClientDomain { window: WindowId, ) -> Fallible> { let remote_tab_id = { - let mut client = self.inner.client.lock().unwrap(); - - let result = client + let result = self + .inner + .client .spawn(Spawn { domain_id: self.inner.remote_domain_id, window_id: self.inner.local_to_remote_window(window), @@ -117,8 +117,7 @@ impl Domain for ClientDomain { fn attach(&self) -> Fallible<()> { let mux = Mux::get().unwrap(); - let mut client = self.inner.client.lock().unwrap(); - let tabs = client.list_tabs().wait()?; + let tabs = self.inner.client.list_tabs().wait()?; log::error!("ListTabs result {:#?}", tabs); for entry in tabs.tabs.iter() { diff --git a/src/server/tab.rs b/src/server/tab.rs index 57945a615..bc1d3df00 100644 --- a/src/server/tab.rs +++ b/src/server/tab.rs @@ -1,6 +1,8 @@ +use crate::frontend::gui_executor; use crate::mux::domain::DomainId; use crate::mux::renderable::Renderable; use crate::mux::tab::{alloc_tab_id, Tab, TabId}; +use crate::server::client::Client; use crate::server::codec::*; use crate::server::domain::ClientInner; use failure::Fallible; @@ -10,17 +12,113 @@ use portable_pty::PtySize; use promise::Future; use std::cell::RefCell; use std::cell::RefMut; +use std::collections::VecDeque; use std::ops::Range; -use std::sync::Arc; +use std::sync::{Arc, Mutex}; use std::time::{Duration, Instant}; use term::color::ColorPalette; use term::selection::SelectionRange; use term::{CursorPosition, Line}; -use term::{KeyCode, KeyModifiers, MouseEvent, TerminalHost}; +use term::{KeyCode, KeyModifiers, MouseButton, MouseEvent, MouseEventKind, TerminalHost}; use termwiz::hyperlink::Hyperlink; use termwiz::input::KeyEvent; use termwiz::surface::{Change, SequenceNo, Surface}; +struct MouseState { + future: Option>, + queue: VecDeque, + selection_range: Arc>>, + something_changed: Arc>, + client: Client, + remote_tab_id: TabId, +} + +impl MouseState { + fn append(&mut self, event: MouseEvent) { + if let Some(last) = self.queue.back_mut() { + if last.modifiers == event.modifiers { + if last.kind == MouseEventKind::Move + && event.kind == MouseEventKind::Move + && last.button == event.button + { + // Collapse any interim moves and just buffer up + // the last of them + *last = event; + return; + } + + // Similarly, for repeated wheel scrolls, add up the deltas + // rather than swamping the queue + match (&last.button, &event.button) { + (MouseButton::WheelUp(a), MouseButton::WheelUp(b)) => { + last.button = MouseButton::WheelUp(a + b); + return; + } + (MouseButton::WheelDown(a), MouseButton::WheelDown(b)) => { + last.button = MouseButton::WheelDown(a + b); + return; + } + _ => {} + } + } + } + self.queue.push_back(event); + log::trace!("MouseEvent {}: queued", self.queue.len()); + } + + fn pop(&mut self) -> Fallible> { + if self.can_send()? { + Ok(self.queue.pop_front()) + } else { + Ok(None) + } + } + + fn can_send(&mut self) -> Fallible { + if self.future.is_none() { + Ok(true) + } else { + let ready = self.future.as_ref().map(Future::is_ready).unwrap_or(false); + if ready { + self.future.take().unwrap().wait()?; + Ok(true) + } else { + Ok(false) + } + } + } + + fn next(state: &Arc>) -> Fallible<()> { + let mut mouse = state.lock().unwrap(); + if let Some(event) = mouse.pop()? { + let selection_range = Arc::clone(&mouse.selection_range); + let something_changed = Arc::clone(&mouse.something_changed); + let state = Arc::clone(state); + + mouse.future = Some( + mouse + .client + .mouse_event(SendMouseEvent { + tab_id: mouse.remote_tab_id, + event, + }) + .then(move |resp| { + if let Ok(r) = resp.as_ref() { + *selection_range.lock().unwrap() = r.selection_range.clone(); + *something_changed.lock().unwrap() = true; + } + Future::with_executor(gui_executor().unwrap(), move || { + Self::next(&state)?; + Ok(()) + }); + Ok(()) + }), + ); + } + Ok(()) + } +} + pub struct ClientTab { client: Arc, local_tab_id: TabId, @@ -28,6 +126,7 @@ pub struct ClientTab { renderable: RefCell, writer: RefCell, reader: Pipe, + mouse: Arc>, } impl ClientTab { @@ -37,6 +136,16 @@ impl ClientTab { client: Arc::clone(client), remote_tab_id, }; + let selection_range = Arc::new(Mutex::new(None)); + let something_changed = Arc::new(Mutex::new(true)); + let mouse = Arc::new(Mutex::new(MouseState { + selection_range: Arc::clone(&selection_range), + something_changed: Arc::clone(&something_changed), + remote_tab_id, + client: client.client.clone(), + future: None, + queue: VecDeque::new(), + })); let render = RenderableState { client: Arc::clone(client), remote_tab_id, @@ -46,13 +155,15 @@ impl ClientTab { surface: RefCell::new(Surface::new(80, 24)), remote_sequence: RefCell::new(0), local_sequence: RefCell::new(0), - selection_range: RefCell::new(None), + selection_range, + something_changed, }; let reader = Pipe::new().expect("Pipe::new failed"); Self { client: Arc::clone(client), + mouse, remote_tab_id, local_tab_id, renderable: RefCell::new(render), @@ -77,8 +188,7 @@ impl Tab for ClientTab { } fn send_paste(&self, text: &str) -> Fallible<()> { - let mut client = self.client.client.lock().unwrap(); - client.send_paste(SendPaste { + self.client.client.send_paste(SendPaste { tab_id: self.remote_tab_id, data: text.to_owned(), }); @@ -100,8 +210,7 @@ impl Tab for ClientTab { .surface .borrow_mut() .resize(size.cols as usize, size.rows as usize); - let mut client = self.client.client.lock().unwrap(); - client.resize(Resize { + self.client.client.resize(Resize { tab_id: self.remote_tab_id, size, }); @@ -109,8 +218,7 @@ impl Tab for ClientTab { } fn key_down(&self, key: KeyCode, mods: KeyModifiers) -> Fallible<()> { - let mut client = self.client.client.lock().unwrap(); - client.key_down(SendKeyDown { + self.client.client.key_down(SendKeyDown { tab_id: self.remote_tab_id, event: KeyEvent { key, @@ -120,21 +228,17 @@ impl Tab for ClientTab { Ok(()) } - fn mouse_event(&self, event: MouseEvent, host: &mut dyn TerminalHost) -> Fallible<()> { - let mut client = self.client.client.lock().unwrap(); - let resp = client - .mouse_event(SendMouseEvent { - tab_id: self.remote_tab_id, - event, - }) - .wait()?; + fn mouse_event(&self, event: MouseEvent, _host: &mut dyn TerminalHost) -> Fallible<()> { + self.mouse.lock().unwrap().append(event); + MouseState::next(&self.mouse)?; + Ok(()) + /* if resp.clipboard.is_some() { host.set_clipboard(resp.clipboard)?; } - *self.renderable.borrow().selection_range.borrow_mut() = resp.selection_range; - - Ok(()) + *self.renderable.borrow().selection_range.lock().unwrap() = resp.selection_range; + */ } fn advance_bytes(&self, _buf: &[u8], _host: &mut dyn TerminalHost) { @@ -158,7 +262,12 @@ impl Tab for ClientTab { } fn selection_range(&self) -> Option { - self.renderable.borrow().selection_range.borrow().clone() + self.renderable + .borrow() + .selection_range + .lock() + .unwrap() + .clone() } } @@ -171,7 +280,8 @@ struct RenderableState { surface: RefCell, remote_sequence: RefCell, local_sequence: RefCell, - selection_range: RefCell>, + selection_range: Arc>>, + something_changed: Arc>, } const POLL_INTERVAL: Duration = Duration::from_millis(50); @@ -214,13 +324,13 @@ impl RenderableState { } { - let mut client = self.client.client.lock().unwrap(); *self.last_poll.borrow_mut() = Instant::now(); - *self.poll_future.borrow_mut() = - Some(client.get_tab_render_changes(GetTabRenderChanges { + *self.poll_future.borrow_mut() = Some(self.client.client.get_tab_render_changes( + GetTabRenderChanges { tab_id: self.remote_tab_id, sequence_no: *self.remote_sequence.borrow(), - })); + }, + )); } Ok(()) } @@ -236,7 +346,8 @@ impl Renderable for RenderableState { let mut surface = self.surface.borrow_mut(); let seq = surface.current_seqno(); surface.flush_changes_older_than(seq); - let selection = *self.selection_range.borrow(); + let selection = *self.selection_range.lock().unwrap(); + *self.something_changed.lock().unwrap() = false; surface .screen_lines() .into_iter() @@ -255,6 +366,9 @@ impl Renderable for RenderableState { if self.poll().is_err() { *self.dead.borrow_mut() = true; } + if *self.something_changed.lock().unwrap() { + return true; + } self.surface .borrow() .has_changes(*self.local_sequence.borrow()) @@ -281,8 +395,8 @@ struct TabWriter { impl std::io::Write for TabWriter { fn write(&mut self, data: &[u8]) -> Result { - let mut client = self.client.client.lock().unwrap(); - client + self.client + .client .write_to_tab(WriteToTab { tab_id: self.remote_tab_id, data: data.to_vec(), diff --git a/term/src/input.rs b/term/src/input.rs index c485ceba4..0b5ce1b72 100644 --- a/term/src/input.rs +++ b/term/src/input.rs @@ -13,8 +13,8 @@ pub enum MouseButton { Left, Middle, Right, - WheelUp, - WheelDown, + WheelUp(usize), + WheelDown(usize), None, } diff --git a/term/src/terminalstate.rs b/term/src/terminalstate.rs index cb8c7c149..ca724e744 100644 --- a/term/src/terminalstate.rs +++ b/term/src/terminalstate.rs @@ -614,10 +614,10 @@ impl TerminalState { } fn mouse_wheel(&mut self, event: MouseEvent, writer: &mut std::io::Write) -> Result<(), Error> { - let (report_button, scroll_delta, key) = if event.button == MouseButton::WheelUp { - (64, -1, KeyCode::UpArrow) - } else { - (65, 1, KeyCode::DownArrow) + let (report_button, scroll_delta, key) = match event.button { + MouseButton::WheelUp(amount) => (64, -(amount as i64), KeyCode::UpArrow), + MouseButton::WheelDown(amount) => (65, amount as i64, KeyCode::DownArrow), + _ => bail!("unexpected mouse event {:?}", event), }; if self.sgr_mouse { @@ -763,12 +763,12 @@ impl TerminalState { match event { MouseEvent { kind: MouseEventKind::Press, - button: MouseButton::WheelUp, + button: MouseButton::WheelUp(_), .. } | MouseEvent { kind: MouseEventKind::Press, - button: MouseButton::WheelDown, + button: MouseButton::WheelDown(_), .. } => self.mouse_wheel(event, host.writer()), MouseEvent {