Fix Properties panel to show selected layers/nodes

document-legacy/src/document_metadata.rs

@@ -215,7 +215,7 @@ impl DocumentMetadata {
 }
 
 fn first_child_layer<'a>(graph: &'a NodeNetwork, node: &DocumentNode) -> Option<(&'a DocumentNode, NodeId)> {
-	graph.primary_flow_from_node(Some(node.inputs[0].as_node()?)).find(|(node, _)| node.is_layer())
+	graph.upstream_flow_back_from_nodes(vec![node.inputs[0].as_node()?], true).find(|(node, _)| node.is_layer())
 }
 
 fn sibling_below<'a>(graph: &'a NodeNetwork, node: &DocumentNode) -> Option<(&'a DocumentNode, NodeId)> {
@@ -259,13 +259,13 @@ impl DocumentMetadata {
 }
 
 pub fn is_artboard(layer: LayerNodeIdentifier, network: &NodeNetwork) -> bool {
-	network.primary_flow_from_node(Some(layer.to_node())).any(|(node, _)| node.name == "Artboard")
+	network.upstream_flow_back_from_nodes(vec![layer.to_node()], true).any(|(node, _)| node.name == "Artboard")
 }
 
 pub fn is_folder(layer: LayerNodeIdentifier, network: &NodeNetwork) -> bool {
 	network.nodes.get(&layer.to_node()).and_then(|node| node.inputs.first()).is_some_and(|input| input.as_node().is_none())
 		|| network
-			.primary_flow_from_node(Some(layer.to_node()))
+			.upstream_flow_back_from_nodes(vec![layer.to_node()], true)
 			.skip(1)
 			.any(|(node, _)| node.name == "Artboard" || node.is_layer())
 }

editor/src/messages/portfolio/document/node_graph/graph_operation_message_handler.rs

@@ -45,8 +45,7 @@ impl<'a> ModifyInputsContext<'a> {
 		}
 	}
 
