From e5a7420b5a995c70a174bcaa0fae4a5c22284a28 Mon Sep 17 00:00:00 2001 From: Adam Obuchowicz Date: Mon, 4 Apr 2022 17:55:55 +0200 Subject: [PATCH] The EnsoGL Component abstraction with special dropping behavior (#3322) In this branch: * The workaround for cursor-not-being-updated-after-closing-searcher bug (discovered while testing #3278) is reverted. * The proper fix was introduced: created an abstraction for EnsoGL component, which, when dropping, will not immediately drop the FRP network and model, but instead put it into the Garbage Collector. The Collector ensures, that all "component hiding" effects and events will be handled, and drops FRP network and model only after that. * I run clippy for wasm32 target out of curiosity. There was one warning, and I fixed it on this branch. --- Cargo.lock | 1 + Cargo.toml | 5 +- app/gui/language/parser/src/jsclient.rs | 2 +- .../view/graph-editor/src/component/node.rs | 23 ++- .../src/component/node/growth_animation.rs | 4 +- .../src/component/node/input/area.rs | 1 - app/gui/view/graph-editor/src/lib.rs | 132 ++++++++---------- .../graph-editor/src/new_node_position.rs | 6 +- app/gui/view/graph-editor/src/selection.rs | 2 +- app/gui/view/src/project.rs | 2 +- integration-test/Cargo.toml | 1 + integration-test/tests/graph_editor.rs | 9 +- .../ensogl/component/flame-graph/src/block.rs | 4 +- .../ensogl/component/gui/src/component.rs | 75 +++++----- lib/rust/ensogl/core/src/display.rs | 1 + lib/rust/ensogl/core/src/display/garbage.rs | 112 +++++++++++++++ .../src/display/render/passes/pixel_read.rs | 35 +++-- lib/rust/ensogl/core/src/display/world.rs | 22 ++- lib/rust/ensogl/core/src/gui.rs | 2 + lib/rust/ensogl/core/src/gui/component.rs | 120 ++++++++++++++++ lib/rust/shortcuts/src/lib.rs | 4 +- 21 files changed, 416 insertions(+), 147 deletions(-) create mode 100644 lib/rust/ensogl/core/src/display/garbage.rs diff --git a/Cargo.lock b/Cargo.lock index 159e0b53123a..f2d24ea76420 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1038,6 +1038,7 @@ dependencies = [ "enso-frp", "enso-gui", "enso-prelude", + "enso-shortcuts", "enso-web", "ensogl", "ordered-float 2.10.0", diff --git a/Cargo.toml b/Cargo.toml index b9540ce89ba1..157a35f0b17d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -41,7 +41,4 @@ debug-assertions = true [profile.integration-test] inherits = "test" -# The integration-test profile was created to be able run integration tests with optimizations (as they took a lot of -# time). There is, however, an issue with running them with optimizations #181740444. -# opt-level = 2 -opt-level = 0 +opt-level = 2 diff --git a/app/gui/language/parser/src/jsclient.rs b/app/gui/language/parser/src/jsclient.rs index eafed3aad242..bacbaa332bf8 100644 --- a/app/gui/language/parser/src/jsclient.rs +++ b/app/gui/language/parser/src/jsclient.rs @@ -84,7 +84,7 @@ impl Client { ) -> api::Result> { let result = || { let json = &parse_with_metadata(program)?; - let module = from_json_str_without_recursion_limit(&json)?; + let module = from_json_str_without_recursion_limit(json)?; Result::Ok(module) }; Ok(result()?) diff --git a/app/gui/view/graph-editor/src/component/node.rs b/app/gui/view/graph-editor/src/component/node.rs index 3bf012a3170a..e7740821de42 100644 --- a/app/gui/view/graph-editor/src/component/node.rs +++ b/app/gui/view/graph-editor/src/component/node.rs @@ -21,6 +21,7 @@ use ensogl::application::Application; use ensogl::data::color; use ensogl::display; use ensogl::display::scene::Layer; +use ensogl::gui; use ensogl::Animation; use ensogl_component::shadow; use ensogl_component::text; @@ -385,12 +386,11 @@ ensogl::define_endpoints_2! { /// complex logically (the events are emitted from node to graph, then processed there and /// emitted back to the right node). /// -/// Currently, the solution "C" (most optimal) is implemented here. +/// Currently, the solution "C" (nearest to optimal) is implemented here. #[derive(Clone, CloneRef, Debug)] #[allow(missing_docs)] pub struct Node { - pub model: Rc, - pub frp: Frp, + widget: gui::Widget, } impl AsRef for Node { @@ -399,10 +399,17 @@ impl AsRef for Node { } } +impl AsRef> for Node { + fn as_ref(&self) -> &gui::Widget { + &self.widget + } +} + + impl Deref for Node { - type Target = Frp; + type Target = gui::Widget; fn deref(&self) -> &Self::Target { - &self.frp + &self.widget } } @@ -949,7 +956,9 @@ impl Node { frp.set_disabled.emit(false); frp.show_quick_action_bar_on_hover.emit(true); - Self { model, frp } + let display_object = model.display_object.clone_ref(); + let widget = gui::Widget::new(app, frp, model, display_object); + Node { widget } } fn error_color(error: &Option, style: &StyleWatch) -> color::Lcha { @@ -969,7 +978,7 @@ impl Node { impl display::Object for Node { fn display_object(&self) -> &display::object::Instance { - &self.model.display_object + self.deref().display_object() } } diff --git a/app/gui/view/graph-editor/src/component/node/growth_animation.rs b/app/gui/view/graph-editor/src/component/node/growth_animation.rs index ad849597d2da..00502a7c0664 100644 --- a/app/gui/view/graph-editor/src/component/node/growth_animation.rs +++ b/app/gui/view/graph-editor/src/component/node/growth_animation.rs @@ -115,14 +115,14 @@ impl GraphEditorModelWithNetwork { /// Move node to the `edited_node` scene layer, so that it is rendered by the separate camera. fn move_node_to_edited_node_layer(&self, node_id: NodeId) { if let Some(node) = self.nodes.get_cloned(&node_id) { - node.model.move_to_edited_node_layer(); + node.model().move_to_edited_node_layer(); } } /// Move node to the `main` scene layer, so that it is rendered by the main camera. fn move_node_to_main_layer(&self, node_id: NodeId) { if let Some(node) = self.nodes.get_cloned(&node_id) { - node.model.move_to_main_layer(); + node.model().move_to_main_layer(); } } } diff --git a/app/gui/view/graph-editor/src/component/node/input/area.rs b/app/gui/view/graph-editor/src/component/node/input/area.rs index 45cf54e08db1..278cfa364a71 100644 --- a/app/gui/view/graph-editor/src/component/node/input/area.rs +++ b/app/gui/view/graph-editor/src/component/node/input/area.rs @@ -676,7 +676,6 @@ impl Area { eval_ mouse_down ([crumbs,frp] frp.source.on_port_press.emit(&crumbs)); - // === Hover === hovered <- bool(&mouse_out,&mouse_over); diff --git a/app/gui/view/graph-editor/src/lib.rs b/app/gui/view/graph-editor/src/lib.rs index 72c7aee7df36..aee70fa89983 100644 --- a/app/gui/view/graph-editor/src/lib.rs +++ b/app/gui/view/graph-editor/src/lib.rs @@ -1098,7 +1098,7 @@ impl Nodes { #[allow(missing_docs)] // FIXME[everyone] All pub functions should have docs. pub fn set_quick_preview(&self, quick: bool) { - self.all.raw.borrow().values().for_each(|node| node.view.frp.quick_preview_vis.emit(quick)) + self.all.raw.borrow().values().for_each(|node| node.view.quick_preview_vis.emit(quick)) } #[allow(missing_docs)] // FIXME[everyone] All pub functions should have docs. @@ -1107,7 +1107,7 @@ impl Nodes { .raw .borrow() .values() - .for_each(|node| node.view.frp.show_quick_action_bar_on_hover.emit(quick)) + .for_each(|node| node.view.show_quick_action_bar_on_hover.emit(quick)) } } @@ -1125,7 +1125,7 @@ impl Nodes { self.selected.remove_item(&node_id) } self.selected.push(node_id); - node.frp.select.emit(()); + node.select.emit(()); } } @@ -1134,7 +1134,7 @@ impl Nodes { let node_id = node_id.into(); if let Some(node) = self.get_cloned_ref(&node_id) { self.selected.remove_item(&node_id); - node.frp.deselect.emit(()); + node.deselect.emit(()); } } @@ -1424,8 +1424,8 @@ impl Default for WayOfCreatingNode { /// Context data required to create a new node. #[derive(Debug)] struct NodeCreationContext<'a> { - pointer_style: &'a frp::Any<(NodeId, cursor::Style)>, - tooltip_update: &'a frp::Any<(NodeId, tooltip::Style)>, + pointer_style: &'a frp::Any, + tooltip_update: &'a frp::Any, output_press: &'a frp::Source, input_press: &'a frp::Source, output: &'a FrpEndpoints, @@ -1464,6 +1464,8 @@ impl GraphEditorModelWithNetwork { use ensogl::application::command::FrpNetworkProvider; let view = component::Node::new(&self.app, self.vis_registry.clone_ref()); let node = Node::new(view); + let node_model = node.model(); + let node_network = &node.frp().network(); let node_id = node.id(); self.add_child(&node); @@ -1477,9 +1479,8 @@ impl GraphEditorModelWithNetwork { output, } = ctx; - let node_network = node.frp.network(); frp::new_bridge_network! { [self.network, node_network] graph_node_bridge - eval_ node.frp.background_press(touch.nodes.down.emit(node_id)); + eval_ node.background_press(touch.nodes.down.emit(node_id)); hovered <- node.output.hover.map (move |t| Some(Switch::new(node_id,*t))); output.source.node_hovered <+ hovered; @@ -1490,56 +1491,57 @@ impl GraphEditorModelWithNetwork { node.set_output_expression_visibility <+ self.frp.nodes_labels_visible; - tooltip_update <+ node.frp.tooltip.map(move |tooltip| (node_id, tooltip.clone())); - pointer_style <+ node.model.input.frp.pointer_style.map(move |s| (node_id, s.clone())); - eval node.model.output.frp.on_port_press ([output_press](crumbs){ + tooltip_update <+ node.tooltip; + pointer_style <+ node_model.input.frp.pointer_style; + + eval node_model.output.frp.on_port_press ([output_press](crumbs){ let target = EdgeEndpoint::new(node_id,crumbs.clone()); output_press.emit(target); }); - eval node.model.input.frp.on_port_press ([input_press](crumbs) + eval node_model.input.frp.on_port_press ([input_press](crumbs) let target = EdgeEndpoint::new(node_id,crumbs.clone()); input_press.emit(target); ); - eval node.model.input.frp.on_port_hover ([model](t) { + eval node_model.input.frp.on_port_hover ([model](t) { let crumbs = t.on(); let target = crumbs.map(|c| EdgeEndpoint::new(node_id,c.clone())); model.frp.source.hover_node_input.emit(target); }); - eval node.model.output.frp.on_port_hover ([model](hover) { + eval node_model.output.frp.on_port_hover ([model](hover) { let output = hover.on().map(|crumbs| EdgeEndpoint::new(node_id,crumbs.clone())); model.frp.source.hover_node_output.emit(output); }); let neutral_color = model.styles_frp.get_color(theme::code::types::any::selection); - _eval <- all_with(&node.model.input.frp.on_port_type_change,&neutral_color, + _eval <- all_with(&node_model.input.frp.on_port_type_change,&neutral_color, f!(((crumbs,_),neutral_color) model.with_input_edge_id(node_id,crumbs,|id| model.refresh_edge_color(id,neutral_color.into()) ) )); - _eval <- all_with(&node.model.input.frp.on_port_type_change,&neutral_color, + _eval <- all_with(&node_model.input.frp.on_port_type_change,&neutral_color, f!(((crumbs,_),neutral_color) model.with_output_edge_id(node_id,crumbs,|id| model.refresh_edge_color(id,neutral_color.into()) ) )); - eval node.frp.expression((t) output.source.node_expression_set.emit((node_id,t.into()))); + eval node.expression((t) output.source.node_expression_set.emit((node_id,t.into()))); // === Actions === - eval node.view.frp.freeze ((is_frozen) { + eval node.view.freeze ((is_frozen) { output.source.node_action_freeze.emit((node_id,*is_frozen)); }); - let set_node_disabled = &node.frp.set_disabled; - eval node.view.frp.skip ([set_node_disabled,output](is_skipped) { + let set_node_disabled = &node.set_disabled; + eval node.view.skip ([set_node_disabled,output](is_skipped) { output.source.node_action_skip.emit((node_id,*is_skipped)); set_node_disabled.emit(is_skipped); }); @@ -1550,18 +1552,18 @@ impl GraphEditorModelWithNetwork { visualization_shown <- node.visualization_visible.gate(&node.visualization_visible); visualization_hidden <- node.visualization_visible.gate_not(&node.visualization_visible); - let vis_is_selected = node.model.visualization.frp.is_selected.clone_ref(); + let vis_is_selected = node_model.visualization.frp.is_selected.clone_ref(); selected <- vis_is_selected.on_true(); deselected <- vis_is_selected.on_false(); output.source.visualization_preprocessor_changed <+ - node.model.visualization.frp.preprocessor.map(move |preprocessor| + node_model.visualization.frp.preprocessor.map(move |preprocessor| (node_id,preprocessor.clone())); output.source.on_visualization_select <+ selected.constant(Switch::On(node_id)); output.source.on_visualization_select <+ deselected.constant(Switch::Off(node_id)); metadata <- any(...); - metadata <+ node.model.visualization.frp.preprocessor.map(visualization::Metadata::new); + metadata <+ node_model.visualization.frp.preprocessor.map(visualization::Metadata::new); // Ensure the graph editor knows about internal changes to the visualisation. If the // visualisation changes that should indicate that the old one has been disabled and a @@ -1598,7 +1600,7 @@ impl GraphEditorModelWithNetwork { node.set_view_mode(self.model.frp.view_mode.value()); let initial_metadata = visualization::Metadata { - preprocessor: node.model.visualization.frp.preprocessor.value(), + preprocessor: node_model.visualization.frp.preprocessor.value(), }; metadata.emit(initial_metadata); init.emit(&()); @@ -1795,14 +1797,14 @@ impl GraphEditorModel { fn enable_visualization_fullscreen(&self, node_id: impl Into) { let node_id = node_id.into(); if let Some(node) = self.nodes.get_cloned_ref(&node_id) { - node.model.visualization.frp.enable_fullscreen.emit(()); + node.model().visualization.frp.enable_fullscreen.emit(()); } } fn disable_visualization_fullscreen(&self, node_id: impl Into) { let node_id = node_id.into(); if let Some(node) = self.nodes.get_cloned_ref(&node_id) { - node.model.visualization.frp.disable_fullscreen.emit(()); + node.model().visualization.frp.disable_fullscreen.emit(()); } } @@ -1811,7 +1813,8 @@ impl GraphEditorModel { &self, node_id: impl Into, ) -> Option { - let frp = &self.nodes.all.get_cloned_ref(&node_id.into())?.model.visualization.frp; + let node = self.nodes.all.get_cloned_ref(&node_id.into())?; + let frp = &node.model().visualization.frp; frp.visible.value().then(|| visualization::Metadata::new(&frp.preprocessor.value())) } @@ -1846,7 +1849,7 @@ impl GraphEditorModel { let node_id = node_id.into(); let expr = expr.into(); if let Some(node) = self.nodes.get_cloned_ref(&node_id) { - node.frp.set_expression.emit(expr); + node.set_expression.emit(expr); } for edge_id in self.node_out_edges(node_id) { self.refresh_edge_source_size(edge_id); @@ -1857,7 +1860,7 @@ impl GraphEditorModel { let node_id = node_id.into(); let comment = comment.into(); if let Some(node) = self.nodes.get_cloned_ref(&node_id) { - node.frp.set_comment.emit(comment); + node.set_comment.emit(comment); } } @@ -2041,17 +2044,17 @@ impl GraphEditorModel { ) { let node_id = node_id.into(); if let Some(node) = self.nodes.get_cloned_ref(&node_id) { - if node.view.model.output.whole_expr_id().contains(&ast_id) { + if node.view.model().output.whole_expr_id().contains(&ast_id) { // TODO[ao]: we must update root output port according to the whole expression type // due to a bug in engine https://github.com/enso-org/enso/issues/1038. let crumbs = span_tree::Crumbs::default(); - node.view.model.output.set_expression_usage_type(crumbs, maybe_type.clone()); + node.view.model().output.set_expression_usage_type(crumbs, maybe_type.clone()); let enso_type = maybe_type.as_ref().map(|tp| enso::Type::new(&tp.0)); - node.view.model.visualization.frp.set_vis_input_type(enso_type); + node.view.model().visualization.frp.set_vis_input_type(enso_type); } - let crumbs = node.view.model.get_crumbs_by_id(ast_id); + let crumbs = node.view.model().get_crumbs_by_id(ast_id); if let Some(crumbs) = crumbs { - node.view.frp.set_expression_usage_type.emit((crumbs, maybe_type)); + node.view.set_expression_usage_type.emit((crumbs, maybe_type)); } } } @@ -2096,8 +2099,8 @@ impl GraphEditorModel { if let Some(edge) = self.edges.get_cloned_ref(&edge_id) { if let Some(edge_source) = edge.source() { if let Some(node) = self.nodes.get_cloned_ref(&edge_source.node_id) { - edge.view.frp.source_width.emit(node.model.width()); - edge.view.frp.source_height.emit(node.model.height()); + edge.view.frp.source_width.emit(node.model().width()); + edge.view.frp.source_height.emit(node.model().height()); edge.view.frp.redraw.emit(()); } } @@ -2124,7 +2127,7 @@ impl GraphEditorModel { if let Some(edge_source) = edge.source() { if let Some(node) = self.nodes.get_cloned_ref(&edge_source.node_id) { edge.mod_position(|p| { - p.x = node.position().x + node.model.width() / 2.0; + p.x = node.position().x + node.model().width() / 2.0; p.y = node.position().y; }); } @@ -2138,7 +2141,7 @@ impl GraphEditorModel { if let Some(edge_target) = edge.target() { if let Some(node) = self.nodes.get_cloned_ref(&edge_target.node_id) { let offset = - node.model.input.port_offset(&edge_target.port).unwrap_or_default(); + node.model().input.port_offset(&edge_target.port).unwrap_or_default(); let pos = node.position().xy() + offset; edge.view.frp.target_position.emit(pos); edge.view.frp.redraw.emit(()); @@ -2270,17 +2273,17 @@ impl GraphEditorModel { } fn edge_source_type(&self, edge_id: EdgeId) -> Option { - self.with_edge_map_source_node(edge_id, |n, c| n.model.output.port_type(&c)).flatten() + self.with_edge_map_source_node(edge_id, |n, c| n.model().output.port_type(&c)).flatten() } fn edge_target_type(&self, edge_id: EdgeId) -> Option { - self.with_edge_map_target_node(edge_id, |n, c| n.model.input.port_type(&c)).flatten() + self.with_edge_map_target_node(edge_id, |n, c| n.model().input.port_type(&c)).flatten() } fn edge_hover_type(&self) -> Option { let hover_tgt = self.frp.hover_node_input.value(); hover_tgt.and_then(|tgt| { - self.with_node(tgt.node_id, |node| node.model.input.port_type(&tgt.port)).flatten() + self.with_node(tgt.node_id, |node| node.model().input.port_type(&tgt.port)).flatten() }) } @@ -2623,8 +2626,8 @@ fn new_graph_editor(app: &Application) -> GraphEditor { frp::extend! { network - node_pointer_style <- any_mut::<(NodeId, cursor::Style)>(); - node_tooltip <- any_mut::<(NodeId, tooltip::Style)>(); + node_pointer_style <- any_mut::(); + node_tooltip <- any_mut::(); let node_input_touch = TouchNetwork::::new(network,mouse); let node_output_touch = TouchNetwork::::new(network,mouse); @@ -2864,12 +2867,12 @@ fn new_graph_editor(app: &Application) -> GraphEditor { eval out.node_editing_started ([model] (id) { if let Some(node) = model.nodes.get_cloned_ref(id) { - node.model.input.frp.set_edit_mode(true); + node.model().input.frp.set_edit_mode(true); } }); eval out.node_editing_finished ([model](id) { if let Some(node) = model.nodes.get_cloned_ref(id) { - node.model.input.set_edit_mode(false); + node.model().input.set_edit_mode(false); } }); } @@ -2886,7 +2889,7 @@ fn new_graph_editor(app: &Application) -> GraphEditor { frp::extend! { network _eval <- all_with(&out.node_hovered,&edit_mode,f!([model](tgt,e) if let Some(tgt) = tgt { - model.with_node(tgt.value,|t| t.model.input.set_edit_ready_mode(*e && tgt.is_on())); + model.with_node(tgt.value,|t| t.model().input.set_edit_ready_mode(*e && tgt.is_on())); } )); _eval <- all_with(&out.node_hovered,&out.some_edge_targets_unset,f!([model](tgt,ok) @@ -2895,7 +2898,7 @@ fn new_graph_editor(app: &Application) -> GraphEditor { let edge_tp = model.first_detached_edge_source_type(); let is_edge_source = model.has_edges_with_detached_targets(node_id); let is_active = *ok && !is_edge_source && tgt.is_on(); - model.with_node(node_id,|t| t.model.input.set_ports_active(is_active,edge_tp)); + model.with_node(node_id,|t| t.model().input.set_ports_active(is_active,edge_tp)); } )); } @@ -2932,16 +2935,6 @@ fn new_graph_editor(app: &Application) -> GraphEditor { eval nodes_to_remove ((node_id) inputs.remove_all_node_edges.emit(node_id)); out.source.node_removed <+ nodes_to_remove; - - // Removed nodes lost their right to set cursor and tooltip styles. - pointer_style_setter_removed <- out.node_removed.map2(&node_pointer_style, - |removed,(setter, _)| removed == setter - ); - tooltip_setter_removed <- out.node_removed.map2(&node_tooltip, |removed, (setter, _)| - removed == setter - ); - node_pointer_style <+ out.node_removed.gate(&pointer_style_setter_removed).constant(default()); - node_tooltip <+ out.node_removed.gate(&tooltip_setter_removed).constant(default()); } @@ -3164,8 +3157,8 @@ fn new_graph_editor(app: &Application) -> GraphEditor { edges.detached_source.for_each(|edge_id| { if let Some(node) = nodes.get_cloned_ref(&target.node_id) { if let Some(edge) = edges.get_cloned_ref(edge_id) { - let node_width = node.view.model.width(); - let node_height = node.view.model.height(); + let node_width = node.view.model().width(); + let node_height = node.view.model().height(); let node_pos = node.position(); edge.view.frp.source_width.emit(node_width); @@ -3192,9 +3185,9 @@ fn new_graph_editor(app: &Application) -> GraphEditor { match (&nodes.get_cloned_ref(node_id), vis_path) { (Some(node), Some(vis_path)) => { let vis_definition = vis_registry.definition_from_path(vis_path); - node.model.visualization.frp.set_visualization.emit(vis_definition); + node.model().visualization.frp.set_visualization.emit(vis_definition); }, - (Some(node), None) => node.model.visualization.frp.set_visualization.emit(None), + (Some(node), None) => node.model().visualization.frp.set_visualization.emit(None), _ => warning!(logger,"Failed to get node: {node_id:?}"), } @@ -3234,13 +3227,13 @@ fn new_graph_editor(app: &Application) -> GraphEditor { eval inputs.set_visualization_data ([nodes]((node_id,data)) { if let Some(node) = nodes.get_cloned(node_id) { - node.model.visualization.frp.set_data.emit(data); + node.model().visualization.frp.set_data.emit(data); } }); eval inputs.set_error_visualization_data ([nodes]((node_id,data)) { if let Some(node) = nodes.get_cloned(node_id) { - node.model.error_visualization.send_data.emit(data); + node.model().error_visualization.send_data.emit(data); } }); @@ -3248,7 +3241,7 @@ fn new_graph_editor(app: &Application) -> GraphEditor { node_to_cycle <- any(nodes_to_cycle,inputs.cycle_visualization); eval node_to_cycle ([model](node_id) { if let Some(node) = model.nodes.get_cloned_ref(node_id) { - node.view.model.visualization.frp.cycle_visualization(); + node.view.model().visualization.frp.cycle_visualization(); } }); @@ -3470,7 +3463,6 @@ fn new_graph_editor(app: &Application) -> GraphEditor { let breadcrumb_style = model.breadcrumbs.pointer_style.clone_ref(); let selection_style = selection_controller.cursor_style.clone_ref(); - node_pointer_style <- node_pointer_style._1(); pointer_style <- all [ pointer_on_drag , selection_style @@ -3511,7 +3503,7 @@ fn new_graph_editor(app: &Application) -> GraphEditor { frp::extend! { network eval cursor.frp.scene_position ((pos) model.tooltip.frp.set_location(pos.xy()) ); - eval node_tooltip (((_,tooltip_update)) model.tooltip.frp.set_style(tooltip_update) ); + eval node_tooltip ((tooltip_update) model.tooltip.frp.set_style(tooltip_update) ); quick_visualization_preview <- bool(&frp.disable_quick_visualization_preview, &frp.enable_quick_visualization_preview); @@ -3691,7 +3683,7 @@ mod graph_editor_tests { let (node_1_id, node_1) = graph_editor.add_node_by_api(); graph_editor.stop_editing(); // Creating edge. - let port = node_1.model.output_port_shape().expect("No output port."); + let port = node_1.model().output_port_shape().expect("No output port."); port.events.mouse_down.emit(PrimaryButton); port.events.mouse_up.emit(PrimaryButton); assert_eq!(graph_editor.edges().len(), 1); @@ -3718,7 +3710,7 @@ mod graph_editor_tests { let (node_id_2, node_2) = graph_editor.add_node_by_api(); graph_editor.stop_editing(); // Creating edge. - let port = node_1.model.output_port_shape().expect("No output port."); + let port = node_1.model().output_port_shape().expect("No output port."); port.events.mouse_down.emit(PrimaryButton); port.events.mouse_up.emit(PrimaryButton); let edge_id = graph_editor.on_edge_add.value(); @@ -3728,8 +3720,8 @@ mod graph_editor_tests { assert_eq!(edges.len(), 1); // Connecting edge. // We need to enable ports. Normally it is done by hovering the node. - node_2.model.input.frp.set_ports_active(true, None); - let port = node_2.model.input_port_shape().expect("No input port."); + node_2.model().input.frp.set_ports_active(true, None); + let port = node_2.model().input_port_shape().expect("No input port."); port.hover.events.mouse_down.emit(PrimaryButton); port.hover.events.mouse_up.emit(PrimaryButton); assert_eq!(edge.source().map(|e| e.node_id), Some(node_id_1)); diff --git a/app/gui/view/graph-editor/src/new_node_position.rs b/app/gui/view/graph-editor/src/new_node_position.rs index df532086e96b..b933018965ee 100644 --- a/app/gui/view/graph-editor/src/new_node_position.rs +++ b/app/gui/view/graph-editor/src/new_node_position.rs @@ -172,7 +172,7 @@ pub fn aligned_if_close_to_node( ) -> Vector2 { let alignment_node = alignment_node.filter(|node| { use theme::graph_editor::alignment_area_around_node as alignment_area_style; - let node_bounding_box = node.frp.bounding_box.value(); + let node_bounding_box = node.bounding_box.value(); let styles = &graph_editor.styles_frp; let left = styles.get_number_or(alignment_area_style::to_the_left_of_node, 0.0); let alignment_area_min_x = node_bounding_box.left() - left.value(); @@ -244,7 +244,7 @@ pub fn on_ray( // `find_free_place` looks for free place for the origin point, and we want to fit not only // the point, but the whole node. let node_areas = nodes.values().map(|node| { - let bounding_box = node.frp.bounding_box.value(); + let bounding_box = node.bounding_box.value(); let left = bounding_box.left() - x_gap - min_spacing; let right = bounding_box.right() + x_gap; let top = bounding_box.top() + node::HEIGHT / 2.0 + y_gap; @@ -269,7 +269,7 @@ fn node_nearest_to_point(graph_editor: &GraphEditorModel, point: Vector2) -> Opt let mut nearest_node = None; let nodes = graph_editor.nodes.all.raw.borrow(); for node in nodes.values() { - let node_bounding_box = node.frp.bounding_box.value(); + let node_bounding_box = node.bounding_box.value(); let distance_squared = node_bounding_box.squared_distance_to_point(point); if distance_squared < min_distance_squared { min_distance_squared = distance_squared; diff --git a/app/gui/view/graph-editor/src/selection.rs b/app/gui/view/graph-editor/src/selection.rs index 90b50faa3150..69e4de10ea6c 100644 --- a/app/gui/view/graph-editor/src/selection.rs +++ b/app/gui/view/graph-editor/src/selection.rs @@ -234,7 +234,7 @@ fn get_nodes_in_bounding_box(bounding_box: &BoundingBox, nodes: &Nodes) -> Vec; +pub type Block = ComponentView; diff --git a/lib/rust/ensogl/component/gui/src/component.rs b/lib/rust/ensogl/component/gui/src/component.rs index 058be9131964..71e255fd654a 100644 --- a/lib/rust/ensogl/component/gui/src/component.rs +++ b/lib/rust/ensogl/component/gui/src/component.rs @@ -1,10 +1,9 @@ -//! UI component consisting of an FRP and a Model. +//! UI Component being and [`application::View`] - the visible GUI element which also registers +//! itself in application and listens for shortcuts. //! -//! Enforces correct ownership of components: Model must not own Frp. Both need to be owned via -//! `Rc` by the parent struct, which itself acts as a smart-pointer to the FRP. -//! -//! Requires both the Frp component, and the Model to implement a trait each, which provide -//! functionality for constructing / initialising the components. +//! The [`ComponentView`] is essentially a [`Widget`] where both the FRP and the Model implement a +//! trait each, which provide functionality for constructing / initialising the component as an +//! [`application::View`]. use crate::prelude::*; use ensogl_core::display::shape::*; @@ -16,6 +15,7 @@ use ensogl_core::application::frp::API; use ensogl_core::application::shortcut; use ensogl_core::application::Application; use ensogl_core::display; +use ensogl_core::gui::Widget; @@ -23,9 +23,9 @@ use ensogl_core::display; // === Model === // ============= -/// Model that can be used in a Component. Requires a constructor that takes an application and -/// returns `Self`. The model will be created with this constructor when constructing the -/// `Component`. +/// Model that can be used in a [`ComponentView`]. Requires a constructor that takes an application +/// and returns `Self`. The model will be created with this constructor when constructing the +/// [`ComponentView`]. pub trait Model { /// Identifier of the Model. Used for initializing the component logger and /// to provide the label for the `command::View` implementation. @@ -41,8 +41,8 @@ pub trait Model { // === FRP === // =========== -/// Frp that can be used in a Component. The FRP requires an initializer that will be called during -/// the construction of the component. `Default` is usually implemented when using +/// Frp that can be used in a [`ComponentView`]. The FRP requires an initializer that will be called +/// during the construction of the component. `Default` is usually implemented when using /// the `ensogl_core::define_endpoints!` macro to create an FRP API. pub trait Frp: Default + API { /// Frp initializer. Should set up the logic for processing inputs and generating outputs @@ -67,65 +67,66 @@ pub trait Frp: Default + API { // === Component === // ================= -/// Base struct for UI components in EnsoGL. Contains the Data/Shape model and the FPR exposing its -/// behaviour. +/// Base struct for visual components in EnsoGL which are [`application::View`]. #[derive(CloneRef, Debug, Derivative)] #[derivative(Clone(bound = ""))] -pub struct Component { - /// Public FRP api of the Component. - frp: Rc, - model: Rc, - logger: Logger, - /// Reference to the application the Component belongs to. Generally required for implementing - /// `application::View` and initialising the `Model` and `Frp` and thus provided by the - /// `Component`. - pub app: Application, +pub struct ComponentView { + widget: Widget, + logger: Logger, } -impl> Component { +impl ComponentView +where + M: Model + display::Object + 'static, + F: Frp + 'static, +{ /// Constructor. pub fn new(app: &Application) -> Self { - let app = app.clone_ref(); let logger = Logger::new(M::label()); - let model = Rc::new(M::new(&app, &logger)); + let model = Rc::new(M::new(app, &logger)); let frp = F::default(); let style = StyleWatchFrp::new(&app.display.default_scene.style_sheet); - F::init(frp.private(), &app, &model, &style); - let frp = Rc::new(frp); - Self { frp, model, app, logger } + F::init(frp.private(), app, &model, &style); + let display_object = model.display_object().clone_ref(); + let widget = Widget::new(app, frp, model, display_object); + Self { widget, logger } } } -impl display::Object for Component { +impl display::Object for ComponentView { fn display_object(&self) -> &display::object::Instance { - self.model.display_object() + self.widget.display_object() } } -impl> Deref for Component { +impl> Deref for ComponentView { type Target = F::Public; fn deref(&self) -> &Self::Target { - self.frp.public() + self.widget.frp().public() } } -impl FrpNetworkProvider for Component { +impl FrpNetworkProvider for ComponentView { fn network(&self) -> &frp::Network { - self.frp.network() + self.widget.frp().network() } } -impl + FrpNetworkProvider> application::View for Component { +impl application::View for ComponentView +where + M: Model + display::Object + 'static, + F: Frp + FrpNetworkProvider + 'static, +{ fn label() -> &'static str { M::label() } fn new(app: &Application) -> Self { - Component::new(app) + ComponentView::new(app) } fn app(&self) -> &Application { - &self.app + self.widget.app() } fn default_shortcuts() -> Vec { diff --git a/lib/rust/ensogl/core/src/display.rs b/lib/rust/ensogl/core/src/display.rs index 4c78fb9b99c3..d3fe70890d90 100644 --- a/lib/rust/ensogl/core/src/display.rs +++ b/lib/rust/ensogl/core/src/display.rs @@ -7,6 +7,7 @@ // ============== pub mod camera; +pub mod garbage; pub mod layout; pub mod navigation; pub mod object; diff --git a/lib/rust/ensogl/core/src/display/garbage.rs b/lib/rust/ensogl/core/src/display/garbage.rs new file mode 100644 index 000000000000..a38f2705a327 --- /dev/null +++ b/lib/rust/ensogl/core/src/display/garbage.rs @@ -0,0 +1,112 @@ +//! A module containing the garbage [`Collector`] structure. +use crate::prelude::*; + + + +// =============== +// === Garbage === +// =============== + +/// The structure with collected garbage arranged in three buckets. +/// +/// For more info, see the docs of [`Collector`]. +#[derive(Debug, Default)] +struct Garbage { + before_pixel_sync: Vec>, + before_pixel_update: Vec>, + before_mouse_events: Vec>, +} + + +/// The Garbage Collector +/// +/// This structure collects the EnsoGL components structures designated to being drop once the +/// component's hiding will be finished and all related events will be handled. Thanks to that, +/// the library's user don't need to handle "is dropped" events separately from "is hidden" events. +/// +/// # Implementation +/// +/// Each collected component FRP network and model must be thus kept long enough until the following +/// sequence will happen: +/// 1. We render the scene without the component; +/// 2. On the new rendered scene, we request the pixel value under the mouse cursor (see +/// [`crate::display::render::passes::PixelReadPass`]); +/// 3. The requested pixel value is loaded to CPU memory after creating fences in GPU; +/// 4. The mouse events are handled (see [`crate::display::scene::SceneData::handle_mouse_events`]). +/// This process may span across several frames, during which the "shape under cursor" property may +/// still contain the old value. +/// +/// After that the component's FRP network will handle or propagate "object hidden" event, and can +/// be then freely dropped. +/// +/// Thus, in the collector, we keep collected objects in three buckets, and when on of the 1-4 +/// points above happens we move objects from appropriate bucket to next one (the points 1 and 2 +/// happen at the same time, thus we have three buckets instead of four), or, if it is the last +/// bucket, we drop the objects. +#[derive(Clone, CloneRef, Debug, Default)] +pub struct Collector { + garbage: Rc>, +} + +impl Collector { + /// Create new, empty Collector. + pub fn new() -> Self { + default() + } + + /// Collect object. + /// + /// The collector is designed to handle EnsoGL component's FRP networks and models, but any + /// structure with static timeline may be put. See [`Collector`] docs for information when + /// the object will be finally dropped. + pub fn collect(&self, object: T) { + self.garbage.borrow_mut().before_pixel_sync.push(Box::new(object)); + } + + /// Pixel value requested (the points 1 and 2 in [`Collector`] docs). + pub fn pixel_synced(&self) { + let mut garbage = self.garbage.borrow_mut(); + let objects_being_moved = std::mem::take(&mut garbage.before_pixel_sync); + garbage.before_pixel_update.extend(objects_being_moved); + } + + /// Pixel value retrieved (the point 3 in [`Collector`] docs). + pub fn pixel_updated(&self) { + let mut garbage = self.garbage.borrow_mut(); + let objects_being_moved = std::mem::take(&mut garbage.before_pixel_update); + garbage.before_mouse_events.extend(objects_being_moved); + } + + /// Mouse events handled (the point 4 in [`Collector`] docs). + pub fn mouse_events_handled(&self) { + let mut garbage = self.garbage.borrow_mut(); + drop(std::mem::take(&mut garbage.before_mouse_events)); + } +} + + + +// ============= +// === Tests === +// ============= + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn garbage_lifetime() { + let collector = Collector::new(); + let garbage = Rc::new(()); + let garbage_weak = Rc::downgrade(&garbage); + + collector.collect(garbage); + assert!(garbage_weak.upgrade().is_some()); + collector.pixel_synced(); + assert!(garbage_weak.upgrade().is_some()); + collector.pixel_updated(); + assert!(garbage_weak.upgrade().is_some()); + collector.mouse_events_handled(); + assert!(garbage_weak.upgrade().is_none()); + } +} diff --git a/lib/rust/ensogl/core/src/display/render/passes/pixel_read.rs b/lib/rust/ensogl/core/src/display/render/passes/pixel_read.rs index c1b48782f7f9..1267c7935e9c 100644 --- a/lib/rust/ensogl/core/src/display/render/passes/pixel_read.rs +++ b/lib/rust/ensogl/core/src/display/render/passes/pixel_read.rs @@ -50,13 +50,15 @@ impl PixelReadPassData { #[derive(Derivative, Clone)] #[derivative(Debug)] pub struct PixelReadPass { - data: Option>, - sync: Option, - position: Uniform>, - threshold: usize, - to_next_read: usize, + data: Option>, + sync: Option, + position: Uniform>, + threshold: usize, + to_next_read: usize, #[derivative(Debug = "ignore")] - callback: Option)>>, + callback: Option)>>, + #[derivative(Debug = "ignore")] + sync_callback: Option>, } impl PixelReadPass { @@ -66,18 +68,28 @@ impl PixelReadPass { let sync = default(); let position = position.clone_ref(); let callback = default(); + let sync_callback = default(); let threshold = 0; let to_next_read = 0; - Self { data, sync, position, threshold, to_next_read, callback } + Self { data, sync, position, threshold, to_next_read, callback, sync_callback } } - /// Sets a callback which will be evaluated after a successful pixel read action. Please note - /// that it will not be evaluated after each run of this pass, as the read is performed in an - /// asynchronous fashion and can take longer than a single frame. + /// Sets a callback which will be evaluated after a successful pixel read action. + /// + /// Please note that it will not be evaluated after each run of this pass, as the read is + /// performed in an asynchronous fashion and can take longer than a single frame. pub fn set_callback) + 'static>(&mut self, f: F) { self.callback = Some(Rc::new(f)); } + /// Sets a callback which will be evaluated after at the beginning of pixel read action. + /// + /// It will not be evaluated after each run of this pass as the read is performed in an + /// asynchronous fashion and can take longer than a single frame. + pub fn set_sync_callback(&mut self, f: F) { + self.sync_callback = Some(Rc::new(f)); + } + /// Sets a threshold of how often the pass should be run. Threshold of 0 means that it will be /// run every time. Threshold of N means that it will be only run every N-th call to the `run` /// function. @@ -171,6 +183,9 @@ impl pass::Definition for PixelReadPass { } if self.sync.is_none() { self.run_not_synced(&instance.context); + if let Some(callback) = &self.sync_callback { + callback() + } } } } diff --git a/lib/rust/ensogl/core/src/display/world.rs b/lib/rust/ensogl/core/src/display/world.rs index d50d4faa5f9f..d31ce016b5d4 100644 --- a/lib/rust/ensogl/core/src/display/world.rs +++ b/lib/rust/ensogl/core/src/display/world.rs @@ -14,6 +14,7 @@ use crate::debug; use crate::debug::stats::Stats; use crate::debug::stats::StatsData; use crate::display; +use crate::display::garbage; use crate::display::render; use crate::display::render::passes::SymbolsRenderPass; use crate::display::scene::DomPath; @@ -187,6 +188,7 @@ pub struct WorldData { stats_draw_handle: callback::Handle, pub on: Callbacks, debug_hotkeys_handle: Rc>>, + garbage_collector: garbage::Collector, } impl WorldData { @@ -201,6 +203,7 @@ impl WorldData { let default_scene = Scene::new(&logger, &stats, on_change); let uniforms = Uniforms::new(&default_scene.variables); let debug_hotkeys_handle = default(); + let garbage_collector = default(); let stats_draw_handle = on.prev_frame_stats.add(f!([stats_monitor] (stats: &StatsData) { stats_monitor.sample_and_draw(stats); @@ -216,6 +219,7 @@ impl WorldData { debug_hotkeys_handle, stats_monitor, stats_draw_handle, + garbage_collector, } .init() } @@ -259,10 +263,13 @@ impl WorldData { fn init_composer(&self) { let mouse_hover_rgba = self.default_scene.mouse.hover_rgba.clone_ref(); + let garbage_collector = &self.garbage_collector; let mut pixel_read_pass = PixelReadPass::::new(&self.default_scene.mouse.position); - pixel_read_pass.set_callback(move |v| { - mouse_hover_rgba.set(Vector4::from_iterator(v.iter().map(|value| *value as u32))) - }); + pixel_read_pass.set_callback(f!([garbage_collector](v) { + mouse_hover_rgba.set(Vector4::from_iterator(v.iter().map(|value| *value as u32))); + garbage_collector.pixel_updated(); + })); + pixel_read_pass.set_sync_callback(f!(garbage_collector.pixel_synced())); // TODO: We may want to enable it on weak hardware. // pixel_read_pass.set_threshold(1); let logger = Logger::new("renderer"); @@ -293,10 +300,19 @@ impl WorldData { self.uniforms.time.set(time.local); self.scene_dirty.unset_all(); self.default_scene.update(time); + self.garbage_collector.mouse_events_handled(); self.default_scene.render(); self.on.after_frame.run_all(time); self.stats.end_frame(); } + + /// Pass object for garbage collection. + /// + /// The collector is designed to handle EnsoGL component's FRP networks and models, but any + /// structure with static timeline may be put. For details, see docs of [`garbage::Collector`]. + pub fn collect_garbage(&self, object: T) { + self.garbage_collector.collect(object); + } } impl Default for WorldData { diff --git a/lib/rust/ensogl/core/src/gui.rs b/lib/rust/ensogl/core/src/gui.rs index c22c668c77c3..3162faae3e1b 100644 --- a/lib/rust/ensogl/core/src/gui.rs +++ b/lib/rust/ensogl/core/src/gui.rs @@ -8,3 +8,5 @@ pub mod component; pub mod cursor; pub mod style; + +pub use component::Widget; diff --git a/lib/rust/ensogl/core/src/gui/component.rs b/lib/rust/ensogl/core/src/gui/component.rs index e3553b09e784..21f7e1de1880 100644 --- a/lib/rust/ensogl/core/src/gui/component.rs +++ b/lib/rust/ensogl/core/src/gui/component.rs @@ -3,6 +3,7 @@ use crate::display::object::traits::*; use crate::prelude::*; +use crate::application::Application; use crate::display; use crate::display::scene; use crate::display::scene::layer::WeakLayer; @@ -13,6 +14,7 @@ use crate::display::shape::primitive::system::DynamicShapeInternals; use crate::display::symbol; + // ============== // === Export === // ============== @@ -167,3 +169,121 @@ impl display::Object for ShapeView { self.shape.display_object() } } + + + +// ============== +// === Widget === +// ============== + +// === WidgetData === + +// We use type bounds here, because Drop implementation requires them +#[derive(Debug)] +struct WidgetData { + app: Application, + display_object: display::object::Instance, + frp: std::mem::ManuallyDrop, + model: std::mem::ManuallyDrop>, +} + +impl WidgetData { + pub fn new( + app: &Application, + frp: Frp, + model: Rc, + display_object: display::object::Instance, + ) -> Self { + Self { + app: app.clone_ref(), + display_object, + frp: std::mem::ManuallyDrop::new(frp), + model: std::mem::ManuallyDrop::new(model), + } + } +} + +impl Drop for WidgetData { + fn drop(&mut self) { + self.display_object.unset_parent(); + // Taking the value from `ManuallyDrop` requires us to not use it anymore. + // This is clearly the case, because the structure will be soon dropped anyway. + #[allow(unsafe_code)] + unsafe { + let frp = std::mem::ManuallyDrop::take(&mut self.frp); + let model = std::mem::ManuallyDrop::take(&mut self.model); + self.app.display.collect_garbage(frp); + self.app.display.collect_garbage(model); + } + } +} + + +// === Widget === + +/// The EnsoGL widget abstraction. +/// +/// The widget is a visible element with FRP logic. The structure contains a FRP network and a +/// model. +/// +/// * The `Frp` structure should own `frp::Network` structure handling the widget's logic. It's +/// recommended to create one with [`crate::application::frp::define_endpoints`] macro. +/// * The `Model` is any structure containing shapes, properties, subwidgets etc. manipulated by +/// the FRP network. +/// +/// # Dropping Widget +/// +/// Upon dropping Widget structure, the object will be hidden, but both FRP and Model will +/// not be dropped at once. Instead, they will be passed to the EnsoGL's garbage collector, +/// because handling all effects of hiding object and emitting appropriate events (for example: +/// the "on_hide" event of [`core::gui::component::ShapeEvents`]) may take a several frames, and +/// we want to have FRP network alive to handle those effects. Thus, both FRP and model will +/// be dropped only after all process of object hiding will be handled. +#[derive(CloneRef, Debug, Derivative)] +#[derivative(Clone(bound = ""))] +pub struct Widget { + data: Rc>, +} + +impl Deref for Widget { + type Target = Frp; + + fn deref(&self) -> &Self::Target { + &self.data.frp + } +} + +impl Widget { + /// Create a new widget. + pub fn new( + app: &Application, + frp: Frp, + model: Rc, + display_object: display::object::Instance, + ) -> Self { + Self { data: Rc::new(WidgetData::new(app, frp, model, display_object)) } + } + + /// Get the FRP structure. It is also a result of deref-ing the widget. + pub fn frp(&self) -> &Frp { + &self.data.frp + } + + /// Get the Model structure. + pub fn model(&self) -> &Model { + &self.data.model + } + + /// Reference to the application the Widget belongs to. It's required for handling model and + /// FRP garbage collection, but also may be helpful when, for example, implementing + /// `application::View`. + pub fn app(&self) -> &Application { + &self.data.app + } +} + +impl display::Object for Widget { + fn display_object(&self) -> &display::object::Instance { + &self.data.display_object + } +} diff --git a/lib/rust/shortcuts/src/lib.rs b/lib/rust/shortcuts/src/lib.rs index c30af20cc44b..f6392223a06e 100644 --- a/lib/rust/shortcuts/src/lib.rs +++ b/lib/rust/shortcuts/src/lib.rs @@ -76,7 +76,9 @@ lazy_static! { static ref SIDE_KEYS_SET: HashSet<&'static str> = SIDE_KEYS.iter().copied().collect(); } -const DOUBLE_EVENT_TIME_MS: f32 = 300.0; +/// The maximum time difference between presses/clicks where they are treated as single +/// `DoublePress`/`DoubleClick` event. +pub const DOUBLE_EVENT_TIME_MS: f32 = 300.0;