From 0d99aedbf81a4bda15a6ae5c116f32ba80ecf4c8 Mon Sep 17 00:00:00 2001 From: Thierry Berger Date: Wed, 11 Sep 2024 11:37:03 +0200 Subject: [PATCH 1/3] add an example of an incorrect graph being displayed when using different system sets --- examples/schedule_graph_child_set.rs | 35 ++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 examples/schedule_graph_child_set.rs diff --git a/examples/schedule_graph_child_set.rs b/examples/schedule_graph_child_set.rs new file mode 100644 index 0000000..02060fb --- /dev/null +++ b/examples/schedule_graph_child_set.rs @@ -0,0 +1,35 @@ +use bevy::{log::LogPlugin, prelude::*}; + +/// A set for rapier's copying bevy_rapier's Bevy components back into rapier. +#[derive(Debug, Hash, PartialEq, Eq, Clone, SystemSet)] +pub struct SystemSet1; + +/// A set for rapier's copying bevy_rapier's Bevy components back into rapier. +#[derive(Debug, Hash, PartialEq, Eq, Clone, SystemSet)] +pub struct SystemSet2; + +fn main() { + let mut app = App::new(); + app.add_plugins(DefaultPlugins.build().disable::()); + app.add_systems( + Update, + ( + system1_no_set, + ( + ( + system_in_child_set1.in_set(SystemSet1), + system_in_child_set1.in_set(SystemSet1), + ), + system_in_child_set2.in_set(SystemSet2), + ) + .chain(), + ) + .chain(), + ); + bevy_mod_debugdump::print_schedule_graph(&mut app, Update); +} + +fn system_in_child_set1() {} + +fn system_in_child_set2() {} +fn system1_no_set() {} From 49d21cac6f8b28ee8368e1838f87010ab66101e5 Mon Sep 17 00:00:00 2001 From: Thierry Berger Date: Tue, 17 Sep 2024 23:48:09 +0200 Subject: [PATCH 2/3] remove transitive dependencies for cleaner hierarchy --- src/schedule_graph/mod.rs | 64 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 63 insertions(+), 1 deletion(-) diff --git a/src/schedule_graph/mod.rs b/src/schedule_graph/mod.rs index 29de489..6e03729 100644 --- a/src/schedule_graph/mod.rs +++ b/src/schedule_graph/mod.rs @@ -14,11 +14,73 @@ use bevy_ecs::{ }; use petgraph::{prelude::DiGraphMap, Direction}; +fn incoming_nodes( + graph: &petgraph::prelude::GraphMap, + u: NodeId, +) -> Vec { + let mut res = vec![]; + res.push(u); + + for neighbor in graph.neighbors_directed(u, Direction::Incoming) { + res.push(neighbor); + res.append(&mut incoming_nodes(graph, neighbor)); + } + + res +} +fn output_nodes( + graph: &petgraph::prelude::GraphMap, + u: NodeId, +) -> Vec { + let mut res = vec![]; + + for neighbor in graph.neighbors_directed(u, Direction::Outgoing) { + res.push(neighbor); + res.append(&mut output_nodes(graph, neighbor)); + } + + res +} + +fn remove_transitive_edges( + graph: &mut petgraph::prelude::GraphMap, +) { + let mut top_nodes = HashSet::new(); + + for n in graph.nodes() { + let mut all_upper_nodes = vec![n]; + + all_upper_nodes.append(&mut incoming_nodes(graph, n)); + top_nodes.insert(all_upper_nodes.last().unwrap().clone()); + } + + for node_up in top_nodes { + let upper = incoming_nodes(graph, node_up); + let down = output_nodes(graph, node_up); + for down_node in down { + if graph.remove_edge(node_up, down_node).is_some() { + let incoming_nodes_to_down = incoming_nodes(graph, down_node); + let mut cancel_removal = true; + for n in incoming_nodes_to_down { + if upper.contains(&n) { + cancel_removal = false; + break; + } + } + if cancel_removal { + graph.add_edge(node_up, down_node, ()); + } + } + } + } +} + /// Formats the schedule into a dot graph. pub fn schedule_graph_dot(schedule: &Schedule, world: &World, settings: &Settings) -> String { let graph = schedule.graph(); let hierarchy = graph.hierarchy().graph(); - let dependency = graph.dependency().graph(); + let dependency = &mut graph.dependency().graph().clone(); + remove_transitive_edges(dependency); let included_systems_sets = included_systems_sets(graph, settings); From 5c06f653686df55791bef36b08d54249d37f888a Mon Sep 17 00:00:00 2001 From: Thierry Berger Date: Wed, 18 Sep 2024 11:50:10 +0200 Subject: [PATCH 3/3] fix implementation of transitive dependencies removal --- src/schedule_graph/mod.rs | 71 +++++++++++++++++---------------------- 1 file changed, 31 insertions(+), 40 deletions(-) diff --git a/src/schedule_graph/mod.rs b/src/schedule_graph/mod.rs index 6e03729..e3b2b80 100644 --- a/src/schedule_graph/mod.rs +++ b/src/schedule_graph/mod.rs @@ -12,31 +12,21 @@ use bevy_ecs::{ system::System, world::World, }; -use petgraph::{prelude::DiGraphMap, Direction}; - -fn incoming_nodes( - graph: &petgraph::prelude::GraphMap, - u: NodeId, -) -> Vec { - let mut res = vec![]; - res.push(u); - - for neighbor in graph.neighbors_directed(u, Direction::Incoming) { - res.push(neighbor); - res.append(&mut incoming_nodes(graph, neighbor)); - } +use petgraph::{ + prelude::DiGraphMap, + Direction::{self, Outgoing}, +}; - res -} -fn output_nodes( +fn dfs_all_neighbours_directed( graph: &petgraph::prelude::GraphMap, u: NodeId, + direction: Direction, ) -> Vec { let mut res = vec![]; - for neighbor in graph.neighbors_directed(u, Direction::Outgoing) { + for neighbor in graph.neighbors_directed(u, direction) { res.push(neighbor); - res.append(&mut output_nodes(graph, neighbor)); + res.append(&mut dfs_all_neighbours_directed(graph, neighbor, direction)); } res @@ -45,31 +35,32 @@ fn output_nodes( fn remove_transitive_edges( graph: &mut petgraph::prelude::GraphMap, ) { + // Holds all root nodes let mut top_nodes = HashSet::new(); for n in graph.nodes() { - let mut all_upper_nodes = vec![n]; - - all_upper_nodes.append(&mut incoming_nodes(graph, n)); - top_nodes.insert(all_upper_nodes.last().unwrap().clone()); - } - - for node_up in top_nodes { - let upper = incoming_nodes(graph, node_up); - let down = output_nodes(graph, node_up); - for down_node in down { - if graph.remove_edge(node_up, down_node).is_some() { - let incoming_nodes_to_down = incoming_nodes(graph, down_node); - let mut cancel_removal = true; - for n in incoming_nodes_to_down { - if upper.contains(&n) { - cancel_removal = false; - break; - } - } - if cancel_removal { - graph.add_edge(node_up, down_node, ()); - } + let mut all_upstream_nodes = vec![n]; + + all_upstream_nodes.append(&mut dfs_all_neighbours_directed( + graph, + n, + Direction::Incoming, + )); + // Save the node which is most upstream + top_nodes.insert(all_upstream_nodes.last().unwrap().clone()); + } + + let toposort = petgraph::algo::toposort(&*graph, None).unwrap(); + + for visiting in toposort { + let direct_heighbours: Vec = graph.neighbors_directed(visiting, Outgoing).collect(); + for n in direct_heighbours { + graph.remove_edge(visiting, n); + // if we still can access a neighbour with a longer path, it's a transitive dependency. + let descendants = dfs_all_neighbours_directed(&graph, visiting, Direction::Outgoing); + if !descendants.contains(&n) { + // No longer path, so we're keeping that edge. + graph.add_edge(visiting, n, ()); } } }