Skip to content

Commit

Permalink
Fix dropdown laziness; also misc fixes (#6991)
Browse files Browse the repository at this point in the history
Several small changes:
- Dropdowns: Populate `GridView` lazily (fixes #6865).
- Clear disconnected edges when editing node (fixes case 1 in #7018).
- Fix regression in node selection rendering (2nd bug in #6975).
- Update profiler docs. The hotkey to *prOfile without exiting* is now `Ctrl+Alt+O` (`Ctrl+Alt+P` has been requisitioned by the CB).

Node selection:
| Pre-`Rectangle` | This PR |
| --- | --- |
| ![image](https://github.com/enso-org/enso/assets/1047859/bec341c1-dbf8-404d-9f2a-5d070c80ff15) | ![image](https://github.com/enso-org/enso/assets/1047859/8161390c-f64b-4bb3-8b7a-b87b2f9b4cd3) |

# Important Notes
- `Rectangle`: When `inset > border`, the extra space is now between the body and the border, not outside the border.
- More robust node layering logic. Now an inconsistent layer order cannot occur, even if something strange happens (like editing an expression and an edge at the same time).
- The dynamic drop down in the `drop_down` example scene doesn't show any entries before (or after) this, so I can't test the dynamic case.
  • Loading branch information
kazcw authored Jun 14, 2023
1 parent 48f0c6f commit bf42ed4
Show file tree
Hide file tree
Showing 13 changed files with 185 additions and 97 deletions.
131 changes: 83 additions & 48 deletions app/gui/view/graph-editor/src/component/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ use ensogl::application::Application;
use ensogl::control::io::mouse;
use ensogl::data::color;
use ensogl::display;
use ensogl::display::scene::Layer;
use ensogl::display::shape::compound::rectangle;
use ensogl::gui;
use ensogl::Animation;
Expand Down Expand Up @@ -416,6 +415,7 @@ pub struct NodeModel {
pub vcs_indicator: vcs::StatusIndicator,
pub style: StyleWatchFrp,
pub comment: text::Text,
pub interaction_state: Rc<Cell<InteractionState>>,
}

impl NodeModel {
Expand Down Expand Up @@ -463,6 +463,8 @@ impl NodeModel {
let comment = text::Text::new(app);
display_object.add_child(&comment);

let interaction_state = default();

let app = app.clone_ref();
Self {
app,
Expand All @@ -478,68 +480,69 @@ impl NodeModel {
vcs_indicator,
style,
comment,
interaction_state,
}
.init()
}

#[profile(Debug)]
fn init(self) -> Self {
self.set_expression(Expression::new_plain("empty"));
self.move_to_main_layer();
self.set_layers_for_state(self.interaction_state.get());
self
}

#[profile(Debug)]
fn set_special_layers(&self, text_layer: &Layer, action_bar_layer: &Layer) {
action_bar_layer.add(&self.action_bar);
self.output.set_label_layer(text_layer);
self.input.set_label_layer(text_layer);
self.profiling_label.set_label_layer(text_layer);
self.comment.add_to_scene_layer(text_layer);
/// Set whether the node is being edited. This is used to adjust the camera.
pub fn set_editing_expression(&self, editing: bool) {
let new_state = self.interaction_state.update(|state| state.editing_expression(editing));
self.set_layers_for_state(new_state);
}

/// Move all sub-components to `edited_node` layer.
///
/// A simple [`Layer::add`] wouldn't work because text rendering in ensogl uses a
/// separate layer management API.
///
/// `action_bar` is moved to the `edited_node` layer as well, though normally it lives on a
/// separate `above_nodes` layer, unlike every other node component.
pub fn move_to_edited_node_layer(&self) {
let scene = &self.app.display.default_scene;
let layer = &scene.layers.edited_node;
let text_layer = &scene.layers.edited_node_text;
let action_bar_layer = &scene.layers.edited_node;
layer.add(&self.display_object);
self.set_special_layers(text_layer, action_bar_layer);
/// Set whether the node is being interacted with by moving an edge with the mouse.
pub fn set_editing_edge(&self, editing: bool) {
let new_state = self.interaction_state.update(|state| state.editing_edge(editing));
self.set_layers_for_state(new_state);
}

/// Move all sub-components to `main` layer.
///
/// A simple [`Layer::add`] wouldn't work because text rendering in ensogl uses a
/// separate layer management API.
///
/// `action_bar` is handled separately, as it uses `above_nodes` scene layer unlike any other
/// node component.
pub fn move_to_main_layer(&self) {
fn set_layers_for_state(&self, new_state: InteractionState) {
let scene = &self.app.display.default_scene;
let layer = &scene.layers.main_nodes_level;
let text_layer = &scene.layers.label;
let action_bar_layer = &scene.layers.above_nodes;
layer.add(&self.display_object);
self.set_special_layers(text_layer, action_bar_layer);
}

/// Move the node to the normal layer used when the node is not connected to a detached edge.
pub fn move_to_resting_node_layer(&self) {
let layer = &self.app.display.default_scene.layers.main_nodes_level;
layer.add(&self.background);
}

/// Move the node to the layer used for nodes that have a detached edge.
pub fn move_to_active_node_layer(&self) {
let layer = &self.app.display.default_scene.layers.main_active_nodes_level;
layer.add(&self.background);
let main_layer;
let background_layer;
let text_layer;
let action_bar_layer;
match new_state {
InteractionState::EditingExpression => {
// Move all sub-components to `edited_node` layer.
//
// `action_bar` is moved to the `edited_node` layer as well, though normally it
// lives on a separate `above_nodes` layer, unlike every other node component.
main_layer = scene.layers.edited_node.default_partition();
background_layer = main_layer.clone();
text_layer = &scene.layers.edited_node_text;
action_bar_layer = &scene.layers.edited_node;
}
InteractionState::EditingEdge => {
main_layer = scene.layers.main_nodes_level.clone();
background_layer = scene.layers.main_active_nodes_level.clone();
text_layer = &scene.layers.label;
action_bar_layer = &scene.layers.above_nodes;
}
InteractionState::Normal => {
main_layer = scene.layers.main_nodes_level.clone();
background_layer = main_layer.clone();
text_layer = &scene.layers.label;
action_bar_layer = &scene.layers.above_nodes;
}
}
main_layer.add(&self.display_object);
background_layer.add(&self.background);
action_bar_layer.add(&self.action_bar);
// For the text layer, [`Layer::add`] can't be used because text rendering currently uses a
// separate layer management API.
self.output.set_label_layer(text_layer);
self.input.set_label_layer(text_layer);
self.profiling_label.set_label_layer(text_layer);
self.comment.add_to_scene_layer(text_layer);
}

#[allow(missing_docs)] // FIXME[everyone] All pub functions should have docs.
Expand Down Expand Up @@ -1014,6 +1017,38 @@ fn bounding_box(
}


// === Interaction state ===

/// Information about how the node is being interacted with.
#[derive(Debug, Copy, Clone, PartialEq, Eq, Default)]
pub enum InteractionState {
/// The node is being edited (with the cursor / Component Browser).
EditingExpression,
/// An edge of the node is being interacted with.
EditingEdge,
/// The node is not being interacted with.
#[default]
Normal,
}

impl InteractionState {
fn editing_expression(self, editing: bool) -> Self {
match editing {
true => Self::EditingExpression,
false => Self::Normal,
}
}

fn editing_edge(self, active: bool) -> Self {
match (self, active) {
(Self::EditingExpression, _) => self,
(_, true) => Self::EditingEdge,
(_, false) => Self::Normal,
}
}
}



// ==================
// === Test Utils ===
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,15 +113,15 @@ impl GraphEditorModel {
#[profile(Debug)]
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().set_editing_expression(true);
}
}

/// Move node to the `main` scene layer, so that it is rendered by the main camera.
#[profile(Debug)]
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().set_editing_expression(false);
}
}
}
7 changes: 5 additions & 2 deletions app/gui/view/graph-editor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
// === Features ===
#![feature(associated_type_defaults)]
#![feature(cell_update)]
#![feature(const_trait_impl)]
#![feature(drain_filter)]
#![feature(entry_insert)]
Expand Down Expand Up @@ -2267,10 +2268,10 @@ impl GraphEditorModel {
let new_sources: HashSet<_> =
detached_target_edges.filter_map(get_edge).filter_map(get_source_id).collect();
for added in new_sources.difference(&old_sources).filter_map(get_node) {
added.model().move_to_active_node_layer();
added.model().set_editing_edge(true);
}
for removed in old_sources.difference(&new_sources).filter_map(get_node) {
removed.model().move_to_resting_node_layer();
removed.model().set_editing_edge(false);
}
self.sources_of_detached_target_edges.raw.replace(new_sources);
}
Expand Down Expand Up @@ -3109,6 +3110,8 @@ fn new_graph_editor(app: &Application) -> GraphEditor {
out.node_edit_mode <+ edit_mode;
out.nodes_labels_visible <+ out.node_edit_mode || node_in_edit_mode;

eval_ out.node_editing (model.clear_all_detached_edges());

eval out.node_editing_started ([model] (id) {
let _profiler = profiler::start_debug!(profiler::APP_LIFETIME, "node_editing_started");
if let Some(node) = model.nodes.get_cloned_ref(id) {
Expand Down
6 changes: 3 additions & 3 deletions app/gui/view/src/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ impl View {
.init_style_toggle_frp()
.init_fullscreen_visualization_frp()
.init_debug_mode_frp()
.init_shortcut_observer()
.init_shortcut_observer(app)
}

fn init_top_bar_frp(self, scene: &Scene) -> Self {
Expand Down Expand Up @@ -659,10 +659,10 @@ impl View {
self
}

fn init_shortcut_observer(self) -> Self {
fn init_shortcut_observer(self, app: &Application) -> Self {
let frp = &self.frp;
frp::extend! { network
frp.source.current_shortcut <+ self.model.app.shortcuts.currently_handled;
frp.source.current_shortcut <+ app.shortcuts.currently_handled;
}

self
Expand Down
3 changes: 2 additions & 1 deletion docs/profiler/profiling.md
Original file line number Diff line number Diff line change
Expand Up @@ -139,14 +139,15 @@ procedure to collect data is:
```shell
./distribution/bin/enso --profile.save=your_profile.json
```
If this option is not provided, the path `profile.json` will be used.
- Perform any activity the profiler should record (the profiler is always
running).
- Press Ctrl+Alt+Q to save profile (to the path specified earlier) and quit.
- To profile in Chrome:
- Open Enso in Chrome.
- Perform any activity the profiler should record (the profiler is always
running).
- Press Ctrl+Alt+P to end profile recording.
- Press Ctrl+Alt+O to end profile recording.
- Open the Browser Console by pressing Ctrl+Alt+J (do not do this before
profiling;
[having the console open can degrade performance](https://developer.chrome.com/blog/wasm-debugging-2020/#profiling)
Expand Down
4 changes: 2 additions & 2 deletions lib/rust/ensogl/app/theme/hardcoded/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -510,8 +510,8 @@ define_themes! { [light:0, dark:1]
background.skipped = graph_editor::node::background , graph_editor::node::background;
selection = selection, selection;
selection {
size = 3.0 , 3.0;
offset = 5.0 , 5.0;
size = 3.5 , 3.5;
offset = 3.75 , 3.75;
}
text = Rgba(0.078,0.067,0.137,0.85) , Lcha(1.0,0.0,0.0,0.7);
text {
Expand Down
22 changes: 16 additions & 6 deletions lib/rust/ensogl/component/drop-down/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
//! entries.
#![recursion_limit = "512"]
// === Features ===
#![feature(let_chains)]
// === Standard Linter Configuration ===
#![deny(non_ascii_idents)]
#![warn(unsafe_code)]
Expand Down Expand Up @@ -189,6 +191,7 @@ impl<T: DropdownValue> Frp<T> {
);
requested_range_ready <- ready_and_request_ranges._0().iter();
requested_range_needed <- ready_and_request_ranges._1().iter();
eval requested_range_needed ((range) model.expect_update_for_range(range.clone()));

output.entries_in_range_needed <+ requested_range_needed;

Expand All @@ -199,7 +202,7 @@ impl<T: DropdownValue> Frp<T> {
});
output.currently_visible_range <+ visible_range;

updated_range <- provided_entries.map4(
requested_ranges_received <- provided_entries.map4(
&visible_range, &max_cache_size, &number_of_entries,
f!([model]((range, entries), visible, max_size, num_entries) {
let range = range.clone();
Expand All @@ -208,11 +211,18 @@ impl<T: DropdownValue> Frp<T> {
})
);

range_to_update <- any(updated_range, requested_range_ready);
model.grid.model_for_entry <+ range_to_update.map(f!([model](range) {
let models_with_index = model.entry_models_for_range(range.clone());
let models_with_cell_pos = models_with_index.map(|(idx, entry)| (idx, 0, entry));
models_with_cell_pos.collect_vec()
ranges_to_update <- any(...);
ranges_to_update <+ requested_range_ready.map(f!([](range) vec![range.clone()]));
ranges_to_update <+ requested_ranges_received;
model.grid.model_for_entry <+ ranges_to_update.map(f!([model](ranges) {
let mut models = vec![];
for range in ranges {
let models_with_index = model.entry_models_for_range(range.clone());
let models_with_cell_pos =
models_with_index.map(|(idx, entry)| (idx, 0, entry));
models.extend(models_with_cell_pos);
}
models
})).iter();


Expand Down
Loading

0 comments on commit bf42ed4

Please sign in to comment.