-	/// Get the node network from the document
+	fn new_with_layer(layer: &'a [LayerId], document: &'a mut Document, node_graph: &'a mut NodeGraphMessageHandler, responses: &'a mut VecDeque<Message>) -> Option<Self> {
-	fn new_layer(layer: &'a [LayerId], document: &'a mut Document, node_graph: &'a mut NodeGraphMessageHandler, responses: &'a mut VecDeque<Message>) -> Option<Self> {
 		let mut document = Self::new(document, node_graph, responses);
 		let Some(mut id) = layer.last().copied() else {
 			error!("Tried to modify root layer");
@@ -299,7 +298,11 @@ impl<'a> ModifyInputsContext<'a> {
 
 	/// Changes the inputs of a specific node
 	fn modify_inputs(&mut self, name: &'static str, skip_rerender: bool, update_input: impl FnOnce(&mut Vec<NodeInput>, NodeId, &DocumentMetadata)) {
-		let existing_node_id = self.network.primary_flow_from_node(self.layer_node).find(|(node, _)| node.name == name).map(|(_, id)| id);
+		let existing_node_id = self
+			.network
+			.upstream_flow_back_from_nodes(self.layer_node.map_or_else(|| self.network.outputs.iter().map(|output| output.node_id).collect(), |id| vec![id]), true)
+			.find(|(node, _)| node.name == name)
+			.map(|(_, id)| id);
 		if let Some(node_id) = existing_node_id {
 			self.modify_existing_node_inputs(node_id, update_input);
 		} else {
@@ -322,7 +325,12 @@ impl<'a> ModifyInputsContext<'a> {
 
 	/// Changes the inputs of a all of the existing instances of a node name
 	fn modify_all_node_inputs(&mut self, name: &'static str, skip_rerender: bool, mut update_input: impl FnMut(&mut Vec<NodeInput>, NodeId, &DocumentMetadata)) {
-		let existing_nodes: Vec<_> = self.network.primary_flow_from_node(self.layer_node).filter(|(node, _)| node.name == name).map(|(_, id)| id).collect();
+		let existing_nodes: Vec<_> = self
+			.network
+			.upstream_flow_back_from_nodes(self.layer_node.map_or_else(|| self.network.outputs.iter().map(|output| output.node_id).collect(), |id| vec![id]), true)
+			.filter(|(node, _)| node.name == name)
+			.map(|(_, id)| id)
+			.collect();
 		for existing_node_id in existing_nodes {
 			self.modify_existing_node_inputs(existing_node_id, &mut update_input);
 		}
@@ -516,7 +524,7 @@ impl<'a> ModifyInputsContext<'a> {
 		}
 
 		let mut delete_nodes = vec![id];
-		for (_node, id) in self.network.primary_flow_from_node(Some(id)) {
+		for (_node, id) in self.network.upstream_flow_back_from_nodes(vec![id], true) {
 			if self.outwards_links.get(&id).is_some_and(|outwards| outwards.len() == 1) {
 				delete_nodes.push(id);
 			}
@@ -528,7 +536,7 @@ impl<'a> ModifyInputsContext<'a> {
 
 		if let Some(node_id) = new_input.as_node() {
 			if let Some(shift) = self.network.nodes.get(&node_id).map(|node| deleted_position - node.metadata.position) {
-				for node_id in self.network.all_dependencies(node_id).map(|(_, id)| id).collect::<Vec<_>>() {
+				for node_id in self.network.upstream_flow_back_from_nodes(vec![node_id], false).map(|(_, id)| id).collect::<Vec<_>>() {
 					let Some(node) = self.network.nodes.get_mut(&node_id) else { continue };
 					node.metadata.position += shift;
 				}
@@ -546,19 +554,19 @@ impl MessageHandler<GraphOperationMessage, (&mut Document, &mut NodeGraphMessage
 	fn process_message(&mut self, message: GraphOperationMessage, responses: &mut VecDeque<Message>, (document, node_graph): (&mut Document, &mut NodeGraphMessageHandler)) {
 		match message {
 			GraphOperationMessage::FillSet { layer, fill } => {
-				if let Some(mut modify_inputs) = ModifyInputsContext::new_layer(&layer, document, node_graph, responses) {
+				if let Some(mut modify_inputs) = ModifyInputsContext::new_with_layer(&layer, document, node_graph, responses) {
 					modify_inputs.fill_set(fill);
 				} else {
 					responses.add(Operation::SetLayerFill { path: layer, fill });
 				}
 			}
 			GraphOperationMessage::UpdateBounds { layer, old_bounds, new_bounds } => {
-				if let Some(mut modify_inputs) = ModifyInputsContext::new_layer(&layer, document, node_graph, responses) {
+				if let Some(mut modify_inputs) = ModifyInputsContext::new_with_layer(&layer, document, node_graph, responses) {
 					modify_inputs.update_bounds(old_bounds, new_bounds);
 				}
 			}
 			GraphOperationMessage::StrokeSet { layer, stroke } => {
-				if let Some(mut modify_inputs) = ModifyInputsContext::new_layer(&layer, document, node_graph, responses) {
+				if let Some(mut modify_inputs) = ModifyInputsContext::new_with_layer(&layer, document, node_graph, responses) {
 					modify_inputs.stroke_set(stroke);
 				} else {
 					responses.add(Operation::SetLayerStroke { path: layer, stroke });
@@ -573,7 +581,7 @@ impl MessageHandler<GraphOperationMessage, (&mut Document, &mut NodeGraphMessage
 				let layer_identifier = LayerNodeIdentifier::new(*layer.last().unwrap(), &document.document_network);
 				let parent_transform = document.metadata.downstream_transform_to_viewport(layer_identifier);
 				let bounds = LayerBounds::new(document, &layer);
-				if let Some(mut modify_inputs) = ModifyInputsContext::new_layer(&layer, document, node_graph, responses) {
+				if let Some(mut modify_inputs) = ModifyInputsContext::new_with_layer(&layer, document, node_graph, responses) {
 					modify_inputs.transform_change(transform, transform_in, parent_transform, bounds, skip_rerender);
 				}
 			}
@@ -588,23 +596,23 @@ impl MessageHandler<GraphOperationMessage, (&mut Document, &mut NodeGraphMessage
 
 				let current_transform = Some(document.metadata.transform_to_viewport(layer_identifier));
 				let bounds = LayerBounds::new(document, &layer);
-				if let Some(mut modify_inputs) = ModifyInputsContext::new_layer(&layer, document, node_graph, responses) {
+				if let Some(mut modify_inputs) = ModifyInputsContext::new_with_layer(&layer, document, node_graph, responses) {
 					modify_inputs.transform_set(transform, transform_in, parent_transform, current_transform, bounds, skip_rerender);
 				}
 			}
 			GraphOperationMessage::TransformSetPivot { layer, pivot } => {
 				let bounds = LayerBounds::new(document, &layer);
-				if let Some(mut modify_inputs) = ModifyInputsContext::new_layer(&layer, document, node_graph, responses) {
+				if let Some(mut modify_inputs) = ModifyInputsContext::new_with_layer(&layer, document, node_graph, responses) {
 					modify_inputs.pivot_set(pivot, bounds);
 				}
 			}
 			GraphOperationMessage::Vector { layer, modification } => {
-				if let Some(mut modify_inputs) = ModifyInputsContext::new_layer(&layer, document, node_graph, responses) {
+				if let Some(mut modify_inputs) = ModifyInputsContext::new_with_layer(&layer, document, node_graph, responses) {
 					modify_inputs.vector_modify(modification);
 				}
 			}
 			GraphOperationMessage::Brush { layer, strokes } => {
-				if let Some(mut modify_inputs) = ModifyInputsContext::new_layer(&layer, document, node_graph, responses) {
+				if let Some(mut modify_inputs) = ModifyInputsContext::new_with_layer(&layer, document, node_graph, responses) {
 					modify_inputs.brush_modify(strokes);
 				}
 			}
@@ -690,7 +698,7 @@ impl MessageHandler<GraphOperationMessage, (&mut Document, &mut NodeGraphMessage
 				document.load_network_structure();
 			}
 			GraphOperationMessage::ResizeArtboard { id, location, dimensions } => {
-				if let Some(mut modify_inputs) = ModifyInputsContext::new_layer(&[id], document, node_graph, responses) {
+				if let Some(mut modify_inputs) = ModifyInputsContext::new_with_layer(&[id], document, node_graph, responses) {
 					modify_inputs.resize_artboard(location, dimensions);
 				}
 			}

editor/src/messages/portfolio/document/node_graph/node_graph_message_handler.rs

@@ -211,7 +211,7 @@ impl NodeGraphMessageHandler {
 	}
 
 	/// Collate the properties panel sections for a node graph
-	pub fn collate_properties(&self, context: &mut NodePropertiesContext, sections: &mut Vec<LayoutGroup>) {
+	pub fn collate_properties(&self, context: &mut NodePropertiesContext) -> Vec<LayoutGroup> {
 		let mut network = context.network;
 		let document = context.document;
 
@@ -219,19 +219,49 @@ impl NodeGraphMessageHandler {
 			network = network.nodes.get(segment).and_then(|node| node.implementation.get_network()).unwrap();
 		}
 
-		// If empty, show all nodes in the network starting with the output
+		// We want:
-		if !document.metadata.has_selected_nodes() {
+		// - If only nodes (no layers) are selected: display each node's properties
-			for (document_node, node_id) in network.primary_flow().collect::<Vec<_>>().into_iter().rev() {
+		// - If one layer is selected, and zero or more of its upstream nodes: display the properties for the layer and its upstream nodes
-				sections.push(node_properties::generate_node_properties(document_node, node_id, context));
+		// - If multiple layers are selected, or one node plus other non-upstream nodes: display nothing
-			}
-		}
-		// Show properties for all selected nodes
-		for node_id in document.metadata.selected_nodes() {
-			let Some(document_node) = network.nodes.get(node_id) else {
-				continue;
-			};
 
-			sections.push(node_properties::generate_node_properties(document_node, *node_id, context));
+		// First, we filter all the selections into layers and nodes
+		let (mut layers, mut nodes) = (Vec::new(), Vec::new());
+		for node_id in document.metadata.selected_nodes() {
+			if let Some(layer_or_node) = network.nodes.get(node_id) {
+				if layer_or_node.is_layer() {
+					layers.push(*node_id);
+				} else {
+					nodes.push(*node_id);
+				}
+			};
+		}
+
+		// Next, we decide what to display based on the number of layers and nodes selected
+		match layers.len() {
+			// If no layers are selected, show properties for all selected nodes
+			0 => nodes
+				.iter()
+				.filter_map(|node_id| network.nodes.get(node_id).map(|node| node_properties::generate_node_properties(node, *node_id, context)))
+				.collect(),
+			// If one layer is selected, filter out all selected nodes that are not upstream of it. If there are no nodes left, show properties for the layer. Otherwise, show nothing.
+			1 => {
+				let nodes_not_upstream_of_layer = nodes
+					.into_iter()
+					.filter(|&selected_node_id| !network.is_node_upstream_of_another_by_primary_flow(layers[0], selected_node_id));
+				if nodes_not_upstream_of_layer.count() > 0 {
+					return Vec::new();
+				}
+
+				// Iterate through all the upstream nodes, but stop when we reach another layer (since that's a point where we switch from horizontal to vertical flow)
+				network
+					.upstream_flow_back_from_nodes(vec![layers[0]], true)
+					.enumerate()
+					.take_while(|(i, (node, _))| if *i == 0 { true } else { !node.is_layer() })
+					.map(|(_, (node, node_id))| node_properties::generate_node_properties(node, node_id, context))
+					.collect()
+			}
+			// If multiple layers and/or nodes are selected, show nothing
+			_ => Vec::new(),
 		}
 	}
 

editor/src/messages/portfolio/document/properties_panel/utility_functions.rs

@@ -102,8 +102,6 @@ pub fn register_artwork_layer_properties(
 			}
 		}
 		LayerDataType::Layer(layer) => {
-			let mut properties_sections = Vec::new();
-
 			let mut context = NodePropertiesContext {
 				persistent_data,
 				document,
@@ -113,7 +111,7 @@ pub fn register_artwork_layer_properties(
 				executor,
 				network: &layer.network,
 			};
-			node_graph_message_handler.collate_properties(&mut context, &mut properties_sections);
+			let properties_sections = node_graph_message_handler.collate_properties(&mut context);
 
 			properties_sections
 		}
@@ -133,8 +131,8 @@ pub fn register_artwork_layer_properties(
 }
 
 pub fn register_document_graph_properties(mut context: NodePropertiesContext, node_graph_message_handler: &NodeGraphMessageHandler, document_name: &str) {
-	let mut properties_sections = Vec::new();
+	let properties_sections = node_graph_message_handler.collate_properties(&mut context);
-	node_graph_message_handler.collate_properties(&mut context, &mut properties_sections);
+
 	let options_bar = vec![LayoutGroup::Row {
 		widgets: vec![
 			IconLabel::new("File").tooltip("Document").widget_holder(),

editor/src/messages/portfolio/portfolio_message_handler.rs

@@ -193,7 +193,7 @@ impl MessageHandler<PortfolioMessage, (&InputPreprocessorMessageHandler, &Prefer
 								active_document.network(),
 								&active_document
 									.network()
-									.all_dependencies(node)
+									.upstream_flow_back_from_nodes(vec![node], false)
 									.enumerate()
 									.map(|(index, (_, node_id))| (node_id, index as NodeId))
 									.collect(),

editor/src/messages/tool/common_functionality/graph_modification_utils.rs

@@ -240,7 +240,7 @@ impl<'a> NodeGraphLayer<'a> {
 
 	/// Return an iterator up the primary flow of the layer
 	pub fn primary_layer_flow(&self) -> impl Iterator<Item = (&'a DocumentNode, u64)> {
-		self.node_graph.primary_flow_from_node(Some(self.layer_node))
+		self.node_graph.upstream_flow_back_from_nodes(vec![self.layer_node], true)
 	}
 
 	/// Does a node exist in the layer's primary flow

editor/src/messages/tool/tool_messages/brush_tool.rs

@@ -305,7 +305,7 @@ impl BrushToolData {
 		};
 
 		self.layer = Some(layer);
-		for (node, node_id) in document.network().primary_flow_from_node(Some(layer.to_node())) {
+		for (node, node_id) in document.network().upstream_flow_back_from_nodes(vec![layer.to_node()], true) {
 			if node.name == "Brush" {
 				let points_input = node.inputs.get(2)?;
 				let NodeInput::Value {

node-graph/graph-craft/src/document.rs

@@ -460,6 +460,7 @@ impl NodeNetwork {
 			previous_outputs: None,
 		}
 	}
+
 	/// A graph with just an input node
 	pub fn new_network() -> Self {
 		Self {
@@ -623,31 +624,18 @@ impl NodeNetwork {
 		self.previous_outputs.as_ref().map(|outputs| outputs.iter().any(|output| output.node_id == node_id))
 	}
 
-	/// A iterator of all nodes connected by primary inputs.
+	/// Gives an iterator to all nodes connected to the given nodes by all inputs (primary or primary + secondary depending on `only_follow_primary` choice), traversing backwards upstream starting from the given node's inputs.
-	///
+	pub fn upstream_flow_back_from_nodes(&self, node_ids: Vec<NodeId>, only_follow_primary: bool) -> impl Iterator<Item = (&DocumentNode, u64)> {
-	/// Used for the properties panel and tools.
-	pub fn primary_flow(&self) -> impl Iterator<Item = (&DocumentNode, u64)> {
 		FlowIter {
-			stack: self.outputs.iter().map(|output| output.node_id).collect(),
+			stack: node_ids,
 			network: self,
-			primary: true,
+			only_follow_primary,
 		}
 	}
 
-	pub fn primary_flow_from_node(&self, id: Option<NodeId>) -> impl Iterator<Item = (&DocumentNode, u64)> {
+	/// In the network `X -> Y -> Z`, `is_node_upstream_of_another_by_primary_flow(Z, X)` returns true.
-		FlowIter {
+	pub fn is_node_upstream_of_another_by_primary_flow(&self, node: NodeId, potentially_upstream_node: NodeId) -> bool {
-			stack: id.map_or_else(|| self.outputs.iter().map(|output| output.node_id).collect(), |id| vec![id]),
+		self.upstream_flow_back_from_nodes(vec![node], true).any(|(_, id)| id == potentially_upstream_node)
-			network: self,
-			primary: true,
-		}
-	}
-
-	pub fn all_dependencies(&self, id: NodeId) -> impl Iterator<Item = (&DocumentNode, u64)> {
-		FlowIter {
-			stack: vec![id],
-			network: self,
-			primary: false,
-		}
 	}
 
 	/// Check there are no cycles in the graph (this should never happen).
@@ -688,16 +676,19 @@ impl NodeNetwork {
 struct FlowIter<'a> {
 	stack: Vec<NodeId>,
 	network: &'a NodeNetwork,
-	primary: bool,
+	only_follow_primary: bool,
 }
 impl<'a> Iterator for FlowIter<'a> {
 	type Item = (&'a DocumentNode, NodeId);
 	fn next(&mut self) -> Option<Self::Item> {
 		loop {
 			let node_id = self.stack.pop()?;
+
 			if let Some(document_node) = self.network.nodes.get(&node_id) {
-				let inputs = document_node.inputs.iter().take(if self.primary { 1 } else { usize::MAX });
+				let take = if self.only_follow_primary { 1 } else { usize::MAX };
-				let node_ids = inputs.filter_map(|input| if let NodeInput::Node { node_id, .. } = input { Some(*node_id) } else { None });
+				let inputs = document_node.inputs.iter().take(take);
+
+				let node_ids = inputs.filter_map(|input| if let NodeInput::Node { node_id, .. } = input { Some(node_id) } else { None });
 
 				self.stack.extend(node_ids);