From 256ac75f1ff71ad76c85e791f41ac9b8137d62fc Mon Sep 17 00:00:00 2001 From: Noah Gold Date: Fri, 5 Aug 2022 16:48:48 -0700 Subject: [PATCH] Revert "x display: ST event emulation for MT events" This reverts commit d98f01af14494bd3e52995fafb6ea1c7d20d102e. NOTE: we are intentionally keeping the changes to use new tracking IDs per contact, and the conversion of wayland to send MT events, as those are not really part of the simulation change, and should be fine to keep. Following discussion w/ drmasquatch@ offline re comments on the OCL, it seems like there are other ways we could solve for the original problem without introducing a simulation layer. Given the complexity introduced and the fact that not all consumers necessarily want this compat layer, a revert seems like the best step. Some options for fixing the original problem in the future: * Modify gpu_display_wl to produce ST events based on a CLI flag. This is pretty easy, and involves enhancing `--display-window-mouse` to take whether the events should be ST or MT, and then generating the ST events in the display backends as specified. The same flag also needs to create a ST device rather than a MT device so that the ST events will be accepted by the guest kernel. * Modify the virtio-input kernel driver. This might be the right place to add a compat layer, if one is needed more broadly. It will also fix the problem everywhere. Original CL: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3330723 TEST=builds BUG=b:191173095 Change-Id: Iad07b6b5a06a3884dc352c49847b2b3c268ee5bd Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3814100 Reviewed-by: Daniel Verkamp Tested-by: Noah Gold Reviewed-by: Ryan Neph Commit-Queue: Noah Gold --- gpu_display/src/event_device.rs | 100 -------------------------------- linux_input_sys/src/lib.rs | 38 ------------ 2 files changed, 138 deletions(-) diff --git a/gpu_display/src/event_device.rs b/gpu_display/src/event_device.rs index dc8660607d..52297d198f 100644 --- a/gpu_display/src/event_device.rs +++ b/gpu_display/src/event_device.rs @@ -2,7 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -use std::cmp::max; use std::collections::VecDeque; use std::io; use std::io::Error; @@ -17,10 +16,6 @@ use base::RawDescriptor; use data_model::DataInit; use linux_input_sys::virtio_input_event; use linux_input_sys::InputEventDecoder; -use linux_input_sys::ABS_MT_POSITION_X; -use linux_input_sys::ABS_MT_POSITION_Y; -use linux_input_sys::ABS_MT_SLOT; -use linux_input_sys::ABS_MT_TRACKING_ID; const EVENT_SIZE: usize = virtio_input_event::SIZE; const EVENT_BUFFER_LEN_MAX: usize = 64 * EVENT_SIZE; @@ -56,15 +51,6 @@ pub struct EventDevice { event_socket: UnixStream, } -// Captures information for emulating MT events. Once more than slot 0 is supported will need to -// store slot information as well. -#[derive(Default, Copy, Clone)] -struct MTEvent { - id: i32, - x: i32, - y: i32, -} - impl EventDevice { pub fn new(kind: EventDeviceKind, event_socket: UnixStream) -> EventDevice { let _ = event_socket.set_nonblocking(true); @@ -114,24 +100,6 @@ impl EventDevice { self.event_buffer.is_empty() } - /// Compares (potential) events and calculates the oldest one - fn capture_oldest_event( - current_mt_event: &Option, - oldest_mt_event: &mut Option, - ) { - if let Some(current) = current_mt_event { - if current.id != -1 { - if let Some(oldest) = oldest_mt_event { - if current.id < oldest.id { - *oldest_mt_event = Some(*current); - } - } else { - *oldest_mt_event = Some(*current); - } - } - } - } - /// Determines if there is space in the event buffer for the given number /// of events. The buffer is capped at `EVENT_BUFFER_LEN_MAX`. #[inline] @@ -153,82 +121,14 @@ impl EventDevice { E::IntoIter: ExactSizeIterator, { let it = events.into_iter(); - let mut emulated_events: Vec = vec![]; - let mut evdev_st_events_present = false; - let mut oldest_mt_event: Option = None; - let mut current_mt_event: Option = None; if !self.can_buffer_events(it.len() + 1) { return Ok(false); } - // Assumptions made for MT emulation: - // * There may exist real ABS_* events coming from e.g. evdev. cancel emulation in this case - // * For current gpu_display_{x, wl} implementations, there is only one slot and all - // available tracking id information is sent. If this assumption is broken in the future, - // `EventDevice` will need to maintain state of the oldest contact for each slot, so that - // this can be compared to any incoming 'sparse' event set. for event in it { let bytes = event.as_slice(); self.event_buffer.extend(bytes.iter()); - - if event.is_valid_st_event() { - // Real evdev event, we shouldnt emulate anything - evdev_st_events_present = true; - } - - // For MT events, we want to also emulate their corresponding ST events - if !evdev_st_events_present { - match event.code.to_native() { - ABS_MT_SLOT => { - // MT packets begin with a SLOT event, begin a new current event and potentially - // store the current as oldest. - EventDevice::capture_oldest_event(¤t_mt_event, &mut oldest_mt_event); - // Only care about slot 0 in current implementation, if more slots are added - // we will need to keep track of events / ids per slot, as well as maintain - // which slot is currently active. - current_mt_event = Some(MTEvent { - ..Default::default() - }) - } - ABS_MT_TRACKING_ID => { - if let Some(mut current) = current_mt_event { - current.id = event.value.to_native(); - } - } - ABS_MT_POSITION_X => { - if let Some(mut current) = current_mt_event { - current.x = event.value.to_native(); - } - } - ABS_MT_POSITION_Y => { - if let Some(mut current) = current_mt_event { - current.y = event.value.to_native(); - } - } - _ => {} - } - } - } - - // Finalize the current event - MT packets have a 'begin' signal but not an 'end' signal, so the - // last collected 'current' event has no chance to be compared in the event loop. - EventDevice::capture_oldest_event(¤t_mt_event, &mut oldest_mt_event); - - if !evdev_st_events_present { - if let Some(oldest_event) = oldest_mt_event { - emulated_events.push(virtio_input_event::touch(true)); - emulated_events.push(virtio_input_event::absolute_x(max(0, oldest_event.x))); - emulated_events.push(virtio_input_event::absolute_y(max(0, oldest_event.y))); - } else { - // No contacts remain, emit lift - emulated_events.push(virtio_input_event::touch(false)); - } - } - - for event in emulated_events.into_iter() { - let bytes = event.as_slice(); - self.event_buffer.extend(bytes.iter()); } self.event_buffer diff --git a/linux_input_sys/src/lib.rs b/linux_input_sys/src/lib.rs index c1159e3f3c..45be1833e1 100644 --- a/linux_input_sys/src/lib.rs +++ b/linux_input_sys/src/lib.rs @@ -194,42 +194,4 @@ impl virtio_input_event { value: SLe32::from(if pressed { 1 } else { 0 }), } } - - #[inline] - pub fn is_valid_mt_event(self) -> bool { - match self.type_.to_native() { - EV_KEY => self.code.to_native() == BTN_TOUCH, - EV_ABS => matches!( - self.code.to_native(), - ABS_MT_SLOT - | ABS_MT_TOUCH_MAJOR - | ABS_MT_TOUCH_MINOR - | ABS_MT_WIDTH_MAJOR - | ABS_MT_WIDTH_MINOR - | ABS_MT_ORIENTATION - | ABS_MT_POSITION_X - | ABS_MT_POSITION_Y - | ABS_MT_TOOL_TYPE - | ABS_MT_BLOB_ID - | ABS_MT_TRACKING_ID - | ABS_MT_PRESSURE - | ABS_MT_DISTANCE - | ABS_MT_TOOL_X - | ABS_MT_TOOL_Y - ), - _ => false, - } - } - - #[inline] - pub fn is_valid_st_event(self) -> bool { - match self.type_.to_native() { - EV_KEY => self.code.to_native() == BTN_TOUCH, - EV_ABS => matches!( - self.code.to_native(), - ABS_X | ABS_Y | ABS_PRESSURE | ABS_TILT_X | ABS_TILT_Y | ABS_TOOL_WIDTH - ), - _ => false, - } - } }