Merge branch 'dorota.czaplejewicz/squeekboard-layouts' into 'layouts'

Check for null buttons

See merge request dorota.czaplejewicz/squeekboard!7

eek/eek-gtk-keyboard.c

@@ -117,7 +117,8 @@ eek_gtk_keyboard_real_draw (GtkWidget *self,
         struct button_place place = squeek_view_find_key(
             view, squeek_button_get_key(head->data)
         );
-        render_pressed_button (self, &place);
+        if (place.button)
+            render_pressed_button (self, &place);
     }
 
     /* redraw locked key */
@@ -128,7 +129,8 @@ eek_gtk_keyboard_real_draw (GtkWidget *self,
                 ((EekModifierKey *)head->data)->button
             )
         );
-        render_locked_button (self, &place);
+        if (place.button)
+            render_locked_button (self, &place);
     }
 
     return FALSE;

eek/eek-renderer.c

@@ -913,14 +913,6 @@ eek_renderer_get_foreground_color (EekRenderer *renderer,
     color->alpha = gcolor.alpha;
 }
 
-struct _FindKeyByPositionCallbackData {
-    EekPoint point;
-    EekPoint origin;
-    gint angle;
-    struct squeek_button *button;
-};
-typedef struct _FindKeyByPositionCallbackData FindKeyByPositionCallbackData;
-
 static gboolean
 sign (EekPoint *p1, EekPoint *p2, EekPoint *p3)
 {
@@ -968,17 +960,6 @@ eek_are_bounds_inside (EekBounds bounds, EekPoint point, EekPoint origin, int32_
     return 0;
 }
 
-static void
-find_button_by_position_row_callback (struct squeek_row *row,
-                                       gpointer user_data)
-{
-    FindKeyByPositionCallbackData *data = user_data;
-    if (data->button) {
-        return;
-    }
-    data->button = squeek_row_find_button_by_position(row, data->point, data->origin);
-}
-
 /**
  * eek_renderer_find_key_by_position:
  * @renderer: The renderer normally used to render the key
@@ -994,30 +975,13 @@ eek_renderer_find_button_by_position (EekRenderer *renderer,
                                    gdouble      x,
                                    gdouble      y)
 {
-    FindKeyByPositionCallbackData data;
-
     g_return_val_if_fail (EEK_IS_RENDERER(renderer), NULL);
 
     EekRendererPrivate *priv = eek_renderer_get_instance_private (renderer);
-    EekBounds bounds = squeek_view_get_bounds (view);
 
-    /* Transform from widget coordinates to keyboard coordinates */
-    x = (x - priv->origin_x)/priv->scale - bounds.x;
-    y = (y - priv->origin_y)/priv->scale - bounds.y;
-
-    if (x < 0 ||
-        y < 0 ||
-        x > bounds.width ||
-        y > bounds.height)
-        return NULL;
-
-    data.point.x = x;
-    data.point.y = y;
-    data.origin.x = 0;
-    data.origin.y = 0;
-    data.button = NULL;
-
-    squeek_view_foreach (view, find_button_by_position_row_callback,
-                        &data);
-    return data.button;
+    EekPoint point = {
+        .x = (x - priv->origin_x)/priv->scale,
+        .y = (y - priv->origin_y)/priv->scale,
+    };
+    return squeek_view_find_button_by_position(view, point);
 }

src/layout.h

@@ -25,7 +25,6 @@ uint32_t squeek_row_contains(struct squeek_row*, struct squeek_button *button);
 
 struct button_place squeek_view_find_key(struct squeek_view*, struct squeek_key *state);
 
