Mostly fix the nested transform cage angle (except leaf layers and skew)

Author: Keavon

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

@@ -237,44 +237,51 @@ impl<'a> ModifyInputsContext<'a> {
 			}
 		})
 	}
+
 	/// Gets the node id of a node with a specific reference that is upstream from the layer node, and optionally creates it if it does not exist.
 	/// The returned node is based on the selection dots in the layer. The right most dot will always insert/access the path that flows directly into the layer.
 	/// Each dot after that represents an existing path node. If there is an existing upstream node, then it will always be returned first.
-	pub fn existing_node_id(&mut self, reference: &'static str, create_if_nonexistent: bool) -> Option<NodeId> {
+	pub fn existing_node_id(&mut self, reference_name: &'static str, create_if_nonexistent: bool) -> Option<NodeId> {
 		// Start from the layer node or export
 		let output_layer = self.get_output_layer()?;
 
-		let existing_node_id = Self::locate_existing_id(output_layer, self.network_interface, reference);
+		let existing_node_id = Self::locate_node_in_layer_chain(reference_name, output_layer, self.network_interface);
 
 		// Create a new node if the node does not exist and update its inputs
 		if create_if_nonexistent {
-			return existing_node_id.or_else(|| self.create_node(reference));
+			return existing_node_id.or_else(|| self.create_node(reference_name));
 		}
 
 		existing_node_id
 	}
 
-	/// Gets the node id of a node with a specific reference that is upstream from the layer node.
-	pub fn locate_existing_id(output_layer: LayerNodeIdentifier, network_interface: &NodeNetworkInterface, reference: &str) -> Option<NodeId> {
-		let upstream = network_interface.upstream_flow_back_from_nodes(vec![output_layer.to_node()], &[], network_interface::FlowType::HorizontalFlow);
+	/// Gets the node id of a node with a specific reference (name) that is upstream (leftward) from the layer node, but before reaching another upstream layer stack.
+	/// For example, if given a group layer, this would find a requested "Transform" or "Boolean Operation" node in its chain, between the group layer and its layer stack child contents.
+	/// It would also travel up an entire layer that's not fed by a stack until reaching the generator node, such as a "Rectangle" or "Path" layer.
+	pub fn locate_node_in_layer_chain(reference_name: &str, left_of_layer: LayerNodeIdentifier, network_interface: &NodeNetworkInterface) -> Option<NodeId> {
+		let upstream = network_interface.upstream_flow_back_from_nodes(vec![left_of_layer.to_node()], &[], network_interface::FlowType::HorizontalFlow);
 
 		// Look at all of the upstream nodes
 		for upstream_node in upstream {
 			// Check if this is the node we have been searching for.
 			if network_interface
 				.reference(&upstream_node, &[])
-				.is_some_and(|node_reference| *node_reference == Some(reference.to_string()))
+				.is_some_and(|node_reference| *node_reference == Some(reference_name.to_string()))
 			{
+				if !network_interface.is_visible(&upstream_node, &[]) {
+					continue;
+				}
+
 				return Some(upstream_node);
 			}
 
-			let is_traversal_start = |node_id: NodeId| output_layer.to_node() == node_id || network_interface.document_network().exports.iter().any(|export| export.as_node() == Some(node_id));
-
 			// Take until another layer node is found (but not the first layer node)
+			let is_traversal_start = |node_id: NodeId| left_of_layer.to_node() == node_id || network_interface.document_network().exports.iter().any(|export| export.as_node() == Some(node_id));
 			if !is_traversal_start(upstream_node) && (network_interface.is_layer(&upstream_node, &[])) {
 				return None;
 			}
 		}
+
 		None
 	}
 

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

@@ -57,7 +57,7 @@ impl OriginalTransforms {
 
 	/// Gets the transform from the most downstream transform node
 	fn get_layer_transform(layer: LayerNodeIdentifier, network_interface: &NodeNetworkInterface) -> Option<DAffine2> {
-		let transform_node_id = ModifyInputsContext::locate_existing_id(layer, network_interface, "Transform")?;
+		let transform_node_id = ModifyInputsContext::locate_node_in_layer_chain("Transform", layer, network_interface)?;
 
 		let document_node = network_interface.document_network().nodes.get(&transform_node_id)?;
 		Some(transform_utils::get_current_transform(&document_node.inputs))

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

@@ -6,7 +6,7 @@ use crate::consts::{
 	SELECTION_DRAG_ANGLE, SELECTION_TOLERANCE,
 };
 use crate::messages::input_mapper::utility_types::input_mouse::ViewportPosition;
-use crate::messages::portfolio::document::graph_operation::utility_types::TransformIn;
+use crate::messages::portfolio::document::graph_operation::utility_types::{ModifyInputsContext, TransformIn};
 use crate::messages::portfolio::document::overlays::utility_types::OverlayContext;
 use crate::messages::portfolio::document::utility_types::document_metadata::LayerNodeIdentifier;
 use crate::messages::portfolio::document::utility_types::misc::{AlignAggregate, AlignAxis, FlipAxis, GroupFolderType};
@@ -23,6 +23,7 @@ use crate::messages::tool::common_functionality::transformation_cage::*;
 use crate::messages::tool::common_functionality::{auto_panning::AutoPanning, measure};
 
 use bezier_rs::Subpath;
+use graph_craft::document::value::TaggedValue;
 use graph_craft::document::NodeId;
 use graphene_core::renderer::Quad;
 use graphene_core::text::load_face;
@@ -523,16 +524,34 @@ impl Fsm for SelectToolFsmState {
 				}
 
 				// Update bounds
-				let transform = document
+				let mut transform = document
 					.network_interface
 					.selected_nodes()
 					.selected_visible_and_unlocked_layers(&document.network_interface)
 					.find(|layer| !document.network_interface.is_artboard(&layer.to_node(), &[]))
-					.map(|layer| document.metadata().transform_to_viewport(layer));
+					.map(|layer| {
+						let transform = document.metadata().transform_to_viewport(layer);
+
+						if let Some(node) =
+							ModifyInputsContext::locate_node_in_layer_chain("Transform", layer, &document.network_interface).and_then(|node_id| document.network_interface.document_node(&node_id, &[]))
+						{
+							if let (Some(&TaggedValue::F64(angle)), Some(&TaggedValue::DVec2(skew))) = (node.inputs[2].as_value(), node.inputs[3].as_value()) {
+								// TODO: Figure out and fix why this incorrectly applies the rotation twice if it's a leaf layer (VectorData, ImageFrame, etc. rather than a group)
+								return DAffine2::from_angle(angle) * transform;
+
+								// TODO: Include skew in the transform cage, since rotation and skew are the two parts that affect the basis for the space in which the bounding box is calculated
+								// 	let mut skew_matrix = DAffine2::IDENTITY;
+								// 	skew_matrix.matrix2.x_axis[0] = skew.x.tan();
+								// 	skew_matrix.matrix2.y_axis[1] = skew.y.tan();
+							}
+						};
+
+						transform
+					})
+					.unwrap_or(DAffine2::IDENTITY);
 
-				let mut transform = transform.unwrap_or(DAffine2::IDENTITY);
-				let mut transform_tampered = false;
 				// Check if the matrix is not invertible
+				let mut transform_tampered = false;
 				if transform.matrix2.determinant() == 0. {
 					transform.matrix2 += DMat2::IDENTITY * 1e-4; // TODO: Is this the cleanest way to handle this?
 					transform_tampered = true;
@@ -549,6 +568,7 @@ impl Fsm for SelectToolFsmState {
 							.bounding_box_with_transform(layer, transform.inverse() * document.metadata().transform_to_viewport(layer))
 					})
 					.reduce(graphene_core::renderer::Quad::combine_bounds);
+
 				if let Some(bounds) = bounds {
 					let bounding_box_manager = tool_data.bounding_box_manager.get_or_insert(BoundingBoxManager::default());
 

editor/src/test_utils.rs

@@ -266,7 +266,7 @@ pub mod test_prelude {
 	pub use graph_craft::document::DocumentNode;
 	pub use graphene_core::raster::{Color, Image};
 	pub use graphene_core::{InputAccessor, InputAccessorSource};
-	pub use graphene_std::{transform::Footprint, GraphicGroup};
+	pub use graphene_std::transform::Footprint;
 
 	#[macro_export]
 	macro_rules! float_eq {

node-graph/gcore/src/graphic_element.rs

@@ -289,7 +289,8 @@ async fn layer(_: impl Ctx, mut stack: GraphicGroupTable, element: GraphicElemen
 	stack
 }
 
-#[node_macro::node(category("General"))]
+// TODO: Once we have nicely working spreadsheet tables, test this and make it nicely user-facing and move it from "Debug" to "General"
+#[node_macro::node(category("Debug"))]
 async fn concatenate<T: Clone>(
 	_: impl Ctx,
 	#[implementations(

node-graph/gcore/src/graphic_element/renderer.rs

@@ -358,7 +358,6 @@ impl GraphicElementRendered for GraphicGroupTable {
 					click_target.apply_transform(*instance.transform)
 				}
 
-				log::debug!("new_click_targets for {graphic_group_id:?}: {new_click_targets:?}");
 				all_upstream_click_targets.extend(new_click_targets);
 			}