Merge branch 'handling' into 'master'

Centralize handling release events

See merge request Librem5/squeekboard!289

src/action.rs

@@ -6,8 +6,8 @@ use std::ffi::CString;
 #[derive(Debug, Clone, PartialEq)]
 pub struct KeySym(pub String);
 
-/// Use to switch layouts
+/// Use to switch views
-type Level = String;
+type View = String;
 
 /// Use to send modified keypresses
 #[derive(Debug, Clone, PartialEq)]
@@ -20,12 +20,12 @@ pub enum Modifier {
 #[derive(Debug, Clone, PartialEq)]
 pub enum Action {
     /// Switch to this view
-    SetLevel(Level),
+    SetView(View),
     /// Switch to a view and latch
-    LockLevel {
+    LockView {
-        lock: Level,
+        lock: View,
         /// When unlocked by pressing it or emitting a key
-        unlock: Level,
+        unlock: View,
     },
     /// Set this modifier TODO: release?
     SetModifier(Modifier),

src/data.rs

@@ -522,7 +522,7 @@ fn create_action<H: WarningHandler>(
     match submission {
         SubmitData::Action(
             Action::SetView(view_name)
-        ) => ::action::Action::SetLevel(
+        ) => ::action::Action::SetView(
             filter_view_name(
                 name, view_name.clone(), &view_names,
                 warning_handler,
@@ -530,7 +530,7 @@ fn create_action<H: WarningHandler>(
         ),
         SubmitData::Action(Action::Locking {
             lock_view, unlock_view
-        }) => ::action::Action::LockLevel {
+        }) => ::action::Action::LockView {
             lock: filter_view_name(
                 name,
                 lock_view.clone(),

src/keyboard.rs

@@ -17,16 +17,39 @@ pub enum PressType {
     Pressed = 1,
 }
 
+pub type KeyCode = u32;
+
 #[derive(Debug, Clone)]
 pub struct KeyState {
     pub pressed: PressType,
     pub locked: bool,
     /// A cache of raw keycodes derived from Action::Sumbit given a keymap
-    pub keycodes: Vec<u32>,
+    pub keycodes: Vec<KeyCode>,
     /// Static description of what the key does when pressed or released
     pub action: Action,
 }
 
+impl KeyState {
+    #[must_use]
+    pub fn into_activated(self) -> KeyState {
+        match self.action {
+            Action::LockView { lock: _, unlock: _ } => KeyState {
+                locked: self.locked ^ true,
+                ..self
+            },
+            _ => self,
+        }
+    }
+
+    #[must_use]
+    pub fn into_released(self) -> KeyState {
+        KeyState {
+            pressed: PressType::Released,
+            ..self
+        }
+    }
+}
+
 /// Sorts an iterator by converting it to a Vector and back
 fn sorted<'a, I: Iterator<Item=&'a str>>(
     iter: I

src/layout.rs

@@ -265,6 +265,11 @@ pub mod c {
             let time = Timestamp(time);
             let layout = unsafe { &mut *layout };
             let virtual_keyboard = VirtualKeyboard(virtual_keyboard);
+            let ui_backend = UIBackend {
+                widget_to_layout,
+                keyboard: ui_keyboard,
+            };
+
             // The list must be copied,
             // because it will be mutated in the loop
             for key in layout.pressed_keys.clone() {
@@ -272,17 +277,16 @@ pub mod c {
                 seat::handle_release_key(
                     layout,
                     &virtual_keyboard,
-                    &widget_to_layout,
+                    Some(&ui_backend),
                     time,
-                    ui_keyboard,
+                    Some(manager),
-                    manager,
                     key,
                 );
             }
             drawing::queue_redraw(ui_keyboard);
         }
 
-        /// Release all buittons but don't redraw
+        /// Release all buttons but don't redraw
         #[no_mangle]
         pub extern "C"
         fn squeek_layout_release_all_only(
@@ -296,10 +300,13 @@ pub mod c {
             // because it will be mutated in the loop
             for key in layout.pressed_keys.clone() {
                 let key: &Rc<RefCell<KeyState>> = key.borrow();
-                layout.release_key(
+                seat::handle_release_key(
+                    layout,
                     &virtual_keyboard,
+                    None, // don't update UI
+                    Timestamp(time),
+                    None, // don't switch layouts
                     &mut key.clone(),
-                    Timestamp(time)
                 );
             }
         }
@@ -353,8 +360,11 @@ pub mod c {
             let time = Timestamp(time);
             let layout = unsafe { &mut *layout };
             let virtual_keyboard = VirtualKeyboard(virtual_keyboard);
-
+            let ui_backend = UIBackend {
-            let point = widget_to_layout.forward(
+                widget_to_layout,
+                keyboard: ui_keyboard,
+            };
+            let point = ui_backend.widget_to_layout.forward(
                 Point { x: x_widget, y: y_widget }
             );
             
@@ -379,10 +389,9 @@ pub mod c {
                         seat::handle_release_key(
                             layout,
                             &virtual_keyboard,
-                            &widget_to_layout,
+                            Some(&ui_backend),
                             time,
-                            ui_keyboard,
+                            Some(manager),
-                            manager,
                             key,
                         );
                     }
@@ -397,10 +406,9 @@ pub mod c {
                     seat::handle_release_key(
                         layout,
                         &virtual_keyboard,
-                        &widget_to_layout,
+                        Some(&ui_backend),
                         time,
-                        ui_keyboard,
+                        Some(manager),
-                        manager,
                         key,
                     );
                 }
@@ -612,6 +620,7 @@ pub struct LayoutData {
     pub margins: Margins,
 }
 
+#[derive(Debug)]
 struct NoSuchView;
 
 // Unfortunately, changes are not atomic due to mutability :(
@@ -644,84 +653,22 @@ impl Layout {
         }
     }
 
-    fn release_key(
-        &mut self,
-        virtual_keyboard: &VirtualKeyboard,
-        mut key: &mut Rc<RefCell<KeyState>>,
-        time: Timestamp,
-    ) {
-        if !self.pressed_keys.remove(&::util::Pointer(key.clone())) {
-            eprintln!("Warning: key {:?} was not pressed", key);
-        }
-        virtual_keyboard.switch(
-            &mut key.borrow_mut(),
-            PressType::Released,
-            time,
-        );
-        self.set_level_from_press(&mut key);
-    }
-    
     fn press_key(
         &mut self,
         virtual_keyboard: &VirtualKeyboard,
-        key: &mut Rc<RefCell<KeyState>>,
+        rckey: &mut Rc<RefCell<KeyState>>,
         time: Timestamp,
     ) {
-        if !self.pressed_keys.insert(::util::Pointer(key.clone())) {
+        if !self.pressed_keys.insert(::util::Pointer(rckey.clone())) {
-            eprintln!("Warning: key {:?} was already pressed", key);
+            eprintln!("Warning: key {:?} was already pressed", rckey);
         }
+        let mut key = rckey.borrow_mut();
         virtual_keyboard.switch(
-            &mut key.borrow_mut(),
+            &key.keycodes,
             PressType::Pressed,
             time,
         );
-    }
+        key.pressed = PressType::Pressed;
-
-    fn set_level_from_press(&mut self, key: &Rc<RefCell<KeyState>>) {
-        let keys = self.locked_keys.clone();
-        for key in &keys {
-            self.locked_keys.remove(key);
-            self.set_state_from_press(key.borrow());
-        }
-
-        // Don't handle the same key twice, but handle it at least once,
-        // because its press is the reason we're here
-        if !keys.contains(&::util::Pointer(key.clone())) {
-            self.set_state_from_press(key);
-        }
-    }
-
-    fn set_state_from_press(&mut self, key: &Rc<RefCell<KeyState>>) {
-        // Action should not hold a reference to key,
-        // because key is later borrowed for mutation. So action is cloned.
-        // RefCell::borrow() is covered up by (dyn Borrow)::borrow()
-        // if used like key.borrow() :(
-        let action = RefCell::borrow(key).action.clone();
-        let view_name = match action {
-            Action::SetLevel(name) => {
-                Some(name.clone())
-            },
-            Action::LockLevel { lock, unlock } => {
-                let locked = {
-                    let mut key = key.borrow_mut();
-                    key.locked ^= true;
-                    key.locked
-                };
-
-                if locked {
-                    self.locked_keys.insert(::util::Pointer(key.clone()));
-                }
-
-                Some(if locked { lock } else { unlock }.clone())
-            },
-            _ => None,
-        };
-
-        if let Some(view_name) = view_name {
-            if let Err(_e) = self.set_view(view_name.clone()) {
-                eprintln!("No such view: {}, ignoring switch", view_name)
-            };
-        };
     }
 
     /// Calculates size without margins
@@ -848,44 +795,161 @@ mod procedures {
     }
 }
 
+pub struct UIBackend {
+    widget_to_layout: c::Transformation,
+    keyboard: c::EekGtkKeyboard,
+}
+
 /// Top level procedures, dispatching to everything
 mod seat {
     use super::*;
 
-    // TODO: turn into release_button
+    fn try_set_view(layout: &mut Layout, view_name: String) {
+        layout.set_view(view_name.clone())
+            .map_err(|e|
+                eprintln!("Bad view {} ({:?}), ignoring", view_name, e)
+            ).ok();
+    }
+
+    /// A vessel holding an obligation to switch view.
+    /// Use with #[must_use]
+    struct ViewChange<'a> {
+        layout: &'a mut Layout,
+        view_name: Option<String>,
+    }
+    
+    impl<'a> ViewChange<'a> {
+        fn choose_view(self, view_name: String) -> ViewChange<'a> {
+            ViewChange {
+                view_name: Some(view_name),
+                ..self
+            }
+        }
+        fn apply(self) {
+            if let Some(name) = self.view_name {
+                try_set_view(self.layout, name);
+            }
+        }
+    }
+
+    /// Find all impermanent view changes and undo them in an arbitrary order.
+    /// Return an obligation to actually switch the view.
+    /// The final view is the "unlock" view
+    /// from one of the currently stuck keys.
+    // As long as only one stuck button is used, this should be fine.
+    // This is guaranteed because pressing a lock button unlocks all others.
+    // TODO: Make some broader guarantee about the resulting view,
+    // e.g. by maintaining a stack of stuck keys.
+    #[must_use]
+    fn unstick_locks(layout: &mut Layout) -> ViewChange {
+        let mut new_view = None;
+        for key in layout.locked_keys.clone() {
+            let key: &Rc<RefCell<KeyState>> = key.borrow();
+            let mut key = RefCell::borrow_mut(key);
+            match &key.action {
+                Action::LockView { lock: _, unlock: view } => {
+                    new_view = Some(view.clone());
+                },
+                a => eprintln!(
+                    "BUG: action {:?} was found inside locked keys",
+                    a
+                ),
+            };
+            key.locked = false;
+        }
+        
+        ViewChange {
+            layout,
+            view_name: new_view,
+        }
+    }
+
     pub fn handle_release_key(
         layout: &mut Layout,
         virtual_keyboard: &VirtualKeyboard,
-        widget_to_layout: &c::Transformation,
+        ui: Option<&UIBackend>,
         time: Timestamp,
-        ui_keyboard: c::EekGtkKeyboard,
+        manager: Option<manager::c::Manager>,
-        manager: manager::c::Manager,
+        rckey: &Rc<RefCell<KeyState>>,
-        key: &Rc<RefCell<KeyState>>,
     ) {
-        layout.release_key(virtual_keyboard, &mut key.clone(), time);
+        let key: KeyState = {
+            RefCell::borrow(rckey).clone()
+        };
+        let action = key.action.clone();
 
-        let view = layout.get_current_view();
+        // update
-        let action = RefCell::borrow(key).action.clone();
+        let key = key.into_released();
-        if let Action::ShowPreferences = action {
+        let key = match action {
-            let places = ::layout::procedures::find_key_places(
+            Action::LockView { lock: _, unlock: _ } => key.into_activated(),
-                view, key
+            _ => key,
+        };
+
+        // process changes
+        match action {
+            Action::Submit { text: _, keys: _ } => {
+                unstick_locks(layout).apply();
+                virtual_keyboard.switch(
+                    &key.keycodes,
+                    PressType::Released,
+                    time,
                 );
-            // getting first item will cause mispositioning
+            },
+            Action::SetView(view) => {
+                try_set_view(layout, view)
+            },
+            Action::LockView { lock, unlock } => {
+                // The button that triggered this will be in the right state
+                // due to commit at the end.
+                unstick_locks(layout)
+                    // It doesn't matter what the resulting view should be,
+                    // it's getting changed anyway.
+                    .choose_view(
+                        match key.locked {
+                            true => lock.clone(),
+                            false => unlock.clone(),
+                        }
+                    )
+                    .apply()
+            },
+            // only show when UI is present
+            Action::ShowPreferences => if let Some(ui) = &ui {
+                // only show when layout manager is available
+                if let Some(manager) = manager {
+                    let view = layout.get_current_view();
+                    let places = ::layout::procedures::find_key_places(
+                        view, &rckey,
+                    );
+                    // Getting first item will cause mispositioning
                     // with more than one button with the same key
-            // on the keyboard
+                    // on the keyboard.
-            if let Some((offset, button)) = places.get(0) {
+                    if let Some((position, button)) = places.get(0) {
                         let bounds = c::Bounds {
-                    x: offset.x, y: offset.y,
+                            x: position.x,
+                            y: position.y,
                             width: button.size.width,
                             height: button.size.height,
                         };
                         ::popover::show(
-                    ui_keyboard,
+                            ui.keyboard,
-                    widget_to_layout.reverse_bounds(bounds),
+                            ui.widget_to_layout.reverse_bounds(bounds),
                             manager,
                         );
                     }
                 }
+            },
+            Action::SetModifier(_) => eprintln!("Modifiers unsupported"),
+        };
+
+        let pointer = ::util::Pointer(rckey.clone());
+        // Apply state changes
+        layout.pressed_keys.remove(&pointer);
+        if key.locked {
+            layout.locked_keys.insert(pointer);
+        } else {
+            layout.locked_keys.remove(&pointer);
+        }
+        // Commit activated button state changes
+        RefCell::replace(rckey, key);
     }
 }
 
@@ -900,7 +964,7 @@ mod test {
             pressed: PressType::Released,
             locked: false,
             keycodes: Vec::new(),
-            action: Action::SetLevel("default".into()),
+            action: Action::SetView("default".into()),
         }))
     }
 

src/submission.rs

@@ -1,6 +1,6 @@
 /*! Managing the events belonging to virtual-keyboard interface. */
 
-use ::keyboard::{ KeyState, PressType };
+use ::keyboard::{ KeyCode, PressType };
 
 /// Gathers stuff defined in C or called by C
 pub mod c {
@@ -33,16 +33,14 @@ impl VirtualKeyboard {
     // TODO: split out keyboard state management
     pub fn switch(
         &self,
-        key: &mut KeyState,
+        keycodes: &Vec<KeyCode>,
         action: PressType,
         timestamp: Timestamp,
     ) {
-        key.pressed = action.clone();
+        let keycodes_count = keycodes.len();
-
+        for keycode in keycodes.iter() {
-        let keycodes_count = key.keycodes.len();
-        for keycode in key.keycodes.iter() {
             let keycode = keycode - 8;
-            match (&key.pressed, keycodes_count) {
+            match (action, keycodes_count) {
                 // Pressing a key made out of a single keycode is simple:
                 // press on press, release on release.
                 (_, 1) => unsafe {