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

If you have a keyboard layout like the following:

A B C D
 E F G
H I J K

The E and G keys here should be pressed when clicking in the empty space
next to them. This is achieved by not checking the bounding boxes of
each key and instead just using the button and row offset to extend
buttons/rows to the edges of the view. Caching for the size and
position of rows is introduced to simplify implementation and possibly
improve performance.

Fixes #191

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 {
-                button,
-                offset: row_offset + c::Point {
-                    x: button_x_offset,
             y: 0.0,
-                },
+            width: self.size.width,
-            })
+            height: self.size.height,
+        };
+        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,
                     Box::new(Button {
                         size: Size { width: 1.0, height: 1.0 },
                         ..*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) {