Placement of newly opened Component Browser dictated by the mouse pointer.

CHANGELOG.md

@@ -2,6 +2,11 @@
 
 #### Visual Environment
 
+- [Nodes created near existing nodes via the <kbd>TAB</kbd> key or by dropping a
+  connection are now repositioned and aligned to existing nodes.][3301] This is
+  to make the resulting graph prettier and avoid overlapping. In such cases,
+  created nodes will be placed below an existing node or on the bottom-left
+  diagonal if there is no space underneath.
 - [Nodes can be added to the graph by double-clicking the output ports of
   existing nodes (or by clicking them with the right mouse button).][3346]
 - [Node Searcher preserves its zoom factor.][3327] The visible size of the node
@@ -106,6 +111,7 @@
 [3285]: https://github.com/enso-org/enso/pull/3285
 [3287]: https://github.com/enso-org/enso/pull/3287
 [3292]: https://github.com/enso-org/enso/pull/3292
+[3301]: https://github.com/enso-org/enso/pull/3301
 [3302]: https://github.com/enso-org/enso/pull/3302
 [3305]: https://github.com/enso-org/enso/pull/3305
 [3309]: https://github.com/enso-org/enso/pull/3309

Cargo.toml

@@ -41,4 +41,7 @@ debug-assertions = true
 
 [profile.integration-test]
 inherits = "test"
-opt-level = 2
+# 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

app/gui/view/graph-editor/src/component/node.rs

@@ -610,21 +610,22 @@ impl NodeModel {
         self.drag_area.size.set(padded_size);
         self.error_indicator.size.set(padded_size);
         self.vcs_indicator.set_size(padded_size);
-        self.backdrop.mod_position(|t| t.x = width / 2.0);
-        self.background.mod_position(|t| t.x = width / 2.0);
-        self.drag_area.mod_position(|t| t.x = width / 2.0);
-        self.error_indicator.set_position_x(width / 2.0);
-        self.vcs_indicator.set_position_x(width / 2.0);
+        let x_offset_to_node_center = x_offset_to_node_center(width);
+        self.backdrop.set_position_x(x_offset_to_node_center);
+        self.background.set_position_x(x_offset_to_node_center);
+        self.drag_area.set_position_x(x_offset_to_node_center);
+        self.error_indicator.set_position_x(x_offset_to_node_center);
+        self.vcs_indicator.set_position_x(x_offset_to_node_center);
 
         let action_bar_width = ACTION_BAR_WIDTH;
         self.action_bar.mod_position(|t| {
-            t.x = width + CORNER_RADIUS + action_bar_width / 2.0;
+            t.x = x_offset_to_node_center + width / 2.0 + CORNER_RADIUS + action_bar_width / 2.0;
         });
         self.action_bar.frp.set_size(Vector2::new(action_bar_width, ACTION_BAR_HEIGHT));
 
-        let visualization_pos = Vector2(width / 2.0, VISUALIZATION_OFFSET_Y);
-        self.error_visualization.set_position_xy(visualization_pos);
-        self.visualization.set_position_xy(visualization_pos);
+        let visualization_offset = visualization_offset(width);
+        self.error_visualization.set_position_xy(visualization_offset);
+        self.visualization.set_position_xy(visualization_offset);
 
         size
     }
@@ -752,14 +753,6 @@ impl Node {
             eval new_size ((t) model.output.frp.set_size.emit(t));
 
 
-            // === Bounding Box ===
-            bounding_box_input <- all2(&new_size,&position);
-            out.bounding_box <+ bounding_box_input.map(|(size,position)| {
-                let position = position - Vector2::new(0.0,size.y / 2.0);
-                BoundingBox::from_position_and_size(position,*size)
-            });
-
-
             // === Action Bar ===
 
             let visualization_enabled = action_bar.action_visibility.clone_ref();
@@ -934,6 +927,16 @@ impl Node {
                 <+ visualization_visible.not().and(&no_error_set);
 
 
+            // === Bounding Box ===
+
+            let visualization_size = &model.visualization.frp.size;
+            // Visualization can be enabled and not visible when the node has an error.
+            visualization_enabled_and_visible <- visualization_enabled && visualization_visible;
+            bbox_input <- all4(
+                &position,&new_size,&visualization_enabled_and_visible,visualization_size);
+            out.bounding_box <+ bbox_input.map(|(a,b,c,d)| bounding_box(*a,*b,*c,*d));
+
+
             // === VCS Handling ===
 
             model.vcs_indicator.frp.set_status <+ input.set_vcs_status;
@@ -971,6 +974,40 @@ impl display::Object for Node {
 }
 
 
+// === Positioning ===
+
+fn x_offset_to_node_center(node_width: f32) -> f32 {
+    node_width / 2.0
+}
+
+/// Calculate a position where to render the [`visualization::Container`] of a node, relative to
+/// the node's origin.
+fn visualization_offset(node_width: f32) -> Vector2 {
+    Vector2(x_offset_to_node_center(node_width), VISUALIZATION_OFFSET_Y)
+}
+
+fn bounding_box(
+    node_position: Vector2,
+    node_size: Vector2,
+    visualization_enabled_and_visible: bool,
+    visualization_size: Vector2,
+) -> BoundingBox {
+    let x_offset_to_node_center = x_offset_to_node_center(node_size.x);
+    let node_bbox_pos = node_position + Vector2(x_offset_to_node_center, 0.0) - node_size / 2.0;
+    let node_bbox = BoundingBox::from_position_and_size(node_bbox_pos, node_size);
+    if visualization_enabled_and_visible {
+        let visualization_offset = visualization_offset(node_size.x);
+        let visualization_pos = node_position + visualization_offset;
+        let visualization_bbox_pos = visualization_pos - visualization_size / 2.0;
+        let visualization_bbox =
+            BoundingBox::from_position_and_size(visualization_bbox_pos, visualization_size);
+        node_bbox.concat_ref(visualization_bbox)
+    } else {
+        node_bbox
+    }
+}
+
+
 
 // ==================
 // === Test Utils ===

app/gui/view/graph-editor/src/lib.rs

@@ -31,8 +31,7 @@ pub mod component;
 
 pub mod builtin;
 pub mod data;
-#[warn(missing_docs)]
-pub mod free_place_finder;
+pub mod new_node_position;
 #[warn(missing_docs)]
 pub mod profiling;
 #[warn(missing_docs)]
@@ -49,8 +48,6 @@ use crate::component::visualization;
 use crate::component::visualization::instance::PreprocessorConfiguration;
 use crate::component::visualization::MockDataGenerator3D;
 use crate::data::enso;
-use crate::free_place_finder::find_free_place;
-use crate::free_place_finder::OccupiedArea;
 pub use crate::node::profiling::Status as NodeProfilingStatus;
 
 use enso_config::ARGS;
@@ -394,6 +391,12 @@ impl<K, V, S> SharedHashMap<K, V, S> {
     where K: Clone {
         self.raw.borrow().keys().cloned().collect_vec()
     }
+
+    /// Get the vector of map's values.
+    pub fn values(&self) -> Vec<V>
+    where V: Clone {
+        self.raw.borrow().values().cloned().collect_vec()
+    }
 }
 
 
@@ -1388,17 +1391,20 @@ impl GraphEditorModelWithNetwork {
 
 // === Node Creation ===
 
+/// Describes the way used to request creation of a new node.
 #[derive(Clone, Debug)]
-enum WayOfCreatingNode {
+pub enum WayOfCreatingNode {
     /// "add_node" FRP event was emitted.
     AddNodeEvent,
     /// "start_node_creation" FRP event was emitted.
     StartCreationEvent,
     /// "start_node_creation_from_port" FRP event was emitted.
+    #[allow(missing_docs)]
     StartCreationFromPortEvent { endpoint: EdgeEndpoint },
     /// add_node_button was clicked.
     ClickingButton,
     /// The edge was dropped on the stage.
+    #[allow(missing_docs)]
     DroppingEdge { edge_id: EdgeId },
 }
 
@@ -1425,36 +1431,33 @@ impl GraphEditorModelWithNetwork {
         way: &WayOfCreatingNode,
         mouse_position: Vector2,
     ) -> (NodeId, Option<NodeSource>, bool) {
-        use WayOfCreatingNode::*;
-        let should_edit = !matches!(way, AddNodeEvent);
         let selection = self.nodes.selected.first_cloned();
-        let source_node = match way {
-            AddNodeEvent => None,
-            StartCreationEvent | ClickingButton => selection,
-            DroppingEdge { edge_id } => self.edge_source_node_id(*edge_id),
-            StartCreationFromPortEvent { endpoint } => Some(endpoint.node_id),
-        };
-        let source = source_node.map(|node| NodeSource { node });
-        let screen_center =
-            self.scene().screen_to_object_space(&self.display_object, Vector2(0.0, 0.0));
-        let position: Vector2 = match way {
-            AddNodeEvent => default(),
-            StartCreationEvent | ClickingButton if selection.is_some() =>
-                self.find_free_place_under(selection.unwrap()),
-            StartCreationEvent => mouse_position,
-            ClickingButton =>
-                self.find_free_place_for_node(screen_center, Vector2(0.0, -1.0)).unwrap(),
-            DroppingEdge { .. } => mouse_position,
-            StartCreationFromPortEvent { endpoint } => self.find_free_place_under(endpoint.node_id),
-        };
+        let position = new_node_position::new_node_position(self, way, selection, mouse_position);
         let node = self.new_node(ctx);
         node.set_position_xy(position);
+        let should_edit = !matches!(way, WayOfCreatingNode::AddNodeEvent);
         if should_edit {
             node.view.set_expression(node::Expression::default());
         }
+        let source = self.data_source_for_new_node(way, selection);
         (node.id(), source, should_edit)
     }
 
+    fn data_source_for_new_node(
+        &self,
+        way: &WayOfCreatingNode,
+        selection: Option<NodeId>,
+    ) -> Option<NodeSource> {
+        use WayOfCreatingNode::*;
+        let source_node = match way {
+            AddNodeEvent => None,
+            StartCreationEvent | ClickingButton => selection,
+            DroppingEdge { edge_id } => self.edge_source_node_id(*edge_id),
+            StartCreationFromPortEvent { endpoint } => Some(endpoint.node_id),
+        };
+        source_node.map(|node| NodeSource { node })
+    }
+
     fn new_node(&self, ctx: &NodeCreationContext) -> Node {
         use ensogl::application::command::FrpNetworkProvider;
         let view = component::Node::new(&self.app, self.vis_registry.clone_ref());
@@ -1722,7 +1725,7 @@ impl GraphEditorModel {
 
     /// Create a new node and place it at a free place below `above` node.
     pub fn add_node_below(&self, above: NodeId) -> NodeId {
-        let pos = self.find_free_place_under(above);
+        let pos = new_node_position::under(self, above);
         self.add_node_at(pos)
     }
 
@@ -1732,46 +1735,6 @@ impl GraphEditorModel {
         self.frp.set_node_position((node_id, pos));
         node_id
     }
-
-    /// Return the first available position for a new node below `node_above` node.
-    pub fn find_free_place_under(&self, node_above: NodeId) -> Vector2 {
-        let above_pos = self.node_position(node_above);
-        let y_gap = self.frp.default_y_gap_between_nodes.value();
-        let y_offset = y_gap + node::HEIGHT;
-        let starting_point = above_pos - Vector2(0.0, y_offset);
-        let direction = Vector2(-1.0, 0.0);
-        self.find_free_place_for_node(starting_point, direction).unwrap()
-    }
-
-    /// Return the first unoccupied point when going along the ray starting from `starting_point`
-    /// and parallel to `direction` vector.
-    pub fn find_free_place_for_node(
-        &self,
-        starting_from: Vector2,
-        direction: Vector2,
-    ) -> Option<Vector2> {
-        let x_gap = self.frp.default_x_gap_between_nodes.value();
-        let y_gap = self.frp.default_y_gap_between_nodes.value();
-        // This is how much horizontal space we are looking for.
-        let min_spacing = self.frp.min_x_spacing_for_new_nodes.value();
-        let nodes = self.nodes.all.raw.borrow();
-        // The "occupied area" for given node consists of:
-        // - area taken by node view (obviously);
-        // - the minimum gap between nodes in all directions, so the new node won't be "glued" to
-        //   another;
-        // - the new node size measured from origin point at each direction accordingly: because
-        //   `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 position = node.position();
-            let left = position.x - x_gap - min_spacing;
-            let right = position.x + node.view.model.width() + x_gap;
-            let top = position.y + node::HEIGHT + y_gap;
-            let bottom = position.y - node::HEIGHT - y_gap;
-            OccupiedArea { x1: left, x2: right, y1: top, y2: bottom }
-        });
-        find_free_place(starting_from, direction, node_areas)
-    }
 }