Use display size as the default size for single touch

When the user specifies the display size through the gpu argument but
not specifies the size of the single touch device, the display size
will be used as the size of these touch devices.

Use default() to initialize the GpuParameters. Allow initialize the
GpuParameters dynamically in the future.

Change-Id: I9fa04f8ff479732370514fbaeb062d737adba319
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2043072
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Kaiyi Li <kaiyili@google.com>
Reviewed-by: Zach Reizner <zachr@chromium.org>

3 files changed, 223 insertions, 57 deletions

diff --git a/src/crosvm.rs b/src/crosvm.rs
index 082e43c..e0ddf06 100644
--- a/src/crosvm.rs
+++ b/src/crosvm.rs
@@ -13,7 +13,7 @@ pub mod plugin;
 use std::collections::BTreeMap;
 use std::net;
 use std::os::unix::io::RawFd;
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 use std::str::FromStr;
 
 use arch::Pstore;
@@ -57,23 +57,58 @@ pub struct GidMap {
     pub count: u32,
 }
 
-const DEFAULT_TOUCH_DEVICE_WIDTH: u32 = 800;
-const DEFAULT_TOUCH_DEVICE_HEIGHT: u32 = 1280;
+pub const DEFAULT_TOUCH_DEVICE_WIDTH: u32 = 800;
+pub const DEFAULT_TOUCH_DEVICE_HEIGHT: u32 = 1280;
 
 pub struct TouchDeviceOption {
-    pub path: PathBuf,
-    pub width: u32,
-    pub height: u32,
+    path: PathBuf,
+    width: Option<u32>,
+    height: Option<u32>,
+    default_width: u32,
+    default_height: u32,
 }
 
 impl TouchDeviceOption {
     pub fn new(path: PathBuf) -> TouchDeviceOption {
         TouchDeviceOption {
             path,
-            width: DEFAULT_TOUCH_DEVICE_WIDTH,
-            height: DEFAULT_TOUCH_DEVICE_HEIGHT,
+            width: None,
+            height: None,
+            default_width: DEFAULT_TOUCH_DEVICE_WIDTH,
+            default_height: DEFAULT_TOUCH_DEVICE_HEIGHT,
         }
     }
+
+    /// Getter for the path to the input event streams.
+    pub fn get_path(&self) -> &Path {
+        self.path.as_path()
+    }
+
+    /// When a user specifies the parameters for a touch device, width and height are optional.
+    /// If the width and height are missing, default values are used. Default values can be set
+    /// dynamically, for example from the display sizes specified by the gpu argument.
+    pub fn set_default_size(&mut self, width: u32, height: u32) {
+        self.default_width = width;
+        self.default_height = height;
+    }
+
+    /// Setter for the width specified by the user.
+    pub fn set_width(&mut self, width: u32) {
+        self.width.replace(width);
+    }
+
+    /// Setter for the height specified by the user.
+    pub fn set_height(&mut self, height: u32) {
+        self.height.replace(height);
+    }
+
+    /// If the user specifies the size, use it. Otherwise, use the default values.
+    pub fn get_size(&self) -> (u32, u32) {
+        (
+            self.width.unwrap_or(self.default_width),
+            self.height.unwrap_or(self.default_height),
+        )
+    }
 }
 
 pub enum SharedDirKind {
diff --git a/src/linux.rs b/src/linux.rs
index 84edf5c..ab6c64c 100644
--- a/src/linux.rs
+++ b/src/linux.rs
@@ -62,7 +62,10 @@ use vm_control::{
     VmRunMode,
 };
 
-use crate::{Config, DiskOption, Executable, SharedDir, SharedDirKind, TouchDeviceOption};
+use crate::{
+    Config, DiskOption, Executable, SharedDir, SharedDirKind, TouchDeviceOption,
+    DEFAULT_TOUCH_DEVICE_HEIGHT, DEFAULT_TOUCH_DEVICE_WIDTH,
+};
 
 use arch::{self, LinuxArch, RunnableLinuxVm, VirtioDeviceStub, VmComponents, VmImage};
 
@@ -473,13 +476,16 @@ fn create_tpm_device(cfg: &Config) -> DeviceResult {
 }
 
 fn create_single_touch_device(cfg: &Config, single_touch_spec: &TouchDeviceOption) -> DeviceResult {
-    let socket = single_touch_spec.path.into_unix_stream().map_err(|e| {
-        error!("failed configuring virtio single touch: {:?}", e);
-        e
-    })?;
-
-    let dev = virtio::new_single_touch(socket, single_touch_spec.width, single_touch_spec.height)
-        .map_err(Error::InputDeviceNew)?;
+    let socket = single_touch_spec
+        .get_path()
+        .into_unix_stream()
+        .map_err(|e| {
+            error!("failed configuring virtio single touch: {:?}", e);
+            e
+        })?;
+
+    let (width, height) = single_touch_spec.get_size();
+    let dev = virtio::new_single_touch(socket, width, height).map_err(Error::InputDeviceNew)?;
     Ok(VirtioDeviceStub {
         dev: Box::new(dev),
         jail: simple_jail(&cfg, "input_device")?,
@@ -487,13 +493,13 @@ fn create_single_touch_device(cfg: &Config, single_touch_spec: &TouchDeviceOptio
 }
 
 fn create_trackpad_device(cfg: &Config, trackpad_spec: &TouchDeviceOption) -> DeviceResult {
-    let socket = trackpad_spec.path.into_unix_stream().map_err(|e| {
+    let socket = trackpad_spec.get_path().into_unix_stream().map_err(|e| {
         error!("failed configuring virtio trackpad: {}", e);
         e
     })?;
 
-    let dev = virtio::new_trackpad(socket, trackpad_spec.width, trackpad_spec.height)
-        .map_err(Error::InputDeviceNew)?;
+    let (width, height) = trackpad_spec.get_size();
+    let dev = virtio::new_trackpad(socket, width, height).map_err(Error::InputDeviceNew)?;
 
     Ok(VirtioDeviceStub {
         dev: Box::new(dev),
@@ -1030,8 +1036,17 @@ fn create_virtio_devices(
                 // TODO(nkgold): the width/height here should match the display's height/width. When
                 // those settings are available as CLI options, we should use the CLI options here
                 // as well.
-                let dev = virtio::new_single_touch(virtio_dev_socket, 1280, 1024)
-                    .map_err(Error::InputDeviceNew)?;
+                let (single_touch_width, single_touch_height) = cfg
+                    .virtio_single_touch
+                    .as_ref()
+                    .map(|single_touch_spec| single_touch_spec.get_size())
+                    .unwrap_or((DEFAULT_TOUCH_DEVICE_WIDTH, DEFAULT_TOUCH_DEVICE_HEIGHT));
+                let dev = virtio::new_single_touch(
+                    virtio_dev_socket,
+                    single_touch_width,
+                    single_touch_height,
+                )
+                .map_err(Error::InputDeviceNew)?;
                 devs.push(VirtioDeviceStub {
                     dev: Box::new(dev),
                     jail: simple_jail(&cfg, "input_device")?,
diff --git a/src/main.rs b/src/main.rs
index fb1be25..1e86e47 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -22,7 +22,7 @@ use crosvm::{
     linux, BindMount, Config, DiskOption, Executable, GidMap, SharedDir, TouchDeviceOption,
 };
 #[cfg(feature = "gpu")]
-use devices::virtio::gpu::{GpuMode, GpuParameters, DEFAULT_GPU_PARAMS};
+use devices::virtio::gpu::{GpuMode, GpuParameters};
 use devices::{SerialParameters, SerialType};
 use disk::QcowFile;
 use msg_socket::{MsgReceiver, MsgSender, MsgSocket};
@@ -114,7 +114,7 @@ fn parse_cpu_set(s: &str) -> argument::Result<Vec<usize>> {
 
 #[cfg(feature = "gpu")]
 fn parse_gpu_options(s: Option<&str>) -> argument::Result<GpuParameters> {
-    let mut gpu_params = DEFAULT_GPU_PARAMS;
+    let mut gpu_params: GpuParameters = Default::default();
 
     if let Some(s) = s {
         let opts = s
@@ -982,12 +982,11 @@ fn set_argument(cfg: &mut Config, name: &str, value: Option<&str>) -> argument::
             let mut single_touch_spec =
                 TouchDeviceOption::new(PathBuf::from(it.next().unwrap().to_owned()));
             if let Some(width) = it.next() {
-                single_touch_spec.width = width.trim().parse().unwrap();
+                single_touch_spec.set_width(width.trim().parse().unwrap());
             }
             if let Some(height) = it.next() {
-                single_touch_spec.height = height.trim().parse().unwrap();
+                single_touch_spec.set_height(height.trim().parse().unwrap());
             }
-
             cfg.virtio_single_touch = Some(single_touch_spec);
         }
         "trackpad" => {
@@ -1001,12 +1000,11 @@ fn set_argument(cfg: &mut Config, name: &str, value: Option<&str>) -> argument::
             let mut trackpad_spec =
                 TouchDeviceOption::new(PathBuf::from(it.next().unwrap().to_owned()));
             if let Some(width) = it.next() {
-                trackpad_spec.width = width.trim().parse().unwrap();
+                trackpad_spec.set_width(width.trim().parse().unwrap());
             }
             if let Some(height) = it.next() {
-                trackpad_spec.height = height.trim().parse().unwrap();
+                trackpad_spec.set_height(height.trim().parse().unwrap());
             }
-
             cfg.virtio_trackpad = Some(trackpad_spec);
         }
         "mouse" => {
@@ -1074,6 +1072,44 @@ fn set_argument(cfg: &mut Config, name: &str, value: Option<&str>) -> argument::
     Ok(())
 }
 
+fn validate_arguments(cfg: &mut Config) -> std::result::Result<(), argument::Error> {
+    if cfg.executable_path.is_none() {
+        return Err(argument::Error::ExpectedArgument("`KERNEL`".to_owned()));
+    }
+    if cfg.host_ip.is_some() || cfg.netmask.is_some() || cfg.mac_address.is_some() {
+        if cfg.host_ip.is_none() {
+            return Err(argument::Error::ExpectedArgument(
+                "`host_ip` missing from network config".to_owned(),
+            ));
+        }
+        if cfg.netmask.is_none() {
+            return Err(argument::Error::ExpectedArgument(
+                "`netmask` missing from network config".to_owned(),
+            ));
+        }
+        if cfg.mac_address.is_none() {
+            return Err(argument::Error::ExpectedArgument(
+                "`mac` missing from network config".to_owned(),
+            ));
+        }
+    }
+    if cfg.plugin_root.is_some() && !executable_is_plugin(&cfg.executable_path) {
+        return Err(argument::Error::ExpectedArgument(
+            "`plugin-root` requires `plugin`".to_owned(),
+        ));
+    }
+    #[cfg(feature = "gpu")]
+    {
+        if let Some(gpu_parameters) = cfg.gpu_parameters.as_ref() {
+            let (width, height) = (gpu_parameters.display_width, gpu_parameters.display_height);
+            if let Some(virtio_single_touch) = cfg.virtio_single_touch.as_mut() {
+                virtio_single_touch.set_default_size(width, height);
+            }
+        }
+    }
+    Ok(())
+}
+
 fn run_vm(args: std::env::Args) -> std::result::Result<(), ()> {
     let arguments =
         &[Argument::positional("KERNEL", "bzImage of kernel to run"),
@@ -1196,34 +1232,7 @@ writeback=BOOL - Indicates whether the VM can use writeback caching (default: fa
     let match_res = set_arguments(args, &arguments[..], |name, value| {
         set_argument(&mut cfg, name, value)
     })
-    .and_then(|_| {
-        if cfg.executable_path.is_none() {
-            return Err(argument::Error::ExpectedArgument("`KERNEL`".to_owned()));
-        }
-        if cfg.host_ip.is_some() || cfg.netmask.is_some() || cfg.mac_address.is_some() {
-            if cfg.host_ip.is_none() {
-                return Err(argument::Error::ExpectedArgument(
-                    "`host_ip` missing from network config".to_owned(),
-                ));
-            }
-            if cfg.netmask.is_none() {
-                return Err(argument::Error::ExpectedArgument(
-                    "`netmask` missing from network config".to_owned(),
-                ));
-            }
-            if cfg.mac_address.is_none() {
-                return Err(argument::Error::ExpectedArgument(
-                    "`mac` missing from network config".to_owned(),
-                ));
-            }
-        }
-        if cfg.plugin_root.is_some() && !executable_is_plugin(&cfg.executable_path) {
-            return Err(argument::Error::ExpectedArgument(
-                "`plugin-root` requires `plugin`".to_owned(),
-            ));
-        }
-        Ok(())
-    });
+    .and_then(|_| validate_arguments(&mut cfg));
 
     match match_res {
         #[cfg(feature = "plugin")]
@@ -1683,6 +1692,7 @@ fn main() {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crosvm::{DEFAULT_TOUCH_DEVICE_HEIGHT, DEFAULT_TOUCH_DEVICE_WIDTH};
 
     #[test]
     fn parse_cpu_set_single() {
@@ -1873,4 +1883,110 @@ mod tests {
         set_argument(&mut config, "plugin-gid-map", Some("1:2:blah"))
             .expect_err("parse should fail because count is not a number");
     }
+
+    #[test]
+    fn single_touch_spec_and_track_pad_spec_default_size() {
+        let mut config = Config::default();
+        config
+            .executable_path
+            .replace(Executable::Kernel(PathBuf::from("kernel")));
+        set_argument(&mut config, "single-touch", Some("/dev/single-touch-test")).unwrap();
+        set_argument(&mut config, "trackpad", Some("/dev/single-touch-test")).unwrap();
+        validate_arguments(&mut config).unwrap();
+        assert_eq!(
+            config.virtio_single_touch.unwrap().get_size(),
+            (DEFAULT_TOUCH_DEVICE_WIDTH, DEFAULT_TOUCH_DEVICE_HEIGHT)
+        );
+        assert_eq!(
+            config.virtio_trackpad.unwrap().get_size(),
+            (DEFAULT_TOUCH_DEVICE_WIDTH, DEFAULT_TOUCH_DEVICE_HEIGHT)
+        );
+    }
+
+    #[cfg(feature = "gpu")]
+    #[test]
+    fn single_touch_spec_default_size_from_gpu() {
+        let width = 12345u32;
+        let height = 54321u32;
+        let mut config = Config::default();
+        config
+            .executable_path
+            .replace(Executable::Kernel(PathBuf::from("kernel")));
+        set_argument(&mut config, "single-touch", Some("/dev/single-touch-test")).unwrap();
+        set_argument(
+            &mut config,
+            "gpu",
+            Some(&format!("width={},height={}", width, height)),
+        )
+        .unwrap();
+        validate_arguments(&mut config).unwrap();
+        assert_eq!(
+            config.virtio_single_touch.unwrap().get_size(),
+            (width, height)
+        );
+    }
+
+    #[test]
+    fn single_touch_spec_and_track_pad_spec_with_size() {
+        let width = 12345u32;
+        let height = 54321u32;
+        let mut config = Config::default();
+        config
+            .executable_path
+            .replace(Executable::Kernel(PathBuf::from("kernel")));
+        set_argument(
+            &mut config,
+            "single-touch",
+            Some(&format!("/dev/single-touch-test:{}:{}", width, height)),
+        )
+        .unwrap();
+        set_argument(
+            &mut config,
+            "trackpad",
+            Some(&format!("/dev/single-touch-test:{}:{}", width, height)),
+        )
+        .unwrap();
+        validate_arguments(&mut config).unwrap();
+        assert_eq!(
+            config.virtio_single_touch.unwrap().get_size(),
+            (width, height)
+        );
+        assert_eq!(config.virtio_trackpad.unwrap().get_size(), (width, height));
+    }
+
+    #[cfg(feature = "gpu")]
+    #[test]
+    fn single_touch_spec_with_size_independent_from_gpu() {
+        let touch_width = 12345u32;
+        let touch_height = 54321u32;
+        let display_width = 1234u32;
+        let display_height = 5432u32;
+        let mut config = Config::default();
+        config
+            .executable_path
+            .replace(Executable::Kernel(PathBuf::from("kernel")));
+        set_argument(
+            &mut config,
+            "single-touch",
+            Some(&format!(
+                "/dev/single-touch-test:{}:{}",
+                touch_width, touch_height
+            )),
+        )
+        .unwrap();
+        set_argument(
+            &mut config,
+            "gpu",
+            Some(&format!(
+                "width={},height={}",
+                display_width, display_height
+            )),
+        )
+        .unwrap();
+        validate_arguments(&mut config).unwrap();
+        assert_eq!(
+            config.virtio_single_touch.unwrap().get_size(),
+            (touch_width, touch_height)
+        );
+    }
 }