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

@@ -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,7 +405,6 @@ impl GliumTerminalWindow {
             Some(tab) => tab,
             None => return Ok(()),
         };
-        for _ in 0..times {
         tab.mouse_event(
             term::MouseEvent {
                 kind: MouseEventKind::Press,
@@ -416,7 +415,6 @@ impl GliumTerminalWindow {
             },
             &mut TabHost::new(&mut *tab.writer(), &mut self.host),
         )?;
-        }
         self.paint_if_needed()?;
 
         Ok(())

src/frontend/xwindows/xwin.rs

@@ -236,8 +236,8 @@ impl X11TerminalWindow {
                         1 => MouseButton::Left,
                         2 => MouseButton::Middle,
                         3 => MouseButton::Right,
-                        4 => MouseButton::WheelUp,
+                        4 => MouseButton::WheelUp(1),
-                        5 => MouseButton::WheelDown,
+                        5 => MouseButton::WheelDown(1),
                         _ => {
                             error!("button {} is not implemented", button_press.detail());
                             return Ok(());

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![

src/server/client.rs

@@ -23,13 +23,14 @@ enum ReaderMessage {
     SendPdu { pdu: Pdu, promise: Promise<Pdu> },
 }
 
+#[derive(Clone)]
 pub struct Client {
     sender: Sender<ReaderMessage>,
 }
 
 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<Pdu> {
+    pub fn send_pdu(&self, pdu: Pdu) -> Future<Pdu> {
         let mut promise = Promise::new();
         let future = promise.get_future().expect("future already taken!?");
         match self.sender.send(ReaderMessage::SendPdu { pdu, promise }) {

src/server/domain.rs

@@ -14,7 +14,7 @@ use std::rc::Rc;
 use std::sync::{Arc, Mutex};
 
 pub struct ClientInner {
-    pub client: Mutex<Client>,
+    pub client: Client,
     pub local_domain_id: DomainId,
     pub remote_domain_id: DomainId,
     remote_to_local_window: Mutex<HashMap<WindowId, WindowId>>,
@@ -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<Rc<dyn Tab>> {
         let remote_tab_id = {
-            let mut client = self.inner.client.lock().unwrap();
+            let result = self
-
+                .inner
-            let result = client
+                .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 = self.inner.client.list_tabs().wait()?;
-        let tabs = client.list_tabs().wait()?;
         log::error!("ListTabs result {:#?}", tabs);
 
         for entry in tabs.tabs.iter() {

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<Future<()>>,
+    queue: VecDeque<MouseEvent>,
+    selection_range: Arc<Mutex<Option<SelectionRange>>>,
+    something_changed: Arc<Mutex<bool>>,
+    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<Option<MouseEvent>> {
+        if self.can_send()? {
+            Ok(self.queue.pop_front())
+        } else {
+            Ok(None)
+        }
+    }
+
+    fn can_send(&mut self) -> Fallible<bool> {
+        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<Mutex<Self>>) -> 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<ClientInner>,
     local_tab_id: TabId,
@@ -28,6 +126,7 @@ pub struct ClientTab {
     renderable: RefCell<RenderableState>,
     writer: RefCell<TabWriter>,
     reader: Pipe,
+    mouse: Arc<Mutex<MouseState>>,
 }
 
 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();
+        self.client.client.send_paste(SendPaste {
-        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();
+        self.client.client.resize(Resize {
-        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();
+        self.client.client.key_down(SendKeyDown {
-        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<()> {
+    fn mouse_event(&self, event: MouseEvent, _host: &mut dyn TerminalHost) -> Fallible<()> {
-        let mut client = self.client.client.lock().unwrap();
+        self.mouse.lock().unwrap().append(event);
-        let resp = client
+        MouseState::next(&self.mouse)?;
-            .mouse_event(SendMouseEvent {
+        Ok(())
-                tab_id: self.remote_tab_id,
-                event,
-            })
-            .wait()?;
 
+        /*
         if resp.clipboard.is_some() {
             host.set_clipboard(resp.clipboard)?;
         }
-        *self.renderable.borrow().selection_range.borrow_mut() = resp.selection_range;
+        *self.renderable.borrow().selection_range.lock().unwrap() = resp.selection_range;
-
+        */
-        Ok(())
     }
 
     fn advance_bytes(&self, _buf: &[u8], _host: &mut dyn TerminalHost) {
@@ -158,7 +262,12 @@ impl Tab for ClientTab {
     }
 
     fn selection_range(&self) -> Option<SelectionRange> {
-        self.renderable.borrow().selection_range.borrow().clone()
+        self.renderable
+            .borrow()
+            .selection_range
+            .lock()
+            .unwrap()
+            .clone()
     }
 }
 
@@ -171,7 +280,8 @@ struct RenderableState {
     surface: RefCell<Surface>,
     remote_sequence: RefCell<SequenceNo>,
     local_sequence: RefCell<SequenceNo>,
-    selection_range: RefCell<Option<SelectionRange>>,
+    selection_range: Arc<Mutex<Option<SelectionRange>>>,
+    something_changed: Arc<Mutex<bool>>,
 }
 
 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() =
+            *self.poll_future.borrow_mut() = Some(self.client.client.get_tab_render_changes(
-                Some(client.get_tab_render_changes(GetTabRenderChanges {
+                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<usize, std::io::Error> {
-        let mut client = self.client.client.lock().unwrap();
+        self.client
-        client
+            .client
             .write_to_tab(WriteToTab {
                 tab_id: self.remote_tab_id,
                 data: data.to_vec(),

term/src/input.rs

@@ -13,8 +13,8 @@ pub enum MouseButton {
     Left,
     Middle,
     Right,
-    WheelUp,
+    WheelUp(usize),
-    WheelDown,
+    WheelDown(usize),
     None,
 }
 

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 {
+        let (report_button, scroll_delta, key) = match event.button {
-            (64, -1, KeyCode::UpArrow)
+            MouseButton::WheelUp(amount) => (64, -(amount as i64), KeyCode::UpArrow),
-        } else {
+            MouseButton::WheelDown(amount) => (65, amount as i64, KeyCode::DownArrow),
-            (65, 1, 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 {