From dd21bfed8da8ba58683416df0407f1fc0bc2dd86 Mon Sep 17 00:00:00 2001
From: Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm>
Date: Sat, 7 Dec 2019 17:10:47 +0000
Subject: [PATCH 1/3] positioning: Calculate sizes instead of storing, move
 position out of widgets

Sizes of widgets can be derived, so storing them was only for C compatibility. Similar with storing position inside of widgets.

Some layout margin and scaling changes could be introduced, meaning a possibility of visual differences.
---
 eek/eek-renderer.c   |  47 -----
 eek/eek-xml-layout.c |   1 -
 src/data.rs          |  99 +++++++----
 src/drawing.rs       |  24 +--
 src/layout.h         |   3 -
 src/layout.rs        | 402 ++++++++++++++++++++-----------------------
 src/tests.rs         |   4 +-
 src/util.rs          |  12 ++
 8 files changed, 273 insertions(+), 319 deletions(-)

diff --git a/eek/eek-renderer.c b/eek/eek-renderer.c
index 2c30a062..c1301c99 100644
--- a/eek/eek-renderer.c
+++ b/eek/eek-renderer.c
@@ -498,53 +498,6 @@ eek_renderer_get_icon_surface (const gchar *icon_name,
     return surface;
 }
 
-static gboolean
-sign (EekPoint *p1, EekPoint *p2, EekPoint *p3)
-{
-    // FIXME: what is this actually checking?
-    return (p1->x - p3->x) * (p2->y - p3->y) -
-        (p2->x - p3->x) * (p1->y - p3->y);
-}
-
-uint32_t
-eek_are_bounds_inside (EekBounds bounds, EekPoint point, EekPoint origin, int32_t angle)
-{
-    EekPoint points[4];
-    gboolean b1, b2, b3;
-
-    points[0].x = bounds.x;
-    points[0].y = bounds.y;
-    points[1].x = points[0].x + bounds.width;
-    points[1].y = points[0].y;
-    points[2].x = points[1].x;
-    points[2].y = points[1].y + bounds.height;
-    points[3].x = points[0].x;
-    points[3].y = points[2].y;
-
-    for (unsigned i = 0; i < G_N_ELEMENTS(points); i++) {
-        eek_point_rotate (&points[i], angle);
-        points[i].x += origin.x;
-        points[i].y += origin.y;
-    }
-
-    b1 = sign (&point, &points[0], &points[1]) < 0.0;
-    b2 = sign (&point, &points[1], &points[2]) < 0.0;
-    b3 = sign (&point, &points[2], &points[0]) < 0.0;
-
-    if (b1 == b2 && b2 == b3) {
-        return 1;
-    }
-
-    b1 = sign (&point, &points[2], &points[3]) < 0.0;
-    b2 = sign (&point, &points[3], &points[0]) < 0.0;
-    b3 = sign (&point, &points[0], &points[2]) < 0.0;
-
-    if (b1 == b2 && b2 == b3) {
-        return 1;
-    }
-    return 0;
-}
-
 struct transformation
 eek_renderer_get_transformation (EekRenderer *renderer) {
     struct transformation failed = {0};
diff --git a/eek/eek-xml-layout.c b/eek/eek-xml-layout.c
index 300d463b..81d0302a 100644
--- a/eek/eek-xml-layout.c
+++ b/eek/eek-xml-layout.c
@@ -34,6 +34,5 @@ eek_xml_layout_real_create_keyboard (const char *keyboard_type,
                                      enum squeek_arrangement_kind t)
 {
     struct squeek_layout *layout = squeek_load_layout(keyboard_type, t);
-    squeek_layout_place_contents(layout);
     return level_keyboard_new(manager, layout);
 }
diff --git a/src/data.rs b/src/data.rs
index cc02948d..54636f57 100644
--- a/src/data.rs
+++ b/src/data.rs
@@ -17,6 +17,7 @@ use ::keyboard::{
     KeyState, PressType,
     generate_keymap, generate_keycodes, FormattingError
 };
+use ::layout;
 use ::layout::ArrangementKind;
 use ::resources;
 use ::util::c::as_str;
@@ -215,6 +216,7 @@ fn load_layout_data_with_fallback(
 #[derive(Debug, Deserialize, PartialEq)]
 #[serde(deny_unknown_fields)]
 pub struct Layout {
+    /// FIXME: deprecate in favor of margins
     bounds: Bounds,
     views: HashMap<String, Vec<ButtonIds>>,
     #[serde(default)] 
@@ -269,6 +271,7 @@ enum Action {
 #[derive(Debug, Clone, Deserialize, PartialEq)]
 #[serde(deny_unknown_fields)]
 struct Outline {
+    /// FIXME: replace with Size
     bounds: Bounds,
 }
 
@@ -303,6 +306,20 @@ impl From<io::Error> for Error {
     }
 }
 
+pub fn add_offsets<'a, I: 'a, T, F: 'a>(iterator: I, get_size: F)
+    -> impl Iterator<Item=(f64, T)> + 'a
+    where I: Iterator<Item=T>,
+        F: Fn(&T) -> f64,
+{
+    let mut offset = 0.0;
+    iterator.map(move |item| {
+        let size = get_size(&item);
+        let value = (offset, item);
+        offset += size;
+        value
+    })
+}
+
 impl Layout {
     pub fn from_resource(name: &str) -> Result<Layout, LoadError> {
         let data = resources::get_keyboard(name)
@@ -403,34 +420,35 @@ impl Layout {
         );
 
         let views = HashMap::from_iter(
-            self.views.iter().map(|(name, view)| {(
-                name.clone(),
-                ::layout::View {
-                    bounds: ::layout::c::Bounds {
-                        x: self.bounds.x,
-                        y: self.bounds.y,
-                        width: self.bounds.width,
-                        height: self.bounds.height,
-                    },
-                    rows: view.iter().map(|row| {
-                        ::layout::Row {
-                            angle: 0,
-                            bounds: None,
-                            buttons: row.split_ascii_whitespace().map(|name| {
-                                Box::new(create_button(
-                                    &self.buttons,
-                                    &self.outlines,
-                                    name,
-                                    button_states_cache.get(name.into())
-                                        .expect("Button state not created")
-                                        .clone(),
-                                    &mut warning_handler,
-                                ))
-                            }).collect(),
-                        }
-                    }).collect(),
-                }
-            )})
+            self.views.iter().map(|(name, view)| {
+                let rows = view.iter().map(|row| {
+                    let buttons = row.split_ascii_whitespace()
+                        .map(|name| {
+                            Box::new(create_button(
+                                &self.buttons,
+                                &self.outlines,
+                                name,
+                                button_states_cache.get(name.into())
+                                    .expect("Button state not created")
+                                    .clone(),
+                                &mut warning_handler,
+                            ))
+                        });
+                    ::layout::Row {
+                        angle: 0,
+                        buttons: add_offsets(
+                            buttons,
+                            |button| button.size.width,
+                        ).collect()
+                    }
+                });
+                let rows = add_offsets(rows, |row| row.get_height())
+                    .collect();
+                (
+                    name.clone(),
+                    layout::View::new(rows)
+                )
+            })
         );
 
         (
@@ -440,6 +458,13 @@ impl Layout {
                     CString::new(keymap_str)
                         .expect("Invalid keymap string generated")
                 },
+                // FIXME: use a dedicated field
+                margins: layout::Margins {
+                    top: self.bounds.x,
+                    left: self.bounds.y,
+                    bottom: 0.0,
+                    right: self.bounds.y,
+                },
             }),
             warning_handler,
         )
@@ -629,13 +654,11 @@ fn create_button<H: WarningHandler>(
             }
         });
 
-    ::layout::Button {
+    layout::Button {
         name: cname,
         outline_name: CString::new(outline_name).expect("Bad outline"),
         // TODO: do layout before creating buttons
-        bounds: ::layout::c::Bounds {
-            x: outline.bounds.x,
-            y: outline.bounds.y,
+        size: layout::Size {
             width: outline.bounds.width,
             height: outline.bounds.height,
         },
@@ -734,8 +757,8 @@ mod tests {
             .unwrap();
         assert_eq!(
             out.views["base"]
-                .rows[0]
-                .buttons[0]
+                .get_rows()[0].1
+                .buttons[0].1
                 .label,
             ::layout::Label::Text(CString::new("test").unwrap())
         );
@@ -749,8 +772,8 @@ mod tests {
             .unwrap();
         assert_eq!(
             out.views["base"]
-                .rows[0]
-                .buttons[0]
+                .get_rows()[0].1
+                .buttons[0].1
                 .label,
             ::layout::Label::Text(CString::new("test").unwrap())
         );
@@ -765,8 +788,8 @@ mod tests {
             .unwrap();
         assert_eq!(
             out.views["base"]
-                .rows[0]
-                .buttons[0]
+                .get_rows()[0].1
+                .buttons[0].1
                 .state.borrow()
                 .keycodes.len(),
             2
diff --git a/src/drawing.rs b/src/drawing.rs
index dfe19702..ead516a7 100644
--- a/src/drawing.rs
+++ b/src/drawing.rs
@@ -48,17 +48,14 @@ mod c {
         let cr = unsafe { cairo::Context::from_raw_none(cr) };
 
         let view = layout.get_current_view();
-        let view_position = view.bounds.get_position();
-        for row in &view.rows {
-            for button in &row.buttons {
+        for (row_offset, row) in &view.get_rows() {
+            for (x_offset, button) in &row.buttons {
                 let state = RefCell::borrow(&button.state).clone();
                 if state.pressed == keyboard::PressType::Pressed || state.locked {
-                    let position = &view_position
-                        + row.bounds.clone().unwrap().get_position()
-                        + button.bounds.get_position();
                     render_button_at_position(
                         renderer, &cr,
-                        position, button.as_ref(),
+                        row_offset + Point { x: *x_offset, y: 0.0 },
+                        button.as_ref(),
                         state.pressed, state.locked,
                     );
                 }
@@ -76,15 +73,12 @@ mod c {
         let layout = unsafe { &mut *layout };
         let cr = unsafe { cairo::Context::from_raw_none(cr) };
         let view = layout.get_current_view();
-        let view_position = view.bounds.get_position();
-        for row in &view.rows {
-            for button in &row.buttons {
-                let position = &view_position
-                    + row.bounds.clone().unwrap().get_position()
-                    + button.bounds.get_position();
+        for (row_offset, row) in &view.get_rows() {
+            for (x_offset, button) in &row.buttons {
                 render_button_at_position(
                     renderer, &cr,
-                    position, button.as_ref(),
+                    row_offset + Point { x: *x_offset, y: 0.0 },
+                    button.as_ref(),
                     keyboard::PressType::Released, false,
                 );
             }
@@ -105,7 +99,7 @@ pub fn render_button_at_position(
     cr.translate(position.x, position.y);
     cr.rectangle(
         0.0, 0.0,
-        button.bounds.width, button.bounds.height
+        button.size.width, button.size.height
     );
     cr.clip();
     unsafe {
diff --git a/src/layout.h b/src/layout.h
index 45748ecc..6754ef1a 100644
--- a/src/layout.h
+++ b/src/layout.h
@@ -28,9 +28,6 @@ struct transformation squeek_layout_calculate_transformation(
         const struct squeek_layout *layout,
         double allocation_width, double allocation_size);
 
-void
-squeek_layout_place_contents(struct squeek_layout*);
-
 struct squeek_layout *squeek_load_layout(const char *name, uint32_t type);
 const char *squeek_layout_get_keymap(const struct squeek_layout*);
 enum squeek_arrangement_kind squeek_layout_get_kind(const struct squeek_layout *);
diff --git a/src/layout.rs b/src/layout.rs
index 80dcb52c..e1cc5570 100644
--- a/src/layout.rs
+++ b/src/layout.rs
@@ -28,6 +28,7 @@ use ::drawing;
 use ::float_ord::FloatOrd;
 use ::keyboard::{ KeyState, PressType };
 use ::submission::{ Timestamp, VirtualKeyboard };
+use ::util::find_max_double;
 
 use std::borrow::Borrow;
 
@@ -40,7 +41,7 @@ pub mod c {
     use std::os::raw::{ c_char, c_void };
     use std::ptr;
 
-    use std::ops::Add;
+    use std::ops::{ Add, Sub };
 
     // The following defined in C
     #[repr(transparent)]
@@ -49,7 +50,7 @@ pub mod c {
 
     /// Defined in eek-types.h
     #[repr(C)]
-    #[derive(Clone, Debug)]
+    #[derive(Clone, Debug, PartialEq)]
     pub struct Point {
         pub x: f64,
         pub y: f64,
@@ -71,6 +72,16 @@ pub mod c {
             }
         }
     }
+    
+    impl Sub<&Point> for Point {
+        type Output = Point;
+        fn sub(self, other: &Point) -> Point {
+            Point {
+                x: self.x - other.x,
+                y: self.y - other.y,
+            }
+        }
+    }
 
     /// Defined in eek-types.h
     #[repr(C)]
@@ -81,13 +92,11 @@ pub mod c {
         pub width: f64,
         pub height: f64
     }
-    
+
     impl Bounds {
-        pub fn get_position(&self) -> Point {
-            Point {
-                x: self.x,
-                y: self.y,
-            }
+        pub fn contains(&self, point: &Point) -> bool {
+            (point.x > self.x && point.x < self.x + self.width
+                && point.y > self.y && point.y < self.y + self.height)
         }
     }
 
@@ -133,7 +142,10 @@ pub mod c {
     pub extern "C"
     fn squeek_button_get_bounds(button: *const ::layout::Button) -> Bounds {
         let button = unsafe { &*button };
-        button.bounds.clone()
+        Bounds {
+            x: 0.0, y: 0.0,
+            width: button.size.width, height: button.size.height
+        }
     }
 
     #[no_mangle]
@@ -193,13 +205,13 @@ pub mod c {
         allocation_height: f64,
     ) -> Transformation {
         let layout = unsafe { &*layout };
-        let bounds = &layout.get_current_view().bounds;
-        let h_scale = allocation_width / bounds.width;
-        let v_scale = allocation_height / bounds.height;
-        let scale = if h_scale > v_scale { h_scale } else { v_scale };
+        let size = layout.calculate_size();
+        let h_scale = allocation_width / size.width;
+        let v_scale = allocation_height / size.height;
+        let scale = if h_scale < v_scale { h_scale } else { v_scale };
         Transformation {
-            origin_x: allocation_width - (scale * bounds.width) / 2.0,
-            origin_y: allocation_height - (scale * bounds.height) / 2.0,
+            origin_x: (allocation_width - (scale * size.width)) / 2.0,
+            origin_y: (allocation_height - (scale * size.height)) / 2.0,
             scale: scale,
         }
     }
@@ -251,37 +263,6 @@ pub mod c {
         #[repr(transparent)]
         pub struct LevelKeyboard(*const c_void);
 
-        #[no_mangle]
-        extern "C" {
-            /// Checks if point falls within bounds,
-            /// which are relative to origin and rotated by angle (I think)
-            pub fn eek_are_bounds_inside (bounds: Bounds,
-                point: Point,
-                origin: Point,
-                angle: i32
-            ) -> u32;
-        }
-
-        /// Places each button in order, starting from 0 on the left,
-        /// keeping the spacing.
-        /// Sizes each button according to outline dimensions.
-        /// Places each row in order, starting from 0 on the top,
-        /// keeping the spacing.
-        /// Sets button and row sizes according to their contents.
-        #[no_mangle]
-        pub extern "C"
-        fn squeek_layout_place_contents(layout: *mut Layout) {
-            let layout = unsafe { &mut *layout };
-            for view in layout.views.values_mut() {
-                let sizes: Vec<Vec<Bounds>> = view.rows.iter().map(|row| {
-                    row.buttons.iter()
-                        .map(|button| button.bounds.clone())
-                        .collect()
-                }).collect();
-                view.place_buttons_with_sizes(sizes);
-            }
-        }
-
         /// Release pointer in the specified position
         #[no_mangle]
         pub extern "C"
@@ -388,9 +369,7 @@ pub mod c {
                 place.map(|place| {(
                     place.button.state.clone(),
                     place.button.clone(),
-                    view.bounds.get_position()
-                        + place.row.bounds.clone().unwrap().get_position()
-                        + place.button.bounds.get_position(),
+                    place.offset,
                 )})
             };
 
@@ -464,9 +443,10 @@ struct ButtonPosition {
 pub struct ButtonPlace<'a> {
     button: &'a Button,
     row: &'a Row,
+    offset: c::Point,
 }
 
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 pub struct Size {
     pub width: f64,
     pub height: f64,
@@ -487,9 +467,7 @@ pub struct Button {
     pub name: CString,
     /// Label to display to the user
     pub label: Label,
-    /// TODO: position the buttons before they get initial bounds
-    /// Position relative to some origin (i.e. parent/row)
-    pub bounds: c::Bounds,
+    pub size: Size,
     /// The name of the visual class applied
     pub outline_name: CString,
     /// current state, shared with other buttons
@@ -498,11 +476,10 @@ pub struct Button {
 
 /// The graphical representation of a row of buttons
 pub struct Row {
-    pub buttons: Vec<Box<Button>>,
+    /// Buttons together with their offset from the left
+    pub buttons: Vec<(f64, Box<Button>)>,
     /// Angle is not really used anywhere...
     pub angle: i32,
-    /// Position relative to some origin (i.e. parent/view origin)
-    pub bounds: Option<c::Bounds>,
 }
 
 impl Row {
@@ -527,6 +504,19 @@ impl Row {
         }).collect()
     }
     
+    pub fn get_height(&self) -> f64 {
+        find_max_double(
+            self.buttons.iter(),
+            |(_offset, button)| button.size.height,
+        )
+    }
+
+    fn get_width(&self) -> f64 {
+        self.buttons.iter().next_back()
+            .map(|(x_offset, button)| button.size.width + x_offset)
+            .unwrap_or(0.0)
+    }
+    
     fn calculate_row_size(positions: &Vec<c::Bounds>) -> Size {
         let max_height = positions.iter().map(
             |bounds| FloatOrd(bounds.height)
@@ -544,20 +534,16 @@ impl Row {
     /// 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)
-        -> Option<&Box<Button>>
+        -> Option<(f64, &Box<Button>)>
     {
-        let row_bounds = self.bounds.as_ref().expect("Missing bounds on row");
-        let origin = c::Point {
-            x: row_bounds.x,
-            y: row_bounds.y,
-        };
-        let angle = self.angle;
-        self.buttons.iter().find(|button| {
-            let bounds = button.bounds.clone();
-            let point = point.clone();
-            let origin = origin.clone();
-            procedures::is_point_inside(bounds, point, origin, angle)
+        self.buttons.iter().find(|(x_offset, button)| {
+            c::Bounds {
+                x: *x_offset, y: 0.0,
+                width: button.size.width,
+                height: button.size.height,
+            }.contains(&point)
         })
+            .map(|(x_offset, button)| (*x_offset, button))
     }
 }
 
@@ -568,85 +554,58 @@ pub struct Spacing {
 }
 
 pub struct View {
-    /// Position relative to keyboard origin
-    pub bounds: c::Bounds,
-    pub rows: Vec<Row>,
+    /// Rows together with their offsets from the top
+    rows: Vec<(f64, Row)>,
 }
 
 impl View {
-    /// Determines the positions of rows based on their sizes
-    /// Each row will be centered horizontally
-    /// The collection of rows will not be altered vertically
-    /// (TODO: maybe make view bounds a constraint,
-    /// and derive a scaling factor that lets contents fit into view)
-    /// (or TODO: blow up view bounds to match contents
-    /// and then scale the entire thing)
-    fn calculate_row_positions(&self, sizes: Vec<Size>) -> Vec<c::Bounds> {
-        let mut y_offset = self.bounds.y;
-        sizes.into_iter().map(|size| {
-            let position = c::Bounds {
-                x: (self.bounds.width - size.width) / 2f64,
-                y: y_offset,
-                width: size.width,
-                height: size.height,
-            };
-            y_offset += size.height;
-            position
-        }).collect()
+    pub fn new(rows: Vec<(f64, Row)>) -> View {
+        View { rows }
     }
-
-    /// Uses button outline information to place all buttons and rows inside.
-    /// The view itself will not be affected by the sizes
-    fn place_buttons_with_sizes(
-        &mut self,
-        button_outlines: Vec<Vec<c::Bounds>>,
-    ) {
-        // Determine all positions
-        let button_positions: Vec<_>
-            = button_outlines.into_iter()
-                .map(|outlines| {
-                    Row::calculate_button_positions(outlines)
-                })
-                .collect();
-        
-        let row_sizes = button_positions.iter()
-            .map(Row::calculate_row_size)
-            .collect();
-
-        let row_positions
-            = self.calculate_row_positions(row_sizes);
-
-        // Apply all positions
-        for ((mut row, row_position), button_positions)
-            in self.rows.iter_mut()
-                .zip(row_positions)
-                .zip(button_positions) {
-            row.bounds = Some(row_position);
-            for (mut button, button_position)
-                in row.buttons.iter_mut()
-                    .zip(button_positions) {
-                button.bounds = button_position;
-            }
-        }
-    }
-
     /// 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>
     {
-        // make point relative to the inside of the view,
-        // which is the origin of all rows
-        let point = c::Point {
-            x: point.x - self.bounds.x,
-            y: point.y - self.bounds.y,
-        };
-
-        self.rows.iter().find_map(|row| {
-            row.find_button_by_position(point.clone())
-                .map(|button| ButtonPlace {row, button})
+        self.get_rows().iter().find_map(|(row_offset, row)| {
+            // make point relative to the inside of the row
+            row.find_button_by_position({
+                c::Point { x: point.x, y: point.y } - row_offset
+            }).map(|(button_x_offset, button)| ButtonPlace {
+                row,
+                button,
+                offset: row_offset + c::Point {
+                    x: button_x_offset,
+                    y: 0.0,
+                },
+            })
         })
     }
+    
+    fn get_width(&self) -> f64 {
+        // No need to call `get_rows()`,
+        // 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())
+    }
+    
+    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)> {
+        let available_width = self.get_width();
+        self.rows.iter().map(|(y_offset, row)| {(
+            c::Point {
+                x: (available_width - row.get_width()) / 2.0,
+                y: *y_offset,
+            },
+            row,
+        )}).collect()
+    }
 }
 
 /// The physical characteristic of layout for the purpose of styling
@@ -656,10 +615,18 @@ pub enum ArrangementKind {
     Wide = 1,
 }
 
+pub struct Margins {
+    pub top: f64,
+    pub bottom: f64,
+    pub left: f64,
+    pub right: f64,
+}
+
 // TODO: split into sth like
 // Arrangement (views) + details (keymap) + State (keys)
 /// State of the UI, contains the backend as well
 pub struct Layout {
+    pub margins: Margins,
     pub kind: ArrangementKind,
     pub current_view: String,
     // Views own the actual buttons which have state
@@ -686,6 +653,7 @@ pub struct Layout {
 pub struct LayoutData {
     pub views: HashMap<String, View>,
     pub keymap_str: CString,
+    pub margins: Margins,
 }
 
 struct NoSuchView;
@@ -703,6 +671,7 @@ impl Layout {
             keymap_str: data.keymap_str,
             pressed_keys: HashSet::new(),
             locked_keys: HashSet::new(),
+            margins: data.margins,
         }
     }
 
@@ -804,49 +773,50 @@ impl Layout {
         let place = view.find_button_by_position(point);
         place.map(|place| ButtonPosition {
             button: place.button.clone(),
-            // Rows have no business being inside a view
-            // if they have no valid bounds.
-            view_position: place.row.bounds.clone().unwrap().get_position()
-                + place.button.bounds.get_position(),
+            view_position: place.offset,
         })
     }
+    
+    fn calculate_size(&self) -> Size {
+        Size {
+            height: find_max_double(
+                self.views.iter(),
+                |(_name, view)| view.get_height(),
+            ),
+            width: find_max_double(
+                self.views.iter(),
+                |(_name, view)| view.get_width(),
+            ),
+        }
+    }
 }
 
 mod procedures {
     use super::*;
 
-    type Path<'v> = (&'v Row, &'v Box<Button>);
+    type Place<'v> = (c::Point, &'v Box<Button>);
 
-    /// Finds all `(row, button)` paths that refer to the specified key `state`
-    pub fn find_key_paths<'v, 's>(
+    /// Finds all buttons referring to the key in `state`,
+    /// together with their offsets within the view.
+    pub fn find_key_places<'v, 's>(
         view: &'v View,
         state: &'s Rc<RefCell<KeyState>>
-    ) -> Vec<Path<'v>> {
-        view.rows.iter().flat_map(|row| {
-            let row_paths: Vec<Path> = row.buttons.iter().filter_map(|button| {
-                if Rc::ptr_eq(&button.state, state) {
-                    Some((row, button))
-                } else {
-                    None
-                }
-            }).collect(); // collecting not to let row references outlive the function
-            row_paths.into_iter()
+    ) -> Vec<Place<'v>> {
+        view.get_rows().iter().flat_map(|(row_offset, row)| {
+            row.buttons.iter()
+                .filter_map(move |(x_offset, button)| {
+                    if Rc::ptr_eq(&button.state, state) {
+                        Some((
+                            row_offset + c::Point { x: *x_offset, y: 0.0 },
+                            button,
+                        ))
+                    } else {
+                        None
+                    }
+                })
         }).collect()
     }
     
-    /// Checks if point is inside bounds which are rotated by angle.
-    /// FIXME: what's origin about?
-    pub fn is_point_inside(
-        bounds: c::Bounds,
-        point: c::Point,
-        origin: c::Point,
-        angle: i32
-    ) -> bool {
-        (unsafe {
-            c::procedures::eek_are_bounds_inside(bounds, point, origin, angle)
-        }) == 1
-    }
-    
     #[cfg(test)]
     mod test {
         use super::*;
@@ -867,63 +837,34 @@ mod procedures {
 
             let button = make_button_with_state("1".into(), state);
             let button_ptr = as_ptr(&button);
-            
-            let row_bounds = Some(c::Bounds {
-                x: 0.1, y: 2.3,
-                width: 4.5, height: 6.7,
-            });
+
             let row = Row {
-                buttons: vec!(button),
+                buttons: vec!((0.1, button)),
                 angle: 0,
-                bounds: row_bounds.clone(),
             };
 
             let view = View {
-                bounds: c::Bounds {
-                    x: 0f64, y: 0f64,
-                    width: 0f64, height: 0f64
-                },
-                rows: vec!(row),
+                rows: vec!((1.2, row)),
             };
 
             assert_eq!(
-                find_key_paths(&view, &state_clone.clone()).iter()
-                    .map(|(row, button)| { (row.bounds.clone(), as_ptr(button)) })
+                find_key_places(&view, &state_clone.clone()).into_iter()
+                    .map(|(place, button)| { (place, as_ptr(button)) })
                     .collect::<Vec<_>>(),
                 vec!(
-                    (row_bounds, button_ptr)
+                    (c::Point { x: 0.1, y: 1.2 }, button_ptr)
                 )
             );
 
             let view = View {
-                bounds: c::Bounds {
-                    x: 0f64, y: 0f64,
-                    width: 0f64, height: 0f64
-                },
                 rows: Vec::new(),
             };
             assert_eq!(
-                find_key_paths(&view, &state_clone.clone()).is_empty(),
+                find_key_places(&view, &state_clone.clone()).is_empty(),
                 true
             );
         }
     }
-    
-    pub fn get_button_bounds(
-        view: &View,
-        row: &Row,
-        button: &Button
-    ) -> Option<c::Bounds> {
-        match &row.bounds {
-            Some(row) => Some(c::Bounds {
-                x: view.bounds.x + row.x + button.bounds.x,
-                y: view.bounds.y + row.y + button.bounds.y,
-                width: button.bounds.width,
-                height: button.bounds.height,
-            }),
-            _ => None,
-        }
-    }
 }
 
 /// Top level procedures, dispatching to everything
@@ -944,19 +885,18 @@ mod seat {
         let view = layout.get_current_view();
         let action = RefCell::borrow(key).action.clone();
         if let Action::ShowPreferences = action {
-            let paths = ::layout::procedures::find_key_paths(
+            let places = ::layout::procedures::find_key_places(
                 view, key
             );
             // getting first item will cause mispositioning
             // with more than one button with the same key
             // on the keyboard
-            if let Some((row, button)) = paths.get(0) {
-                let bounds = ::layout::procedures::get_button_bounds(
-                    view, row, button
-                ).unwrap_or_else(|| {
-                    eprintln!("BUG: Clicked button has no position?");
-                    c::Bounds { x: 0f64, y: 0f64, width: 0f64, height: 0f64 }
-                });
+            if let Some((offset, button)) = places.get(0) {
+                let bounds = c::Bounds {
+                    x: offset.x, y: offset.y,
+                    width: button.size.width,
+                    height: button.size.height,
+                };
                 ::popover::show(
                     ui_keyboard,
                     widget_to_layout.reverse_bounds(bounds)
@@ -987,12 +927,48 @@ mod test {
     ) -> Box<Button> {
         Box::new(Button {
             name: CString::new(name.clone()).unwrap(),
-            bounds: c::Bounds {
-                x: 0f64, y: 0f64, width: 0f64, height: 0f64
-            },
+            size: Size { width: 0f64, height: 0f64 },
             outline_name: CString::new("test").unwrap(),
             label: Label::Text(CString::new(name).unwrap()),
             state: state,
         })
     }
+    
+    #[test]
+    fn check_centering() {
+        //    foo
+        // ---bar---
+        let view = View::new(vec![
+            (
+                0.0,
+                Row {
+                    angle: 0,
+                    buttons: vec![(
+                        0.0,
+                        Box::new(Button {
+                            size: Size { width: 10.0, height: 10.0 },
+                            ..*make_button_with_state("foo".into(), make_state())
+                        }),
+                    )]
+                },
+            ),
+            (
+                10.0,
+                Row {
+                    angle: 0,
+                    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()
+        );
+    }
 }
diff --git a/src/tests.rs b/src/tests.rs
index 19922fe7..d243eab4 100644
--- a/src/tests.rs
+++ b/src/tests.rs
@@ -56,8 +56,8 @@ fn check_layout(layout: Layout) {
     
     // "Press" each button with keysyms
     for view in layout.views.values() {
-        for row in &view.rows {
-            for button in &row.buttons {
+        for (_y, row) in &view.get_rows() {
+            for (_x, button) in &row.buttons {
                 let keystate = button.state.borrow();
                 for keycode in &keystate.keycodes {
                     match state.key_get_one_sym(*keycode) {
diff --git a/src/util.rs b/src/util.rs
index 891e9d28..8bd156cc 100644
--- a/src/util.rs
+++ b/src/util.rs
@@ -2,6 +2,8 @@
 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;
@@ -142,6 +144,16 @@ pub fn hash_map_map<K, V, F, K1, V1>(map: HashMap<K, V>, mut f: F)
     )
 }
 
+pub fn find_max_double<T, I, F>(iterator: I, get: F)
+    -> f64
+    where I: Iterator<Item=T>,
+        F: Fn(&T) -> f64
+{
+    iterator.map(|value| FloatOrd(get(&value)))
+        .max().unwrap_or(FloatOrd(0f64))
+        .0
+}
+
 /// Compares pointers but not internal values of Rc
 pub struct Pointer<T>(pub Rc<T>);
 

From d80758fcd17872e52c6bfaf9c36d1ac5c334745e Mon Sep 17 00:00:00 2001
From: Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm>
Date: Sat, 7 Dec 2019 21:33:38 +0000
Subject: [PATCH 2/3] positioning: Clean up unused code

---
 src/layout.rs | 76 +++------------------------------------------------
 1 file changed, 4 insertions(+), 72 deletions(-)

diff --git a/src/layout.rs b/src/layout.rs
index e1cc5570..4262c82c 100644
--- a/src/layout.rs
+++ b/src/layout.rs
@@ -25,7 +25,6 @@ use std::vec::Vec;
 
 use ::action::Action;
 use ::drawing;
-use ::float_ord::FloatOrd;
 use ::keyboard::{ KeyState, PressType };
 use ::submission::{ Timestamp, VirtualKeyboard };
 use ::util::find_max_double;
@@ -241,22 +240,6 @@ pub mod c {
         use super::*;
 
         use ::submission::c::ZwpVirtualKeyboardV1;
-        
-        #[repr(C)]
-        #[derive(PartialEq, Debug)]
-        pub struct CButtonPlace {
-            row: *const Row,
-            button: *const Button,
-        }
-
-        impl<'a> From<ButtonPlace<'a>> for CButtonPlace {
-            fn from(value: ButtonPlace<'a>) -> CButtonPlace {
-                CButtonPlace {
-                    row: value.row as *const Row,
-                    button: value.button as *const Button,
-                }
-            }
-        }
 
         // This is constructed only in C, no need for warnings
         #[allow(dead_code)]
@@ -329,8 +312,9 @@ pub mod c {
                 Point { x: x_widget, y: y_widget }
             );
 
-            if let Some(position) = layout.get_button_at_point(point) {
-                let mut state = position.button.state.clone();
+            let view = layout.get_current_view();
+            if let Some(place) = view.find_button_by_position(point) {
+                let mut state = place.button.state.clone();
                 layout.press_key(
                     &VirtualKeyboard(virtual_keyboard),
                     &mut state,
@@ -434,15 +418,8 @@ pub mod c {
     }
 }
 
-/// Relative to `View`
-struct ButtonPosition {
-    view_position: c::Point,
-    button: Button,
-}
-
 pub struct ButtonPlace<'a> {
     button: &'a Button,
-    row: &'a Row,
     offset: c::Point,
 }
 
@@ -482,28 +459,7 @@ pub struct Row {
     pub angle: i32,
 }
 
-impl Row {
-    fn last(positions: &Vec<c::Bounds>) -> Option<&c::Bounds> {
-        let len = positions.len();
-        match len {
-            0 => None,
-            l => Some(&positions[l - 1])
-        }
-    }
-    
-    fn calculate_button_positions(outlines: Vec<c::Bounds>) -> Vec<c::Bounds> {
-        let mut x_offset = 0f64;
-        outlines.iter().map(|outline| {
-            x_offset += outline.x; // account for offset outlines
-            let position = c::Bounds {
-                x: x_offset,
-                ..outline.clone()
-            };
-            x_offset += outline.width;
-            position
-        }).collect()
-    }
-    
+impl Row {    
     pub fn get_height(&self) -> f64 {
         find_max_double(
             self.buttons.iter(),
@@ -516,20 +472,6 @@ impl Row {
             .map(|(x_offset, button)| button.size.width + x_offset)
             .unwrap_or(0.0)
     }
-    
-    fn calculate_row_size(positions: &Vec<c::Bounds>) -> Size {
-        let max_height = positions.iter().map(
-            |bounds| FloatOrd(bounds.height)
-        ).max()
-            .unwrap_or(FloatOrd(0f64))
-            .0;
-        
-        let total_width = match Row::last(positions) {
-            Some(position) => position.x + position.width,
-            None => 0f64,
-        };
-        Size { width: total_width, height: max_height }
-    }
 
     /// Finds the first button that covers the specified point
     /// relative to row's position's origin
@@ -572,7 +514,6 @@ impl View {
             row.find_button_by_position({
                 c::Point { x: point.x, y: point.y } - row_offset
             }).map(|(button_x_offset, button)| ButtonPlace {
-                row,
                 button,
                 offset: row_offset + c::Point {
                     x: button_x_offset,
@@ -768,15 +709,6 @@ impl Layout {
         };
     }
 
-    fn get_button_at_point(&self, point: c::Point) -> Option<ButtonPosition> {
-        let view = self.get_current_view();
-        let place = view.find_button_by_position(point);
-        place.map(|place| ButtonPosition {
-            button: place.button.clone(),
-            view_position: place.offset,
-        })
-    }
-    
     fn calculate_size(&self) -> Size {
         Size {
             height: find_max_double(

From b639c7f3f0ec6c4c206926a6dd58d90fb75ff542 Mon Sep 17 00:00:00 2001
From: Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm>
Date: Sat, 7 Dec 2019 22:27:36 +0000
Subject: [PATCH 3/3] Fix old Rust woes

---
 src/layout.rs | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/layout.rs b/src/layout.rs
index 4262c82c..b1d654fd 100644
--- a/src/layout.rs
+++ b/src/layout.rs
@@ -312,9 +312,13 @@ pub mod c {
                 Point { x: x_widget, y: y_widget }
             );
 
-            let view = layout.get_current_view();
-            if let Some(place) = view.find_button_by_position(point) {
-                let mut state = place.button.state.clone();
+            let state = {
+                let view = layout.get_current_view();
+                view.find_button_by_position(point)
+                    .map(|place| place.button.state.clone())
+            };
+            
+            if let Some(mut state) = state {
                 layout.press_key(
                     &VirtualKeyboard(virtual_keyboard),
                     &mut state,
@@ -322,7 +326,7 @@ pub mod c {
                 );
                 // maybe TODO: draw on the display buffer here
                 drawing::queue_redraw(ui_keyboard);
-            }
+            };
         }
 
         // FIXME: this will work funny