Fix input configuration implementation (#3407)

Some of this code was written with an incorrect understanding (that multiple click and scroll modes could be active at once). That required an API change. Also some improved logging and tidying up. No test changes, as the functionality isn't actually affected.
canonical · Jun 5, 2024 · fdeafdf · fdeafdf
2 parents bc301b6 + 0613d64
commit fdeafdf
Show file tree

Hide file tree

Showing 20 changed files with 339 additions and 189 deletions.
diff --git a/debian/control b/debian/control
@@ -107,7 +107,7 @@ Section: libdevel
 Architecture: linux-any
 Multi-Arch: same
 Pre-Depends: ${misc:Pre-Depends}
-Depends: libmircommon10 (= ${binary:Version}),
+Depends: libmircommon11 (= ${binary:Version}),
          libmircore-dev (= ${binary:Version}),
          libxkbcommon-dev,
          ${misc:Depends},
@@ -268,7 +268,7 @@ Description: Display server for Ubuntu - shared library
  .
  Contains the shared libraries required for the Mir server and client.
 
-Package: libmircommon10
+Package: libmircommon11
 Section: libs
 Architecture: linux-any
 Multi-Arch: same
@@ -383,7 +383,7 @@ Description: Display server for Ubuntu - Nvidia driver metapackage
  .
  This package depends on a full set of graphics and input drivers for Nvidia systems.
 
-Package: mir-platform-input-evdev9
+Package: mir-platform-input-evdev10
 Section: libs
 Architecture: linux-any
 Multi-Arch: same
@@ -422,7 +422,7 @@ Multi-Arch: same
 Pre-Depends: ${misc:Pre-Depends}
 Depends: ${misc:Depends},
          mir-platform-graphics-gbm-kms22,
-         mir-platform-input-evdev9,
+         mir-platform-input-evdev10,
          mir-platform-rendering-egl-generic,
 Description: Display server for Ubuntu - gbm-kms driver metapackage
  Mir is a display server running on linux systems, with a focus on efficiency,
@@ -438,7 +438,7 @@ Multi-Arch: same
 Pre-Depends: ${misc:Pre-Depends}
 Depends: ${misc:Depends},
          mir-platform-graphics-eglstream-kms22,
-         mir-platform-input-evdev9,
+         mir-platform-input-evdev10,
 Description: Display server for Ubuntu - eglstream-kms driver metapackage
  Mir is a display server running on linux systems, with a focus on efficiency,
  robust operation and a well-defined driver model.

diff --git a/debian/libmircommon10.install b/debian/libmircommon10.install
diff --git a/debian/libmircommon11.install b/debian/libmircommon11.install
@@ -0,0 +1 @@
+usr/lib/*/libmircommon.so.11
diff --git a/debian/mir-platform-input-evdev10.install b/debian/mir-platform-input-evdev10.install
@@ -0,0 +1 @@
+usr/lib/*/mir/server-platform/input-evdev.so.10
diff --git a/debian/mir-platform-input-evdev9.install b/debian/mir-platform-input-evdev9.install
diff --git a/examples/mir_demo_server/CMakeLists.txt b/examples/mir_demo_server/CMakeLists.txt
@@ -1,3 +1,5 @@
+add_definitions(-DMIR_LOG_COMPONENT_FALLBACK="demo_server")
+
 add_library(exampleserverconfig STATIC
   server_example_input_device_config.cpp
   server_example_input_event_filter.cpp

diff --git a/examples/mir_demo_server/server_example_input_device_config.cpp b/examples/mir_demo_server/server_example_input_device_config.cpp
diff --git a/examples/mir_demo_server/server_example_input_device_config.h b/examples/mir_demo_server/server_example_input_device_config.h
@@ -19,6 +19,7 @@
 
 #include "mir/input/input_device_observer.h"
 #include "mir_toolkit/mir_input_device_types.h"
+#include <optional>
 
 namespace mir
 {
@@ -36,27 +37,29 @@ void add_input_device_configuration_options_to(Server& server);
 class InputDeviceConfig : public mir::input::InputDeviceObserver
 {
 public:
-    InputDeviceConfig(bool disable_while_typing,
-                      MirPointerAcceleration mouse_profile,
-                      double mouse_cursor_acceleration_bias,
-                      double mouse_scroll_speed_scale,
-                      double touchpad_cursor_acceleration_bias,
-                      double touchpad_scroll_speed_scale,
-                      MirTouchpadClickModes click_mode,
-                      MirTouchpadScrollModes scroll_mode);
+    InputDeviceConfig(std::optional<bool> disable_while_typing,
+                      std::optional<bool> tap_to_click,
+                      std::optional<MirPointerAcceleration> mouse_profile,
+                      std::optional<double> mouse_cursor_acceleration_bias,
+                      std::optional<double> mouse_scroll_speed_scale,
+                      std::optional<double> touchpad_cursor_acceleration_bias,
+                      std::optional<double> touchpad_scroll_speed_scale,
+                      std::optional<MirTouchpadClickMode> click_mode,
+                      std::optional<MirTouchpadScrollMode> scroll_mode);
     void device_added(std::shared_ptr<input::Device> const& device) override;
-    void device_changed(std::shared_ptr<input::Device> const&) override {};
-    void device_removed(std::shared_ptr<input::Device> const&) override {};
+    void device_changed(std::shared_ptr<input::Device> const&) override {}
+    void device_removed(std::shared_ptr<input::Device> const&) override {}
     void changes_complete() override {}
 private:
-    bool disable_while_typing;
-    MirPointerAcceleration mouse_profile;
-    double mouse_cursor_acceleration_bias;
-    double mouse_scroll_speed_scale;
-    double touchpad_cursor_acceleration_bias;
-    double touchpad_scroll_speed_scale;
-    MirTouchpadClickModes click_mode;
-    MirTouchpadScrollModes scroll_mode;
+    std::optional<bool> const disable_while_typing;
+    std::optional<MirPointerAcceleration> const mouse_profile;
+    std::optional<double> const mouse_cursor_acceleration_bias;
+    std::optional<double> const mouse_scroll_speed_scale;
+    std::optional<double> const touchpad_cursor_acceleration_bias;
+    std::optional<double> const touchpad_scroll_speed_scale;
+    std::optional<MirTouchpadClickMode> const click_mode;
+    std::optional<MirTouchpadScrollMode> const scroll_mode;
+    std::optional<bool> const tap_to_click;
 };
 
 }

diff --git a/include/common/mir/input/mir_touchpad_config.h b/include/common/mir/input/mir_touchpad_config.h
@@ -30,8 +30,8 @@ struct MirTouchpadConfig
     MirTouchpadConfig(MirTouchpadConfig const& other);
     MirTouchpadConfig& operator=(MirTouchpadConfig const& other);
     ~MirTouchpadConfig();
-    MirTouchpadConfig(MirTouchpadClickModes click_mode,
-                          MirTouchpadScrollModes scroll_mode,
+    MirTouchpadConfig(MirTouchpadClickMode click_mode,
+                          MirTouchpadScrollMode scroll_mode,
                           int button_down_scroll_button,
                           bool tap_to_click,
                           bool disable_while_typing,
@@ -41,13 +41,13 @@ struct MirTouchpadConfig
     /*!
      * The click mode defines when the touchpad generates software emulated button events.
      */
-    MirTouchpadClickModes click_mode() const;
-    void click_mode(MirTouchpadClickModes) ;
+    MirTouchpadClickMode click_mode() const;
+    void click_mode(MirTouchpadClickMode mode) ;
     /*!
      * The scroll mode defines when the touchpad generates scroll events instead of pointer motion events.
      */
-    MirTouchpadScrollModes scroll_mode() const;
-    void scroll_mode(MirTouchpadScrollModes);
+    MirTouchpadScrollMode scroll_mode() const;
+    void scroll_mode(MirTouchpadScrollMode mode);
 
     /*!
      * Configures the button used for the on-button-down scroll mode

diff --git a/include/core/mir_toolkit/mir_input_device_types.h b/include/core/mir_toolkit/mir_input_device_types.h
@@ -63,7 +63,7 @@ typedef enum MirTouchpadClickMode
     mir_touchpad_click_mode_area_to_click = 1 << 0,
     mir_touchpad_click_mode_finger_count  = 1 << 1
 } MirTouchpadClickMode;
-typedef unsigned int MirTouchpadClickModes;
+[[deprecated("UNUSED since Mir 2.18")]] typedef unsigned int MirTouchpadClickModes;
 
 /**
  * MirTouchpadScrollMode configures how the touchpad should generate scroll
@@ -83,7 +83,7 @@ typedef enum MirTouchpadScrollMode
     mir_touchpad_scroll_mode_edge_scroll        = 1 << 1,
     mir_touchpad_scroll_mode_button_down_scroll = 1 << 2
 } MirTouchpadScrollMode;
-typedef unsigned int MirTouchpadScrollModes;
+[[deprecated("UNUSED since Mir 2.18")]] typedef unsigned int MirTouchpadScrollModes;
 
 /**
  * Mapping modes for touchscreen devices. The mode defines how coordinates

diff --git a/include/platform/mir/input/pointer_settings.h b/include/platform/mir/input/pointer_settings.h
@@ -50,15 +50,10 @@ struct PointerSettings
      * Scale vertical scrolling linearly
      */
     double vertical_scroll_scale{1.0};
-};
-
-inline bool operator==(PointerSettings const& lhs, PointerSettings const& rhs)
-{
-    return lhs.handedness == rhs.handedness && lhs.cursor_acceleration_bias == rhs.cursor_acceleration_bias &&
-           lhs.acceleration == rhs.acceleration && lhs.horizontal_scroll_scale == rhs.horizontal_scroll_scale &&
-           lhs.vertical_scroll_scale == rhs.vertical_scroll_scale;
-}
 
+    bool operator==(PointerSettings const& rhs) const = default;
+    bool operator!=(PointerSettings const& rhs) const = default;
+};
 }
 }
 

