Code cleanup

Author: adamgerhant

editor/src/messages/layout/layout_message_handler.rs

@@ -186,7 +186,7 @@ impl LayoutMessageHandler {
 						let callback_message = (number_input.on_update.callback)(number_input);
 						responses.add(callback_message);
 					}
-					//TODO: This crashes when the cursor is in a text box, such as in the Text node, and the transform node is clicked (https://github.com/GraphiteEditor/Graphite/issues/1761)
+					// TODO: This crashes when the cursor is in a text box, such as in the Text node, and the transform node is clicked (https://github.com/GraphiteEditor/Graphite/issues/1761)
 					Value::String(str) => match str.as_str() {
 						"Increment" => responses.add((number_input.increment_callback_increase.callback)(number_input)),
 						"Decrement" => responses.add((number_input.increment_callback_decrease.callback)(number_input)),

editor/src/messages/portfolio/document/document_message_handler.rs

@@ -317,7 +317,7 @@ impl MessageHandler<DocumentMessage, DocumentMessageData<'_>> for DocumentMessag
 
 				responses.add_front(BroadcastEvent::SelectionChanged);
 				for path in self.metadata().shallowest_unique_layers(self.selected_nodes.selected_layers(self.metadata())) {
-					//Path will never include ROOT_PARENT, so this is safe
+					// Path will never include ROOT_PARENT, so this is safe
 					responses.add_front(DocumentMessage::DeleteLayer { layer: *path.last().unwrap() });
 				}
 			}
@@ -455,7 +455,7 @@ impl MessageHandler<DocumentMessage, DocumentMessageData<'_>> for DocumentMessag
 					});
 
 					// Insert node right above the folder
-					//TODO: downstream node can be none if it is the root node
+					// TODO: downstream node can be none if it is the root node
 					let (folder_downstream_node_id, folder_downstream_input_index) =
 						DocumentMessageHandler::get_downstream_node(&self.network, &self.metadata, first_unselected_parent_folder).unwrap_or((self.network.exports_metadata.0, 0));
 
@@ -481,12 +481,12 @@ impl MessageHandler<DocumentMessage, DocumentMessageData<'_>> for DocumentMessag
 				responses.add(GraphOperationMessage::NewCustomLayer {
 					id: folder_id,
 					nodes: HashMap::new(),
-					parent: parent,
+					parent,
 					insert_index: calculated_insert_index,
 					alias: String::new(),
 				});
 
-				let parent: LayerNodeIdentifier = LayerNodeIdentifier::new_unchecked(folder_id);
+				let parent = LayerNodeIdentifier::new_unchecked(folder_id);
 				responses.add(GraphOperationMessage::MoveSelectedSiblingsToChild { new_parent: parent });
 
 				responses.add(NodeGraphMessage::SelectedNodesSet { nodes: vec![folder_id] });
@@ -539,7 +539,7 @@ impl MessageHandler<DocumentMessage, DocumentMessageData<'_>> for DocumentMessag
 				if selected_layers.iter().any(|&layer| parent.ancestors(self.metadata()).any(|ancestor| ancestor == layer)) {
 					return;
 				}
-				// Artboards can only have None as the parent.
+				// Artboards can only have ROOT_PARENT as the parent.
 				if selected_layers.iter().any(|&layer| self.metadata.is_artboard(layer)) && parent != LayerNodeIdentifier::ROOT_PARENT {
 					return;
 				}
@@ -985,7 +985,7 @@ impl MessageHandler<DocumentMessage, DocumentMessageData<'_>> for DocumentMessag
 					});
 
 					// Set the primary input for the node downstream of folder to the first layer node
