Merge branch 'extend-keys-to-bounding-box' into 'master'

Expand key press detection to the edges of the view's bounding box

Closes #191

See merge request Librem5/squeekboard!379

src/data.rs

@@ -459,14 +459,14 @@ impl Layout {
                                 &mut warning_handler,
                             ))
                         });
-                    layout::Row {
+                    layout::Row::new(
-                        buttons: add_offsets(
+                        add_offsets(
                             buttons,
                             |button| button.size.width,
                         ).collect()
-                    }
+                    )
                 });
-                let rows = add_offsets(rows, |row| row.get_height())
+                let rows = add_offsets(rows, |row| row.get_size().height)
                     .collect();
                 (
                     name.clone(),
@@ -484,8 +484,8 @@ impl Layout {
                 name,
                 (
                     layout::c::Point {
-                        x: (total_size.width - view.get_width()) / 2.0,
+                        x: (total_size.width - view.get_size().width) / 2.0,
-                        y: (total_size.height - view.get_height()) / 2.0,
+                        y: (total_size.height - view.get_size().height) / 2.0,
                     },
                     view,
                 ),
@@ -824,7 +824,7 @@ mod tests {
         assert_eq!(
             out.views["base"].1
                 .get_rows()[0].1
-                .buttons[0].1
+                .get_buttons()[0].1
                 .label,
             ::layout::Label::Text(CString::new("test").unwrap())
         );
@@ -839,7 +839,7 @@ mod tests {
         assert_eq!(
             out.views["base"].1
                 .get_rows()[0].1
-                .buttons[0].1
+                .get_buttons()[0].1
                 .label,
             ::layout::Label::Text(CString::new("test").unwrap())
         );
@@ -855,7 +855,7 @@ mod tests {
         assert_eq!(
             out.views["base"].1
                 .get_rows()[0].1
-                .buttons[0].1
+                .get_buttons()[0].1
                 .state.borrow()
                 .keycodes.len(),
             2

src/layout.rs

@@ -493,38 +493,63 @@ pub struct Button {
 }
 
 /// The graphical representation of a row of buttons
+#[derive(Clone, Debug)]
 pub struct Row {
-    /// Buttons together with their offset from the left
+    /// Buttons together with their offset from the left relative to the row.
-    pub buttons: Vec<(f64, Box<Button>)>,
+    /// ie. the first button always start at 0.
+    buttons: Vec<(f64, Box<Button>)>,
+
+    /// Total size of the row
+    size: Size,
 }
 
 impl Row {
-    pub fn get_height(&self) -> f64 {
+    pub fn new(buttons: Vec<(f64, Box<Button>)>) -> Row {
-        find_max_double(
+        // Make sure buttons are sorted by offset.
-            self.buttons.iter(),
+        debug_assert!({
+            let mut sorted = buttons.clone();
+            sorted.sort_by(|(f1, _), (f2, _)| f1.partial_cmp(f2).unwrap());
+
+            sorted.iter().map(|(f, _)| *f).collect::<Vec<_>>()
+                == buttons.iter().map(|(f, _)| *f).collect::<Vec<_>>()
+        });
+
+        let width = buttons.iter().next_back()
+            .map(|(x_offset, button)| button.size.width + x_offset)
+            .unwrap_or(0.0);
+
+        let height = find_max_double(
+            buttons.iter(),
             |(_offset, button)| button.size.height,
-        )
+        );
+
+        Row { buttons, size: Size { width, height } }
     }
 
-    fn get_width(&self) -> f64 {
+    pub fn get_size(&self) -> Size {
-        self.buttons.iter().next_back()
+        self.size.clone()
-            .map(|(x_offset, button)| button.size.width + x_offset)
+    }
-            .unwrap_or(0.0)
+
+    pub fn get_buttons(&self) -> &Vec<(f64, Box<Button>)> {
+        &self.buttons
     }
 
     /// Finds the first button that covers the specified point
     /// relative to row's position's origin
-    fn find_button_by_position(&self, point: c::Point)
+    fn find_button_by_position(&self, x: f64) -> &(f64, Box<Button>)
-        -> Option<(f64, &Box<Button>)>
     {
-        self.buttons.iter().find(|(x_offset, button)| {
+        // Buttons are sorted so we can use a binary search to find the clicked
-            c::Bounds {
+        // button. Note this doesn't check whether the point is actually within
-                x: *x_offset, y: 0.0,
+        // a button. This is on purpose as we want a click past the left edge of
-                width: button.size.width,
+        // the left-most button to register as a click.
-                height: button.size.height,
+        let result = self.buttons.binary_search_by(
-            }.contains(&point)
+            |&(f, _)| f.partial_cmp(&x).unwrap()
-        })
+        );
-            .map(|(x_offset, button)| (*x_offset, button))
+
+        let index = result.unwrap_or_else(|r| r);
+        let index = if index > 0 { index - 1 } else { 0 };
+
+        &self.buttons[index]
     }
 }
 
@@ -535,56 +560,85 @@ pub struct Spacing {
 }
 
 pub struct View {
-    /// Rows together with their offsets from the top
+    /// Rows together with their offsets from the top left
-    rows: Vec<(f64, Row)>,
+    rows: Vec<(c::Point, Row)>,
+
+    /// Total size of the view
+    size: Size,
 }
 
 impl View {
     pub fn new(rows: Vec<(f64, Row)>) -> View {
-        View { rows }
+        // Make sure rows are sorted by offset.
+        debug_assert!({
+            let mut sorted = rows.clone();
+            sorted.sort_by(|(f1, _), (f2, _)| f1.partial_cmp(f2).unwrap());
+
+            sorted.iter().map(|(f, _)| *f).collect::<Vec<_>>()
+                == rows.iter().map(|(f, _)| *f).collect::<Vec<_>>()
+        });
+
+        // No need to call `get_rows()`,
+        // as the biggest row is the most far reaching in both directions
+        // because they are all centered.
+        let width = find_max_double(rows.iter(), |(_offset, row)| row.size.width);
+
+        let height = rows.iter().next_back()
+            .map(|(y_offset, row)| row.size.height + y_offset)
+            .unwrap_or(0.0);
+
+        // Center the rows
+        let rows = rows.into_iter().map(|(y_offset, row)| {(
+                c::Point {
+                    x: (width - row.size.width) / 2.0,
+                    y: y_offset,
+                },
+                row,
+            )}).collect::<Vec<_>>();
+
+        View { rows, size: Size { width, height } }
     }
     /// Finds the first button that covers the specified point
     /// relative to view's position's origin
     fn find_button_by_position(&self, point: c::Point)
         -> Option<ButtonPlace>
     {
-        self.get_rows().iter().find_map(|(row_offset, row)| {
+        // Only test bounds of the view here, letting rows/column search extend
-            // make point relative to the inside of the row
+        // to the edges of these bounds.
-            row.find_button_by_position({
+        let bounds = c::Bounds {
-                c::Point { x: point.x, y: point.y } - row_offset
+            x: 0.0,
-            }).map(|(button_x_offset, button)| ButtonPlace {
+            y: 0.0,
-                button,
+            width: self.size.width,
-                offset: row_offset + c::Point {
+            height: self.size.height,
-                    x: button_x_offset,
+        };
-                    y: 0.0,
+        if !bounds.contains(&point) {
-                },
+            return None;
-            })
+        }
+
+        // Rows are sorted so we can use a binary search to find the row.
+        let result = self.rows.binary_search_by(
+            |(f, _)| f.y.partial_cmp(&point.y).unwrap()
+        );
+
+        let index = result.unwrap_or_else(|r| r);
+        let index = if index > 0 { index - 1 } else { 0 };
+
+        let row = &self.rows[index];
+        let button = row.1.find_button_by_position(point.x - row.0.x);
+
+        Some(ButtonPlace {
+            button: &button.1,
+            offset: &row.0 + c::Point { x: button.0, y: 0.0 },
         })
     }
     
-    pub fn get_width(&self) -> f64 {
+    pub fn get_size(&self) -> Size {
-        // No need to call `get_rows()`,
+        self.size.clone()
-        // as the biggest row is the most far reaching in both directions
-        // because they are all centered.
-        find_max_double(self.rows.iter(), |(_offset, row)| row.get_width())
-    }
-    
-    pub fn get_height(&self) -> f64 {
-        self.rows.iter().next_back()
-            .map(|(y_offset, row)| row.get_height() + y_offset)
-            .unwrap_or(0.0)
     }
     
     /// Returns positioned rows, with appropriate x offsets (centered)
-    pub fn get_rows(&self) -> Vec<(c::Point, &Row)> {
+    pub fn get_rows(&self) -> &Vec<(c::Point, Row)> {
-        let available_width = self.get_width();
+        &self.rows
-        self.rows.iter().map(|(y_offset, row)| {(
-            c::Point {
-                x: (available_width - row.get_width()) / 2.0,
-                y: *y_offset,
-            },
-            row,
-        )}).collect()
     }
 
     /// Returns a size which contains all the views
@@ -593,11 +647,11 @@ impl View {
         Size {
             height: find_max_double(
                 views.iter(),
-                |view| view.get_height(),
+                |view| view.size.height,
             ),
             width: find_max_double(
                 views.iter(),
-                |view| view.get_width(),
+                |view| view.size.width,
             ),
         }
     }
@@ -745,7 +799,7 @@ impl Layout {
         where F: FnMut(c::Point, &Box<Button>)
     {
         let (view_offset, view) = self.get_current_view_position();
-        for (row_offset, row) in &view.get_rows() {
+        for (row_offset, row) in view.get_rows() {
             for (x_offset, button) in &row.buttons {
                 let offset = view_offset
                     + row_offset.clone()
@@ -758,7 +812,7 @@ impl Layout {
     pub fn get_locked_keys(&self) -> Vec<Rc<RefCell<KeyState>>> {
         let mut out = Vec::new();
         let view = self.get_current_view();
-        for (_, row) in &view.get_rows() {
+        for (_, row) in view.get_rows() {
             for (_, button) in &row.buttons {
                 let locked = {
                     let state = RefCell::borrow(&button.state).clone();
@@ -820,13 +874,9 @@ mod procedures {
             let button = make_button_with_state("1".into(), state);
             let button_ptr = as_ptr(&button);
 
-            let row = Row {
+            let row = Row::new(vec!((0.1, button)));
-                buttons: vec!((0.1, button)),
-            };
 
-            let view = View {
+            let view = View::new(vec!((1.2, row)));
-                rows: vec!((1.2, row)),
-            };
 
             assert_eq!(
                 find_key_places(&view, &state_clone.clone()).into_iter()
@@ -837,9 +887,7 @@ mod procedures {
                 )
             );
 
-            let view = View {
+            let view = View::new(vec![]);
-                rows: Vec::new(),
-            };
             assert_eq!(
                 find_key_places(&view, &state_clone.clone()).is_empty(),
                 true
@@ -1082,37 +1130,56 @@ mod test {
     
     #[test]
     fn check_centering() {
-        //    foo
+        //    A B
         // ---bar---
         let view = View::new(vec![
             (
                 0.0,
-                Row {
+                Row::new(vec![
-                    buttons: vec![(
+                    (
                         0.0,
                         Box::new(Button {
-                            size: Size { width: 10.0, height: 10.0 },
+                            size: Size { width: 5.0, height: 10.0 },
-                            ..*make_button_with_state("foo".into(), make_state())
+                            ..*make_button_with_state("A".into(), make_state())
                         }),
-                    )]
+                    ),
-                },
+                    (
+                        5.0,
+                        Box::new(Button {
+                            size: Size { width: 5.0, height: 10.0 },
+                            ..*make_button_with_state("B".into(), make_state())
+                        }),
+                    ),
+                ]),
             ),
             (
                 10.0,
-                Row {
+                Row::new(vec![
-                    buttons: vec![(
+                    (
                         0.0,
                         Box::new(Button {
                             size: Size { width: 30.0, height: 10.0 },
                             ..*make_button_with_state("bar".into(), make_state())
                         }),
-                    )]
+                    ),
-                },
+                ]),
             )
         ]);
         assert!(
             view.find_button_by_position(c::Point { x: 5.0, y: 5.0 })
-                .is_none()
+                .unwrap().button.name.to_str().unwrap() == "A"
+        );
+        assert!(
+            view.find_button_by_position(c::Point { x: 14.99, y: 5.0 })
+                .unwrap().button.name.to_str().unwrap() == "A"
+        );
+        assert!(
+            view.find_button_by_position(c::Point { x: 15.01, y: 5.0 })
+                .unwrap().button.name.to_str().unwrap() == "B"
+        );
+        assert!(
+            view.find_button_by_position(c::Point { x: 25.0, y: 5.0 })
+                .unwrap().button.name.to_str().unwrap() == "B"
         );
     }
 
@@ -1122,15 +1189,13 @@ mod test {
         let view = View::new(vec![
             (
                 0.0,
-                Row {
+                Row::new(vec![(
-                    buttons: vec![(
+                    0.0,
-                        0.0,
+                    Box::new(Button {
-                        Box::new(Button {
+                        size: Size { width: 1.0, height: 1.0 },
-                            size: Size { width: 1.0, height: 1.0 },
+                        ..*make_button_with_state("foo".into(), make_state())
-                            ..*make_button_with_state("foo".into(), make_state())
+                    }),
-                        }),
+                )]),
-                    )]
-                },
             ),
         ]);
         let layout = Layout {

src/tests.rs

@@ -61,8 +61,8 @@ fn check_layout(layout: Layout) {
     
     // "Press" each button with keysyms
     for (_pos, view) in layout.views.values() {
-        for (_y, row) in &view.get_rows() {
+        for (_y, row) in view.get_rows() {
-            for (_x, button) in &row.buttons {
+            for (_x, button) in row.get_buttons() {
                 let keystate = button.state.borrow();
                 for keycode in &keystate.keycodes {
                     match state.key_get_one_sym(*keycode) {