diff --git a/include/platform/mir/input/touchpad_settings.h b/include/platform/mir/input/touchpad_settings.h
@@ -28,23 +28,18 @@ int const no_scroll_button = 0;
 struct TouchpadSettings
 {
     TouchpadSettings() {}
-    MirTouchpadClickModes click_mode{mir_touchpad_click_mode_finger_count};
-    MirTouchpadScrollModes scroll_mode{mir_touchpad_scroll_mode_two_finger_scroll};
+
+    bool operator==(TouchpadSettings const& rhs) const = default;
+    bool operator!=(TouchpadSettings const& rhs) const = default;
+
+    MirTouchpadClickMode click_mode{mir_touchpad_click_mode_finger_count};
+    MirTouchpadScrollMode scroll_mode{mir_touchpad_scroll_mode_two_finger_scroll};
     int button_down_scroll_button{no_scroll_button};
     bool tap_to_click{true};
     bool disable_while_typing{false};
     bool disable_with_mouse{false};
     bool middle_mouse_button_emulation{true};
 };
-
-inline bool operator==(TouchpadSettings const& lhs, TouchpadSettings const& rhs)
-{
-    return lhs.click_mode == rhs.click_mode && lhs.scroll_mode == rhs.scroll_mode &&
-           lhs.button_down_scroll_button == rhs.button_down_scroll_button && lhs.tap_to_click == rhs.tap_to_click &&
-           lhs.disable_while_typing == rhs.disable_while_typing && lhs.disable_with_mouse == rhs.disable_with_mouse &&
-           lhs.middle_mouse_button_emulation == rhs.middle_mouse_button_emulation;
-}
-
 }
 }
 

