Skip to content

Commit

Permalink
Coalesce graph editor view invalidations (#6786)
Browse files Browse the repository at this point in the history
Only invalidate the graph editor view at most once per frame. On develop, this saves about 70ms (2%). Testing a recent backend without #6755 as a stress-test, this saves about 5s (45%). This reflects better scalability to large numbers of `SuggestionUpdate` messages.

Fixes #6630.

# Important Notes
- Also fix intermittent profiling failures occurring since the introduction of microtasks.
  • Loading branch information
kazcw authored May 29, 2023
1 parent 6eb4737 commit 7e6a919
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 3 deletions.
14 changes: 12 additions & 2 deletions app/gui/src/presenter/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,7 @@ impl Model {
}

/// Look through all graph's nodes in AST and set position where it is missing.
#[profile(Debug)]
fn initialize_nodes_positions(&self, default_gap_between_nodes: f32) {
match self.controller.graph().nodes() {
Ok(nodes) => {
Expand Down Expand Up @@ -562,6 +563,7 @@ struct ViewUpdate {

impl ViewUpdate {
/// Create ViewUpdate information from Graph Presenter's model.
#[profile(Debug)]
fn new(model: &Model) -> FallibleResult<Self> {
let state = model.state.clone_ref();
let nodes = model.controller.graph().nodes()?;
Expand All @@ -572,11 +574,13 @@ impl ViewUpdate {
}

/// Remove nodes from the state and return node views to be removed.
#[profile(Debug)]
fn remove_nodes(&self) -> Vec<ViewNodeId> {
self.state.update_from_controller().retain_nodes(&self.node_ids().collect())
}

/// Returns number of nodes view should create.
#[profile(Debug)]
fn count_nodes_to_add(&self) -> usize {
self.node_ids().filter(|n| self.state.view_id_of_ast_node(*n).is_none()).count()
}
Expand Down Expand Up @@ -616,6 +620,7 @@ impl ViewUpdate {
/// input for nodes where position changed.
///
/// The nodes not having views are also updated in the state.
#[profile(Debug)]
fn set_node_positions(&self) -> Vec<(ViewNodeId, Vector2)> {
self.nodes
.iter()
Expand All @@ -629,6 +634,7 @@ impl ViewUpdate {
.collect()
}

#[profile(Debug)]
fn set_node_visualizations(&self) -> Vec<(ViewNodeId, Option<visualization_view::Path>)> {
self.nodes
.iter()
Expand All @@ -640,11 +646,13 @@ impl ViewUpdate {
}

/// Remove connections from the state and return views to be removed.
#[profile(Debug)]
fn remove_connections(&self) -> Vec<ViewConnection> {
self.state.update_from_controller().retain_connections(&self.connections)
}

/// Add connections to the state and return endpoints of connections to be created in views.
#[profile(Debug)]
fn add_connections(&self) -> Vec<(EdgeEndpoint, EdgeEndpoint)> {
let ast_conns = self.connections.iter();
ast_conns
Expand All @@ -654,6 +662,7 @@ impl ViewUpdate {
.collect()
}

#[profile(Debug)]
fn node_ids(&self) -> impl Iterator<Item = AstNodeId> + '_ {
self.nodes.iter().map(controller::graph::Node::id)
}
Expand Down Expand Up @@ -699,10 +708,11 @@ impl Graph {

frp::extend! { network
update_view <- source::<()>();
update_view_once <- update_view.debounce();
// Position initialization should go before emitting `update_data` event.
update_with_gap <- view.default_y_gap_between_nodes.sample(&update_view);
update_with_gap <- view.default_y_gap_between_nodes.sample(&update_view_once);
eval update_with_gap ((gap) model.initialize_nodes_positions(*gap));
update_data <- update_view.map(f_!([model] match ViewUpdate::new(&model) {
update_data <- update_view_once.map(f_!([model] match ViewUpdate::new(&model) {
Ok(update) => Rc::new(update),
Err(err) => {
error!("Failed to update view: {err:?}");
Expand Down
2 changes: 2 additions & 0 deletions app/gui/view/graph-editor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2029,6 +2029,7 @@ impl GraphEditorModel {

/// Warning! This function does not remove connected edges. It needs to be handled by the
/// implementation.
#[profile(Debug)]
fn remove_node(&self, node_id: impl Into<NodeId>) {
let node_id = node_id.into();
self.nodes.remove(&node_id);
Expand Down Expand Up @@ -2276,6 +2277,7 @@ impl GraphEditorModel {

impl GraphEditorModel {
#[allow(missing_docs)] // FIXME[everyone] All pub functions should have docs.
#[profile(Debug)]
pub fn set_node_position(&self, node_id: impl Into<NodeId>, position: Vector2) {
let node_id = node_id.into();
if let Some(node) = self.nodes.get_cloned_ref(&node_id) {
Expand Down
1 change: 0 additions & 1 deletion lib/rust/ensogl/core/src/animation/loops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,6 @@ fn on_frame_closure(
.then(move || on_after_animations.emit(time_info))
.then(move || on_before_layout.emit(time_info))
.then(move || on_before_rendering.emit(time_info))
.then(move || drop(_profiler))
.schedule();
}
}
Expand Down

0 comments on commit 7e6a919

Please sign in to comment.