Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Short circuit graph simulation if all nodes are fixed #8549

Merged
merged 3 commits into from
Dec 20, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 38 additions & 8 deletions crates/viewer/re_view_graph/src/layout/provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,15 @@ pub fn update_simulation(
}

pub struct ForceLayoutProvider {
simulation: fj::Simulation,
// If all nodes are fixed, we can skip the simulation.
simulation: Option<fj::Simulation>,
pub request: LayoutRequest,
}

fn requires_simulation(request: &LayoutRequest) -> bool {
request.all_nodes().any(|(_, v)| v.fixed_position.is_none())
}
grtlr marked this conversation as resolved.
Show resolved Hide resolved

fn considered_edges(request: &LayoutRequest) -> Vec<(usize, usize)> {
let node_index: ahash::HashMap<NodeId, usize> = request
.all_nodes()
Expand All @@ -117,6 +122,13 @@ fn considered_edges(request: &LayoutRequest) -> Vec<(usize, usize)> {

impl ForceLayoutProvider {
pub fn new(request: LayoutRequest, params: &ForceLayoutParams) -> Self {
if !requires_simulation(&request) {
return Self {
simulation: None,
request,
};
}

let nodes = request.all_nodes().map(|(_, v)| fj::Node::from(v));
let radii = request
.all_nodes()
Expand All @@ -129,7 +141,7 @@ impl ForceLayoutProvider {
let simulation = update_simulation(simulation, params, edges, radii);

Self {
simulation,
simulation: Some(simulation),
request,
}
}
Expand All @@ -139,6 +151,13 @@ impl ForceLayoutProvider {
layout: &Layout,
params: &ForceLayoutParams,
) -> Self {
if !requires_simulation(&request) {
return Self {
simulation: None,
request,
};
}

let nodes = request.all_nodes().map(|(id, template)| {
let node = fj::Node::from(template);

Expand All @@ -161,24 +180,32 @@ impl ForceLayoutProvider {
let simulation = update_simulation(simulation, params, edges, radii);

Self {
simulation,
simulation: Some(simulation),
request,
}
}

fn layout(&self) -> Layout {
// We make use of the fact here that the simulation is stable, i.e. the
// order of the nodes is the same as in the `request`.
let mut positions = self.simulation.positions();
let positions: &mut dyn Iterator<Item = Pos2> = if let Some(simulation) = &self.simulation {
&mut simulation
.positions()
.map(|[x, y]| Pos2::new(x as f32, y as f32))
} else {
&mut self.request.all_nodes().map(|(_, v)| {
v.fixed_position
.expect("if there is no simulation, all nodes have to be fixed")
grtlr marked this conversation as resolved.
Show resolved Hide resolved
})
};
grtlr marked this conversation as resolved.
Show resolved Hide resolved

let mut layout = Layout::empty();

for (entity, graph) in &self.request.graphs {
let mut current_rect = Rect::NOTHING;

for (node, template) in &graph.nodes {
let [x, y] = positions.next().expect("positions has to match the layout");
let pos = Pos2::new(x as f32, y as f32);
let pos = positions.next().expect("positions has to match the layout");
grtlr marked this conversation as resolved.
Show resolved Hide resolved
let extent = Rect::from_center_size(pos, template.size);
current_rect = current_rect.union(extent);
layout.nodes.insert(*node, extent);
Expand Down Expand Up @@ -306,12 +333,15 @@ impl ForceLayoutProvider {

/// Returns `true` if finished.
pub fn tick(&mut self) -> Layout {
self.simulation.tick(1);
if let Some(simulation) = self.simulation.as_mut() {
simulation.tick(1);
}

self.layout()
}

pub fn is_finished(&self) -> bool {
self.simulation.is_finished()
self.simulation.as_ref().map_or(true, |s| s.is_finished())
}
}

Expand Down
4 changes: 4 additions & 0 deletions crates/viewer/re_view_graph/src/visualizers/nodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,10 @@ impl VisualizerSystem for NodeVisualizer {
text: label.clone(),
color,
},
(None, true) => Label::Text {
text: node.0 .0.clone(),
color,
},
_ => Label::Circle {
// Radius is negative for UI radii, but we don't handle this here.
radius: radius.unwrap_or(FALLBACK_RADIUS).abs(),
Expand Down
Loading