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

Implement incremental graph layouts #8308

Merged
merged 9 commits into from
Dec 4, 2024
23 changes: 19 additions & 4 deletions crates/viewer/re_space_view_graph/src/layout/provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,26 @@ pub struct ForceLayoutProvider {

impl ForceLayoutProvider {
pub fn new(request: LayoutRequest) -> Self {
Self::new_impl(request, None)
}

pub fn new_with_previous(request: LayoutRequest, layout: &Layout) -> Self {
Self::new_impl(request, Some(layout))
}

// TODO(grtlr): Consider consuming the old layout to avoid re-allocating the extents.
// That logic has to be revised when adding the blueprints anyways.
fn new_impl(request: LayoutRequest, layout: Option<&Layout>) -> Self {
let nodes = request.graphs.iter().flat_map(|(_, graph_template)| {
graph_template
.nodes
.iter()
.map(|n| (n.0, fj::Node::from(n.1)))
graph_template.nodes.iter().map(|n| {
let mut fj_node = fj::Node::from(n.1);
if let Some(rect) = layout.and_then(|l| l.get_node(n.0)) {
let pos = rect.center();
fj_node = fj_node.position(pos.x as f64, pos.y as f64);
}

(n.0, fj_node)
})
});

let mut node_index = ahash::HashMap::default();
Expand Down
18 changes: 14 additions & 4 deletions crates/viewer/re_space_view_graph/src/ui/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,15 +104,25 @@ impl LayoutState {
self // no op
}
// We need to recompute the layout.
Self::None | Self::Finished { .. } => {
Self::None => {
let provider = ForceLayoutProvider::new(new_request);
let layout = provider.init();

Self::InProgress { layout, provider }
}
Self::InProgress { provider, .. } if provider.request != new_request => {
let provider = ForceLayoutProvider::new(new_request);
let layout = provider.init();
Self::Finished { layout, .. } => {
let mut provider = ForceLayoutProvider::new_with_previous(new_request, &layout);
let mut layout = provider.init();
provider.tick(&mut layout);

Self::InProgress { layout, provider }
}
Self::InProgress {
layout, provider, ..
} if provider.request != new_request => {
let mut provider = ForceLayoutProvider::new_with_previous(new_request, &layout);
let mut layout = provider.init();
provider.tick(&mut layout);
Comment on lines +113 to +125
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these two match arms look like they can be lumped together:

Suggested change
Self::Finished { layout, .. } => {
let mut provider = ForceLayoutProvider::new_with_previous(new_request, &layout);
let mut layout = provider.init();
provider.tick(&mut layout);
Self::InProgress { layout, provider }
}
Self::InProgress {
layout, provider, ..
} if provider.request != new_request => {
let mut provider = ForceLayoutProvider::new_with_previous(new_request, &layout);
let mut layout = provider.init();
provider.tick(&mut layout);
Self::Finished { layout, .. } | Self::InProgress {
layout, provider, ..
} if provider.request != new_request => {
let mut provider = ForceLayoutProvider::new_with_previous(new_request, &layout);
let mut layout = provider.init();
provider.tick(&mut layout);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that's possible due to the if-guard on the Self::InProgress arm.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh ok. I didn't know there was such a restriction.


Self::InProgress { layout, provider }
}
Expand Down
61 changes: 61 additions & 0 deletions tests/python/release_checklist/check_graph_time_layout.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
from __future__ import annotations

import os
import random
from argparse import Namespace
from uuid import uuid4

import rerun as rr
import rerun.blueprint as rrb

README = """\
# Graph view

Please check the following:
* Run the graph view in an endless loop and see if how it looks good (TM).
grtlr marked this conversation as resolved.
Show resolved Hide resolved
* Try scrubbing the timeline to see how the graph layout changes over time.
grtlr marked this conversation as resolved.
Show resolved Hide resolved
"""


def log_readme() -> None:
rr.log("readme", rr.TextDocument(README, media_type=rr.MediaType.MARKDOWN), static=True)


def log_graphs() -> None:
nodes = ["root"]
edges = []

# Randomly add nodes and edges to the graph
for i in range(50):
existing = random.choice(nodes)
new_node = str(i)
nodes.append(new_node)
edges.append((existing, new_node))

rr.set_time_sequence("frame", i)
rr.log("graph", rr.GraphNodes(nodes, labels=nodes), rr.GraphEdges(edges, graph_type=rr.GraphType.Directed))

rr.send_blueprint(
rrb.Blueprint(
rrb.Grid(
rrb.GraphView(origin="graph", name="Graph"),
rrb.TextDocumentView(origin="readme", name="Instructions"),
)
)
)


def run(args: Namespace) -> None:
rr.script_setup(args, f"{os.path.basename(__file__)}", recording_id=uuid4())

log_readme()
log_graphs()


if __name__ == "__main__":
import argparse

parser = argparse.ArgumentParser(description="Interactive release checklist")
rr.script_add_args(parser)
args = parser.parse_args()
run(args)
Loading