enso-org/enso
1
1
Fork 0
mirror of https://github.com/enso-org/enso.git synced 2024-12-23 08:53:31 +03:00
Code Issues Projects Releases Wiki Activity

Fix dropdown laziness; also misc fixes (#6991)

Browse Source 
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.
This commit is contained in:
Kaz Wesley 2023-06-14 10:58:03 -07:00 committed by GitHub
parent 48f0c6f5e8
commit bf42ed482d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 185 additions and 97 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -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;
@ -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 {
@ -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,
@ -478,6 +480,7 @@ impl NodeModel {
vcs_indicator,
style,
comment,
interaction_state,
}
.init()
}
@ -485,63 +488,63 @@ impl NodeModel {
#[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) {
/// 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);
}
/// 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);
}
fn set_layers_for_state(&self, new_state: InteractionState) {
let scene = &self.app.display.default_scene;
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);
}
/// 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);
}
/// 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) {
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);
}
#[allow(missing_docs)] // FIXME[everyone] All pub functions should have docs.
pub fn width(&self) -> f32 {
self.input.width.value()
@ -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 ===

4
app/gui/view/graph-editor/src/component/node/growth_animation.rs
View File

@ -113,7 +113,7 @@ 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);
}
}
@ -121,7 +121,7 @@ impl GraphEditorModel {
#[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
app/gui/view/graph-editor/src/lib.rs
View File

@ -4,6 +4,7 @@
// === Features ===
#![feature(associated_type_defaults)]
#![feature(cell_update)]
#![feature(const_trait_impl)]
#![feature(drain_filter)]
#![feature(entry_insert)]
@ -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);
}
@ -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) {

6
app/gui/view/src/project.rs
View File

@ -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 {
@ -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

3
docs/profiler/profiling.md
View File

@ -139,6 +139,7 @@ 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.
@ -146,7 +147,7 @@ procedure to collect data is:
- 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)

4
lib/rust/ensogl/app/theme/hardcoded/src/lib.rs
View File

@ -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 {

20
lib/rust/ensogl/component/drop-down/src/lib.rs
View File

@ -2,6 +2,8 @@
//! entries.
#![recursion_limit = "512"]
// === Features ===
#![feature(let_chains)]
// === Standard Linter Configuration ===
#![deny(non_ascii_idents)]
#![warn(unsafe_code)]
@ -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;
@ -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();
@ -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) {
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_with_cell_pos.collect_vec()
let models_with_cell_pos =
models_with_index.map(|(idx, entry)| (idx, 0, entry));
models.extend(models_with_cell_pos);
}
models
})).iter();

62
lib/rust/ensogl/component/drop-down/src/model.rs
View File