-struct squeek_button *squeek_row_find_button_by_position(struct squeek_row *row, EekPoint point, EekPoint origin);
 
 typedef void (*ButtonCallback) (struct squeek_button *button, gpointer user_data);
 void squeek_row_foreach(struct squeek_row*,
@@ -74,4 +73,6 @@ struct squeek_row *squeek_view_get_row(struct squeek_view *view,
 
 void
 squeek_view_place_contents(struct squeek_view *view, LevelKeyboard *keyboard);
+
+struct squeek_button *squeek_view_find_button_by_position(struct squeek_view *view, EekPoint point);
 #endif

src/layout.rs

@@ -1,3 +1,22 @@
+/**
+ * Layout-related data.
+ * 
+ * The `View` contains `Row`s and each `Row` contains `Button`s.
+ * They carry data relevant to their positioning only,
+ * except the Button, which also carries some data
+ * about its appearance and function.
+ * 
+ * The layout is determined bottom-up, by measuring `Button` sizes,
+ * deriving `Row` sizes from them, and then centering them within the `View`.
+ * 
+ * That makes the `View` position immutable,
+ * and therefore different than the other positions.
+ * 
+ * Note that it might be a better idea
+ * to make `View` position depend on its contents,
+ * and let the renderer scale and center it within the widget.
+ */
+
 use std::cell::RefCell;
 use std::rc::Rc;
 use std::vec::Vec;
@@ -310,7 +329,7 @@ pub mod c {
     }
     
     /// Entry points for more complex procedures and algoithms which span multiple modules
-    mod procedures {
+    pub mod procedures {
         use super::*;
         
         #[repr(transparent)]
@@ -332,7 +351,7 @@ pub mod c {
 
             /// Checks if point falls within bounds,
             /// which are relative to origin and rotated by angle (I think)
-            fn eek_are_bounds_inside (bounds: Bounds,
+            pub fn eek_are_bounds_inside (bounds: Bounds,
                 point: Point,
                 origin: Point,
                 angle: i32
@@ -369,36 +388,6 @@ pub mod c {
             view.place_buttons_with_sizes(sizes);
         }
 
-        #[no_mangle]
-        pub extern "C"
-        fn squeek_row_find_button_by_position(
-            row: *mut Row, point: Point, origin: Point
-        ) -> *mut Button {
-            let row = unsafe { &mut *row };
-            let row_bounds = row.bounds
-                .as_ref().expect("Missing bounds on row");
-            let origin = Point {
-                x: origin.x + row_bounds.x,
-                y: origin.y + row_bounds.y,
-            };
-            let angle = row.angle;
-            let result = row.buttons.iter_mut().find(|button| {
-                let bounds = button.bounds
-                    .as_ref().expect("Missing bounds on button")
-                    .clone();
-                let point = point.clone();
-                let origin = origin.clone();
-                unsafe {
-                    eek_are_bounds_inside(bounds, point, origin, angle) == 1
-                }
-            });
-            
-            match result {
-                Some(button) => button.as_mut() as *mut Button,
-                None => ptr::null_mut(),
-            }
-        }
-        
         fn squeek_row_contains(row: &Row, needle: *const Button) -> bool {
             row.buttons.iter().position(
                 // TODO: wrap Button properly in Rc; this comparison is unreliable
@@ -447,6 +436,20 @@ pub mod c {
             ButtonPlace { row, button }
         }
 
+
+        #[no_mangle]
+        pub extern "C"
+        fn squeek_view_find_button_by_position(
+            view: *mut View, point: Point
+        ) -> *mut Button {
+            let view = unsafe { &mut *view };
+            let result = view.find_button_by_position(point);
+            match result {
+                Some(button) => button.as_mut() as *mut Button,
+                None => ptr::null_mut(),
+            }
+        }
+        
         #[cfg(test)]
         mod test {
             use super::*;
@@ -559,12 +562,13 @@ pub struct Size {
 pub struct Button {
     oref: c::OutlineRef,
     /// TODO: abolish Option, buttons should be created with bounds fully formed
+    /// Position relative to some origin (i.e. parent/row)
     bounds: Option<c::Bounds>,
     /// current state, shared with other buttons
     pub state: Rc<RefCell<KeyState>>,
 }
 
-
+// FIXME: derive from the style/margin/padding
 const BUTTON_SPACING: f64 = 4.0;
 const ROW_SPACING: f64 = 7.0;
 
@@ -573,6 +577,7 @@ pub struct Row {
     buttons: Vec<Box<Button>>,
     /// Angle is not really used anywhere...
     angle: i32,
+    /// Position relative to some origin (i.e. parent/view origin)
     bounds: Option<c::Bounds>,
 }
 
@@ -621,6 +626,27 @@ impl Row {
         };
         Size { width: total_width, height: max_height }
     }
+
+    /// Finds the first button that covers the specified point
+    /// relative to row's position's origin
+    fn find_button_by_position(&mut self, point: c::Point)
+        -> Option<&mut 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_mut().find(|button| {
+            let bounds = button.bounds
+                .as_ref().expect("Missing bounds on button")
+                .clone();
+            let point = point.clone();
+            let origin = origin.clone();
+            procedures::is_point_inside(bounds, point, origin, angle)
+        })
+    }
 }
 
 pub struct View {
@@ -633,8 +659,10 @@ 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: make view bounds a constraint,
+    /// (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| {
@@ -680,6 +708,23 @@ impl View {
             }
         }
     }
+
+    /// Finds the first button that covers the specified point
+    /// relative to view's position's origin
+    fn find_button_by_position(&mut self, point: c::Point)
+        -> Option<&mut Box<Button>>
+    {
+        // 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_mut().find_map(
+            |row| row.find_button_by_position(point.clone())
+        )
+    }
 }
 
 mod procedures {
@@ -703,4 +748,17 @@ mod procedures {
             row_paths.into_iter()
         }).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
+    }
 }