Instance tables refactor part 6: unwrap VectorData and ImageFrame from single-row to multi-row tables

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

@@ -3292,7 +3292,7 @@ mod document_message_handler_tests {
 		let document = editor.active_document();
 		let rect_bbox_before = document.metadata().bounding_box_viewport(rect_layer).unwrap();
 
-		// Moving rectangle from folder1 --> folder2
+		// Moving rectangle from folder1 to folder2
 		editor.handle_message(DocumentMessage::MoveSelectedLayersTo { parent: folder2, insert_index: 0 }).await;
 
 		// Rectangle's viewport position after moving
@@ -3304,6 +3304,10 @@ mod document_message_handler_tests {
 		let after_center = (rect_bbox_after[0] + rect_bbox_after[1]) / 2.;
 		let distance = before_center.distance(after_center);
 
-		assert!(distance < 1., "Rectangle should maintain its viewport position after moving between transformed groups");
+		assert!(
+			distance < 1.,
+			"Rectangle should maintain its viewport position after moving between transformed groups\n\
+			Before: {before_center:?}, After: {after_center:?}, Distance: {distance} (should be < 1)"
+		);
 	}
 }

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

@@ -413,13 +413,10 @@ impl<'a> ModifyInputsContext<'a> {
 	pub fn transform_set_direct(&mut self, transform: DAffine2, skip_rerender: bool, transform_node_id: Option<NodeId>) {
 		// If the Transform node didn't exist yet, create it now
 		let Some(transform_node_id) = transform_node_id.or_else(|| {
-			// Check if the transform is the identity transform and if so, don't create a new Transform node
-			if let Some((scale, angle, translation)) = (transform.matrix2.determinant() != 0.).then(|| transform.to_scale_angle_translation()) {
-				// Check if the transform is the identity transform within an epsilon
-				if scale.x.abs() < 1e-6 && scale.y.abs() < 1e-6 && angle.abs() < 1e-6 && translation.x.abs() < 1e-6 && translation.y.abs() < 1e-6 {
-					// We don't want to pollute the graph with an unnecessary Transform node, so we avoid creating and setting it by returning None
-					return None;
-				}
+			// Check if the transform is the identity transform (within an epsilon) and if so, don't create a new Transform node
+			if transform.abs_diff_eq(DAffine2::IDENTITY, 1e-6) {
+				// We don't want to pollute the graph with an unnecessary Transform node, so we avoid creating and setting it by returning None
+				return None;
 			}
 
 			// Create the Transform node
@@ -453,7 +450,7 @@ impl<'a> ModifyInputsContext<'a> {
 
 	pub fn brush_modify(&mut self, strokes: Vec<BrushStroke>) {
 		let Some(brush_node_id) = self.existing_node_id("Brush", true) else { return };
-		self.set_input_with_refresh(InputConnector::node(brush_node_id, 2), NodeInput::value(TaggedValue::BrushStrokes(strokes), false), false);
+		self.set_input_with_refresh(InputConnector::node(brush_node_id, 1), NodeInput::value(TaggedValue::BrushStrokes(strokes), false), false);
 	}
 
 	pub fn resize_artboard(&mut self, location: IVec2, dimensions: IVec2) {

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

@@ -636,7 +636,7 @@ fn static_nodes() -> Vec<DocumentNodeDefinition> {
 						.collect(),
 						..Default::default()
 					}),
-					inputs: vec![NodeInput::value(TaggedValue::ImageFrame(ImageFrameTable::one_empty_image()), true)],
+					inputs: vec![NodeInput::value(TaggedValue::ImageFrame(ImageFrameTable::default()), true)],
 					..Default::default()
 				},
 				persistent_node_metadata: DocumentNodePersistentMetadata {
@@ -893,7 +893,7 @@ fn static_nodes() -> Vec<DocumentNodeDefinition> {
 						.collect(),
 						..Default::default()
 					}),
-					inputs: vec![NodeInput::value(TaggedValue::ImageFrame(ImageFrameTable::one_empty_image()), true)],
+					inputs: vec![NodeInput::value(TaggedValue::ImageFrame(ImageFrameTable::default()), true)],
 					..Default::default()
 				},
 				persistent_node_metadata: DocumentNodePersistentMetadata {
@@ -978,7 +978,7 @@ fn static_nodes() -> Vec<DocumentNodeDefinition> {
 
 						..Default::default()
 					}),
-					inputs: vec![NodeInput::value(TaggedValue::ImageFrame(ImageFrameTable::one_empty_image()), true)],
+					inputs: vec![NodeInput::value(TaggedValue::ImageFrame(ImageFrameTable::default()), true)],
 					..Default::default()
 				},
 				persistent_node_metadata: DocumentNodePersistentMetadata {
@@ -1019,6 +1019,7 @@ fn static_nodes() -> Vec<DocumentNodeDefinition> {
 			description: Cow::Borrowed("TODO"),
 			properties: None,
 		},
+		// TODO: Remove this and just use the proto node definition directly
 		DocumentNodeDefinition {
 			identifier: "Brush",
 			category: "Raster",
@@ -1029,9 +1030,8 @@ fn static_nodes() -> Vec<DocumentNodeDefinition> {
 						nodes: vec![DocumentNode {
 							inputs: vec![
 								NodeInput::network(concrete!(ImageFrameTable<Color>), 0),
-								NodeInput::network(concrete!(ImageFrameTable<Color>), 1),
-								NodeInput::network(concrete!(Vec<graphene_core::vector::brush_stroke::BrushStroke>), 2),
-								NodeInput::network(concrete!(BrushCache), 3),
+								NodeInput::network(concrete!(Vec<graphene_core::vector::brush_stroke::BrushStroke>), 1),
+								NodeInput::network(concrete!(BrushCache), 2),
 							],
 							manual_composition: Some(concrete!(Context)),
 							implementation: DocumentNodeImplementation::ProtoNode(ProtoNodeIdentifier::new("graphene_std::brush::BrushNode")),
@@ -1044,15 +1044,14 @@ fn static_nodes() -> Vec<DocumentNodeDefinition> {
 						..Default::default()
 					}),
 					inputs: vec![
-						NodeInput::value(TaggedValue::ImageFrame(ImageFrameTable::one_empty_image()), true),
-						NodeInput::value(TaggedValue::ImageFrame(ImageFrameTable::one_empty_image()), false),
+						NodeInput::value(TaggedValue::ImageFrame(ImageFrameTable::default()), true),
 						NodeInput::value(TaggedValue::BrushStrokes(Vec::new()), false),
 						NodeInput::value(TaggedValue::BrushCache(BrushCache::new_proto()), false),
 					],
 					..Default::default()
 				},
 				persistent_node_metadata: DocumentNodePersistentMetadata {
-					input_properties: vec![("Background", "TODO").into(), ("Bounds", "TODO").into(), ("Trace", "TODO").into(), ("Cache", "TODO").into()],
+					input_properties: vec![("Background", "TODO").into(), ("Trace", "TODO").into(), ("Cache", "TODO").into()],
 					output_names: vec!["Image".to_string()],
 					network_metadata: Some(NodeNetworkMetadata {
 						persistent_metadata: NodeNetworkPersistentMetadata {
@@ -1084,7 +1083,7 @@ fn static_nodes() -> Vec<DocumentNodeDefinition> {
 			node_template: NodeTemplate {
 				document_node: DocumentNode {
 					implementation: DocumentNodeImplementation::proto("graphene_core::memo::MemoNode"),
-					inputs: vec![NodeInput::value(TaggedValue::ImageFrame(ImageFrameTable::one_empty_image()), true)],
+					inputs: vec![NodeInput::value(TaggedValue::ImageFrame(ImageFrameTable::default()), true)],
 					manual_composition: Some(concrete!(Context)),
 					..Default::default()
 				},
@@ -1103,7 +1102,7 @@ fn static_nodes() -> Vec<DocumentNodeDefinition> {
 			node_template: NodeTemplate {
 				document_node: DocumentNode {
 					implementation: DocumentNodeImplementation::proto("graphene_core::memo::ImpureMemoNode"),
-					inputs: vec![NodeInput::value(TaggedValue::ImageFrame(ImageFrameTable::one_empty_image()), true)],
+					inputs: vec![NodeInput::value(TaggedValue::ImageFrame(ImageFrameTable::default()), true)],
 					manual_composition: Some(concrete!(Context)),
 					..Default::default()
 				},
@@ -1803,7 +1802,7 @@ fn static_nodes() -> Vec<DocumentNodeDefinition> {
 						.collect(),
 						..Default::default()
 					}),
-					inputs: vec![NodeInput::value(TaggedValue::ImageFrame(ImageFrameTable::one_empty_image()), true)],
+					inputs: vec![NodeInput::value(TaggedValue::ImageFrame(ImageFrameTable::default()), true)],
 					..Default::default()
 				},
 				persistent_node_metadata: DocumentNodePersistentMetadata {
@@ -2685,7 +2684,7 @@ fn static_nodes() -> Vec<DocumentNodeDefinition> {
 // 				..Default::default()
 // 			}),
 // 			inputs: vec![
-// 				NodeInput::value(TaggedValue::ImageFrame(ImageFrameTable::one_empty_image()), true),
+// 				NodeInput::value(TaggedValue::ImageFrame(ImageFrameTable::default()), true),
 // 				NodeInput::scope("editor-api"),
 // 				NodeInput::value(TaggedValue::ImaginateController(Default::default()), false),
 // 				NodeInput::value(TaggedValue::F64(0.), false), // Remember to keep index used in `ImaginateRandom` updated with this entry's index

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

@@ -635,17 +635,17 @@ impl<'a> Selected<'a> {
 	}
 
 	pub fn apply_transformation(&mut self, transformation: DAffine2, transform_operation: Option<TransformOperation>) {
-		if !self.selected.is_empty() {
-			// TODO: Cache the result of `shallowest_unique_layers` to avoid this heavy computation every frame of movement, see https://github.com/GraphiteEditor/Graphite/pull/481
-			for layer in self.network_interface.shallowest_unique_layers(&[]) {
-				match &mut self.original_transforms {
-					OriginalTransforms::Layer(layer_transforms) => {
-						Self::transform_layer(self.network_interface.document_metadata(), layer, layer_transforms.get(&layer), transformation, self.responses)
-					}
-					OriginalTransforms::Path(path_transforms) => {
-						if let Some(initial_points) = path_transforms.get_mut(&layer) {
-							Self::transform_path(self.network_interface.document_metadata(), layer, initial_points, transformation, self.responses, transform_operation)
-						}
+		if self.selected.is_empty() {
+			return;
+		}
+
+		// TODO: Cache the result of `shallowest_unique_layers` to avoid this heavy computation every frame of movement, see https://github.com/GraphiteEditor/Graphite/pull/481
+		for layer in self.network_interface.shallowest_unique_layers(&[]) {
+			match &mut self.original_transforms {
+				OriginalTransforms::Layer(layer_transforms) => Self::transform_layer(self.network_interface.document_metadata(), layer, layer_transforms.get(&layer), transformation, self.responses),
+				OriginalTransforms::Path(path_transforms) => {
+					if let Some(initial_points) = path_transforms.get_mut(&layer) {
+						Self::transform_path(self.network_interface.document_metadata(), layer, initial_points, transformation, self.responses, transform_operation)
 					}
 				}
 			}

editor/src/messages/portfolio/portfolio_message_handler.rs

@@ -930,6 +930,23 @@ impl MessageHandler<PortfolioMessage, PortfolioMessageData<'_>> for PortfolioMes
 						document.network_interface.set_input(&InputConnector::node(*node_id, 2), old_inputs[2].clone(), network_path);
 						// We have removed the last input, so we don't add index 3
 					}
+
+					if reference == "Brush" && inputs_count == 4 {
+						let node_definition = resolve_document_node_type(reference).unwrap();
+						let new_node_template = node_definition.default_node_template();
+						let document_node = new_node_template.document_node;
+						document.network_interface.replace_implementation(node_id, network_path, document_node.implementation.clone());
+						document
+							.network_interface
+							.replace_implementation_metadata(node_id, network_path, new_node_template.persistent_node_metadata);
+
+						let old_inputs = document.network_interface.replace_inputs(node_id, document_node.inputs.clone(), network_path);
+
+						document.network_interface.set_input(&InputConnector::node(*node_id, 0), old_inputs[0].clone(), network_path);
+						// We have removed the second input ("bounds"), so we don't add index 1 and we shift the rest of the inputs down by one
+						document.network_interface.set_input(&InputConnector::node(*node_id, 1), old_inputs[2].clone(), network_path);
+						document.network_interface.set_input(&InputConnector::node(*node_id, 2), old_inputs[3].clone(), network_path);
+					}
 				}
 
 				// TODO: Eventually remove this document upgrade code

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

@@ -276,15 +276,16 @@ impl BrushToolData {
 			let Some(reference) = document.network_interface.reference(&node_id, &[]) else {
 				continue;
 			};
+
 			if *reference == Some("Brush".to_string()) && node_id != layer.to_node() {
-				let points_input = node.inputs.get(2)?;
-				let Some(TaggedValue::BrushStrokes(strokes)) = points_input.as_value() else {
-					continue;
-				};
+				let points_input = node.inputs.get(1)?;
+				let Some(TaggedValue::BrushStrokes(strokes)) = points_input.as_value() else { continue };
 				self.strokes.clone_from(strokes);
 
 				return Some(layer);
-			} else if *reference == Some("Transform".to_string()) {
+			}
+
+			if *reference == Some("Transform".to_string()) {
 				let upstream = document.metadata().upstream_transform(node_id);
 				let pivot = DAffine2::from_translation(upstream.transform_point2(get_current_normalized_pivot(&node.inputs)));
 				self.transform = pivot * get_current_transform(&node.inputs) * pivot.inverse() * self.transform;

editor/src/messages/tool/transform_layer/transform_layer_message_handler.rs

@@ -397,31 +397,28 @@ impl MessageHandler<TransformLayerMessage, TransformData<'_>> for TransformLayer
 					return;
 				}
 
-				let Some(vector_data) = selected_layers.first().and_then(|&layer| document.network_interface.compute_modified_vector(layer)) else {
-					selected.original_transforms.clear();
-					return;
-				};
-
-				if let [point] = selected_points.as_slice() {
-					if matches!(point, ManipulatorPointId::Anchor(_)) {
-						if let Some([handle1, handle2]) = point.get_handle_pair(&vector_data) {
-							let handle1_length = handle1.length(&vector_data);
-							let handle2_length = handle2.length(&vector_data);
-
-							if (handle1_length == 0. && handle2_length == 0. && !using_select_tool) || (handle1_length == f64::MAX && handle2_length == f64::MAX && !using_select_tool) {
-								// G should work for this point but not R and S
-								if matches!(transform_type, TransformType::Rotate | TransformType::Scale) {
-									selected.original_transforms.clear();
-									return;
+				if let Some(vector_data) = selected_layers.first().and_then(|&layer| document.network_interface.compute_modified_vector(layer)) {
+					if let [point] = selected_points.as_slice() {
+						if matches!(point, ManipulatorPointId::Anchor(_)) {
+							if let Some([handle1, handle2]) = point.get_handle_pair(&vector_data) {
+								let handle1_length = handle1.length(&vector_data);
+								let handle2_length = handle2.length(&vector_data);
+
+								if (handle1_length == 0. && handle2_length == 0. && !using_select_tool) || (handle1_length == f64::MAX && handle2_length == f64::MAX && !using_select_tool) {
+									// G should work for this point but not R and S
+									if matches!(transform_type, TransformType::Rotate | TransformType::Scale) {
+										selected.original_transforms.clear();
+										return;
+									}
 								}
 							}
-						}
-					} else {
-						let handle_length = point.as_handle().map(|handle| handle.length(&vector_data));
+						} else {
+							let handle_length = point.as_handle().map(|handle| handle.length(&vector_data));
 
-						if handle_length == Some(0.) {
-							selected.original_transforms.clear();
-							return;
+							if handle_length == Some(0.) {
+								selected.original_transforms.clear();
+								return;
+							}
 						}
 					}
 				}

editor/src/node_graph_executor/runtime.rs

@@ -14,6 +14,7 @@ use graphene_core::renderer::{RenderSvgSegmentList, SvgSegment};
 use graphene_core::text::FontCache;
 use graphene_core::vector::style::ViewMode;
 use graphene_std::Context;
+use graphene_std::instances::Instance;
 use graphene_std::vector::{VectorData, VectorDataTable};
 use graphene_std::wasm_application_io::{WasmApplicationIo, WasmEditorApi};
 use interpreted_executor::dynamic_executor::{DynamicExecutor, IntrospectError, ResolvedDocumentNodeTypesDelta};
@@ -293,9 +294,16 @@ impl NodeRuntime {
 				Self::process_graphic_element(&mut self.thumbnail_renders, parent_network_node_id, &io.output, responses, update_thumbnails)
 			// Insert the vector modify if we are dealing with vector data
 			} else if let Some(record) = introspected_data.downcast_ref::<IORecord<Context, VectorDataTable>>() {
-				self.vector_modify.insert(parent_network_node_id, record.output.one_instance_ref().instance.clone());
+				let default = Instance {
+					instance: VectorData::empty(),
+					..Default::default()
+				};
+				self.vector_modify.insert(
+					parent_network_node_id,
+					record.output.instance_ref_iter().next().unwrap_or_else(|| default.to_instance_ref()).instance.clone(),
+				);
 			} else {
-				log::warn!("failed to downcast monitor node output {parent_network_node_id:?}");
+				log::warn!("Failed to downcast monitor node output {parent_network_node_id:?}");
 			}
 		}
 	}

node-graph/gcore/src/application_io.rs

@@ -1,6 +1,6 @@
 use crate::instances::Instances;
 use crate::text::FontCache;
-use crate::transform::{Footprint, Transform, TransformMut};
+use crate::transform::Footprint;
 use crate::vector::style::ViewMode;
 use alloc::sync::Arc;
 use core::fmt::Debug;
@@ -37,17 +37,6 @@ impl Hash for SurfaceFrame {
 	}
 }
 
-impl Transform for SurfaceFrame {
-	fn transform(&self) -> DAffine2 {
-		self.transform
-	}
-}
-impl TransformMut for SurfaceFrame {
-	fn transform_mut(&mut self) -> &mut DAffine2 {
-		&mut self.transform
-	}
-}
-
 #[cfg(feature = "dyn-any")]
 unsafe impl StaticType for SurfaceFrame {
 	type Static = SurfaceFrame;
@@ -152,18 +141,6 @@ unsafe impl<T: 'static> StaticType for SurfaceHandleFrame<T> {
 	type Static = SurfaceHandleFrame<T>;
 }
 
-impl<T> Transform for SurfaceHandleFrame<T> {
-	fn transform(&self) -> DAffine2 {
-		self.transform
-	}
-}
-
-impl<T> TransformMut for SurfaceHandleFrame<T> {
-	fn transform_mut(&mut self) -> &mut DAffine2 {
-		&mut self.transform
-	}
-}
-
 // TODO: think about how to automatically clean up memory
 /*
 impl<'a, Surface> Drop for SurfaceHandle<'a, Surface> {