-					//TODO: downstream node can be none if it is the root node
+					// TODO: downstream node can be none if it is the root node. A layer group connected directly to the export cannot be ungrouped
 					let Some((downstream_node_id, downstream_input_index)) = DocumentMessageHandler::get_downstream_node(&self.network, &self.metadata, folder) else {
 						log::error!("Downstream node should always exist when moving layer");
 						continue;
@@ -1219,20 +1219,20 @@ impl DocumentMessageHandler {
 	}
 
 	pub fn deserialize_document(serialized_content: &str) -> Result<Self, EditorError> {
-		// serde_json::from_str(serialized_content).map_err(|e| EditorError::DocumentDeserialization(e.to_string()))
-		// TODO: Comment out after upgrading demo artwork
-		match serde_json::from_str::<Self>(serialized_content).map_err(|e| EditorError::DocumentDeserialization(e.to_string())) {
-			Ok(mut document) => {
-				for (_, node) in &mut document.network.nodes {
-					let node_definition = crate::messages::portfolio::document::node_graph::document_node_types::resolve_document_node_type(&node.name).unwrap();
-					let default_definition_node = node_definition.default_document_node();
-
-					node.implementation = default_definition_node.implementation.clone();
-				}
-				Ok(document)
-			}
-			Err(e) => Err(e),
-		}
+		serde_json::from_str(serialized_content).map_err(|e| EditorError::DocumentDeserialization(e.to_string()))
+		// Used for upgrading old internal networks for demo artwork nodes. Will reset all node internals for any opened file
+		// match serde_json::from_str::<Self>(serialized_content).map_err(|e| EditorError::DocumentDeserialization(e.to_string())) {
+		// 	Ok(mut document) => {
+		// 		for (_, node) in &mut document.network.nodes {
+		// 			let node_definition = crate::messages::portfolio::document::node_graph::document_node_types::resolve_document_node_type(&node.name).unwrap();
+		// 			let default_definition_node = node_definition.default_document_node();
+
+		// 			node.implementation = default_definition_node.implementation.clone();
+		// 		}
+		// 		Ok(document)
+		// 	}
+		// 	Err(e) => Err(e),
+		// }
 	}
 
 	pub fn with_name(name: String, ipp: &InputPreprocessorMessageHandler, responses: &mut VecDeque<Message>) -> Self {
@@ -1253,7 +1253,6 @@ impl DocumentMessageHandler {
 	/// Called recursively by the entry function [`serialize_root`].
 	fn serialize_structure(&self, folder: LayerNodeIdentifier, structure_section: &mut Vec<u64>, data_section: &mut Vec<u64>, path: &mut Vec<LayerNodeIdentifier>) {
 		let mut space = 0;
-
 		for layer_node in folder.children(self.metadata()) {
 			data_section.push(layer_node.to_node().0);
 			space += 1;
@@ -1307,7 +1306,7 @@ impl DocumentMessageHandler {
 	pub fn serialize_root(&self) -> RawBuffer {
 		let mut structure_section = vec![NodeId(0).0];
 		let mut data_section = Vec::new();
-		self.serialize_structure(self.metadata().root(), &mut structure_section, &mut data_section, &mut vec![]);
+		self.serialize_structure(LayerNodeIdentifier::ROOT_PARENT, &mut structure_section, &mut data_section, &mut vec![]);
 
 		// Remove the ROOT element. Prepend `L`, the length (excluding the ROOT) of the structure section (which happens to be where the ROOT element was).
 		structure_section[0] = structure_section.len() as u64 - 1;
@@ -1458,15 +1457,6 @@ impl DocumentMessageHandler {
 			})
 	}
 
-	/// When working with an insert index, deleting the layers may cause the insert index to point to a different location (if the layer being deleted was located before the insert index).
-	///
-	/// This function updates the insert index so that it points to the same place after the specified `layers` are deleted.
-	// fn update_insert_index(&self, layers: &[LayerNodeIdentifier], parent: LayerNodeIdentifier, insert_index: isize) -> usize {
-	// 	let take_amount = if insert_index < 0 { usize::MAX } else { insert_index as usize };
-	// 	let layer_ids_above = parent.children(self.metadata()).take(take_amount);
-	// 	layer_ids_above.filter(|layer_id| !layers.contains(layer_id)).count() as usize
-	// }
-
 	/// Finds the parent folder which, based on the current selections, should be the container of any newly added layers.
 	pub fn new_layer_parent(&self, include_self: bool) -> LayerNodeIdentifier {
 		self.metadata()

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

@@ -158,7 +158,7 @@ impl MessageHandler<GraphOperationMessage, GraphOperationMessageData<'_>> for Gr
 					return;
 				};
 
-				let tagged_value = ModifyInputsContext::get_input_tagged_value(document_network, &Vec::new(), node_id, &node_graph.resolved_types, input_index);
+				let tagged_value = TaggedValue::from_type(&ModifyInputsContext::get_input_type(document_network, &Vec::new(), node_id, &node_graph.resolved_types, input_index));
 
 				let mut input = NodeInput::value(tagged_value, true);
 				if let NodeInput::Value { exposed, .. } = &mut input {

editor/src/messages/portfolio/document/graph_operation/utility_types.rs

@@ -1,15 +1,11 @@
-use std::hash::{DefaultHasher, Hash, Hasher};
-
 use crate::messages::portfolio::document::node_graph::document_node_types::resolve_document_node_type;
 use crate::messages::portfolio::document::utility_types::document_metadata::{DocumentMetadata, LayerNodeIdentifier};
+use crate::messages::portfolio::document::utility_types::nodes::SelectedNodes;
 use crate::messages::prelude::*;
 
-use crate::messages::portfolio::document::utility_types::nodes::SelectedNodes;
-use bezier_rs::Subpath;
 use graph_craft::concrete;
 use graph_craft::document::value::TaggedValue;
-use graph_craft::document::Previewing;
-use graph_craft::document::{generate_uuid, DocumentNode, DocumentNodeImplementation, NodeId, NodeInput, NodeNetwork};
+use graph_craft::document::{generate_uuid, DocumentNode, DocumentNodeImplementation, NodeId, NodeInput, NodeNetwork, Previewing};
 use graphene_core::raster::{BlendMode, ImageFrame};
 use graphene_core::text::Font;
 use graphene_core::uuid::ManipulatorGroupId;
@@ -19,9 +15,11 @@ use graphene_core::Type;
 use graphene_core::{Artboard, Color};
 use graphene_std::vector::ManipulatorPointId;
 use interpreted_executor::dynamic_executor::ResolvedDocumentNodeTypes;
+use interpreted_executor::node_registry::NODE_REGISTRY;
 
+use bezier_rs::Subpath;
 use glam::{DAffine2, DVec2, IVec2};
-use interpreted_executor::node_registry::NODE_REGISTRY;
+use std::hash::{DefaultHasher, Hash, Hasher};
 
 use super::transform_utils::{self, LayerBounds};
 
@@ -48,8 +46,7 @@ pub enum VectorDataModification {
 	UpdateSubpaths { subpaths: Vec<Subpath<ManipulatorGroupId>> },
 }
 
-// TODO: Generalize for any network, rewrite as static functions since there only a few fields are used for each function, so when calling only the necessary data will
-// be provided
+// TODO: Generalize for any network, rewrite as static functions since there only a few fields are used for each function, so when calling only the necessary data will be provided
 /// NodeGraphMessage or GraphOperationMessage cannot be added in ModifyInputsContext, since the functions are called by both messages handlers
 pub struct ModifyInputsContext<'a> {
 	pub document_metadata: &'a mut DocumentMetadata,
@@ -139,7 +136,7 @@ impl<'a> ModifyInputsContext<'a> {
 		assert!(!document_network.nodes.contains_key(&id), "Creating already existing node");
 		if let Some(root_node) = document_network.get_root_node() {
 			let previous_root_node = document_network.nodes.get_mut(&root_node.id).expect("Root node should always exist");
-			//Insert whatever non artboard node previously fed into export as a child of the new node
+			// Insert whatever non artboard node previously fed into export as a child of the new node
 			let node_input_index = if new_node.is_artboard() && !previous_root_node.is_artboard() { 1 } else { 0 };
 			new_node.inputs[node_input_index] = NodeInput::node(root_node.id, root_node.output_index);
 			ModifyInputsContext::shift_upstream(document_network, root_node.id, IVec2::new(8, 0), true);
@@ -278,7 +275,7 @@ impl<'a> ModifyInputsContext<'a> {
 				self.insert_node_before(new_id, post_node_id, post_node_input_index, new_layer_node, offset);
 			};
 		} else {
-			//If post_node does not exist, then network is empty
+			// If post_node does not exist, then network is empty
 			ModifyInputsContext::insert_node_as_primary_export(self.document_network, new_id, new_layer_node);
 		}
 
@@ -462,7 +459,6 @@ impl<'a> ModifyInputsContext<'a> {
 			self.modify_new_node(name, update_input);
 		}
 
-		//self.node_graph.network.clear();
 		self.responses.add(PropertiesPanelMessage::Refresh);
 
 		if !skip_rerender {
@@ -503,7 +499,7 @@ impl<'a> ModifyInputsContext<'a> {
 		}
 	}
 
-	/// Returns a true if the network structure is updated
+	/// Returns true if the network structure is updated
 	pub fn set_input(network: &mut NodeNetwork, node_id: NodeId, input_index: usize, input: NodeInput, is_document_network: bool) -> bool {
 		let mut load_network_structure = false;
 		if let Some(node) = network.nodes.get_mut(&node_id) {
@@ -888,7 +884,7 @@ impl<'a> ModifyInputsContext<'a> {
 				can_reconnect = false;
 			} else {
 				//Disconnect input
-				let tagged_value = ModifyInputsContext::get_input_tagged_value(document_network, network_path, node_id, resolved_types, input_index);
+				let tagged_value = TaggedValue::from_type(&ModifyInputsContext::get_input_type(document_network, network_path, node_id, resolved_types, input_index));
 				let value_input = NodeInput::value(tagged_value, true);
 				nodes_to_set_input.push((node_id, input_index, Some(value_input)));
 			}
@@ -943,12 +939,13 @@ impl<'a> ModifyInputsContext<'a> {
 		true
 	}
 
+	/// Get the Type for any node id and input index. Network path is the path to the encapsulating node (including the encapsulating node), node_id is the selected node
 	pub fn get_input_type(document_network: &NodeNetwork, network_path: &Vec<NodeId>, node_id: NodeId, resolved_types: &ResolvedDocumentNodeTypes, input_index: usize) -> Type {
 		let Some(network) = document_network.nested_network(&network_path) else {
 			log::error!("Could not get network in get_tagged_value");
 			return concrete!(());
 		};
-		//TODO: Store types for all document nodes, not just the compiled proto nodes, which currently skips isolated nodes
+		// TODO: Store types for all document nodes, not just the compiled proto nodes, which currently skips isolated nodes
 		let node_id_path = &[&network_path[..], &[node_id]].concat();
 		let input_type = resolved_types.inputs.get(&graph_craft::document::Source {
 			node: node_id_path.clone(),
@@ -1037,8 +1034,4 @@ impl<'a> ModifyInputsContext<'a> {
 			get_type_from_node(node, input_index)
 		}
 	}
-	/// Get the tagged_value for any node id and input index. Network path is the path to the encapsulating node (including the encapsulating node), node_id is the selected node
-	pub fn get_input_tagged_value(document_network: &NodeNetwork, network_path: &Vec<NodeId>, node_id: NodeId, resolved_types: &ResolvedDocumentNodeTypes, input_index: usize) -> TaggedValue {
-		TaggedValue::from_type(&ModifyInputsContext::get_input_type(document_network, network_path, node_id, resolved_types, input_index))
-	}
 }