From 9f9dd71e91d951431e0abda06ce346c14c24c673 Mon Sep 17 00:00:00 2001 From: Keavon Chambers Date: Tue, 10 Mar 2026 00:58:51 -0700 Subject: [PATCH] Fix vector drawing tool transform space handling (#3872) * Fix vector drawing tool transform space handling * Review fixes * Fix test --- .../document/graph_operation/utility_types.rs | 4 +- .../messages/portfolio/document_migration.rs | 126 ++++++++++++++++++ .../shapes/arrow_shape.rs | 44 +++--- .../common_functionality/shapes/line_shape.rs | 116 ++++++++-------- .../tool/tool_messages/freehand_tool.rs | 38 +++++- .../tool/tool_messages/gradient_tool.rs | 4 +- .../messages/tool/tool_messages/pen_tool.rs | 30 ++++- .../messages/tool/tool_messages/shape_tool.rs | 26 ++-- .../tool/tool_messages/spline_tool.rs | 58 +++++++- .../messages/tool/tool_messages/text_tool.rs | 28 ++-- editor/src/test_utils.rs | 5 +- .../nodes/vector/src/generator_nodes.rs | 25 ++-- 12 files changed, 380 insertions(+), 124 deletions(-) diff --git a/editor/src/messages/portfolio/document/graph_operation/utility_types.rs b/editor/src/messages/portfolio/document/graph_operation/utility_types.rs index cb8c4608..16c70b11 100644 --- a/editor/src/messages/portfolio/document/graph_operation/utility_types.rs +++ b/editor/src/messages/portfolio/document/graph_operation/utility_types.rs @@ -201,8 +201,8 @@ impl<'a> ModifyInputsContext<'a> { Some(NodeInput::value(TaggedValue::F64(typesetting.character_spacing), false)), Some(NodeInput::value(TaggedValue::Bool(typesetting.max_width.is_some()), false)), Some(NodeInput::value(TaggedValue::F64(typesetting.max_width.unwrap_or(100.)), false)), - Some(NodeInput::value(TaggedValue::Bool(typesetting.max_width.is_some()), false)), - Some(NodeInput::value(TaggedValue::F64(typesetting.max_width.unwrap_or(100.)), false)), + Some(NodeInput::value(TaggedValue::Bool(typesetting.max_height.is_some()), false)), + Some(NodeInput::value(TaggedValue::F64(typesetting.max_height.unwrap_or(100.)), false)), Some(NodeInput::value(TaggedValue::F64(typesetting.tilt), false)), Some(NodeInput::value(TaggedValue::TextAlign(typesetting.align), false)), ]); diff --git a/editor/src/messages/portfolio/document_migration.rs b/editor/src/messages/portfolio/document_migration.rs index e704b165..58c33c55 100644 --- a/editor/src/messages/portfolio/document_migration.rs +++ b/editor/src/messages/portfolio/document_migration.rs @@ -1708,6 +1708,132 @@ fn migrate_node(node_id: &NodeId, node: &DocumentNode, network_path: &[NodeId], document.network_interface.set_input(&InputConnector::node(*node_id, 1), old_inputs[2].clone(), network_path); } + // Migrate old Arrow node from (start, end, shaft_width, head_width, head_length) to (arrow_to, shaft_width, head_width, head_length) with a Transform node for positioning + if reference == DefinitionIdentifier::ProtoNode(graphene_std::vector_nodes::arrow::IDENTIFIER) && inputs_count == 6 { + // Read old start and end values + let start = match node.inputs.get(1)? { + NodeInput::Value { tagged_value, .. } => { + if let TaggedValue::DVec2(v) = *tagged_value.clone().into_inner() { + v + } else { + DVec2::ZERO + } + } + _ => DVec2::ZERO, + }; + let end = match node.inputs.get(2)? { + NodeInput::Value { tagged_value, .. } => { + if let TaggedValue::DVec2(v) = *tagged_value.clone().into_inner() { + v + } else { + DVec2::new(100., 0.) + } + } + _ => DVec2::new(100., 0.), + }; + + // Replace inputs with the new node definition (primary + arrow_to + shaft_width + head_width + head_length) + let mut node_template = resolve_document_node_type(&reference)?.default_node_template(); + let old_inputs = document.network_interface.replace_inputs(node_id, network_path, &mut node_template)?; + + // Preserve primary input connection + document.network_interface.set_input(&InputConnector::node(*node_id, 0), old_inputs[0].clone(), network_path); + // Set arrow_to = end - start + document + .network_interface + .set_input(&InputConnector::node(*node_id, 1), NodeInput::value(TaggedValue::DVec2(end - start), false), network_path); + // Preserve shaft_width, head_width, head_length (shifted from indices 3,4,5 to 2,3,4) + document.network_interface.set_input(&InputConnector::node(*node_id, 2), old_inputs[3].clone(), network_path); + document.network_interface.set_input(&InputConnector::node(*node_id, 3), old_inputs[4].clone(), network_path); + document.network_interface.set_input(&InputConnector::node(*node_id, 4), old_inputs[5].clone(), network_path); + + // Find downstream connection to insert Transform node + let downstream = document + .network_interface + .outward_wires(network_path) + .and_then(|wires| wires.get(&OutputConnector::node(*node_id, 0))) + .and_then(|connections| connections.first().cloned()); + + if let Some(downstream_input) = downstream { + // Create a Transform node with translation = start + let Some(transform_node_type) = resolve_network_node_type("Transform") else { + log::error!("Transform node definition not found during Arrow migration"); + return None; + }; + let mut transform_template = transform_node_type.default_node_template(); + transform_template.document_node.inputs[1] = NodeInput::value(TaggedValue::DVec2(start), false); + + let transform_id = NodeId::new(); + + // Position the Transform node to the right of the Arrow node + let arrow_position = document.network_interface.position(node_id, network_path).unwrap_or_default(); + document.network_interface.insert_node(transform_id, transform_template, network_path); + document.network_interface.shift_absolute_node_position(&transform_id, arrow_position + IVec2::new(7, 0), network_path); + document.network_interface.insert_node_between(&transform_id, &downstream_input, 0, network_path); + } + } + + // Migrate old Line node from (start, end) to (line_to) with a Transform node for positioning + if reference == DefinitionIdentifier::ProtoNode(graphene_std::vector::generator_nodes::line::IDENTIFIER) && inputs_count == 3 { + // Read old start and end values + let start = match node.inputs.get(1)? { + NodeInput::Value { tagged_value, .. } => { + if let TaggedValue::DVec2(v) = *tagged_value.clone().into_inner() { + v + } else { + DVec2::ZERO + } + } + _ => DVec2::ZERO, + }; + let end = match node.inputs.get(2)? { + NodeInput::Value { tagged_value, .. } => { + if let TaggedValue::DVec2(v) = *tagged_value.clone().into_inner() { + v + } else { + DVec2::new(100., 100.) + } + } + _ => DVec2::new(100., 100.), + }; + + // Replace inputs with the new node definition (primary + line_to) + let mut node_template = resolve_document_node_type(&reference)?.default_node_template(); + let old_inputs = document.network_interface.replace_inputs(node_id, network_path, &mut node_template)?; + + // Preserve primary input connection + document.network_interface.set_input(&InputConnector::node(*node_id, 0), old_inputs[0].clone(), network_path); + // Set line_to = end - start + document + .network_interface + .set_input(&InputConnector::node(*node_id, 1), NodeInput::value(TaggedValue::DVec2(end - start), false), network_path); + + // Find downstream connection to insert Transform node + let downstream = document + .network_interface + .outward_wires(network_path) + .and_then(|wires| wires.get(&OutputConnector::node(*node_id, 0))) + .and_then(|connections| connections.first().cloned()); + + if let Some(downstream_input) = downstream { + // Create a Transform node with translation = start + let Some(transform_node_type) = resolve_network_node_type("Transform") else { + log::error!("Transform node definition not found during Line migration"); + return None; + }; + let mut transform_template = transform_node_type.default_node_template(); + transform_template.document_node.inputs[1] = NodeInput::value(TaggedValue::DVec2(start), false); + + let transform_id = NodeId::new(); + + // Position the Transform node to the right of the Line node + let line_position = document.network_interface.position(node_id, network_path).unwrap_or_default(); + document.network_interface.insert_node(transform_id, transform_template, network_path); + document.network_interface.shift_absolute_node_position(&transform_id, line_position + IVec2::new(7, 0), network_path); + document.network_interface.insert_node_between(&transform_id, &downstream_input, 0, network_path); + } + } + // Add context features to nodes that don't have them (fine-grained context caching migration) if node.context_features == graphene_std::ContextDependencies::default() && let Some(reference) = document.network_interface.reference(node_id, network_path).clone() diff --git a/editor/src/messages/tool/common_functionality/shapes/arrow_shape.rs b/editor/src/messages/tool/common_functionality/shapes/arrow_shape.rs index 0858eddf..d62472c3 100644 --- a/editor/src/messages/tool/common_functionality/shapes/arrow_shape.rs +++ b/editor/src/messages/tool/common_functionality/shapes/arrow_shape.rs @@ -1,12 +1,13 @@ use super::shape_utility::ShapeToolModifierKey; use super::*; -use crate::messages::portfolio::document::node_graph::document_node_definitions::resolve_proto_node_type; +use crate::messages::portfolio::document::graph_operation::utility_types::TransformIn; +use crate::messages::portfolio::document::node_graph::document_node_definitions::{DefinitionIdentifier, resolve_document_node_type}; 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::network_interface::{InputConnector, NodeTemplate}; use crate::messages::prelude::*; use crate::messages::tool::common_functionality::graph_modification_utils; -use glam::DVec2; +use glam::{DAffine2, DVec2}; use graph_craft::document::NodeInput; use graph_craft::document::value::TaggedValue; use std::collections::VecDeque; @@ -15,16 +16,15 @@ use std::collections::VecDeque; pub struct Arrow; impl Arrow { - pub fn create_node(document: &DocumentMessageHandler, drag_start: DVec2, shaft_width: f64, head_width: f64, head_length: f64) -> NodeTemplate { - let node_type = resolve_proto_node_type(graphene_std::vector_nodes::arrow::IDENTIFIER).expect("Arrow node does not exist"); - let viewport_pos = document.metadata().document_to_viewport.transform_point2(drag_start); + pub fn create_node(shaft_width: f64, head_width: f64, head_length: f64) -> NodeTemplate { + let identifier = DefinitionIdentifier::ProtoNode(graphene_std::vector_nodes::arrow::IDENTIFIER); + let node_type = resolve_document_node_type(&identifier).expect("Arrow node can't be found"); node_type.node_template_input_override([ None, - Some(NodeInput::value(TaggedValue::DVec2(viewport_pos), false)), // start - Some(NodeInput::value(TaggedValue::DVec2(viewport_pos), false)), // end - Some(NodeInput::value(TaggedValue::F64(shaft_width), false)), // shaft_width - Some(NodeInput::value(TaggedValue::F64(head_width), false)), // head_width - Some(NodeInput::value(TaggedValue::F64(head_length), false)), // head_length + Some(NodeInput::value(TaggedValue::DVec2(DVec2::ZERO), false)), // arrow_to + Some(NodeInput::value(TaggedValue::F64(shaft_width), false)), // shaft_width + Some(NodeInput::value(TaggedValue::F64(head_width), false)), // head_width + Some(NodeInput::value(TaggedValue::F64(head_length), false)), // head_length ]) } @@ -40,15 +40,13 @@ impl Arrow { // Track current mouse position in viewport space tool_data.line_data.drag_current = input.mouse.position; - // Convert both points to document space (matching Line tool pattern) + // Compute arrow_to in document space let document_to_viewport = document.metadata().document_to_viewport; let start_document = tool_data.data.drag_start; - let end_document = document_to_viewport.inverse().transform_point2(tool_data.line_data.drag_current); + let end_document = document_to_viewport.inverse().transform_point2(input.mouse.position); + let arrow_to = end_document - start_document; - // Calculate length in document space for validation - let delta = end_document - start_document; - let length_document = delta.length(); - if length_document < 1e-6 { + if arrow_to.length() < 1e-6 { return; } @@ -56,14 +54,18 @@ impl Arrow { return; }; - // Update Arrow node start and end points with document space coordinates + // Update Arrow node arrow_to in document space responses.add(NodeGraphMessage::SetInput { input_connector: InputConnector::node(node_id, 1), - input: NodeInput::value(TaggedValue::DVec2(start_document), false), + input: NodeInput::value(TaggedValue::DVec2(arrow_to), false), }); - responses.add(NodeGraphMessage::SetInput { - input_connector: InputConnector::node(node_id, 2), - input: NodeInput::value(TaggedValue::DVec2(end_document), false), + let downstream = document.metadata().downstream_transform_to_viewport(layer); + let scope = downstream.inverse() * document_to_viewport; + responses.add(GraphOperationMessage::TransformSet { + layer, + transform: DAffine2::from_translation(start_document), + transform_in: TransformIn::Scope { scope }, + skip_rerender: false, }); responses.add(NodeGraphMessage::RunDocumentGraph); diff --git a/editor/src/messages/tool/common_functionality/shapes/line_shape.rs b/editor/src/messages/tool/common_functionality/shapes/line_shape.rs index d05b0bb3..839c0466 100644 --- a/editor/src/messages/tool/common_functionality/shapes/line_shape.rs +++ b/editor/src/messages/tool/common_functionality/shapes/line_shape.rs @@ -1,5 +1,6 @@ use super::shape_utility::ShapeToolModifierKey; use crate::consts::{BOUNDS_SELECT_THRESHOLD, LINE_ROTATE_SNAP_ANGLE}; +use crate::messages::portfolio::document::graph_operation::utility_types::TransformIn; use crate::messages::portfolio::document::node_graph::document_node_definitions::{DefinitionIdentifier, resolve_document_node_type}; use crate::messages::portfolio::document::overlays::utility_types::OverlayContext; use crate::messages::portfolio::document::utility_types::document_metadata::LayerNodeIdentifier; @@ -9,7 +10,7 @@ pub use crate::messages::tool::common_functionality::graph_modification_utils::N use crate::messages::tool::common_functionality::snapping::{SnapCandidatePoint, SnapConstraint, SnapData, SnapTypeConfiguration}; use crate::messages::tool::tool_messages::shape_tool::ShapeToolData; use crate::messages::tool::tool_messages::tool_prelude::*; -use glam::DVec2; +use glam::{DAffine2, DVec2}; use graph_craft::document::NodeInput; use graph_craft::document::value::TaggedValue; use std::collections::VecDeque; @@ -36,14 +37,10 @@ pub struct LineToolData { pub struct Line; impl Line { - pub fn create_node(document: &DocumentMessageHandler, drag_start: DVec2) -> NodeTemplate { + pub fn create_node() -> NodeTemplate { let identifier = DefinitionIdentifier::ProtoNode(graphene_std::vector::generator_nodes::line::IDENTIFIER); let node_type = resolve_document_node_type(&identifier).expect("Line node can't be found"); - node_type.node_template_input_override([ - None, - Some(NodeInput::value(TaggedValue::DVec2(document.metadata().document_to_viewport.transform_point2(drag_start)), false)), - Some(NodeInput::value(TaggedValue::DVec2(document.metadata().document_to_viewport.transform_point2(drag_start)), false)), - ]) + node_type.node_template_input_override([None, Some(NodeInput::value(TaggedValue::DVec2(DVec2::ZERO), false))]) } pub fn update_shape( @@ -72,15 +69,22 @@ impl Line { return; }; + // Compute line_to in document space + let line_to = document_points[1] - document_points[0]; + responses.add(NodeGraphMessage::SetInput { input_connector: InputConnector::node(node_id, 1), - input: NodeInput::value(TaggedValue::DVec2(document_points[0]), false), + input: NodeInput::value(TaggedValue::DVec2(line_to), false), }); - responses.add(NodeGraphMessage::SetInput { - input_connector: InputConnector::node(node_id, 2), - input: NodeInput::value(TaggedValue::DVec2(document_points[1]), false), + let document_to_viewport = document.metadata().document_to_viewport; + let downstream = document.metadata().downstream_transform_to_viewport(layer); + let scope = downstream.inverse() * document_to_viewport; + responses.add(GraphOperationMessage::TransformSet { + layer, + transform: DAffine2::from_translation(document_points[0]), + transform_in: TransformIn::Scope { scope }, + skip_rerender: false, }); - responses.add(NodeGraphMessage::RunDocumentGraph); } pub fn overlays(document: &DocumentMessageHandler, shape_tool_data: &mut ShapeToolData, overlay_context: &mut OverlayContext) { @@ -92,18 +96,22 @@ impl Line { let node_inputs = NodeGraphLayer::new(layer, &document.network_interface).find_node_inputs(&DefinitionIdentifier::ProtoNode(graphene_std::vector::generator_nodes::line::IDENTIFIER))?; - let (Some(&TaggedValue::DVec2(start)), Some(&TaggedValue::DVec2(end))) = (node_inputs[1].as_value(), node_inputs[2].as_value()) else { + let Some(&TaggedValue::DVec2(line_to)) = node_inputs[1].as_value() else { return None; }; - let [viewport_start, viewport_end] = [start, end].map(|point| document.metadata().transform_to_viewport(layer).transform_point2(point)); - if !start.abs_diff_eq(end, f64::EPSILON * 1000.) { + // Line goes from local origin (0,0) to line_to, positioned by the Transform node + let transform = document.metadata().transform_to_viewport(layer); + let viewport_start = transform.transform_point2(DVec2::ZERO); + let viewport_end = transform.transform_point2(line_to); + if !line_to.abs_diff_eq(DVec2::ZERO, f64::EPSILON * 1000.) { overlay_context.line(viewport_start, viewport_end, None, None); overlay_context.square(viewport_start, Some(6.), None, None); overlay_context.square(viewport_end, Some(6.), None, None); } - Some((layer, [start, end])) + // Store local-space positions for endpoint editing + Some((layer, [DVec2::ZERO, line_to])) }) .collect::>(); } @@ -177,10 +185,14 @@ pub fn clicked_on_line_endpoints(layer: LayerNodeIdentifier, document: &Document return false; }; - let (Some(&TaggedValue::DVec2(document_start)), Some(&TaggedValue::DVec2(document_end))) = (node_inputs[1].as_value(), node_inputs[2].as_value()) else { + let Some(&TaggedValue::DVec2(line_to)) = node_inputs[1].as_value() else { return false; }; + // Line goes from local origin (0,0) to line_to, positioned by the Transform node + let local_start = DVec2::ZERO; + let local_end = line_to; + let transform = document.metadata().transform_to_viewport(layer); let viewport_x = transform.transform_vector2(DVec2::X).normalize_or_zero() * BOUNDS_SELECT_THRESHOLD; let viewport_y = transform.transform_vector2(DVec2::Y).normalize_or_zero() * BOUNDS_SELECT_THRESHOLD; @@ -188,14 +200,16 @@ pub fn clicked_on_line_endpoints(layer: LayerNodeIdentifier, document: &Document let threshold_y = transform.inverse().transform_vector2(viewport_y).length(); let drag_start = input.mouse.position; - let [start, end] = [document_start, document_end].map(|point| transform.transform_point2(point)); + let [start, end] = [local_start, local_end].map(|point| transform.transform_point2(point)); let start_click = (drag_start.y - start.y).abs() < threshold_y && (drag_start.x - start.x).abs() < threshold_x; let end_click = (drag_start.y - end.y).abs() < threshold_y && (drag_start.x - end.x).abs() < threshold_x; if start_click || end_click { shape_tool_data.line_data.dragging_endpoint = Some(if end_click { LineEnd::End } else { LineEnd::Start }); - shape_tool_data.data.drag_start = if end_click { document_start } else { document_end }; + // Convert the anchor endpoint (the one NOT being dragged) to document space for drag_start + let anchor_local = if end_click { local_start } else { local_end }; + shape_tool_data.data.drag_start = document.metadata().transform_to_document(layer).transform_point2(anchor_local); shape_tool_data.line_data.editing_layer = Some(layer); return true; } @@ -210,7 +224,9 @@ mod test_line_tool { use glam::DAffine2; use graph_craft::document::value::TaggedValue; - async fn get_line_node_inputs(editor: &mut EditorTestUtils) -> Option<(DVec2, DVec2)> { + /// Get the line's document-space start and end points by reading line_to from the node + /// and computing the actual positions via the layer's transform. + async fn get_line_endpoints_document(editor: &mut EditorTestUtils) -> Option<(DVec2, DVec2)> { let document = editor.active_document(); let network_interface = &document.network_interface; @@ -219,10 +235,14 @@ mod test_line_tool { .selected_visible_and_unlocked_layers(network_interface) .filter_map(|layer| { let node_inputs = NodeGraphLayer::new(layer, network_interface).find_node_inputs(&DefinitionIdentifier::ProtoNode(graphene_std::vector::generator_nodes::line::IDENTIFIER))?; - let (Some(&TaggedValue::DVec2(start)), Some(&TaggedValue::DVec2(end))) = (node_inputs[1].as_value(), node_inputs[2].as_value()) else { + let Some(&TaggedValue::DVec2(line_to)) = node_inputs[1].as_value() else { return None; }; - Some((start, end)) + + let transform_to_doc = document.metadata().transform_to_document(layer); + let doc_start = transform_to_doc.transform_point2(DVec2::ZERO); + let doc_end = transform_to_doc.transform_point2(line_to); + Some((doc_start, doc_end)) }) .next() } @@ -232,13 +252,9 @@ mod test_line_tool { let mut editor = EditorTestUtils::create(); editor.new_document().await; editor.drag_tool(ToolType::Line, 0., 0., 100., 100., ModifierKeys::empty()).await; - if let Some((start_input, end_input)) = get_line_node_inputs(&mut editor).await { - match (start_input, end_input) { - (start_input, end_input) => { - assert!((start_input - DVec2::ZERO).length() < 1., "Start point should be near (0,0)"); - assert!((end_input - DVec2::new(100., 100.)).length() < 1., "End point should be near (100,100)"); - } - } + if let Some((start, end)) = get_line_endpoints_document(&mut editor).await { + assert!((start - DVec2::ZERO).length() < 1., "Start point should be near (0,0)"); + assert!((end - DVec2::new(100., 100.)).length() < 1., "End point should be near (100,100)"); } } @@ -250,7 +266,7 @@ mod test_line_tool { editor.handle_message(NavigationMessage::CanvasPan { delta: DVec2::new(100., 50.) }).await; editor.handle_message(NavigationMessage::CanvasTiltSet { angle_radians: 30_f64.to_radians() }).await; editor.drag_tool(ToolType::Line, 0., 0., 100., 100., ModifierKeys::empty()).await; - if let Some((start_input, end_input)) = get_line_node_inputs(&mut editor).await { + if let Some((start, end)) = get_line_endpoints_document(&mut editor).await { let document = editor.active_document(); let document_to_viewport = document.metadata().document_to_viewport; let viewport_to_document = document_to_viewport.inverse(); @@ -259,12 +275,12 @@ mod test_line_tool { let expected_end = viewport_to_document.transform_point2(DVec2::new(100., 100.)); assert!( - (start_input - expected_start).length() < 1., - "Start point should match expected document coordinates. Got {start_input:?}, expected {expected_start:?}" + (start - expected_start).length() < 1., + "Start point should match expected document coordinates. Got {start:?}, expected {expected_start:?}" ); assert!( - (end_input - expected_end).length() < 1., - "End point should match expected document coordinates. Got {end_input:?}, expected {expected_end:?}" + (end - expected_end).length() < 1., + "End point should match expected document coordinates. Got {end:?}, expected {expected_end:?}" ); } else { panic!("Line was not created successfully with transformed viewport"); @@ -276,11 +292,11 @@ mod test_line_tool { let mut editor = EditorTestUtils::create(); editor.new_document().await; editor.drag_tool(ToolType::Line, 0., 0., 100., 100., ModifierKeys::CONTROL).await; - if let Some((start_input, end_input)) = get_line_node_inputs(&mut editor).await { - let line_vec = end_input - start_input; + if let Some((start, end)) = get_line_endpoints_document(&mut editor).await { + let line_vec = end - start; let original_angle = line_vec.angle_to(DVec2::X); editor.drag_tool(ToolType::Line, 0., 0., 200., 50., ModifierKeys::CONTROL).await; - if let Some((updated_start, updated_end)) = get_line_node_inputs(&mut editor).await { + if let Some((updated_start, updated_end)) = get_line_endpoints_document(&mut editor).await { let updated_line_vec = updated_end - updated_start; let updated_angle = updated_line_vec.angle_to(DVec2::X); print!("{original_angle:?}"); @@ -299,11 +315,11 @@ mod test_line_tool { let mut editor = EditorTestUtils::create(); editor.new_document().await; editor.drag_tool(ToolType::Line, 100., 100., 200., 100., ModifierKeys::ALT).await; - if let Some((start_input, end_input)) = get_line_node_inputs(&mut editor).await { + if let Some((start, end)) = get_line_endpoints_document(&mut editor).await { let expected_start = DVec2::new(0., 100.); let expected_end = DVec2::new(200., 100.); - assert!((start_input - expected_start).length() < 1., "Start point should be near (0, 100)"); - assert!((end_input - expected_end).length() < 1., "End point should be near (200, 100)"); + assert!((start - expected_start).length() < 1., "Start point should be near (0, 100)"); + assert!((end - expected_end).length() < 1., "End point should be near (200, 100)"); } } @@ -312,17 +328,13 @@ mod test_line_tool { let mut editor = EditorTestUtils::create(); editor.new_document().await; editor.drag_tool(ToolType::Line, 100., 100., 150., 120., ModifierKeys::ALT | ModifierKeys::SHIFT).await; - if let Some((start_input, end_input)) = get_line_node_inputs(&mut editor).await { - match (start_input, end_input) { - (start_input, end_input) => { - let line_vec = end_input - start_input; - let angle_radians = line_vec.angle_to(DVec2::X); - let angle_degrees = angle_radians.to_degrees(); - let nearest_angle = (angle_degrees / 15.).round() * 15.; + if let Some((start, end)) = get_line_endpoints_document(&mut editor).await { + let line_vec = end - start; + let angle_radians = line_vec.angle_to(DVec2::X); + let angle_degrees = angle_radians.to_degrees(); + let nearest_angle = (angle_degrees / 15.).round() * 15.; - assert!((angle_degrees - nearest_angle).abs() < 1., "Angle should snap to the nearest 15 degrees"); - } - } + assert!((angle_degrees - nearest_angle).abs() < 1., "Angle should snap to the nearest 15 degrees"); } } @@ -345,9 +357,9 @@ mod test_line_tool { editor.drag_tool(ToolType::Line, 50., 50., 150., 150., ModifierKeys::empty()).await; - let (start_input, end_input) = get_line_node_inputs(&mut editor).await.expect("Line was not created successfully within transformed artboard"); + let (start, end) = get_line_endpoints_document(&mut editor).await.expect("Line was not created successfully within transformed artboard"); // The line should still be diagonal with equal change in x and y - let line_vector = end_input - start_input; + let line_vector = end - start; // Verifying the line is approximately 100*sqrt(2) units in length (diagonal of 100x100 square) let line_length = line_vector.length(); assert!( diff --git a/editor/src/messages/tool/tool_messages/freehand_tool.rs b/editor/src/messages/tool/tool_messages/freehand_tool.rs index f6fc27ed..27d1792d 100644 --- a/editor/src/messages/tool/tool_messages/freehand_tool.rs +++ b/editor/src/messages/tool/tool_messages/freehand_tool.rs @@ -1,5 +1,6 @@ use super::tool_prelude::*; use crate::consts::DEFAULT_STROKE_WIDTH; +use crate::messages::portfolio::document::graph_operation::utility_types::TransformIn; use crate::messages::portfolio::document::node_graph::document_node_definitions::resolve_network_node_type; use crate::messages::portfolio::document::overlays::utility_functions::path_endpoint_overlays; use crate::messages::portfolio::document::overlays::utility_types::OverlayContext; @@ -219,6 +220,9 @@ struct FreehandToolData { dragged: bool, weight: f64, layer: Option, + /// Viewport-space start position for newly created layers, used to compute local-space + /// positions before the deferred TransformSet has been reflected in metadata. + new_layer_viewport_start: Option, } impl Fsm for FreehandToolFsmState { @@ -255,6 +259,7 @@ impl Fsm for FreehandToolFsmState { tool_data.dragged = false; tool_data.end_point = None; tool_data.weight = tool_options.line_weight; + tool_data.new_layer_viewport_start = None; // Extend an endpoint of the selected path let selected_nodes = document.network_interface.selected_nodes(); @@ -295,13 +300,42 @@ impl Fsm for FreehandToolFsmState { tool_options.stroke.apply_stroke(tool_data.weight, layer, responses); tool_options.fill.apply_fill(layer, responses); tool_data.layer = Some(layer); + tool_data.new_layer_viewport_start = Some(input.mouse.position); + + // Position the layer at the initial mouse position via Transform + responses.add(DeferMessage::AfterGraphRun { + messages: vec![ + GraphOperationMessage::TransformSet { + layer, + transform: DAffine2::from_translation(input.mouse.position), + transform_in: TransformIn::Viewport, + skip_rerender: false, + } + .into(), + NodeGraphMessage::RunDocumentGraph.into(), + ], + }); FreehandToolFsmState::Drawing } (FreehandToolFsmState::Drawing, FreehandToolMessage::PointerMove) => { if let Some(layer) = tool_data.layer { let transform = document.metadata().transform_to_viewport(layer); - let position = transform.inverse().transform_point2(input.mouse.position); + + // For newly created layers, the deferred TransformSet may not yet be reflected + // in the metadata, so compute local position from the known viewport start. + // Once the metadata catches up (origin maps to start), switch to using it so + // that mid-stroke pan/tilt/zoom works correctly. + if let Some(start) = tool_data.new_layer_viewport_start + && transform.transform_point2(DVec2::ZERO).abs_diff_eq(start, 1e-5) + { + tool_data.new_layer_viewport_start = None; + } + let position = if let Some(start) = tool_data.new_layer_viewport_start { + input.mouse.position - start + } else { + transform.inverse().transform_point2(input.mouse.position) + }; extend_path_with_next_segment(tool_data, position, true, responses); } @@ -317,6 +351,7 @@ impl Fsm for FreehandToolFsmState { tool_data.end_point = None; tool_data.layer = None; + tool_data.new_layer_viewport_start = None; FreehandToolFsmState::Ready } @@ -324,6 +359,7 @@ impl Fsm for FreehandToolFsmState { responses.add(DocumentMessage::AbortTransaction); tool_data.layer = None; tool_data.end_point = None; + tool_data.new_layer_viewport_start = None; FreehandToolFsmState::Ready } diff --git a/editor/src/messages/tool/tool_messages/gradient_tool.rs b/editor/src/messages/tool/tool_messages/gradient_tool.rs index 8f16b2a2..f1823618 100644 --- a/editor/src/messages/tool/tool_messages/gradient_tool.rs +++ b/editor/src/messages/tool/tool_messages/gradient_tool.rs @@ -1187,8 +1187,8 @@ impl Fsm for GradientToolFsmState { } // Convert drag_start from document space to effective viewport space - let d2v = document.metadata().document_to_viewport; - let drag_start_viewport = d2v.transform_point2(tool_data.drag_start) + tool_data.auto_pan_shift; + let document_to_viewport = document.metadata().document_to_viewport; + let drag_start_viewport = document_to_viewport.transform_point2(tool_data.drag_start) + tool_data.auto_pan_shift; tool_data.auto_pan_shift = DVec2::ZERO; selected_gradient.update_gradient( diff --git a/editor/src/messages/tool/tool_messages/pen_tool.rs b/editor/src/messages/tool/tool_messages/pen_tool.rs index b83bda18..954272c2 100644 --- a/editor/src/messages/tool/tool_messages/pen_tool.rs +++ b/editor/src/messages/tool/tool_messages/pen_tool.rs @@ -1,6 +1,7 @@ use super::tool_prelude::*; use crate::consts::{COLOR_OVERLAY_BLUE, COLOR_OVERLAY_BLUE_05, DEFAULT_STROKE_WIDTH, HIDE_HANDLE_DISTANCE, LINE_ROTATE_SNAP_ANGLE, SEGMENT_OVERLAY_SIZE}; use crate::messages::input_mapper::utility_types::input_mouse::MouseKeys; +use crate::messages::portfolio::document::graph_operation::utility_types::TransformIn; use crate::messages::portfolio::document::node_graph::document_node_definitions::resolve_network_node_type; use crate::messages::portfolio::document::overlays::utility_functions::path_overlays; use crate::messages::portfolio::document::overlays::utility_types::{DrawHandles, OverlayContext}; @@ -1297,9 +1298,34 @@ impl PenToolData { self.prior_segments = None; responses.add(NodeGraphMessage::SelectedNodesSet { nodes: vec![layer.to_node()] }); - // It is necessary to defer this until the transform of the layer can be accurately computed (quite hacky) + // Set up the first point at local origin (0,0) and position the layer at the viewport location via Transform + let id = PointId::generate(); + self.add_point(LastPoint { + id, + pos: DVec2::ZERO, + in_segment: None, + handle_start: DVec2::ZERO, + }); + self.next_point = DVec2::ZERO; + self.next_handle_start = DVec2::ZERO; + self.handle_end = None; + + // Defer the transform setup and point insertion until after the layer is created responses.add(DeferMessage::AfterGraphRun { - messages: vec![PenToolMessage::AddPointLayerPosition { layer, viewport: viewport_vec }.into()], + messages: vec![ + GraphOperationMessage::TransformSet { + layer, + transform: DAffine2::from_translation(viewport_vec), + transform_in: TransformIn::Viewport, + skip_rerender: false, + } + .into(), + GraphOperationMessage::Vector { + layer, + modification_type: VectorModificationType::InsertPoint { id, position: DVec2::ZERO }, + } + .into(), + ], }); responses.add(NodeGraphMessage::RunDocumentGraph); } diff --git a/editor/src/messages/tool/tool_messages/shape_tool.rs b/editor/src/messages/tool/tool_messages/shape_tool.rs index dea5418d..f6a47e49 100644 --- a/editor/src/messages/tool/tool_messages/shape_tool.rs +++ b/editor/src/messages/tool/tool_messages/shape_tool.rs @@ -928,14 +928,8 @@ impl Fsm for ShapeToolFsmState { ShapeType::Grid => Grid::create_node(tool_options.grid_type), ShapeType::Rectangle => Rectangle::create_node(), ShapeType::Ellipse => Ellipse::create_node(), - ShapeType::Arrow => Arrow::create_node( - document, - tool_data.data.drag_start, - tool_options.arrow_shaft_width, - tool_options.arrow_head_width, - tool_options.arrow_head_length, - ), - ShapeType::Line => Line::create_node(document, tool_data.data.drag_start), + ShapeType::Arrow => Arrow::create_node(tool_options.arrow_shaft_width, tool_options.arrow_head_width, tool_options.arrow_head_length), + ShapeType::Line => Line::create_node(), }; let nodes = vec![(NodeId(0), node)]; @@ -956,12 +950,28 @@ impl Fsm for ShapeToolFsmState { tool_options.fill.apply_fill(layer, defered_responses); } ShapeType::Arrow => { + let viewport_drag_start = tool_data.data.viewport_drag_start(document); + defered_responses.add(GraphOperationMessage::TransformSet { + layer, + transform: DAffine2::from_scale_angle_translation(DVec2::ONE, 0., viewport_drag_start), + transform_in: TransformIn::Viewport, + skip_rerender: false, + }); + tool_data.line_data.weight = tool_options.line_weight; tool_data.line_data.editing_layer = Some(layer); tool_options.stroke.apply_stroke(tool_options.line_weight, layer, defered_responses); tool_options.fill.apply_fill(layer, defered_responses); } ShapeType::Line => { + let viewport_drag_start = tool_data.data.viewport_drag_start(document); + defered_responses.add(GraphOperationMessage::TransformSet { + layer, + transform: DAffine2::from_scale_angle_translation(DVec2::ONE, 0., viewport_drag_start), + transform_in: TransformIn::Viewport, + skip_rerender: false, + }); + tool_data.line_data.weight = tool_options.line_weight; tool_data.line_data.editing_layer = Some(layer); tool_options.stroke.apply_stroke(tool_options.line_weight, layer, defered_responses); diff --git a/editor/src/messages/tool/tool_messages/spline_tool.rs b/editor/src/messages/tool/tool_messages/spline_tool.rs index b5ef6c6b..62e9b8e6 100644 --- a/editor/src/messages/tool/tool_messages/spline_tool.rs +++ b/editor/src/messages/tool/tool_messages/spline_tool.rs @@ -1,6 +1,7 @@ use super::tool_prelude::*; use crate::consts::{DEFAULT_STROKE_WIDTH, DRAG_THRESHOLD, PATH_JOIN_THRESHOLD, SNAP_POINT_TOLERANCE}; use crate::messages::input_mapper::utility_types::input_mouse::MouseKeys; +use crate::messages::portfolio::document::graph_operation::utility_types::TransformIn; use crate::messages::portfolio::document::node_graph::document_node_definitions::{resolve_network_node_type, resolve_proto_node_type}; use crate::messages::portfolio::document::overlays::utility_functions::path_endpoint_overlays; use crate::messages::portfolio::document::overlays::utility_types::OverlayContext; @@ -255,11 +256,15 @@ struct SplineToolData { merge_endpoints: Vec<(EndpointPosition, PointId)>, snap_manager: SnapManager, auto_panning: AutoPanning, + /// Viewport-space start position for newly created layers, used to compute local-space + /// positions before the deferred TransformSet has been reflected in metadata. + new_layer_viewport_start: Option, } impl SplineToolData { fn cleanup(&mut self) { self.current_layer = None; + self.new_layer_viewport_start = None; self.merge_layers = HashSet::new(); self.merge_endpoints = Vec::new(); self.preview_point = None; @@ -270,9 +275,7 @@ impl SplineToolData { /// Get the snapped point while ignoring current layer fn snapped_point(&mut self, document: &DocumentMessageHandler, input: &InputPreprocessorMessageHandler, viewport: &ViewportMessageHandler) -> SnappedPoint { - let metadata = document.metadata(); - let transform = self.current_layer.map_or(metadata.document_to_viewport, |layer| metadata.transform_to_viewport(layer)); - let point = SnapCandidatePoint::handle(transform.inverse().transform_point2(input.mouse.position)); + let point = SnapCandidatePoint::handle(document.metadata().document_to_viewport.inverse().transform_point2(input.mouse.position)); let ignore = if let Some(layer) = self.current_layer { vec![layer] } else { vec![] }; let snap_data = SnapData::ignore(document, input, viewport, &ignore); self.snap_manager.free_snap(&snap_data, &point, SnapTypeConfiguration::default()) @@ -400,6 +403,21 @@ impl Fsm for SplineToolFsmState { tool_options.stroke.apply_stroke(tool_data.weight, layer, responses); tool_options.fill.apply_fill(layer, responses); tool_data.current_layer = Some(layer); + tool_data.new_layer_viewport_start = Some(viewport_vec); + + // Position the layer at the initial mouse position via Transform + responses.add(DeferMessage::AfterGraphRun { + messages: vec![ + GraphOperationMessage::TransformSet { + layer, + transform: DAffine2::from_translation(viewport_vec), + transform_in: TransformIn::Viewport, + skip_rerender: false, + } + .into(), + NodeGraphMessage::RunDocumentGraph.into(), + ], + }); SplineToolFsmState::Drawing } @@ -409,10 +427,25 @@ impl Fsm for SplineToolFsmState { tool_data.extend = false; return SplineToolFsmState::Drawing; } - if tool_data.current_layer.is_none() { + let Some(layer) = tool_data.current_layer else { return SplineToolFsmState::Ready; }; - tool_data.next_point = tool_data.snapped_point(document, input, viewport).snapped_point_document; + + // Convert snapped document-space position to layer-local space + let snapped_document = tool_data.snapped_point(document, input, viewport).snapped_point_document; + let document_to_viewport = document.metadata().document_to_viewport; + let viewport_pos = document_to_viewport.transform_point2(snapped_document); + + // For newly created layers, the deferred TransformSet may not yet be reflected + // in the metadata, so compute local position from the known viewport start. + tool_data.next_point = if let Some(start) = tool_data.new_layer_viewport_start { + viewport_pos - start + } else { + let transform = document.metadata().transform_to_viewport(layer); + transform.inverse().transform_point2(viewport_pos) + }; + tool_data.new_layer_viewport_start = None; + if tool_data.points.last().is_none_or(|last_pos| last_pos.1.distance(tool_data.next_point) > DRAG_THRESHOLD) { let preview_point = tool_data.preview_point; extend_spline(tool_data, false, responses); @@ -430,13 +463,24 @@ impl Fsm for SplineToolFsmState { let ignore = |cp: PointId| tool_data.preview_point.is_some_and(|pp| pp == cp) || tool_data.points.last().is_some_and(|(ep, _)| *ep == cp); let join_point = closest_point(document, input.mouse.position, PATH_JOIN_THRESHOLD, vec![layer].into_iter(), ignore); - // Endpoints snapping + // Endpoints snapping - closest_point returns local-space positions if let Some((_, _, point)) = join_point { tool_data.next_point = point; tool_data.snap_manager.clear_indicator(); } else { + // Convert snapped document-space position to layer-local space let snapped_point = tool_data.snapped_point(document, input, viewport); - tool_data.next_point = snapped_point.snapped_point_document; + let document_to_viewport = document.metadata().document_to_viewport; + let viewport_pos = document_to_viewport.transform_point2(snapped_point.snapped_point_document); + + // For newly created layers, the deferred TransformSet may not yet be reflected + // in the metadata, so compute local position from the known viewport start. + tool_data.next_point = if let Some(start) = tool_data.new_layer_viewport_start { + viewport_pos - start + } else { + let transform = document.metadata().transform_to_viewport(layer); + transform.inverse().transform_point2(viewport_pos) + }; tool_data.snap_manager.update_indicator(snapped_point); } diff --git a/editor/src/messages/tool/tool_messages/text_tool.rs b/editor/src/messages/tool/tool_messages/text_tool.rs index c2a35bb6..c5471f0b 100644 --- a/editor/src/messages/tool/tool_messages/text_tool.rs +++ b/editor/src/messages/tool/tool_messages/text_tool.rs @@ -490,25 +490,31 @@ impl TextToolData { }); responses.add(GraphOperationMessage::FillSet { layer: self.layer, - fill: if editing_text.color.is_some() { - Fill::Solid(editing_text.color.unwrap().to_gamma_srgb()) - } else { - Fill::None - }, - }); - responses.add(GraphOperationMessage::TransformSet { - layer: self.layer, - transform: editing_text.transform, - transform_in: TransformIn::Viewport, - skip_rerender: true, + fill: if let Some(color) = editing_text.color { Fill::Solid(color.to_gamma_srgb()) } else { Fill::None }, }); + let transform = editing_text.transform; self.editing_text = Some(editing_text); self.set_editing(true, font_cache, responses); responses.add(NodeGraphMessage::SelectedNodesSet { nodes: vec![self.layer.to_node()] }); + // Defer TransformSet until after the graph has run so that downstream_transform_to_viewport + // has correct metadata for the new layer (needed for proper placement in transformed parents). + let layer = self.layer; responses.add(NodeGraphMessage::RunDocumentGraph); + responses.add(DeferMessage::AfterGraphRun { + messages: vec![ + GraphOperationMessage::TransformSet { + layer, + transform, + transform_in: TransformIn::Viewport, + skip_rerender: false, + } + .into(), + NodeGraphMessage::RunDocumentGraph.into(), + ], + }); } fn check_click(document: &DocumentMessageHandler, input: &InputPreprocessorMessageHandler, font_cache: &FontCache) -> Option { diff --git a/editor/src/test_utils.rs b/editor/src/test_utils.rs index 1a796407..40d6c30a 100644 --- a/editor/src/test_utils.rs +++ b/editor/src/test_utils.rs @@ -35,15 +35,16 @@ impl EditorTestUtils { // An inner function is required since async functions in traits are a bit weird async fn run<'a>(editor: &'a mut Editor, runtime: &'a mut NodeRuntime) -> Result { let portfolio = &mut editor.dispatcher.message_handlers.portfolio_message_handler; + let document_id = portfolio.active_document_id.unwrap(); let exector = &mut portfolio.executor; - let document = portfolio.documents.get_mut(&portfolio.active_document_id.unwrap()).unwrap(); + let document = portfolio.documents.get_mut(&document_id).unwrap(); let instrumented = match exector.update_node_graph_instrumented(document) { Ok(instrumented) => instrumented, Err(e) => return Err(format!("update_node_graph_instrumented failed\n\n{e}")), }; - if let Err(e) = exector.submit_current_node_graph_evaluation(document, DocumentId(0), UVec2::ONE, 1., Default::default(), DVec2::ZERO) { + if let Err(e) = exector.submit_current_node_graph_evaluation(document, document_id, UVec2::ONE, 1., Default::default(), DVec2::ZERO) { return Err(format!("submit_current_node_graph_evaluation failed\n\n{e}")); } runtime.run().await; diff --git a/node-graph/nodes/vector/src/generator_nodes.rs b/node-graph/nodes/vector/src/generator_nodes.rs index 41ed3311..66c8009e 100644 --- a/node-graph/nodes/vector/src/generator_nodes.rs +++ b/node-graph/nodes/vector/src/generator_nodes.rs @@ -1,5 +1,5 @@ use core_types::Ctx; -use core_types::registry::types::{Angle, PixelSize}; +use core_types::registry::types::{Angle, PixelLength, PixelSize}; use core_types::table::Table; use dyn_any::DynAny; use glam::DVec2; @@ -257,29 +257,22 @@ fn qr_code( Table::new_from_element(vector) } -/// Generates a line with endpoints at the two chosen coordinates. +/// Generates an arrow from the origin to the chosen coordinate. #[node_macro::node(category("Vector: Shape"))] fn arrow( _: impl Ctx, _primary: (), - #[default(0., 0.)] start: PixelSize, - #[default(100., 0.)] end: PixelSize, - #[unit(" px")] - #[default(10)] - shaft_width: f64, - #[unit(" px")] - #[default(30)] - head_width: f64, - #[unit(" px")] - #[default(20)] - head_length: f64, + #[default(100., 0.)] arrow_to: PixelSize, + #[default(10)] shaft_width: PixelLength, + #[default(30)] head_width: PixelLength, + #[default(20)] head_length: PixelLength, ) -> Table { - Table::new_from_element(Vector::from_subpath(subpath::Subpath::new_arrow(start, end, shaft_width, head_width, head_length))) + Table::new_from_element(Vector::from_subpath(subpath::Subpath::new_arrow(DVec2::ZERO, arrow_to, shaft_width, head_width, head_length))) } #[node_macro::node(category("Vector: Shape"))] -fn line(_: impl Ctx, _primary: (), #[default(0., 0.)] start: PixelSize, #[default(100., 100.)] end: PixelSize) -> Table { - Table::new_from_element(Vector::from_subpath(subpath::Subpath::new_line(start, end))) +fn line(_: impl Ctx, _primary: (), #[default(100., 100.)] line_to: PixelSize) -> Table { + Table::new_from_element(Vector::from_subpath(subpath::Subpath::new_line(DVec2::ZERO, line_to))) } trait GridSpacing {