layout: Separate button info from state

Splitting out state into a dedicated place lets keep all the immutable metadata in one place, and all state in another.

src/data/parsing.rs

@@ -17,13 +17,12 @@ use xkbcommon::xkb;
 use super::{ Error, LoadError };
 
 use ::action;
-use ::keyboard::{
+use crate::keyboard::{
-    KeyState, PressType,
+    Key, KeyState, PressType,
     generate_keymaps, generate_keycodes, KeyCode, FormattingError
 };
 use ::layout;
 use ::logging;
-use ::util::hash_map_map;
 use ::resources;
 
 // traits, derives
@@ -183,7 +182,7 @@ impl Layout {
             extract_symbol_names(&button_actions)
         );
 
-        let button_states = HashMap::<String, KeyState>::from_iter(
+        let button_states = HashMap::<String, Key>::from_iter(
             button_actions.into_iter().map(|(name, action)| {
                 let keycodes = match &action {
                     ::action::Action::Submit { text: _, keys } => {
@@ -208,8 +207,7 @@ impl Layout {
                 };
                 (
                     name.into(),
-                    KeyState {
+                    Key {
-                        pressed: PressType::Released,
                         keycodes,
                         action,
                     }
@@ -222,13 +220,7 @@ impl Layout {
             Ok(v) => v,
         };
 
-        let button_states_cache = hash_map_map(
+        let button_states_cache = button_states;
-            button_states,
-            |name, state| {(
-                name,
-                Rc::new(RefCell::new(state))
-            )}
-        );
 
         let views: Vec<_> = self.views.iter()
             .map(|(name, view)| {
@@ -461,7 +453,7 @@ fn create_button<H: logging::Handler>(
     button_info: &HashMap<String, ButtonMeta>,
     outlines: &HashMap<String, Outline>,
     name: &str,
-    state: Rc<RefCell<KeyState>>,
+    data: Key,
     warning_handler: &mut H,
 ) -> ::layout::Button {
     let cname = CString::new(name.clone())
@@ -523,7 +515,11 @@ fn create_button<H: logging::Handler>(
             height: outline.height,
         },
         label: label,
-        state: state,
+        action: data.action,
+        keycodes: data.keycodes,
+        state: Rc::new(
+            RefCell::new(KeyState { pressed: PressType::Released })
+        ),
     }
 }
 
@@ -677,7 +673,6 @@ mod tests {
             out.views["base"].1
                 .get_rows()[0].1
                 .get_buttons()[0].1
-                .state.borrow()
                 .keycodes.len(),
             2
         );
@@ -694,7 +689,6 @@ mod tests {
             out.views["base"].1
                 .get_rows()[0].1
                 .get_buttons()[0].1
-                .state.borrow()
                 .keycodes.len(),
             1
         );

src/drawing.rs

@@ -88,7 +88,7 @@ mod c {
             let state = RefCell::borrow(&button.state).clone();
 
             let locked = LockedStyle::from_action(
-                &state.action,
+                &button.action,
                 &active_modifiers,
                 layout.get_view_latched(),
                 &layout.state.current_view,

src/keyboard.rs

@@ -1,6 +1,8 @@
 /*! State of the emulated keyboard and keys.
  * Regards the keyboard as if it was composed of switches. */
 
+use crate::action::Action;
+use crate::util;
 use std::cell::RefCell;
 use std::collections::HashMap;
 use std::fmt;
@@ -10,9 +12,6 @@ use std::ptr;
 use std::rc::Rc;
 use std::string::FromUtf8Error;
 
-use ::action::Action;
-use ::util;
-
 // Traits
 use std::io::Write;
 use std::iter::{ FromIterator, IntoIterator };
@@ -24,7 +23,7 @@ pub enum PressType {
 }
 
 /// The extended, unambiguous layout-keycode
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, PartialEq)]
 pub struct KeyCode {
     pub code: u32,
     pub keymap_idx: usize,
@@ -53,15 +52,19 @@ bitflags!{
 #[derive(Clone, PartialEq)]
 pub struct KeyStateId(*const KeyState);
 
-#[derive(Debug, Clone)]
+#[derive(Clone)]
-pub struct KeyState {
+pub struct Key {
-    pub pressed: PressType,
     /// A cache of raw keycodes derived from Action::Submit given a keymap
     pub keycodes: Vec<KeyCode>,
     /// Static description of what the key does when pressed or released
     pub action: Action,
 }
 
+#[derive(Debug, Clone)]
+pub struct KeyState {
+    pub pressed: PressType,
+}
+
 impl KeyState {
     #[must_use]
     pub fn into_released(self) -> KeyState {

src/layout.rs

@@ -29,7 +29,7 @@ use crate::action::Action;
 use crate::actors;
 use crate::drawing;
 use crate::float_ord::FloatOrd;
-use crate::keyboard::KeyState;
+use crate::keyboard::{KeyState, KeyCode};
 use crate::logging;
 use crate::popover;
 use crate::receiver;
@@ -466,7 +466,7 @@ pub enum Label {
     IconName(CString),
 }
 
-/// The graphical representation of a button
+/// The definition of an interactive button
 #[derive(Clone, Debug)]
 pub struct Button {
     /// ID string, e.g. for CSS
@@ -476,7 +476,12 @@ pub struct Button {
     pub size: Size,
     /// The name of the visual class applied
     pub outline_name: CString,
-    /// current state, shared with other buttons
+    // action-related stuff
+    /// A cache of raw keycodes derived from Action::Submit given a keymap
+    pub keycodes: Vec<KeyCode>,
+    /// Static description of what the key does when pressed or released
+    pub action: Action,
+    /// Current state, shared with other buttons
     pub state: Rc<RefCell<KeyState>>,
 }
 
@@ -489,7 +494,7 @@ impl Button {
     }
 }
 
-/// The graphical representation of a row of buttons
+/// The representation of a row of buttons
 #[derive(Clone, Debug)]
 pub struct Row {
     /// Buttons together with their offset from the left relative to the row.
@@ -758,10 +763,14 @@ impl fmt::Display for NoSuchView {
 
 impl LayoutData {
     fn get_key(&self, button: &ButtonPosition) -> Option<&Rc<RefCell<KeyState>>> {
+        self.get_button(button).map(|button| &button.state)
+    }
+    
+    fn get_button(&self, button: &ButtonPosition) -> Option<&Button> {
         let (_, view) = self.views.get(&button.view)?;
         let (_, row) = view.rows.get(button.row)?;
         let (_, key) = row.buttons.get(button.position_in_row)?;
-        Some(&key.state)
+        Some(key)
     }
 
     /// Calculates size without margins
@@ -1029,7 +1038,10 @@ mod procedures {
             let state = make_state();
             let state_clone = state.clone();
 
-            let button = make_button_with_state("1".into(), state);
+            let button = Box::new(Button {
+                state,
+                ..make_button("1".into())
+            });
             let button_ptr = as_ptr(&button);
 
             let row = Row::new(vec!((0.1, button)));
@@ -1078,12 +1090,13 @@ mod seat {
                 "Button {:?} was already pressed", button,
             );
         } else {
-            layout.state.pressed_buttons.push(button);
+            layout.state.pressed_buttons.push(button.clone());
         }
         let key: KeyState = {
             RefCell::borrow(rckey).clone()
         };
-        let action = key.action.clone();
+        let button = layout.shape.get_button(&button).unwrap();
+        let action = button.action.clone();
         match action {
             Action::Submit {
                 text: Some(text),
@@ -1091,7 +1104,7 @@ mod seat {
             } => submission.handle_press(
                 KeyState::get_id(rckey),
                 SubmitData::Text(&text),
-                &key.keycodes,
+                &button.keycodes,
                 time,
             ),
             Action::Submit {
@@ -1100,13 +1113,13 @@ mod seat {
             } => submission.handle_press(
                 KeyState::get_id(rckey),
                 SubmitData::Keycodes,
-                &key.keycodes,
+                &button.keycodes,
                 time,
             ),
             Action::Erase => submission.handle_press(
                 KeyState::get_id(rckey),
                 SubmitData::Erase,
-                &key.keycodes,
+                &button.keycodes,
                 time,
             ),
             _ => {},
@@ -1125,12 +1138,13 @@ mod seat {
         // and passed always.
         manager: Option<(&actors::popover::State, receiver::State)>,
         rckey: &Rc<RefCell<KeyState>>,
-        button: ButtonPosition,
+        button_pos: ButtonPosition,
     ) {
         let key: KeyState = {
             RefCell::borrow(rckey).clone()
         };
-        let action = key.action.clone();
+        let button = layout.shape.get_button(&button_pos).unwrap();
+        let action = button.action.clone();
 
         layout.apply_view_transition(&action);
 
@@ -1188,11 +1202,12 @@ mod seat {
         };
 
         // Apply state changes
-        let pos = layout.state.pressed_buttons.iter().position(|b| b == &button);
+        let pos = layout.state.pressed_buttons.iter()
+            .position(|b| b == &button_pos);
         if let Some(pos) = pos {
             layout.state.pressed_buttons.remove(pos);
         } else {
-            log_print!(logging::Level::Bug, "No button to remove from pressed list: {:?}", button);                
+            log_print!(logging::Level::Bug, "No button to remove from pressed list: {:?}", button_pos);                
         }
         // Commit activated button state changes
         RefCell::replace(rckey, key);
@@ -1204,33 +1219,26 @@ mod test {
     use super::*;
 
     use std::ffi::CString;
-    use ::keyboard::PressType;
+    use crate::keyboard::{PressType, KeyState};
 
-    pub fn make_state_with_action(action: Action)
+    pub fn make_state() -> Rc<RefCell<KeyState>> {
-        -> Rc<RefCell<::keyboard::KeyState>>
+        Rc::new(RefCell::new(KeyState {
-    {
-        Rc::new(RefCell::new(::keyboard::KeyState {
             pressed: PressType::Released,
-            keycodes: Vec::new(),
-            action,
         }))
     }
 
-    pub fn make_state() -> Rc<RefCell<::keyboard::KeyState>> {
+    pub fn make_button(
-        make_state_with_action(Action::SetView("default".into()))
-    }
-
-    pub fn make_button_with_state(
         name: String,
-        state: Rc<RefCell<::keyboard::KeyState>>,
+    ) -> Button {
-    ) -> Box<Button> {
+        Button {
-        Box::new(Button {
             name: CString::new(name.clone()).unwrap(),
             size: Size { width: 0f64, height: 0f64 },
             outline_name: CString::new("test").unwrap(),
             label: Label::Text(CString::new(name).unwrap()),
-            state: state,
+            action: Action::SetView("default".into()),
-        })
+            keycodes: Vec::new(),
+            state: make_state(),
+        }
     }
 
     #[test]
@@ -1279,17 +1287,17 @@ mod test {
             Row::new(vec![
                 (
                     0.0,
-                    make_button_with_state(
+                    Box::new(Button {
-                        "switch".into(),
+                        action: switch.clone(),
-                        make_state_with_action(switch.clone())
+                        ..make_button("switch".into())
-                    ),
+                    }),
                 ),
                 (
                     1.0,
-                    make_button_with_state(
+                    Box::new(Button {
-                        "submit".into(),
+                        action: submit.clone(),
-                        make_state_with_action(submit.clone())
+                        ..make_button("submit".into())
-                    ),
+                    }),
                 ),
             ]),
         )]);
@@ -1359,17 +1367,17 @@ mod test {
             Row::new(vec![
                 (
                     0.0,
-                    make_button_with_state(
+                    Box::new(Button {
-                        "switch".into(),
+                        action: switch.clone(),
-                        make_state_with_action(switch.clone())
+                        ..make_button("switch".into())
-                    ),
+                    }),
                 ),
                 (
                     1.0,
-                    make_button_with_state(
+                    Box::new(Button {
-                        "submit".into(),
+                        action: submit.clone(),
-                        make_state_with_action(submit.clone())
+                        ..make_button("submit".into())
-                    ),
+                    }),
                 ),
             ]),
         )]);
@@ -1430,17 +1438,17 @@ mod test {
             Row::new(vec![
                 (
                     0.0,
-                    make_button_with_state(
+                    Box::new(Button {
-                        "switch".into(),
+                        action: switch.clone(),
-                        make_state_with_action(switch.clone())
+                        ..make_button("switch".into())
-                    ),
+                    }),
                 ),
                 (
                     1.0,
-                    make_button_with_state(
+                    Box::new(Button {
-                        "submit".into(),
+                        action: submit.clone(),
-                        make_state_with_action(submit.clone())
+                        ..make_button("submit".into())
-                    ),
+                    }),
                 ),
             ]),
         )]);
@@ -1496,14 +1504,14 @@ mod test {
                         0.0,
                         Box::new(Button {
                             size: Size { width: 5.0, height: 10.0 },
-                            ..*make_button_with_state("A".into(), make_state())
+                            ..make_button("A".into())
                         }),
                     ),
                     (
                         5.0,
                         Box::new(Button {
                             size: Size { width: 5.0, height: 10.0 },
-                            ..*make_button_with_state("B".into(), make_state())
+                            ..make_button("B".into())
                         }),
                     ),
                 ]),
@@ -1515,7 +1523,7 @@ mod test {
                         0.0,
                         Box::new(Button {
                             size: Size { width: 30.0, height: 10.0 },
-                            ..*make_button_with_state("bar".into(), make_state())
+                            ..make_button("bar".into())
                         }),
                     ),
                 ]),
@@ -1549,7 +1557,7 @@ mod test {
                     0.0,
                     Box::new(Button {
                         size: Size { width: 1.0, height: 1.0 },
-                        ..*make_button_with_state("foo".into(), make_state())
+                        ..make_button("foo".into())
                     }),
                 )]),
             ),
@@ -1599,7 +1607,7 @@ mod test {
                     0.0,
                     Box::new(Button {
                         size: Size { width: 1.0, height: 1.0 },
-                        ..*make_button_with_state("foo".into(), make_state())
+                        ..make_button("foo".into())
                     }),
                 )]),
             ),

src/tests.rs

@@ -109,8 +109,7 @@ fn check_layout(layout: Layout, allow_missing_return: bool) {
     for (_pos, view) in layout.views.values() {
         for (_y, row) in view.get_rows() {
             for (_x, button) in row.get_buttons() {
-                let keystate = button.state.borrow();
+                for keycode in &button.keycodes {
-                for keycode in &keystate.keycodes {
                     match xkb_states[keycode.keymap_idx].key_get_one_sym(keycode.code) {
                         xkb::KEY_NoSymbol => {
                             eprintln!(

src/util.rs

@@ -1,12 +1,10 @@
 /*! Assorted helpers */
-use std::collections::HashMap;
 use std::rc::Rc;
 
 use ::float_ord::FloatOrd;
 
 use std::borrow::Borrow;
 use std::hash::{ Hash, Hasher };
-use std::iter::FromIterator;
 use std::ops::Mul;
 
 pub mod c {
@@ -138,16 +136,6 @@ pub trait CloneOwned {
     fn clone_owned(&self) -> Self::Owned;
 }
 
-pub fn hash_map_map<K, V, F, K1, V1>(map: HashMap<K, V>, mut f: F)
-    -> HashMap<K1, V1>
-    where F: FnMut(K, V) -> (K1, V1),
-        K1: std::cmp::Eq + std::hash::Hash
-{
-    HashMap::from_iter(
-        map.into_iter().map(|(key, value)| f(key, value))
-    )
-}
-
 pub fn find_max_double<T, I, F>(iterator: I, get: F)
     -> f64
     where I: Iterator<Item=T>,