Merge branch 'fixmods' into 'master'

virtual_keyboard: Fix desynced modifiers state

See merge request Librem5/squeekboard!362

eek/eek-gtk-keyboard.c

@@ -126,8 +126,8 @@ eek_gtk_keyboard_real_size_allocate (GtkWidget     *self,
                 (uint32_t)(allocation->height - allocation->y) * scale);
     if (priv->layout->arrangement != new_type) {
         priv->layout->arrangement = new_type;
-
+        uint32_t time = gdk_event_get_time(NULL);
-        eekboard_context_service_use_layout(priv->eekboard_context, priv->layout);
+        eekboard_context_service_use_layout(priv->eekboard_context, priv->layout, time);
     }
 
     if (priv->renderer)

eekboard/eekboard-context-service.c

@@ -116,7 +116,7 @@ settings_get_layout(GSettings *settings, char **type, char **layout)
 }
 
 void
-eekboard_context_service_use_layout(EekboardContextService *context, struct squeek_layout_state *state) {
+eekboard_context_service_use_layout(EekboardContextService *context, struct squeek_layout_state *state, uint32_t timestamp) {
     gchar *layout_name = state->overlay_name;
 
     if (layout_name == NULL) {
@@ -148,7 +148,7 @@ eekboard_context_service_use_layout(EekboardContextService *context, struct sque
     // Update the keymap if necessary.
     // TODO: Update submission on change event
     if (context->priv->submission) {
-        submission_set_keyboard(context->priv->submission, keyboard);
+        submission_set_keyboard(context->priv->submission, keyboard, timestamp);
     }
 
     // Update UI
@@ -174,7 +174,8 @@ static void eekboard_context_service_update_settings_layout(EekboardContextServi
             context->layout->layout_name = g_strdup(keyboard_layout);
         }
         // This must actually update the UI.
-        eekboard_context_service_use_layout(context, context->layout);
+        uint32_t time = gdk_event_get_time(NULL);
+        eekboard_context_service_use_layout(context, context->layout, time);
     }
 }
 
@@ -299,7 +300,8 @@ void eekboard_context_service_set_hint_purpose(EekboardContextService *context,
     if (context->layout->hint != hint || context->layout->purpose != purpose) {
         context->layout->hint = hint;
         context->layout->purpose = purpose;
-        eekboard_context_service_use_layout(context, context->layout);
+        uint32_t time = gdk_event_get_time(NULL);
+        eekboard_context_service_use_layout(context, context->layout, time);
     }
 }
 
@@ -308,7 +310,8 @@ eekboard_context_service_set_overlay(EekboardContextService *context, const char
     if (g_strcmp0(context->layout->overlay_name, name)) {
         g_free(context->layout->overlay_name);
         context->layout->overlay_name = g_strdup(name);
-        eekboard_context_service_use_layout(context, context->layout);
+        uint32_t time = gdk_event_get_time(NULL);
+        eekboard_context_service_use_layout(context, context->layout, time);
     }
 }
 
@@ -322,14 +325,16 @@ EekboardContextService *eekboard_context_service_new(struct squeek_layout_state
     EekboardContextService *context = g_object_new (EEKBOARD_TYPE_CONTEXT_SERVICE, NULL);
     context->layout = state;
     eekboard_context_service_update_settings_layout(context);
-    eekboard_context_service_use_layout(context, context->layout);
+    uint32_t time = gdk_event_get_time(NULL);
+    eekboard_context_service_use_layout(context, context->layout, time);
     return context;
 }
 
 void eekboard_context_service_set_submission(EekboardContextService *context, struct submission *submission) {
     context->priv->submission = submission;
     if (context->priv->submission) {
-        submission_set_keyboard(context->priv->submission, context->priv->keyboard);
+        uint32_t time = gdk_event_get_time(NULL);
+        submission_set_keyboard(context->priv->submission, context->priv->keyboard, time);
     }
 }
 

eekboard/eekboard-context-service.h

@@ -95,6 +95,6 @@ void eekboard_context_service_set_hint_purpose(EekboardContextService *context,
                                                uint32_t hint,
                                                uint32_t purpose);
 void
-eekboard_context_service_use_layout(EekboardContextService *context, struct squeek_layout_state *layout);
+eekboard_context_service_use_layout(EekboardContextService *context, struct squeek_layout_state *layout, uint32_t timestamp);
 G_END_DECLS
 #endif  /* EEKBOARD_CONTEXT_SERVICE_H */

src/keyboard.rs

@@ -43,7 +43,7 @@ bitflags!{
 
 /// When the submitted actions of keys need to be tracked,
 /// they need a stable, comparable ID
-#[derive(PartialEq)]
+#[derive(Clone, PartialEq)]
 pub struct KeyStateId(*const KeyState);
 
 #[derive(Debug, Clone)]

src/submission.h

@@ -15,5 +15,5 @@ struct submission* get_submission(struct zwp_input_method_manager_v2 *immanager,
 // Defined in Rust
 struct submission* submission_new(struct zwp_input_method_v2 *im, struct zwp_virtual_keyboard_v1 *vk, EekboardContextService *state);
 void submission_set_ui(struct submission *self, ServerContextService *ui_context);
-void submission_set_keyboard(struct submission *self, LevelKeyboard *keyboard);
+void submission_set_keyboard(struct submission *self, LevelKeyboard *keyboard, uint32_t time);
 #endif

src/submission.rs

@@ -23,6 +23,7 @@ use ::action::Modifier;
 use ::imservice;
 use ::imservice::IMService;
 use ::keyboard::{ KeyCode, KeyStateId, Modifiers, PressType };
+use ::layout::c::LevelKeyboard;
 use ::util::vec_remove;
 use ::vkeyboard::VirtualKeyboard;
 
@@ -36,7 +37,6 @@ pub mod c {
     use std::os::raw::c_void;
 
     use ::imservice::c::InputMethod;
-    use ::layout::c::LevelKeyboard;
     use ::vkeyboard::c::ZwpVirtualKeyboardV1;
 
     // The following defined in C
@@ -91,18 +91,23 @@ pub mod c {
 
     #[no_mangle]
     pub extern "C"
-    fn submission_set_keyboard(submission: *mut Submission, keyboard: LevelKeyboard) {
+    fn submission_set_keyboard(
+        submission: *mut Submission,
+        keyboard: LevelKeyboard,
+        time: u32,
+    ) {
         if submission.is_null() {
             panic!("Null submission pointer");
         }
         let submission: &mut Submission = unsafe { &mut *submission };
-        submission.virtual_keyboard.update_keymap(keyboard);
+        submission.update_keymap(keyboard, Timestamp(time));
     }
 }
 
 #[derive(Clone, Copy)]
 pub struct Timestamp(pub u32);
 
+#[derive(Clone)]
 enum SubmittedAction {
     /// A collection of keycodes that were pressed
     VirtualKeyboard(Vec<KeyCode>),
@@ -243,4 +248,44 @@ impl Submission {
             self.modifiers_active.iter().map(|(_id, m)| m.clone())
         )
     }
+
+    fn clear_all_modifiers(&mut self) {
+        // Looks like an optimization,
+        // but preemptive cleaning is needed before setting a new keymap,
+        // so removing this check would break keymap setting.
+        if self.modifiers_active.is_empty() {
+            return;
+        }
+        self.modifiers_active = Vec::new();
+        self.virtual_keyboard.set_modifiers_state(Modifiers::empty())
+    }
+
+    fn release_all_virtual_keys(&mut self, time: Timestamp) {
+        let virtual_pressed = self.pressed
+            .clone().into_iter()
+            .filter_map(|(id, action)| {
+                match action {
+                    SubmittedAction::VirtualKeyboard(_) => Some(id),
+                    _ => None,
+                }
+            });
+        for id in virtual_pressed {
+            self.handle_release(id, time);
+        }
+    }
+
+    /// Changes keymap and clears pressed keys and modifiers.
+    ///
+    /// It's not obvious if clearing is the right thing to do, 
+    /// but keymap update may (or may not) do that,
+    /// possibly putting self.modifiers_active and self.pressed out of sync,
+    /// so a consistent stance is adopted to avoid that.
+    /// Alternatively, modifiers could be restored on the new keymap.
+    /// That approach might be difficult
+    /// due to modifiers meaning different things in different keymaps.
+    pub fn update_keymap(&mut self, keyboard: LevelKeyboard, time: Timestamp) {
+        self.clear_all_modifiers();
+        self.release_all_virtual_keys(time);
+        self.virtual_keyboard.update_keymap(keyboard);
+    }
 }