@ -49,6 +49,7 @@ pub struct Model<T> {
pub grid: Grid,
selected_entries: Rc<RefCell<HashSet<T>>>,
cache: Rc<RefCell<EntryCache<T>>>,
expected_indices: Rc<RefCell<HashSet<usize>>>,
}
impl<T> component::Model for Model<T> {
@ -72,7 +73,14 @@ impl<T> component::Model for Model<T> {
grid.scroll_frp().set_corner_radius(inner_corners_radius);
grid.set_entries_size(Vector2(min_width, ENTRY_HEIGHT));
Model { background, grid, display_object, selected_entries: default(), cache: default() }
Model {
background,
grid,
display_object,
selected_entries: default(),
cache: default(),
expected_indices: default(),
}
}
}
@ -138,30 +146,33 @@ impl<T: DropdownValue> Model<T> {
requested_indices: &[usize],
) -> (Vec<Range<usize>>, Vec<Range<usize>>) {
let cache = self.cache.borrow();
let sorted_indices = {
let mut indices = requested_indices.to_owned();
indices.sort_unstable();
indices
};
let mut request_ranges: Vec<Range<usize>> = Vec::new();
let mut ready_ranges: Vec<Range<usize>> = Vec::new();
for &index in requested_indices {
for index in sorted_indices {
let modify_ranges = match cache.contains_key(index) {
true => &mut ready_ranges,
false => &mut request_ranges,
};
let mut new_range = Range { start: index, end: index + 1 };
modify_ranges.retain(|range| {
let ranges_overlap = new_range.start <= range.end && range.start <= new_range.end;
if ranges_overlap {
new_range.start = range.start.min(new_range.start);
new_range.end = range.end.max(new_range.end);
if let Some(range) = modify_ranges.last_mut() && range.end == index {
range.end = index + 1;
} else {
modify_ranges.push(Range { start: index, end: index + 1 });
}
!ranges_overlap
});
modify_ranges.push(new_range);
}
(ready_ranges, request_ranges)
}
/// Add the specified values to the set of indices that have been requested by the [`GridView`]
/// before their data has become available.
pub fn expect_update_for_range(&self, range: Range<usize>) {
self.expected_indices.borrow_mut().extend(range);
}
/// Accepts entry at given index, modifying selection. If entry is already selected, it will be
/// unselected, unless it is the last selected entry and `allow_empty` is false. For
/// single-select dropdowns, previously selected entry will be unselected.
@ -202,7 +213,8 @@ impl<T: DropdownValue> Model<T> {
})
}
/// Update cache with new entries at given range. Returns range of indices that were updated.
/// Update cache with new entries at given range. Returns ranges of indices that were previously
/// marked as expected and have now become available.
#[profile(Debug)]
pub fn insert_entries_in_range(
&self,
@ -211,7 +223,7 @@ impl<T: DropdownValue> Model<T> {
visible_range: Range<usize>,
max_cache_size: usize,
num_entries: usize,
) -> Range<usize> {
) -> Vec<Range<usize>> {
let update_start = updated_range.start.min(num_entries);
let update_end = updated_range.end.min(num_entries);
let truncated_range = update_start..update_end;
@ -219,7 +231,23 @@ impl<T: DropdownValue> Model<T> {
let mut cache = self.cache.borrow_mut();
cache.insert(truncated_range.clone(), truncated_entries, visible_range, max_cache_size);
truncated_range
let mut updated_ranges = vec![];
let mut new_range: Option<Range<usize>> = None;
let mut expected = self.expected_indices.borrow_mut();
for i in truncated_range {
let was_expected = expected.remove(&i);
if was_expected {
if let Some(new_range) = new_range.as_mut() && new_range.end == i {
new_range.end = i + 1;
} else {
let ended_range = new_range.replace(Range { start: i, end: i + 1 });
updated_ranges.extend(ended_range);
}
}
}
updated_ranges.extend(new_range);
updated_ranges
}
/// Prune selection according to changed multiselect mode. Returns true if the selection was

1
lib/rust/ensogl/component/grid-view/src/lib.rs
View File

@ -370,6 +370,7 @@ impl<E: Entry> Model<E, E::Params> {
entry_frp.set_model(model);
}
#[profile(Debug)]
fn update_after_column_resize(
&self,
resized_column: Col,

10
lib/rust/ensogl/core/src/display/scene.rs
View File

@ -587,16 +587,14 @@ impl Renderer {
// === Layers ===
// ==============
type RectLayerPartition = Rc<LayerSymbolPartition<rectangle::Shape>>;
type RectLayerPartition = LayerSymbolPartition<rectangle::Shape>;
/// Create a new layer partition with the given name, wrapped in an `Rc` for use in the
/// [`HardcodedLayers`] structure.
/// Create a new layer partition with the given name.
fn partition_layer<S: display::shape::primitive::system::Shape>(
base_layer: &Layer,
name: &str,
) -> Rc<LayerSymbolPartition<S>> {
let partition = base_layer.create_symbol_partition::<S>(name);
Rc::new(partition)
) -> LayerSymbolPartition<S> {
base_layer.create_symbol_partition::<S>(name)
}
/// Please note that currently the `Layers` structure is implemented in a hacky way. It assumes the

15
lib/rust/ensogl/core/src/display/scene/layer.rs
View File

@ -943,11 +943,18 @@ impl LayerModel {
let system_id = ShapeSystem::<S>::id();
let mut partitions = self.symbol_buffer_partitions.borrow_mut();
let index = partitions.entry(system_id).or_default();
let id = SymbolPartitionId { index: *index };
let id = Immutable(SymbolPartitionId { index: *index });
*index += 1;
LayerSymbolPartition { layer: WeakLayer { model: self.downgrade() }, id, shape: default() }
}
/// Return some symbol partition for a particular symbol in the layer. This can be used to refer
/// to the only partition in an unpartitioned layer.
pub fn default_partition<S: Shape>(self: &Rc<Self>) -> LayerSymbolPartition<S> {
let layer = WeakLayer { model: self.downgrade() };
LayerSymbolPartition { layer, id: default(), shape: default() }
}
/// The layer's mask, if any.
pub fn mask(&self) -> Option<Layer> {
self.mask.borrow().as_ref().and_then(|t| t.upgrade())
@ -1067,11 +1074,11 @@ impl std::borrow::Borrow<LayerModel> for Layer {
///
/// Symbol partitions determine the depth-order of instances of a particular symbol within a layer.
/// Any other symbol added to a symbol partition will be treated as if present in the parent layer.
#[derive(Debug, Derivative)]
#[derive(Debug, Derivative, CloneRef)]
#[derivative(Clone(bound = ""))]
pub struct LayerSymbolPartition<S> {
layer: WeakLayer,
id: SymbolPartitionId,
id: Immutable<SymbolPartitionId>,
shape: PhantomData<*const S>,
}
@ -1120,7 +1127,7 @@ impl AnySymbolPartition {
impl<S: Shape> From<&'_ LayerSymbolPartition<S>> for AnySymbolPartition {
fn from(value: &'_ LayerSymbolPartition<S>) -> Self {
let shape = ShapeSystem::<S>::id();
let id = value.id;
let id = *value.id;
Self { shape, id }
}
}

13
lib/rust/ensogl/core/src/display/shape/compound/rectangle.rs
View File

@ -60,7 +60,8 @@ pub mod shape {
let body = body.fill(color);
// === Border ===
let border = body.grow(border.px());
let padded_body = body.grow((inset - &border).px());
let border = padded_body.grow(border.px()) - padded_body;
let border_color = Var::<color::Rgba>::from(border_color);
let border = border.fill(border_color);
@ -152,9 +153,13 @@ impl Rectangle {
self.set_corner_radius(max_radius)
}
/// Set the padding between edge of the frame and shape itself. If you want to use border, you
/// should always set the inset at least of the size of the border. If you do not want the
/// border to be animated, you can use [`Self::set_inset_border`] instead.
/// Set the padding between edge of the frame and main shape.
///
/// This value should not be less than the width of the border. To set it to the same width as
/// the border, you can use [`Self::set_inset_border`].
///
/// If this value is greater than the border width, the extra padding will be between the body
/// and the border.
pub fn set_inset(&self, inset: f32) -> &Self {
self.modify_view(|view| view.inset.set(inset))
}

2
lib/rust/ensogl/core/src/display/world.rs
View File

@ -505,7 +505,7 @@ impl WorldData {
let key = event.code();
if key == "Backquote" {
stats_monitor.toggle()
} else if key == "KeyP" {
} else if key == "KeyO" {
if event.shift_key() {
let forwarding_incrementally = emit_measurements_handle.borrow().is_some();
// If we are submitting the data continuously, the hotkey is redundant.