Fix list demo and make text input less crash-happy

If a user allocated a TextInputState on the heap, previously the text input would simply crash. Now it just emits a "On::KeyDown" signal to signify that it can receive KeyDown events, so that a user can call TextInput::on_key_down() from outside the component. The fix was to always submit a rectangle to the display list, independent of the background color. Otherwise, the hit-testing function couldn't find the rectangle to test against.
fschutt · Oct 17, 2018 · 25dcc41 · 25dcc41
1 parent 0a2ca5c
commit 25dcc41
Show file tree

Hide file tree

Showing 8 changed files with 144 additions and 114 deletions.
diff --git a/examples/list.rs b/examples/list.rs
@@ -8,6 +8,8 @@ struct List {
     selected: Option<usize>,
 }
 
+const CUSTOM_CSS: &str = ".selected { background-color: black; color: white; }";
+
 impl Layout for List {
     fn layout(&self, _: WindowInfo<Self>) -> Dom<Self> {
         let mut set = BTreeSet::new();
@@ -25,7 +27,6 @@ impl Layout for List {
 }
 
 fn print_which_item_was_selected(app_state: &mut AppState<List>, event: WindowEvent<List>) -> UpdateScreen {
-    println!("mouse click!");
 
     let selected = event.get_first_hit_child(event.hit_dom_node, On::MouseDown).and_then(|x| Some(x.0));
     let mut should_redraw = UpdateScreen::DontRedraw;
@@ -42,7 +43,7 @@ fn print_which_item_was_selected(app_state: &mut AppState<List>, event: WindowEv
 }
 
 fn main() {
-    let app = App::new(List {
+    let data = List {
         items: vec![
             "Hello",
             "World",
@@ -53,6 +54,10 @@ fn main() {
             "Ipsum",
         ],
         selected: None,
-    }, AppConfig::default());
-    app.run(Window::new(WindowCreateOptions::default(), Css::native()).unwrap()).unwrap();
+    };
+
+    let app = App::new(data, AppConfig::default());
+    let css = Css::override_native(CUSTOM_CSS).unwrap();
+    let window = Window::new(WindowCreateOptions::default(), css).unwrap();
+    app.run(window).unwrap();
 }
diff --git a/src/app.rs b/src/app.rs
@@ -690,12 +690,6 @@ fn do_hit_test_and_call_callbacks<T: Layout>(
     app_state.windows[window_id.id].set_keyboard_state(&window.state.keyboard_state);
     app_state.windows[window_id.id].set_mouse_state(&window.state.mouse_state);
 
-    if !hit_test_results.items.is_empty() {
-        println!("hit test result: {:?}", hit_test_results);
-        println!("tags to callbacks:{:?}", ui_state_cache[window_id.id].tag_ids_to_callbacks);
-        println!("tags to noop callbacks:\n{:?}", ui_state_cache[window_id.id].tag_ids_to_noop_callbacks);
-    }
-
     // Run all default callbacks - **before** the user-defined callbacks are run!
     // TODO: duplicated code!
     {

diff --git a/src/css_parser.rs b/src/css_parser.rs
@@ -613,6 +613,13 @@ fn parse_float_value(input: &str)
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]
 pub struct BackgroundColor(pub ColorU);
 
+impl Default for BackgroundColor {
+    fn default() -> Self {
+        // Transparent color
+        BackgroundColor(ColorU::new(255, 255, 255, 255))
+    }
+}
+
 fn parse_css_background_color<'a>(input: &'a str)
 -> Result<BackgroundColor, CssColorParseError<'a>>
 {

diff --git a/src/display_list.rs b/src/display_list.rs
@@ -468,9 +468,10 @@ fn displaylist_handle_rect<'a,'b,'c,'d,'e,'f, T: Layout>(
         referenced_mutable_content.builder.push_clip_id(id);
     }
 
-    if let Some(ref bg_col) = rect.style.background_color {
-        push_rect(&info, referenced_mutable_content.builder, bg_col);
-    }
+    // We always have to push the rect, otherwise the hit-testing gets confused
+    push_rect(&info,
+              referenced_mutable_content.builder,
+              &rect.style.background_color.unwrap_or_default());
 
     if let Some(ref bg) = rect.style.background {
         push_background(

diff --git a/src/dom.rs b/src/dom.rs
@@ -22,7 +22,7 @@ use {
     text_layout::{Words, FontMetrics},
 };
 
-static TAG_ID: AtomicUsize = AtomicUsize::new(0);
+static TAG_ID: AtomicUsize = AtomicUsize::new(1);
 
 pub(crate) type TagId = u64;
 
@@ -804,8 +804,10 @@ impl<T: Layout> Dom<T> {
         node_ids_to_tag_ids: &mut BTreeMap<NodeId, TagId>,
         tag_ids_to_node_ids: &mut BTreeMap<TagId, NodeId>)
     {
-        for item in self.root.traverse(&*self.arena.borrow()) {
-            let node_id = item.inner_value();
+        let arena = self.arena.borrow();
+
+        for node_id in arena.linear_iter() {
+
             let item = &self.arena.borrow()[node_id];
 
             let mut node_tag_id = None;
@@ -822,23 +824,21 @@ impl<T: Layout> Dom<T> {
                 node_tag_id = Some(tag_id);
             }
 
-            if let Some(tag_id) = node_tag_id {
-                tag_ids_to_node_ids.insert(tag_id, node_id);
-                node_ids_to_tag_ids.insert(node_id, tag_id);
-            }
-/*
             // Force-enabling hit-testing is important for child nodes that don't have any
             // callbacks attached themselves, but their parents need them to be hit-tested
             if !item.data.force_enable_hit_test.is_empty() {
-                let new_tag_id = new_tag_id();
-                tag_ids_to_noop_callbacks.insert(new_tag_id, item.data.force_enable_hit_test.clone());
-                tag_ids_to_node_ids.insert(new_tag_id, node_id);
+                let tag_id = node_tag_id.unwrap_or(new_tag_id());
+                tag_ids_to_noop_callbacks.insert(tag_id, item.data.force_enable_hit_test.clone());
+                node_tag_id = Some(tag_id);
             }
-*/
 
+            if let Some(tag_id) = node_tag_id {
+                tag_ids_to_node_ids.insert(tag_id, node_id);
+                node_ids_to_tag_ids.insert(node_id, tag_id);
+            }
         }
 
-        TAG_ID.swap(0, Ordering::SeqCst);
+        TAG_ID.swap(1, Ordering::SeqCst);
     }
 }
 

diff --git a/src/id_tree.rs b/src/id_tree.rs
@@ -22,7 +22,7 @@ mod node_id {
     };
 
     /// A node identifier within a particular `Arena`.
-    #[derive(Debug, Copy, Clone, PartialOrd, Ord, PartialEq, Eq, Hash)]
+    #[derive(Copy, Clone, PartialOrd, Ord, PartialEq, Eq, Hash)]
     pub struct NodeId {
         index: NonZeroUsize,
     }
@@ -80,6 +80,12 @@ mod node_id {
     }
 }
 
+impl fmt::Debug for NodeId {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "NodeId({})", self.index())
+    }
+}
+
 #[derive(Clone, PartialEq)]
 pub struct Node<T> {
     pub(crate) parent: Option<NodeId>,

diff --git a/src/widgets/table_view.rs b/src/widgets/table_view.rs
@@ -60,85 +60,94 @@ impl TableView {
     }
 
     pub fn dom<T: Layout>(&self, data: &TableViewState, t: &T) -> Dom<T> {
-        Dom::new(NodeType::IFrame((IFrameCallback(render_table_callback), StackCheckedPointer::new(t, data).unwrap())))
+        if let Some(ptr) =  StackCheckedPointer::new(t, data) {
+            Dom::new(NodeType::IFrame((IFrameCallback(render_table_callback), ptr)))
+        } else {
+            Dom::new(NodeType::Label(
+                "Cannot create table from heap-allocated TableViewState, \
+                 please call TableViewState::render_dom manually".into())
+            )
+        }
     }
 }
 
 fn render_table_callback<T: Layout>(ptr: &StackCheckedPointer<T>, info: WindowInfo<T>, dimensions: HidpiAdjustedBounds)
 -> Dom<T>
 {
-    unsafe { ptr.invoke_mut_iframe(render_table, info, dimensions) }
+    unsafe { ptr.invoke_mut_iframe(TableViewState::render, info, dimensions) }
 }
 
-fn render_table<T: Layout>(state: &mut TableViewState, _info: WindowInfo<T>, dimensions: HidpiAdjustedBounds)
--> Dom<T>
-{
-    let necessary_columns = (dimensions.logical_size.width as f32 / state.column_width).ceil() as usize;
-    let necessary_rows = (dimensions.logical_size.height as f32 / state.row_height).ceil() as usize;
-
-    // div.__azul-native-table-container
-    //     |-> div.__azul-native-table-column (Column 0)
-    //         |-> div.__azul-native-table-top-left-rect .__azul-native-table-column-name
-    //         '-> div.__azul-native-table-row-numbers .__azul-native-table-row
-    //
-    //     |-> div.__azul-native-table-column-container
-    //         |-> div.__azul-native-table-column (Column 1 ...)
-    //             |-> div.__azul-native-table-column-name
-    //             '-> div.__azul-native-table-row
-    //                 '-> div.__azul-native-table-cell
-
-    Dom::new(NodeType::Div)
-    .with_class("__azul-native-table-container")
-    .with_child(
+impl TableViewState {
+    pub fn render<T: Layout>(state: &mut TableViewState, _info: WindowInfo<T>, dimensions: HidpiAdjustedBounds)
+    -> Dom<T>
+    {
+        let necessary_columns = (dimensions.logical_size.width as f32 / state.column_width).ceil() as usize;
+        let necessary_rows = (dimensions.logical_size.height as f32 / state.row_height).ceil() as usize;
+
+        // div.__azul-native-table-container
+        //     |-> div.__azul-native-table-column (Column 0)
+        //         |-> div.__azul-native-table-top-left-rect .__azul-native-table-column-name
+        //         '-> div.__azul-native-table-row-numbers .__azul-native-table-row
+        //
+        //     |-> div.__azul-native-table-column-container
+        //         |-> div.__azul-native-table-column (Column 1 ...)
+        //             |-> div.__azul-native-table-column-name
+        //             '-> div.__azul-native-table-row
+        //                 '-> div.__azul-native-table-cell
+
         Dom::new(NodeType::Div)
-        .with_class("__azul-native-table-row-number-wrapper")
+        .with_class("__azul-native-table-container")
         .with_child(
-            // Empty rectangle at the top left of the table
             Dom::new(NodeType::Div)
-            .with_class("__azul-native-table-top-left-rect")
-        )
-        .with_child(
-            // Rows - "1", "2", "3"
-            (0..necessary_rows.saturating_sub(1))
-            .map(|row_idx|
-                NodeData {
-                    node_type: NodeType::Label(format!("{}", row_idx + 1)),
-                    classes: vec![String::from("__azul-native-table-row")],
-                    .. Default::default()
-                }
+            .with_class("__azul-native-table-row-number-wrapper")
+            .with_child(
+                // Empty rectangle at the top left of the table
+                Dom::new(NodeType::Div)
+                .with_class("__azul-native-table-top-left-rect")
             )
-            .collect::<Dom<T>>()
-            .with_class("__azul-native-table-row-numbers")
-        )
-    )
-    .with_child(
-        (0..necessary_columns)
-        .map(|col_idx|
-            // Column name
-            Dom::new(NodeType::Div)
-            .with_class("__azul-native-table-column")
-            .with_child(Dom::new(NodeType::Label(column_name_from_number(col_idx))).with_class("__azul-native-table-column-name"))
             .with_child(
-                // Actual rows - if no content is given, they are simply empty
-                (0..necessary_rows)
+                // Rows - "1", "2", "3"
+                (0..necessary_rows.saturating_sub(1))
                 .map(|row_idx|
                     NodeData {
-                        node_type: if let Some(data) = state.work_sheet.data.get(&col_idx).and_then(|col| col.get(&row_idx)) {
-                            NodeType::Label(data.clone())
-                        } else {
-                            NodeType::Div
-                        },
-                        classes: vec![String::from("__azul-native-table-cell")],
+                        node_type: NodeType::Label(format!("{}", row_idx + 1)),
+                        classes: vec![String::from("__azul-native-table-row")],
                         .. Default::default()
                     }
                 )
                 .collect::<Dom<T>>()
-                .with_class("__azul-native-table-rows")
+                .with_class("__azul-native-table-row-numbers")
+            )
+        )
+        .with_child(
+            (0..necessary_columns)
+            .map(|col_idx|
+                // Column name
+                Dom::new(NodeType::Div)
+                .with_class("__azul-native-table-column")
+                .with_child(Dom::new(NodeType::Label(column_name_from_number(col_idx))).with_class("__azul-native-table-column-name"))
+                .with_child(
+                    // Actual rows - if no content is given, they are simply empty
+                    (0..necessary_rows)
+                    .map(|row_idx|
+                        NodeData {
+                            node_type: if let Some(data) = state.work_sheet.data.get(&col_idx).and_then(|col| col.get(&row_idx)) {
+                                NodeType::Label(data.clone())
+                            } else {
+                                NodeType::Div
+                            },
+                            classes: vec![String::from("__azul-native-table-cell")],
+                            .. Default::default()
+                        }
+                    )
+                    .collect::<Dom<T>>()
+                    .with_class("__azul-native-table-rows")
+                )
             )
+            .collect::<Dom<T>>()
+            .with_class("__azul-native-table-column-container")
         )
-        .collect::<Dom<T>>()
-        .with_class("__azul-native-table-column-container")
-    )
+    }
 }
 
 /// Maps an index number to a value, necessary for creating the column name: