Merge branch 'splitl' into 'master'

layout: cleanups

See merge request World/Phosh/squeekboard!577

src/data/loading.rs

@@ -192,7 +192,7 @@ fn iter_layout_sources(
 }
 
 fn load_layout_data(source: DataSource)
-    -> Result<::layout::LayoutData, LoadError>
+    -> Result<::layout::LayoutParseData, LoadError>
 {
     let handler = logging::Print {};
     match source {
@@ -217,7 +217,7 @@ fn load_layout_data_with_fallback(
     kind: ArrangementKind,
     purpose: ContentPurpose,
     overlay: Option<&str>,
-) -> (ArrangementKind, layout::LayoutData) {
+) -> (ArrangementKind, layout::LayoutParseData) {
 
     // Build the path to the right keyboard layout subdirectory
     let path = env::var_os("SQUEEKBOARD_KEYBOARDSDIR")

src/data/parsing.rs

@@ -4,12 +4,10 @@
 
 /*! Parsing of the data files containing layouts */
 
-use std::cell::RefCell;
 use std::collections::{ HashMap, HashSet };
 use std::ffi::CString;
 use std::fs;
 use std::path::PathBuf;
-use std::rc::Rc;
 use std::vec::Vec;
 
 use xkbcommon::xkb;
@@ -17,13 +15,11 @@ use xkbcommon::xkb;
 use super::{ Error, LoadError };
 
 use ::action;
-use ::keyboard::{
+use crate::keyboard::{
-    KeyState, PressType,
+    Key, generate_keymaps, generate_keycodes, KeyCode, FormattingError
-    generate_keymaps, generate_keycodes, KeyCode, FormattingError
 };
 use ::layout;
 use ::logging;
-use ::util::hash_map_map;
 use ::resources;
 
 // traits, derives
@@ -157,7 +153,7 @@ impl Layout {
     }
 
     pub fn build<H: logging::Handler>(self, mut warning_handler: H)
-        -> (Result<::layout::LayoutData, FormattingError>, H)
+        -> (Result<::layout::LayoutParseData, FormattingError>, H)
     {
         let button_names = self.views.values()
             .flat_map(|rows| {
@@ -183,7 +179,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 +204,7 @@ impl Layout {
                 };
                 (
                     name.into(),
-                    KeyState {
+                    Key {
-                        pressed: PressType::Released,
                         keycodes,
                         action,
                     }
@@ -222,20 +217,14 @@ 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)| {
                 let rows = view.iter().map(|row| {
                     let buttons = row.split_ascii_whitespace()
                         .map(|name| {
-                            Box::new(create_button(
+                            create_button(
                                 &self.buttons,
                                 &self.outlines,
                                 name,
@@ -243,7 +232,7 @@ impl Layout {
                                     .expect("Button state not created")
                                     .clone(),
                                 &mut warning_handler,
-                            ))
+                            )
                         });
                     layout::Row::new(
                         add_offsets(
@@ -279,7 +268,7 @@ impl Layout {
         };
 
         (
-            Ok(::layout::LayoutData {
+            Ok(layout::LayoutParseData {
                 views: views,
                 keymaps: keymaps.into_iter().map(|keymap_str|
                     CString::new(keymap_str)
@@ -461,7 +450,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 +512,8 @@ fn create_button<H: logging::Handler>(
             height: outline.height,
         },
         label: label,
-        state: state,
+        action: data.action,
+        keycodes: data.keycodes,
     }
 }
 
@@ -677,7 +667,6 @@ mod tests {
             out.views["base"].1
                 .get_rows()[0].1
                 .get_buttons()[0].1
-                .state.borrow()
                 .keycodes.len(),
             2
         );
@@ -694,7 +683,6 @@ mod tests {
             out.views["base"].1
                 .get_rows()[0].1
                 .get_buttons()[0].1
-                .state.borrow()
                 .keycodes.len(),
             1
         );

src/drawing.rs

@@ -1,11 +1,10 @@
 /*! Drawing the UI */
 
 use cairo;
-use std::cell::RefCell;
 
 use ::action::{ Action, Modifier };
 use ::keyboard;
-use ::layout::{ Button, Label, LatchedState, Layout };
+use crate::layout::{ Button, ButtonPosition, Label, LatchedState, Layout };
 use ::layout::c::{ Bounds, EekGtkKeyboard, Point };
 use ::submission::c::Submission as CSubmission;
 
@@ -84,14 +83,21 @@ mod c {
         let cr = unsafe { cairo::Context::from_raw_none(cr) };
         let active_modifiers = submission.get_active_modifiers();
 
-        layout.foreach_visible_button(|offset, button| {
+        layout.foreach_visible_button(|offset, button, (row, position_in_row)| {
-            let state = RefCell::borrow(&button.state).clone();
+            // TODO: this iterator copies string indices way too much.
+            // For efficiency, it would be better to draw pressed buttons from the list first,
+            // and then iterate the rest without having to look up their indices.
+            let state = layout.state.active_buttons.get(&ButtonPosition {
+                view: layout.state.current_view.clone(),
+                row,
+                position_in_row,
+            });
 
             let locked = LockedStyle::from_action(
-                &state.action,
+                &button.action,
                 &active_modifiers,
                 layout.get_view_latched(),
-                &layout.current_view,
+                &layout.state.current_view,
             );
             if state.pressed == keyboard::PressType::Pressed
                 || locked != LockedStyle::Free
@@ -99,7 +105,7 @@ mod c {
                 render_button_at_position(
                     renderer, &cr,
                     offset,
-                    button.as_ref(),
+                    button,
                     state.pressed, locked,
                 );
             }
@@ -116,11 +122,11 @@ mod c {
         let layout = unsafe { &mut *layout };
         let cr = unsafe { cairo::Context::from_raw_none(cr) };
         
-        layout.foreach_visible_button(|offset, button| {
+        layout.foreach_visible_button(|offset, button, _index| {
             render_button_at_position(
                 renderer, &cr,
                 offset,
-                button.as_ref(),
+                button,
                 keyboard::PressType::Released,
                 LockedStyle::Free,
             );

src/keyboard.rs

@@ -1,18 +1,16 @@
 /*! State of the emulated keyboard and keys.
  * Regards the keyboard as if it was composed of switches. */
 
-use std::cell::RefCell;
+use crate::action::Action;
+use crate::layout;
+use crate::util;
 use std::collections::HashMap;
 use std::fmt;
 use std::io;
 use std::mem;
 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 +22,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,
@@ -49,19 +47,30 @@ bitflags!{
 }
 
 /// When the submitted actions of keys need to be tracked,
-/// they need a stable, comparable ID
+/// they need a stable, comparable ID.
+/// With layout::ButtonPosition, the IDs are unique within layouts.
 #[derive(Clone, PartialEq)]
-pub struct KeyStateId(*const KeyState);
+pub struct KeyStateId(layout::ButtonPosition);
 
-#[derive(Debug, Clone)]
+impl From<&layout::ButtonPosition> for KeyStateId {
-pub struct KeyState {
+    fn from(v: &layout::ButtonPosition) -> Self {
-    pub pressed: PressType,
+        Self(v.clone())
+    }
+}
+
+#[derive(Clone)]
+pub struct Key {
     /// 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 {
@@ -78,12 +87,6 @@ impl KeyState {
             ..self
         }
     }
-
-    /// KeyStates instances are the unique identifiers of pressed keys,
-    /// and the actions submitted with them.
-    pub fn get_id(keystate: &Rc<RefCell<KeyState>>) -> KeyStateId {
-        KeyStateId(keystate.as_ptr() as *const KeyState)
-    }
 }
 
 /// Sorts an iterator by converting it to a Vector and back

src/submission.rs

@@ -52,7 +52,7 @@ pub mod c {
         let submission = submission.clone_ref();
         let mut submission = submission.borrow_mut();
         let layout = unsafe { &*layout };
-        submission.use_layout(layout, Timestamp(time));
+        submission.use_layout(&layout.shape, Timestamp(time));
     }
 
     #[no_mangle]
@@ -303,7 +303,7 @@ impl Submission {
         }
     }
     
-    pub fn use_layout(&mut self, layout: &layout::Layout, time: Timestamp) {
+    pub fn use_layout(&mut self, layout: &layout::LayoutData, time: Timestamp) {
         self.keymap_fds = layout.keymaps.iter()
             .map(|keymap_str| vkeyboard::c::KeyMap::from_cstr(
                 keymap_str.as_c_str()

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>,