diff --git a/src/common/CMakeLists.txt b/src/common/CMakeLists.txt
@@ -44,7 +44,7 @@ set(
   PARENT_SCOPE)
 
 # TODO we need a place to manage ABI and related versioning but use this as placeholder
-set(MIRCOMMON_ABI 10)
+set(MIRCOMMON_ABI 11)
 set(symbol_map ${CMAKE_CURRENT_SOURCE_DIR}/symbols.map)
 
 add_library(mircommon SHARED

diff --git a/src/common/input/mir_touchpad_config.cpp b/src/common/input/mir_touchpad_config.cpp
@@ -20,8 +20,8 @@
 struct MirTouchpadConfig::Implementation
 {
     Implementation() = default;
-    Implementation(MirTouchpadClickModes click_mode,
-                   MirTouchpadScrollModes scroll_mode,
+    Implementation(MirTouchpadClickMode click_mode,
+                   MirTouchpadScrollMode scroll_mode,
                    int button_down_scroll_button,
                    bool tap_to_click,
                    bool middle_mouse_button_emulation,
@@ -36,8 +36,8 @@ struct MirTouchpadConfig::Implementation
           disable_while_typing{disable_while_typing}
     {
     }
-    MirTouchpadClickModes click_mode{mir_touchpad_click_mode_finger_count};
-    MirTouchpadScrollModes scroll_mode{mir_touchpad_scroll_mode_two_finger_scroll};
+    MirTouchpadClickMode click_mode{mir_touchpad_click_mode_finger_count};
+    MirTouchpadScrollMode scroll_mode{mir_touchpad_scroll_mode_two_finger_scroll};
     int button_down_scroll_button{0};
     bool tap_to_click{true};
     bool middle_mouse_button_emulation{true};
@@ -68,13 +68,14 @@ MirTouchpadConfig& MirTouchpadConfig::operator=(MirTouchpadConfig const& other)
 
 MirTouchpadConfig::~MirTouchpadConfig() = default;
 
-MirTouchpadConfig::MirTouchpadConfig(MirTouchpadClickModes click_mode,
-                                                 MirTouchpadScrollModes scroll_mode,
-                                                 int button_down_scroll_button,
-                                                 bool tap_to_click,
-                                                 bool disable_while_typing,
-                                                 bool disable_with_mouse,
-                                                 bool middle_mouse_button_emulation)
+MirTouchpadConfig::MirTouchpadConfig(
+    MirTouchpadClickMode click_mode,
+    MirTouchpadScrollMode scroll_mode,
+    int button_down_scroll_button,
+    bool tap_to_click,
+    bool disable_while_typing,
+    bool disable_with_mouse,
+    bool middle_mouse_button_emulation)
     : impl{std::make_unique<Implementation>(
             click_mode,
             scroll_mode,
@@ -87,24 +88,24 @@ MirTouchpadConfig::MirTouchpadConfig(MirTouchpadClickModes click_mode,
 {
 }
 
-MirTouchpadClickModes MirTouchpadConfig::click_mode() const
+MirTouchpadClickMode MirTouchpadConfig::click_mode() const
 {
     return impl->click_mode;
 }
 
-void MirTouchpadConfig::click_mode(MirTouchpadClickModes modes)
+void MirTouchpadConfig::click_mode(MirTouchpadClickMode mode)
 {
-    impl->click_mode = modes;
+    impl->click_mode = mode;
 }
 
-MirTouchpadScrollModes MirTouchpadConfig::scroll_mode() const
+MirTouchpadScrollMode MirTouchpadConfig::scroll_mode() const
 {
     return impl->scroll_mode;
 }
 
-void MirTouchpadConfig::scroll_mode(MirTouchpadScrollModes modes)
+void MirTouchpadConfig::scroll_mode(MirTouchpadScrollMode mode)
 {
-    impl->scroll_mode = modes;
+    impl->scroll_mode = mode;
 }
 
 int MirTouchpadConfig::button_down_scroll_button() const

diff --git a/src/platforms/CMakeLists.txt b/src/platforms/CMakeLists.txt
@@ -3,7 +3,7 @@
 # This ABI is much smaller than the full libmirplatform ABI.
 #
 # TODO: Add an extra driver-ABI check target.
-set(MIR_SERVER_INPUT_PLATFORM_ABI 9)
+set(MIR_SERVER_INPUT_PLATFORM_ABI 10)
 set(MIR_SERVER_INPUT_PLATFORM_STANZA_VERSION 0.27)
 set(MIR_SERVER_INPUT_PLATFORM_ABI ${MIR_SERVER_INPUT_PLATFORM_ABI} PARENT_SCOPE)
 set(MIR_SERVER_INPUT_PLATFORM_VERSION "MIR_INPUT_PLATFORM_${MIR_SERVER_INPUT_PLATFORM_STANZA_VERSION}")

diff --git a/src/platforms/evdev/CMakeLists.txt b/src/platforms/evdev/CMakeLists.txt
@@ -3,6 +3,8 @@ include_directories(
   ${PROJECT_SOURCE_DIR}/src/include/platform # udev Context
   )
 
+add_definitions(-DMIR_LOG_COMPONENT_FALLBACK="evdev-input")
+
 add_library(mirevdevutilsobjects OBJECT
     button_utils.cpp
 )

diff --git a/src/platforms/evdev/fd_store.cpp b/src/platforms/evdev/fd_store.cpp
@@ -16,7 +16,6 @@
 
 #include "fd_store.h"
 
-#define MIR_LOG_COMPONENT "evdev-input"
 #include "mir/log.h"
 
 #include <algorithm>
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		usr/lib/*/mir/server-platform/input-evdev.so.10