Skip to content

Commit

Permalink
Short circuit graph simulation if all nodes are fixed (#8549)
Browse files Browse the repository at this point in the history
### What

Title.

Wonder if we should even show the forces section in the selection panel
in this case—but that might be confusing for the user too.

<!--
Make sure the PR title and labels are set to maximize their usefulness
for the CHANGELOG,
and our `git log`.

If you have noticed any breaking changes, include them in the migration
guide.

We track various metrics at <https://build.rerun.io>.

For maintainers:
* To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.
* To deploy documentation changes immediately after merging this PR, add
the `deploy docs` label.
-->
  • Loading branch information
grtlr authored Dec 20, 2024
1 parent 7596891 commit 19e93a6
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 8 deletions.
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -6648,6 +6648,7 @@ dependencies = [
"ahash",
"egui",
"fjadra",
"itertools 0.13.0",
"nohash-hasher",
"re_chunk",
"re_data_ui",
Expand Down
1 change: 1 addition & 0 deletions crates/viewer/re_view_graph/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,5 @@ re_viewport_blueprint.workspace = true
ahash.workspace = true
egui.workspace = true
fjadra.workspace = true
itertools.workspace = true
nohash-hasher.workspace = true
52 changes: 44 additions & 8 deletions crates/viewer/re_view_graph/src/layout/provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

use egui::{Pos2, Rect, Vec2};
use fjadra::{self as fj, Simulation};
use re_log::error_once;

use crate::graph::{EdgeId, NodeId};

Expand Down Expand Up @@ -98,7 +99,8 @@ 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,
}

Expand All @@ -117,6 +119,13 @@ fn considered_edges(request: &LayoutRequest) -> Vec<(usize, usize)> {

impl ForceLayoutProvider {
pub fn new(request: LayoutRequest, params: &ForceLayoutParams) -> Self {
if request.all_nodes_fixed() {
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 +138,7 @@ impl ForceLayoutProvider {
let simulation = update_simulation(simulation, params, edges, radii);

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

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

Expand All @@ -161,24 +177,41 @@ 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 mut positions = if let Some(simulation) = &self.simulation {
itertools::Either::Left(
simulation
.positions()
.map(|[x, y]| Pos2::new(x as f32, y as f32)),
)
} else {
itertools::Either::Right(self.request.all_nodes().filter_map(|(_, v)| {
debug_assert!(
v.fixed_position.is_some(),
"if there is no simulation, all nodes should have fixed positions"
);
v.fixed_position
}))
};

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().unwrap_or_else(|| {
debug_assert!(false, "not enough positions returned for layout request");
error_once!("not enough positions returned for layout request");
Pos2::ZERO
});
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 +339,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
5 changes: 5 additions & 0 deletions crates/viewer/re_view_graph/src/layout/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,11 @@ impl LayoutRequest {
request
}

/// Returns `true` if all nodes in this request have fixed positions.
pub(super) fn all_nodes_fixed(&self) -> bool {
self.all_nodes().all(|(_, v)| v.fixed_position.is_some())
}

/// Returns all nodes from all graphs in this request.
pub(super) fn all_nodes(&self) -> impl Iterator<Item = (NodeId, &NodeTemplate)> + '_ {
self.graphs
Expand Down

0 comments on commit 19e93a6

Please sign in to comment.