diff --git a/editor/src/messages/portfolio/document/utility_types/network_interface/editor_delta.rs b/editor/src/messages/portfolio/document/utility_types/network_interface/editor_delta.rs index ef8281df0e..e688482f14 100644 --- a/editor/src/messages/portfolio/document/utility_types/network_interface/editor_delta.rs +++ b/editor/src/messages/portfolio/document/utility_types/network_interface/editor_delta.rs @@ -23,24 +23,67 @@ pub struct ConstructedOps { pub declaration_bytes: DeclarationBytes, } -impl EditorDelta { - pub fn to_registry_deltas(&self, working: &Registry, resources: &ResourceRegistry, peer: document_graph_storage::PeerId) -> Result { - let resolver = PathResolver::new(peer); - let mut ops = Vec::new(); - let mut declaration_bytes = DeclarationBytes::new(); +pub fn construct_batch(deltas: &[EditorDelta], working: &Registry, resources: &ResourceRegistry, peer: document_graph_storage::PeerId) -> Result { + let resolver = PathResolver::new(peer); + let mut ops = Vec::new(); + let mut declaration_bytes = DeclarationBytes::new(); + let mut batch_removed_nodes = Vec::new(); + let mut batch_removed_networks = Vec::new(); + let mut batch_added_resources = HashSet::new(); + for delta in deltas { + if let EditorDelta::Graph(RuntimeDelta::RemoveNode { network_path, node_id } | RuntimeDelta::ReplaceNode { network_path, node_id, .. }) = delta { + collect_removal_closure(resolver.node_id(network_path, *node_id), working, &mut batch_removed_nodes, &mut batch_removed_networks); + } + } + batch_removed_nodes.sort(); + batch_removed_nodes.dedup(); + batch_removed_networks.sort(); + batch_removed_networks.dedup(); + + for delta in deltas { + delta.construct(working, resources, peer, &resolver, &batch_removed_nodes, &mut batch_added_resources, &mut ops, &mut declaration_bytes)?; + } + + construct_resource_removals(&batch_removed_nodes, working, &mut ops); + Ok(ConstructedOps { ops, declaration_bytes }) +} + +impl EditorDelta { + #[allow(clippy::too_many_arguments)] + fn construct( + &self, + working: &Registry, + resources: &ResourceRegistry, + peer: document_graph_storage::PeerId, + resolver: &PathResolver, + batch_removed_nodes: &[document_graph_storage::NodeId], + batch_added_resources: &mut HashSet, + ops: &mut Vec, + declaration_bytes: &mut DeclarationBytes, + ) -> Result<(), ConversionError> { match self { EditorDelta::Graph(RuntimeDelta::AddNode { network_path, node_id, node }) => { - declaration_bytes = construct_structural_additions(network_path, *node_id, node, working, resources, peer, &mut ops)?; + construct_structural_additions(network_path, *node_id, node, working, resources, peer, batch_added_resources, ops, declaration_bytes)?; } EditorDelta::Graph(RuntimeDelta::ReplaceNode { network_path, node_id, node }) => { - construct_removals(resolver.node_id(network_path, *node_id), working, &mut ops); - declaration_bytes = construct_structural_additions(network_path, *node_id, node, working, resources, peer, &mut ops)?; + construct_removals(resolver.node_id(network_path, *node_id), working, ops); + construct_structural_additions(network_path, *node_id, node, working, resources, peer, batch_added_resources, ops, declaration_bytes)?; } EditorDelta::Graph(RuntimeDelta::RemoveNode { network_path, node_id }) => { - construct_removals(resolver.node_id(network_path, *node_id), working, &mut ops); + construct_removals(resolver.node_id(network_path, *node_id), working, ops); + } + + EditorDelta::Graph(RuntimeDelta::SetVisibility { network_path, node_id, visible }) => { + ops.push(RegistryDelta::ChangeNodeAttribute { + id: resolver.node_id(network_path, *node_id), + delta: AttributeDelta { + key: node_attr::VISIBLE.to_string(), + value: (!visible).then_some(serde_json::Value::Bool(false)), + }, + }); } EditorDelta::Graph(RuntimeDelta::SetInput { @@ -65,14 +108,16 @@ impl EditorDelta { } EditorDelta::NodeMetadata { network_path, node_id, metadata } => { - construct_metadata_changes(network_path, *node_id, metadata, working, &resolver, &mut ops)?; + construct_metadata_changes(network_path, *node_id, metadata, working, resolver, ops)?; } } - Ok(ConstructedOps { ops, declaration_bytes }) + let _ = batch_removed_nodes; + Ok(()) } } +#[allow(clippy::too_many_arguments)] fn construct_structural_additions( network_path: &[NodeId], node_id: NodeId, @@ -80,12 +125,14 @@ fn construct_structural_additions( working: &Registry, resources: &ResourceRegistry, peer: document_graph_storage::PeerId, + batch_added_resources: &mut HashSet, ops: &mut Vec, -) -> Result { + declaration_bytes: &mut DeclarationBytes, +) -> Result<(), ConversionError> { let mut scoped = ScopedConversion::new(&NoMetadata, peer); let mut scratch = Registry::default(); scoped.convert_node_at(&mut scratch, network_path, node_id, node, true)?; - let declaration_bytes = scoped.finish(); + declaration_bytes.extend(scoped.finish()); let mut networks: Vec<_> = scratch.networks.iter().collect(); networks.sort_by_key(|(id, _)| **id); @@ -102,7 +149,9 @@ fn construct_structural_additions( let mut new_resources: Vec<_> = scratch.resources.iter().filter(|(id, _)| !working.resources.contains_key(id)).collect(); new_resources.sort_by_key(|(id, _)| **id); for (id, entry) in new_resources { - ops.push(RegistryDelta::AddResource { id: *id, entry: entry.clone() }); + if batch_added_resources.insert(*id) { + ops.push(RegistryDelta::AddResource { id: *id, entry: entry.clone() }); + } } let mut tagged: Vec = scratch.node_instances.values().flat_map(node_value_resource_refs).collect(); tagged.sort(); @@ -110,13 +159,15 @@ fn construct_structural_additions( for id in tagged { if !working.resources.contains_key(&id) && !scratch.resources.contains_key(&id) + && !batch_added_resources.contains(&id) && let Some(entry) = convert_resource_entry(resources, id, peer)? { + batch_added_resources.insert(id); ops.push(RegistryDelta::AddResource { id, entry }); } } - Ok(declaration_bytes) + Ok(()) } fn construct_metadata_changes( @@ -231,9 +282,11 @@ fn construct_removals(node_id: document_graph_storage::NodeId, working: &Registr snapshot: working.networks[id].clone(), }); } +} - let removed_node_set: HashSet<_> = removed_nodes.iter().copied().collect(); - let mut candidates: Vec = removed_nodes +fn construct_resource_removals(batch_removed_nodes: &[document_graph_storage::NodeId], working: &Registry, ops: &mut Vec) { + let removed_node_set: HashSet<_> = batch_removed_nodes.iter().copied().collect(); + let mut candidates: Vec = batch_removed_nodes .iter() .flat_map(|id| { let node = &working.node_instances[id]; diff --git a/editor/src/messages/portfolio/document/utility_types/network_interface/editor_delta_tests.rs b/editor/src/messages/portfolio/document/utility_types/network_interface/editor_delta_tests.rs index e51fcc4881..256c748390 100644 --- a/editor/src/messages/portfolio/document/utility_types/network_interface/editor_delta_tests.rs +++ b/editor/src/messages/portfolio/document/utility_types/network_interface/editor_delta_tests.rs @@ -1,5 +1,5 @@ use super::InputConnector; -use super::editor_delta::EditorDelta; +use super::editor_delta::{EditorDelta, construct_batch}; use super::storage_metadata::StorageMetadataView; use crate::test_utils::test_prelude::*; use document_graph_storage::delta::compute_deltas; @@ -27,9 +27,8 @@ fn convert(editor: &EditorTestUtils) -> Registry { .registry } -fn construct(editor: &EditorTestUtils, delta: &EditorDelta, working: &Registry) -> Vec { - delta - .to_registry_deltas(working, &editor.active_document().resources.registry, PEER) +fn construct(editor: &EditorTestUtils, deltas: &[EditorDelta], working: &Registry) -> Vec { + construct_batch(deltas, working, &editor.active_document().resources.registry, PEER) .expect("construction should succeed") .ops } @@ -84,7 +83,7 @@ async fn set_input_value_constructs_the_exact_diff_op() { input, }); - let constructed = construct(&editor, &delta, &working); + let constructed = construct(&editor, std::slice::from_ref(&delta), &working); let diffed = compute_deltas(&working, &convert(&editor)); assert_eq!(constructed, diffed, "A value edit should construct exactly the diff's op"); } @@ -105,7 +104,7 @@ async fn wiring_and_export_edits_construct_the_exact_diff_ops() { input_index: 1, input: wire, }); - let constructed = construct(&editor, &delta, &working); + let constructed = construct(&editor, std::slice::from_ref(&delta), &working); let diffed = compute_deltas(&working, &convert(&editor)); assert_eq!(constructed, diffed, "A wiring edit should construct exactly the diff's op"); @@ -117,7 +116,7 @@ async fn wiring_and_export_edits_construct_the_exact_diff_ops() { export_index: 0, input: export, }); - let constructed = construct(&editor, &delta, &working); + let constructed = construct(&editor, std::slice::from_ref(&delta), &working); let diffed = compute_deltas(&working, &convert(&editor)); assert_eq!(constructed, diffed, "An export edit should construct exactly the diff's op"); } @@ -148,7 +147,7 @@ async fn adding_a_node_as_structure_plus_metadata_matches_the_diff() { }, ]; - let constructed = deltas.iter().flat_map(|delta| construct(&editor, delta, &working)).collect(); + let constructed = construct(&editor, &deltas, &working); let diffed = compute_deltas(&working, &convert(&editor)); assert_same_stored_effect(&working, constructed, diffed, "node addition"); } @@ -163,12 +162,21 @@ async fn metadata_edits_construct_the_exact_diff_ops() { { let network_interface = &mut editor.active_document_mut().network_interface; network_interface.set_display_name(&node, "Renamed".to_string(), &[]); + network_interface.set_visibility(&node, &[], false); network_interface.set_locked(&node, &[], true); network_interface.set_pinned(&node, &[], true); network_interface.shift_node(&node, glam::IVec2::new(3, 5), &[]); } - let constructed = construct(&editor, &node_metadata_delta(&editor, node), &working); + let deltas = [ + node_metadata_delta(&editor, node), + EditorDelta::Graph(RuntimeDelta::SetVisibility { + network_path: Vec::new(), + node_id: node, + visible: false, + }), + ]; + let constructed = construct(&editor, &deltas, &working); let diffed = compute_deltas(&working, &convert(&editor)); assert_eq!(constructed, diffed, "Metadata edits should construct exactly the diff's attribute ops"); } @@ -227,12 +235,7 @@ async fn removing_a_nested_network_node_matches_the_diff() { } } - let without_resource_ops = |ops: Vec| { - ops.into_iter() - .filter(|op| !matches!(op, RegistryDelta::RemoveResource { .. } | RegistryDelta::AddResource { .. })) - .collect::>() - }; - let constructed = without_resource_ops(deltas.iter().flat_map(|delta| construct(&editor, delta, &working)).collect()); - let diffed = without_resource_ops(compute_deltas(&working, &convert(&editor))); + let constructed = construct(&editor, &deltas, &working); + let diffed = compute_deltas(&working, &convert(&editor)); assert_same_stored_effect(&working, constructed, diffed, "nested network removal"); } diff --git a/node-graph/graph-craft/src/runtime_delta.rs b/node-graph/graph-craft/src/runtime_delta.rs index e3eb24aff6..50dff8147e 100644 --- a/node-graph/graph-craft/src/runtime_delta.rs +++ b/node-graph/graph-craft/src/runtime_delta.rs @@ -27,4 +27,9 @@ pub enum RuntimeDelta { export_index: usize, input: NodeInput, }, + SetVisibility { + network_path: Vec, + node_id: NodeId, + visible: bool, + }, }